feat(site-deploy): deploy a static build as an immutable release - #1776
Conversation
Adds @constructive-io/site-deploy: walk/hash a build, upload only the bytes the server does not already have, commit one manifest, and optionally publish or point a preview ref at the resulting commit. Builds on @constructive-io/upload-client for the generic presigned PUT; the site-specific half (CAS keys, manifest, release row, publish/preview) lives here so the UI and CI do not each reimplement it.
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Review complete. 🟡 4 medium 💬 Inline comments (4)
🧹 Nitpicks (2) — 🟢 2 low
This PR introduces the
The main risks are edge-case handling: Reviewed commit: 909e0a6 |
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
There was a problem hiding this comment.
Adds a new site-deploy package that walks a static build directory, content-addresses files, uploads them to a CAS bucket, and publishes a manifest/release via GraphQL, with several edge-case correctness and security gaps.
Key findings
- 🟡 skipIfUnchanged silently skips requested publish/preview — deploy.ts:112
- 🟡 Symlink-following reads arbitrary files into deploy — walk.ts:39
- 🟡 dryRun still performs a server-side write — deploy.ts:140
- 🟡 Empty (0-byte) files fail the whole deploy — manifest.ts:104
| ) { | ||
| onProgress?.({ type: 'unchanged', commitId: existing.commitId }); | ||
| return { | ||
| commitId: existing.commitId, | ||
| storeId: existing.storeId, | ||
| releaseId: existing.id, | ||
| manifest, | ||
| files: manifest.file_count, | ||
| uploaded: 0, | ||
| skipped: manifest.file_count, | ||
| bytesUploaded: 0, | ||
| published: false, | ||
| previewUrl: null, | ||
| unchanged: true, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🟡 bug · medium
skipIfUnchanged silently skips requested publish/preview
When skipIfUnchanged matches an existing manifest, deploySite returns early at deploy.ts:112 with published: false and previewUrl: null, never executing the publish or preview pointer moves even when those options were set. A caller that runs deploy({ publish: true, skipIfUnchanged: true }) on an unchanged tree receives a success result while site.activeCommitId is never moved, leaving the site unpublished despite an explicit go-live request.
📋 Prompt for AI Agents
In packages/site-deploy/src/deploy.ts lines 112-127, the skipIfUnchanged early-return bypasses the publish and preview pointer moves. After the unchanged check returns the existing release, if options.publish is true still call publishCommit(api, names, siteId, existing.commitId) and set published: true; if options.preview !== undefined still call pointPreview(...) and set previewUrl. Do not unconditionally hardcode published: false/previewUrl: null in this branch, because a caller requesting publish must not get a silent no-op go-live.
| // Symlinks to files are followed by readFile; anything that is neither a | ||
| // file nor a directory (socket, fifo) has no meaning in a static site. | ||
| if (!entry.isFile() && !entry.isSymbolicLink()) continue; | ||
| yield { path: logical, bytes: new Uint8Array(await readFile(child)) }; |
There was a problem hiding this comment.
🟡 security · medium
Symlink-following reads arbitrary files into deploy
walkDirectory treats any symlink as a file and reads it with readFile (packages/site-deploy/src/walk.ts:39), so a symlink inside the build tree that points outside the directory (e.g. .env or source) is read and uploaded to the site's public bucket. A build influenced by a compromised dependency or untrusted artifact can therefore exfiltrate arbitrary local files into a public release.
📋 Prompt for AI Agents
In packages/site-deploy/src/walk.ts around lines 36-39, stop following symlinks that escape the build directory. Resolve each entry with fs.realpath and verify the resolved path stays within realpath(dir) before reading; skip (or throw a DeployError) for any symlink whose target resolves outside the root. This prevents a malicious build from reading arbitrary local files into the deployed site. Update the comment on lines 36-37 to reflect the new behavior.
| const diff = await diffCas(files, uploadOptions); | ||
| onProgress?.({ | ||
| type: 'diffed', | ||
| files: manifest.file_count, | ||
| toUpload: diff.missing.length, | ||
| skipped: diff.deduplicated.length, | ||
| bytesToUpload: diff.missing.reduce((sum, entry) => sum + entry.file.size, 0), | ||
| }); |
There was a problem hiding this comment.
🟡 bug · medium
dryRun still performs a server-side write
The dryRun path in deploy.ts calls diffCas (line 140) before returning at line 147, and diffCas executes the uploadFiles mutation, which the server documents as creating file rows (graphile/graphile-presigned-url-plugin/src/plugin.ts:346). A run documented as 'hash and diff only' therefore still writes rows on the server, leaving orphaned file rows behind when the bytes are never PUT.
📋 Prompt for AI Agents
In packages/site-deploy/src/deploy.ts, the dryRun branch (around lines 140-147) still calls diffCas, which runs the uploadFiles mutation that creates file rows server-side (see graphile/graphile-presigned-url-plugin/src/plugin.ts:346). Make dryRun side-effect-free: when dryRun is true, skip the diffCas mutation (or derive the diff from a read-only mechanism) so no server write occurs, then return the hashed/diffed result as before. Update the dryRun test to assert no uploadFiles call is made.
| entries[file.path] = { | ||
| hash: file.hash, | ||
| content_type: file.contentType, | ||
| size: file.size, | ||
| }; |
There was a problem hiding this comment.
🟡 bug · medium
Empty (0-byte) files fail the whole deploy
buildManifest accepts a 0-byte file and emits a manifest entry with size: 0, but the server's processSingleFile rejects size <= 0 with INVALID_FILE_SIZE (graphile/graphile-presigned-url-plugin/src/plugin.ts:613), so any empty file in a build aborts the entire deploy via diffCas. A static build can legitimately ship an empty placeholder (e.g. .gitkeep), and the client gives no signal that it is unsupported.
📋 Prompt for AI Agents
In packages/site-deploy/src/manifest.ts inside buildManifest (around lines 100-115), before writing entries[file.path], check if (file.size <= 0) and throw new DeployError('INVALID_PATH', ...) with a message explaining that empty (0-byte) files are rejected by the server's upload surface, so the deploy fails early and clearly rather than surfacing the server's INVALID_FILE_SIZE from diffCas.
1 participant
Summary
@constructive-io/site-deployturns a built static site into one immutable release. The server already owned everything hard — commits,preview/<name>refs, manifest versioning, CAS reads, the atomicactiveCommitIdswitch — but nothing shipped the four mechanical client steps, so the UI and CI were each about to reimplement hash-and-upload.It walks the build, sha256s each file, diffs against the bucket in batched
uploadFilescalls, PUTs only the bytes the server reports as notdeduplicated, then writes onepath -> {hash, content_type, size}manifest row — whose insert trigger produces thecommitIdthat is the release. Publish and preview are separate steps, so a preview never touches production and a failed publish leaves the release intact. Layering follows the generic/specific split:upload-clientstays site-agnostic,site-deployowns thecas/sha256/<hash>convention, manifests, releases, previews, dedupe, batching, retries and progress; the dependency goes one way.Failure modes are hard failures, not silent drift: a missing upload URL, a failed PUT after retries, or a returned key that isn't the expected
cas/sha256/<hash>aborts before any manifest is written.Also adds
examples/site-deploy-ssg: a ~100-line generator producing a 5-file site, deployed to aplaypreview via the workspace package, with tests covering preview-without-publish, publish, one-file incremental redeploy (uploaded: 1, skipped: 4), rollback and unchanged-build skip. Both packages are registered in thepackages-coretest batch.Verification beyond the 49 mock tests: the built package was driven against a live constructive-db harness (real Postgres, real MinIO presigned PUTs, in-process static gateway) — first deploy created the release and
preview/playwhile production stayed unserved, an edited page re-uploaded one file and deduped four, the preview host served the new bytes while production still served the published commit, and publish/rollback moved the pointer both ways. That test isn't committed here: it lives in constructive-db and depends on this package being published.Link to Devin session: https://app.devin.ai/sessions/ecf6201520ef4ac984f4dd610ceea5b3
Requested by: @pyramation