Add verified compilation download

[raghubetina]

Summary

• add firstdraft plan compile --output <absent-path> for one conditional, pinned Compilation
• validate the deterministic artifact and materialize a verified binary-safe tree without overwriting an output
• preserve machine-readable recovery states for ambiguous starts, status and domain failures, artifact failures, and local publication
• exercise the real local HTTP journey through the packed executable

Boundary

This is the local smoke-test slice. It does not add cancellation or resume commands, MCP, remote publication, deployment, authentication, or retries for ambiguous mutations.

Verification

• asdf exec npm run check
• 110 tests
• typecheck, lint, formatting, and package allowlist
• offline packed install and real packed-executable local HTTP compilation and materialization

[raghubetina]

Technical review

The riskiest surface in this CLI so far: it takes bytes from a server and writes a directory tree onto the user's disk. npm run check exits 0 with 110 tests and npm audit reports 0 vulnerabilities.

Most of this review is an audit of the write path, because that is where a mistake stops being a bug and starts being a vulnerability.

Path validation is complete

COMPONENT_PATTERN alone would not be enough. It admits . and .., since both match [A-Za-z0-9._][A-Za-z0-9._-]*. So the first thing I looked for was an explicit dot-segment rejection, and it is there:

component.length === 0 ||
component === "." ||
component === ".." ||
Buffer.byteLength(component) > MAX_COMPONENT_BYTES ||
!COMPONENT_PATTERN.test(component) ||
component.endsWith(".") ||
component.toLowerCase() === ".git"

Plus Windows device names on the stem. That covers the classic archive-extraction attack, where a server-supplied path like ../../.ssh/authorized_keys escapes the destination.

Two of those go beyond the server-side equivalent I read in firstdraft/firstdraft#236. The trailing-dot rejection matters on Windows, where foo. and foo resolve to the same file. And excluding .git case-insensitively stops an artifact from writing into a repository's git directory, which is the kind of thing you only think of after seeing it exploited.

The write path is defended at four independent layers

Any one of these would help. Together they mean a compromised or buggy server cannot quietly corrupt the destination.

Contents are verified before acceptance. Each file's SHA-256 is checked against the manifest during parsing, so bytes that do not match their declared digest never reach the filesystem.

Writes cannot clobber. flag: "wx" fails if the path exists rather than overwriting.

Symlinks are seen, not followed. lstatSync throughout rather than statSync.

The tree is verified after writing, then published atomically:

writeArtifactTree(artifact.files, temporaryDirectory);
chmodSync(temporaryDirectory, DIRECTORY_MODE);
verifyArtifactTree(artifact.files, temporaryDirectory);
assertTargetStillAvailable(target, parent);
renameSync(temporaryDirectory, target);

The output path never holds a partial or unverified tree. It does not exist, and then it exists complete.

Two details in that sequence are easy to get wrong and are right here. The temporary directory is created inside the target's own parent:

const prefix = path.join(parent, `.firstdraft-${path.basename(target)}-`);

Had it gone to os.tmpdir(), the final renameSync would fail with EXDEV whenever the output landed on a different filesystem, which is exactly the sort of thing that works on the author's laptop and fails for a user with an external drive. Same-parent means the rename is always intra-filesystem.

And assertTargetStillAvailable is called twice, once before the work and once immediately before the rename. That does not eliminate the race between check and publish, since nothing short of an atomic directory create can, but narrowing the window to a single syscall is the right amount of effort for a local CLI. assertOwnedTemporaryDirectory additionally confirms mkdtemp returned a path inside the expected parent with the expected prefix.

The cli#8 finding is closed

On #8 I noted that the body claimed to "exercise the packed executable against a real local endpoint," but the packed smoke had zero network references and the real server lived only in the source-level tests. The two halves were separate.

They are joined now. scripts/smoke-package.js imports createServer from node:http, stands up a real server, and drives the installed executable against it:

const server = createServer(async (request, response) => { /* ... */ });
// ...
["plan", "compile", "--output", output],   // via spawnPackedCli

So the artifact a user would actually install now performs a real HTTP compilation and materialization in CI. That is a stronger claim than the one I questioned, and this time the diff supports it.

No findings

I went looking specifically for the ways this class of code fails and did not find one. Path traversal, overwrite, symlink following, unverified contents, partial output on failure, and cross-filesystem rename are all handled.

The boundary in the body is also accurate to what I read: no cancellation, resume, remote publication, authentication, or retry-after-ambiguous-mutation. The last of those is the right omission, since retrying a mutation whose outcome is unknown is precisely what the error contract from #6 exists to prevent.

[raghubetina]

Lesson: writing somebody else's files to your disk

Your code downloads an archive and extracts it. Or accepts an upload and saves it. Or, like this PR, fetches a generated project and writes it to a directory.

The filename came from somewhere else. That one fact changes everything about how you write the next twenty lines.

The attack that has been working since 1991

The archive contains a file named:

../../../../home/you/.ssh/authorized_keys

Your extractor joins that onto the destination directory and writes it. The path resolves out of the destination, up into the home directory, and now somebody else can log in as you.

This is Zip Slip, and it is remarkable mostly for how long it keeps working. It has hit archive libraries in every major language, and it hits again every time someone writes their own extraction loop and reaches for the obvious code:

writeFileSync(path.join(destination, entry.name), entry.contents);  // no

path.join is not a security boundary. It happily resolves .. for you, because that is its job.

Rejecting, not sanitizing

The tempting fix is to clean the path. Strip ../, maybe. Do not do this. Stripping is a filter you have to get exhaustively right, and attackers are creative: ....//, URL-encoded separators, backslashes on Windows, Unicode characters that normalize to a separator.

Validate instead, and reject anything that is not obviously fine. This PR splits on / and checks every component:

component.length === 0 ||
component === "." ||
component === ".." ||
Buffer.byteLength(component) > MAX_COMPONENT_BYTES ||
!COMPONENT_PATTERN.test(component) ||
component.endsWith(".") ||
component.toLowerCase() === ".git"

Allowlist the characters, reject the dot segments by name, and refuse anything oversized. Reject unknown input rather than repairing it, the same instinct as strong parameters.

Two of those checks are experience talking. component.endsWith(".") matters because Windows resolves foo. and foo to the same file, so a trailing dot lets one entry collide with another. And .git is excluded because writing into a repository's git directory is a way to get code executed later, via hooks.

If you are in Ruby, Pathname#relative_path_from and friends will not save you either. The reliable check is: expand the joined path, and confirm it still starts with the expanded destination.

full = File.expand_path(File.join(destination, entry_name))
raise "unsafe path" unless full.start_with?(File.expand_path(destination) + File::SEPARATOR)

Three more things that bite

Overwriting. The default write mode replaces whatever is there. If the destination has files, an extraction can silently destroy them. Open exclusively instead, so an existing file is an error:

writeFileSync(destination, contents, { flag: "wx" });   // fails if it exists

Symlinks. If an attacker can get a symlink into the destination first, writing "through" it lands somewhere else entirely. Use lstat, which describes the link itself, rather than stat, which describes what it points at. This PR uses lstatSync everywhere and treats any symlink it finds as a failure.

Partial output. Something fails halfway. Now the destination holds half a project, and the next thing to look at it has no way to know. Write somewhere private, verify, and publish atomically:

temporaryDirectory = mkdtempSync(prefix);
writeArtifactTree(files, temporaryDirectory);
verifyArtifactTree(files, temporaryDirectory);
renameSync(temporaryDirectory, target);

rename within one filesystem is atomic. The destination does not exist, and then it exists complete. There is no moment where it exists half-written.

One detail there is worth copying exactly. The temporary directory is created inside the target's parent, not in /tmp. Rename across filesystems fails with EXDEV, so a temp directory in /tmp works on your machine and breaks for the user whose output is on an external drive. Same parent, same filesystem, always.

Verify before you publish, not after

The other habit in this code: every file's SHA-256 is checked against the manifest before anything is written, and the whole tree is verified again before the rename.

That ordering matters. Verifying after publishing means a user can already be looking at bad output when you discover it. Verifying inside the temporary directory means a failure leaves nothing behind but a directory you delete.

The short version

Any time a path comes from outside your process, treat it as hostile:

• validate each component against an allowlist, reject . and .. by name
• create exclusively so you cannot clobber
• lstat, never stat, and refuse symlinks
• write to a temporary directory beside the target, verify, then rename
• check digests before publishing, not after

Five rules, and every one of them exists because somebody skipped it.

[raghubetina]

Windows CI resolution

The three Windows failures had one root cause: the materializer compared exact POSIX 0644/0755 bits after writing, but Node's Windows filesystem contract can manipulate only write permission and does not implement the owner/group/other distinction. The generated tree was otherwise valid.

The amended commit keeps the trust boundary intact:

• artifact-declared modes are still restricted to 0644 or 0755 on every platform
• POSIX still applies and verifies exact directory and file modes
• Windows skips only the mode operation and comparison it cannot represent
• file type, symlink rejection, exact tree shape, bytes, and SHA-256 verification remain unconditional

The platform-aware test now sets a restrictive 0077 umask on POSIX before materialization, proving the explicit chmod produces 0755 directories and artifact-declared file modes rather than passing because of the usual 0022 default. Windows exercises the same successful materialization path while asserting every entry's expected type. Integration and packed-executable smoke tests preserve their exact mode assertions where supported.

Node documents the limitation in its filesystem mode guidance.

Verification on amended head c58325b:

• asdf exec npm run check
• 111 tests
• typecheck, ESLint, Prettier, package allowlist, and packed real-HTTP compilation/materialization smoke

The hosted OS matrix has restarted on the amended commit.