fix: spec-store open-once read + fail-closed spec_id scan (iss-68) - #37
Merged
Conversation
- P7: readRepoFile did Lstat then os.ReadFile, re-resolving the path (symlink-swap TOCTOU). It now opens once with O_NOFOLLOW|O_NONBLOCK and validates the same fd (regular file, size cap) before reading. O_NONBLOCK keeps a FIFO leaf from blocking the open (regression guard: TestLoadRejectsFifoSpecFile). - P5: maxIntentSpecNum silently dropped a non-null intent spec_id from which no reservation number could be parsed (specNum zeroed it), so its slot could be missed and NextID could mint a colliding id. It now fails closed on a spec_id with no parseable number (spc-, spc-abc). A spc-N or spc-N-<slug> form still reserves N — record-lint's planned rule (prefix ^spc-) and the spec_lifecycle specNum parse both tolerate the trailing slug, so the two gates agree and a lint-green record never bricks the mint path. - seed4: abcd intent plan seeds a new spec with a clear author-guidance placeholder instead of a bare 'TODO'. Document-accepted (trusted-worktree model, honest comments): the ensureDir ancestor-symlink guard (seed5) and the Close Lstat-then-Rename clobber check (P8). Assisted-by: Claude:claude-opus-4-8
Assisted-by: Claude:claude-opus-4-8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves iss-68 (native ledger) — the last of the iss-65..72 sweep set.
Fixes (
internal/core/spec)readRepoFiledidLstatthenos.ReadFile, re-resolving the path (symlink-swap window). It now opens once withO_NOFOLLOW|O_NONBLOCKand validates the same fd (regular file, size cap) before reading.O_NONBLOCKkeeps a FIFO/device leaf from blocking the open — a regression guard (TestLoadRejectsFifoSpecFile) enforces it, since the new open-first flow would otherwise hang where the oldLstat-first flow didn't.maxIntentSpecNumsilently dropped a non-null intentspec_idfrom which no reservation number could be parsed, so its slot could be missed andNextIDcould mint a colliding id. It now fails closed on aspec_idwith no parseable number (spc-,spc-abc). Aspc-Norspc-N-<slug>form still reserves N — matching record-lint's planned prefix rule (^spc-) and thespec_lifecyclespecNumtrailing-slug tolerance, so the two gates agree and a lint-green record never bricks the mint path.abcd intent planseeds a spec with a clear author-guidance placeholder in## Summary, not a bareTODO.Document-accepted (trusted-worktree model, honest comments)
ensureDirguards the leaf but not a symlinked ancestor; planting one needs repo write access equal to editing the record directly.Close'sLstat→Renameclobber check is racy, but the atomic same-filesystem rename is preferred over a non-atomic no-clobberlink+removea crash could leave half-done; there is no concurrent adversary in-model.Review
make preflight+make record-lintgreen; spec package green under-race. Reviewed over two rounds: ruthless SHIP; security BLOCK then PASS — the first cut used spec's anchored^spc-[0-9]+$(with a false comment that record-lint required it), which would have rejected a lint-validspc-N-<slug>and hard-failedabcd intent planrepo-wide. Narrowed tospecNumRe(has-a-number), corrected the comment, reverted the masking fixture, re-verified PASS.Assisted-by: Claude:claude-opus-4-8