bug(storage): upgrade reclaim unlinks by exact path, so a parent symlink escapes the Artifact root and a case alias can hit a live record

[jackwener]

What happened

reclaimUpgradeResidue() decides what to unlink by comparing recorded orphan paths against a set of live record paths as exact strings, then calling unlink(join(artifactRoot, recordedPath)) directly. Two properties fall out of that, one reachable today with an out-of-band change to the state tree and one latent:

1. A parent-directory symlink lets the reclaim delete bytes outside the Artifact root. The lexical guard accepts session-1/retired-payload.txt. Replacing artifacts/session-1 with a symlink to a directory elsewhere makes the reclaim unlink the same-named file there and then discharge the note. A leaf symlink is handled correctly — only the link is removed, not its referent — and .., absolute paths, empty segments, ., backslash traversal and URLs are all correctly rejected.

2. The live-record protection is case-sensitive while the filesystem often is not. claimed is a JavaScript Set of exact path strings. On a case-insensitive filesystem (default APFS, and Windows), a recorded orphan path and a live record's path that differ only in case name the same file, the claimed lookup misses, and the reclaim unlinks the live file while its metadata stays. The Session then reads not_found for an Artifact the catalog still lists, and the orphan note is discharged, so nothing retries.

Reachability, and why this is not filed as a data-loss bug

Property 2 was originally graded P1 and that was wrong; the correction is worth stating because it is the whole reason this is not urgent. The alias needs the same artifactId prefix with a differently-cased name, and no released client produces that. The artifact id is attachment-${sha256(sessionId\0uploadId).slice(0, 32)}, so aliasing requires the same (sessionId, uploadId) to be ingested twice under names differing only in case. Across released writers:

• v0.1.0–v0.1.4: the Desktop attachment path calls create() with no id, so every upload gets a fresh random one.
• v0.1.5–v0.1.11 and v0.2.0-dev.9–.22: everything goes through DesktopRuntimeHostClient.ingestAttachment(), which resolves input.uploadId ?? randomUUID(), and no non-test caller passes a uploadId. A reconnect inside one call can reuse the id, but it carries the same input name; a fresh send re-enters the method and draws a new id.
• sanitizeArtifactName has never altered case. A trailing-dot sanitizer change did land between v0.1.0/.1 and v0.1.2, and pre-SQLite JSONL rows do migrate in, but that still does not produce the same id written again under a different canonical name.
• Unicode NFC/NFD and trailing dot/space cannot substitute for the case difference either: the stored path is ${sessionId}/${artifactId}-${name}, and two different released uploads have different ASCII id prefixes, so normalising the suffix cannot make the whole path alias.

The Host protocol does allow a custom client to reuse (sessionId, uploadId) with a different name once the old row is gone, so the invariant is real — it simply has no producer in this repository today. Filed as a latent invariant rather than a live defect.

Property 1 needs an out-of-band mutation or transplant of the state tree before it applies: the official backup path refuses symlinks and no ordinary writer produces that layout. It grants no privilege the caller did not already have, but it does unlink bytes outside the Artifact root, which is why it is worth fixing rather than documenting.

Suggested fix

Both properties come from the same shortcut — deriving the target with join() and trusting an exact-string comparison. The ordinary purge path already solves this, with real-parent containment plus a filesystem-identity comparison against the live record. Reusing that removal-identity mechanism in the upgrade reclaim closes both at once and adds no second definition of "the same file" to keep in step.

Verified alongside

Two adjacent behaviours were checked and are correct, so a fix should preserve them:

• Exact path reuse is safe. The reclaim reloads metadata inside the same writer lock before comparing, so a real v1 → v3 → same-path rebuild keeps the new bytes.
• A create in flight is safe, and the safety comes from the lock rather than from timing. Pausing 750 ms between the production file sync and the metadata commit still leaves a second store's reclaim behind the same root writer lock. Changing the production reclaim's enqueueMutation to enqueue makes the identical probe fail with ENOENT, which is what shows the lock is load-bearing.

Environment

• Affected code: reclaimUpgradeResidue() in packages/storage/src/artifact-store.ts, on main since fix(storage): reclaim the bytes the v1 upgrade orphaned #4867
• Reviewed at 8c523c413, re-bound to 07f6027ac with the four relevant Storage files byte-identical
• Property 2 observed on default case-insensitive APFS; the same aliasing applies on Windows

---

Automated review notice: This issue was filed by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

[liuxiaocs7]

take