Development record
record_stores values are not containment-checked so a committed config walks the record gate outside the repository
record_stores values are not containment-checked so a committed config walks the record gate outside the repository
Found by the round-2 adversarial security review of build/itd-189. PRE-EXISTING -- reproduces identically on 932629f9 and on main.
A PR that edits .abcd/record-lint.json to "adr": "../outside/decisions"
reaches filepath.Join(repoRoot, ...) at schema.go:1007 with no .. or
IsAbs rejection, and the gate then reads and echoes .md frontmatter from
outside the checkout. Reproduced on both HEAD and base:
../outside/decisions/0001-outside-secret.md:2: [BLOCKER record_schema]
filename claims id 'adr-1' but frontmatter declares 'adr-77'
validateRecordStores checks only prefix membership and inter-store layout,
not containment.
Mitigating factor, and why this is major rather than critical: the edit is glaringly visible in the PR diff, and a config change is exactly the thing review looks at. This is a defence-in-depth gap, not an invisible one.
Remedy: reject any record_stores value that is absolute or that escapes the
repo root after filepath.Clean.
Sibling: iss-2608301203521317 (the store walk's raw os.ReadFile). Both are the
same underlying shape -- the GATE reads attacker-influenceable paths without
the guard the repo already owns -- and fsutil.ReadGuarded closes the read
half of both at one call site.
Grounds
- pursued: a gate at the config door cannot be forgotten by a field added later, where the six per-site containedRepoPath guards were opt-in and silent when omitted; the anti-vacuity guard (this repository's own record-lint.json and docs-lint.json still load) is what would show the predicate too strict, and a managed repo whose legitimate config spells a path uncleanly ('./docs') being refused is what would show the strictness misjudged.
Frontmatter
| id | iss-2608301308367566 |
|---|---|
| type | issue |
| lifecycle | resolved |
| severity | major |
.abcd/work/issues/resolved/iss-2608301308367566-record-stores-values-are-not-containment.md
open on GitHub ↗ · commit history ↗
mentions6
- adr-1Three-layer mental model (brief / intent / spec)
- iss-2608301203521317record_schema's store walk reads every record with os.ReadDir and os.ReadFile, with no symlink refusal and no size cap, while the sibling reading walk in readingoutstanding.go reads the same trees through fsutil.ReadGuarded — so a symlinked or oversized record is followed by the gate and declined by the report, and a symlink's target frontmatter can surface in lint output. Pre-existing across all four original stores; noted during the itd-189 security review and left out of that branch's scope. Read the store walk through the same guarded read the report uses.
- iss-2608301423540137the orchestrator committed record bodies to three build branches without running the record and site gates, turning site-render red on two of them
- iss-2609012037137250Sibling of GHSA-865x-5m7q-qm79 (CWE-59), found on the sweep and not fixed there: intent ensureRealDir (internal/core/intent/lifecycle.go) and spec ensureDir (internal/core/spec/store.go) refuse a symlinked LEAF directory and then os.MkdirAll the path, so a committed symlink at an ancestor (.abcd, .abcd/development, or intents/ and specs/ themselves) redirects the store create and every subsequent write — including the draft that capture promote mints through intent.CreateDraft — outside the checkout. Both functions carry a NOTE naming this as a low-severity follow-up under the trusted-worktree model; that model exists only in those comments and a research note, not in an ADR, and the same class is already ruled in scope for memory (ad2f1a8e), ahoy (81f81f67) and now the issue ledger. The fix is the per-segment Lstat walk from repoRoot that memory.memoryDir and the capture ledger use, one level at a time with os.Mkdir, with a test per store that symlinks .abcd/development out of tree and asserts a refusal and an empty target. Record-store containment as a structural rule is iss-2608301308367566; this record names the two create sites that rule would close.
- iss-2609012037143368The capture package's ledger writes are absolute-path operations — createPlaceholder through syscall.Open, the .iss-alloc.lock flock path, fsutil.WriteFileAtomicPreserveMode for the commit and the stamps, os.Mkdir for the directories — each guarded by its own Lstat or O_NOFOLLOW check. The GHSA-865x fix adds a per-segment ancestor walk, which closes the committed-symlink case but leaves the lstat-to-mkdir window a local racer could use, the same window memory.memoryDir accepts. The durable closure is os.Root: fsutil already carries CreateExclusiveIn, WriteFileAtomicInRoot and ReadGuardedInRoot for exactly this class, and a store opened once as a root cannot be redirected between a check and a write. Converting the capture store to it is a package-wide refactor of every write site and the lock path, not a one-site fix, and it is the shape a structural record-store containment rule (iss-2608301308367566) would want every store to take. Recorded as the follow-up the advisory fix does not attempt.
- itd-189What the widening reading proposes is admitted or declined on the record — grounds on admission, dispositions on declines, and surprises as their own entries