TASK-531: Audit remediation: harden scanner, viewer, and store surfaces (37 triple-validated findings)

[docxology]

What this is

An external, adversarial security and correctness audit of groma.md v0.5.0 (HEAD 5882073a), run with a three-pass methodology:

1. Discovery (R1) — independent reviewers swept the scanner/plugin surface, core lifecycle, storage/watching, docs-truth, and the web/TUI viewer for defects.
2. Adversarial re-derivation (R2) — a separate reviewer per claim received only the claim text (not the discovery reasoning) and re-derived each finding from source; overclaims and drifted citations were corrected here.
3. Reproduction (R3) — every surviving finding was re-verified against the actual lines on HEAD, with minimal reproductions where feasible.

Result: 37 triple-validated findings — 28 fixed, 9 reported on this branch. Each finding has its own issue with full evidence, failure scenario, remediation, and validation notes:

• Fixed findings are linked in the first table below; each links its issue.
• Findings left for maintainer decision are itemized at the bottom.

Triple-validated: independent discovery (R1), adversarial re-derivation by a separate reviewer given only the claim text (R2), and source reproduction on HEAD 5882073.

Fixed on this branch (28)

ID	Finding	Severity	Issue
D-sc-02	Attacker-controlled source file names are interpolated unescaped into groma/relationships.md rows, forging rows and permanently breaking architecture loading (INVALID_RELATIONSHIP)	medium	docxology#2
D-sc-03	Repository listing keeps symlinks that escape the scanned tree; scanner reads follow them (device/FIFO DoS, host-file probing)	medium	docxology#3
D-sc-04	One malformed package.json/angular.json/project.json in an active project fails the whole TypeScript/Angular/React observation with a raw SyntaxError	medium	docxology#4
D-sc-05	TypeScript scanner: dangling projectReference throws raw ENOENT and references are followed outside the repository	low	docxology#5
D-sc-07	CONTRIBUTING.md documents a verify command for a file that no longer exists	low	docxology#7
D-sc-08	Swift adapter buffers worker output with maxBuffer: Infinity — unbounded memory on crafted source	low	docxology#8
D-sc-09	Discovery attributes dependency versions resolved outside the repository to the project	low	docxology#9
D2-1	Store and AGENTS.md writes are non-atomic direct writeFile — truncation leaves an unloadable store	medium	docxology#11
D2-3	Root, export, and instructions-interactive actions lack try/catch: raw stack instead of message + exit 1	medium	docxology#13
D2-4	Welcome-loop action errors escape as unhandled rejection (raw stack) while the identical groma scan failure reports cleanly	low	docxology#14
D2-5	Draft relation help marks --description optional but addRelation requires --description and --technology	low	docxology#15
D2-7	Welcome status bar hardcodes "Architecture ready" while the footer can simultaneously show a scanner error notice	low	docxology#17
D2-8	stopOnSignal has no .catch on close(): a rejected close crashes with a raw stack instead of exiting cleanly	low	docxology#18
D-work-01	Backlog work-source shim can echo cmd.exe metacharacters into an unquoted /c string (command injection, Windows shim path)	medium	docxology#19
D-work-02	Backlog stream parser dies permanently on pre-snapshot noise, negative depth, or unbalanced quotes (watcher killed, stale data for the session)	medium	docxology#20
D-work-03	Degenerate task fields flow unvalidated into pins (crashes in map-session publishWork and TUI pullWork paths)	medium	docxology#21
D-work-04	stopProcess sends one SIGTERM with no escalation: a wedged backlog child hangs CLI exit, and EPERM rethrow becomes an unhandled shutdown rejection	low	docxology#22
D-work-05	Dotted task ids sort and color incorrectly (parseFloat merges 416.10 with 416.1; digit-less ids give NaN)	low	docxology#23
D-work-06	CONTRIBUTING.md instructs running a test file that was deleted (test-bun/parcel-bytecode.test.ts)	low	docxology#24
D5-0	product-model.md claims exports contain task details and diffs; export ships EMPTY_WORK_SNAPSHOT	medium	docxology#25
D5-1	component-markdown.md links to groma/systems/groma/system.md and groma/drafts/mvp.md which do not exist	low	docxology#26
D5-2	relationship-inference.md self-counts drifted from the live tree (docs: 220/83/113/99/14; live: 119 elements, 42 authored, 21 derived rows)	low	docxology#27
D5-4	inspect.md implies --json works across paged commands; view/lint reject --json	low	docxology#29
D5-5	release.yml commits to main via unpinned third-party actions with contents:write + id-token:write; all third-party actions float tags	high	docxology#30
WEB-01	DNS rebinding: web viewer accepts any Host header and has no token auth on any endpoint	high	docxology#33
WEB-02	CSRF: cross-site form POSTs reach every mutation endpoint (no Origin/Referer check, no token)	high	docxology#34
WEB-03	Path traversal via stored code references: /source.json and comparison reads do readFile(join(root, reference.file)) with no containment validation	high	docxology#35
WEB-04	Stored XSS from repo content: unescaped interpolation of backlog workflow status (and taskId) into innerHTML in the work island	high	docxology#36

Reported for maintainer decision (9)

These are real and confirmed, but the right fix is a design/product decision, so this PR deliberately does not change them:

ID	Finding	Severity	Issue
D-sc-01	Untrusted repository's groma/scanners.json executes attacker code in the groma process (local-plugin import + settings-driven worker paths)	high	docxology#1
D-sc-06	Web viewer server binds all interfaces unauthenticated and exposes scanner installation/init over the network	high	docxology#6
D2-0	Crash mid-rewrite leaves a write-then-remove window; the move variant leaves an unloadable duplicate-id store (the rename variant does not)	medium	docxology#10
D2-2	No cross-process locking: CLI, scan --watch, and web writes are unserialized whole-file read-modify-write (last-writer-wins)	medium	docxology#12
D2-6	groma init overwrites the stored project title with no confirmation prompt	low	docxology#16
D5-3	historical-investigations.md cites SHAs and branches absent from the clone — and absent from origin too	low	docxology#28
D5-6	Typecheck gate and shipped scanner runtime pinned to a dated TypeScript dev build (7.1.0-dev.20260924.1)	medium	docxology#31
D5-7	architecture.yml runs npm-fetched scanner code on every main push; the global backlog.md install is unconsumed	low	docxology#32
WEB-05	DoS bounds: unauthenticated per-request full-repo git-archive extraction + complete map re-layout, and unbounded SSE client registry	medium	docxology#37

Fixed by area

• Scanner surface (D-sc-02/03/04/05/07/08/09): symlink escape from the repository root, unescaped file names in relationship rows (could permanently brick the store), malformed-manifest scan kills, dangling tsconfig references, stale CONTRIBUTING verify command, Swift maxBuffer: Infinity, out-of-tree dependency version attribution.
• Web viewer (WEB-01/02/03/04): loopback Host gate (DNS rebinding), Origin/Referer CSRF gate, stored-reference path traversal containment at parse time and read time, work-island XSS (status + chip now textContent).
• Core CLI lifecycle (D2-1/3/4/5/7/8): atomic store writes at the GromaFileSystem.write choke point, try/catch on root/export/instructions actions, welcome-loop action errors, draft-relation help text, status-bar/notice contradiction, signal-shutdown close() crash.
• Backlog work source (D-work-01/02/03/04/05): task-id validation + shim quoting, stream resync, degenerate-field coercion + publishWork catch-reset, SIGKILL escalation on close, segment-aware pin ordering.
• Docs (D5-0/1/2/4): export contract, dead example links, re-measured self-counts, --json scoping.
• CI (D5-5): all 12 third-party actions pinned by full commit SHA (tags kept as comments).

Amended pinned test (needs review)

test-bun/scanner-source-listing.test.ts incidentally pinned the audited D-sc-03 behavior: its fixture symlinks Shared.swift to a file outside the repository root (a sibling temp dir) and asserted the symlink stays listed. Under the fix, out-of-tree symlinks are excluded (that is the point of D-sc-03), so the expected list was amended to drop Shared.swift; the test's actual subject — the in-tree symlink deduped to its listed target — is untouched and still passes.

Checks run

bun run check on this branch: 763 pass / 7 fail (+26 new tests, all passing). The 7 fails are the pre-existing environment-dependent set on this arm64 macOS host with no Java/SwiftSyntax toolchain — 6 Swift (worker needs bundled SwiftParser host modules that ship via CI's setup-swift) and 1 Java (JDK-version-sensitive assertion; verified identical pre-fix at 737/7 with JDK 21 and 763/7 with JDK 27, so none are regressions). CI validates these.

Checked and cleared (not filed, for completeness)

• D2-9 (REFUTED): loadWelcomeModel cannot crash on corrupt scanner settings — readScannerSettings catches all settings-read errors and returns an error notice (src/scanner/modules/settings.ts:84-91); backlogPlugin.readiness() is a Bun.which lookup.
• INF-001 (PARTIAL, positive control): escaped() is attribute-context safe and all server-rendered sinks route through it; the register's stale citations were re-anchored; the uncovered sinks are the client-side island ones reported as WEB-04.
• INF-002 (REFUTED): a sanitizer exists and is wired into the only raw-HTML sink (comark security plugin, src/viewers/web/project/editor.ts:1-11); "0 hits for sanitize" was a keyword artifact.
• WEB-06 (REFUTED): TUI escape injection — all content paints via opentui cell-buffer chunks with consoleMode disabled; no raw stdout write of scanned strings found (renderer binary not executed; medium confidence).
• WEB-07 (REFUTED): no static file serving exists to traverse; export writes fixed filenames; exported script islands are escaped (JSON.stringify + </ replacement).
• WEB-08 (REFUTED): no exec in the open-source-file flow; complete spawn-site enumeration shows fixed argv skeletons and validated revision ids.
• WEB-09 (REFUTED): every URL param is identity/range-validated before use; all paint paths are textContent/setAttribute.

---

Linked task: TASK-531 ("Harden audited surfaces and correct documentation drift") in backlog/tasks/.

[github-actions]

Groma architecture comparison

Changes	Added	Modified	Removed
Components	0	16	0
Relationships	0	0	0

Open before / after comparison

Compared b759b19 (merge base) → 37337a0.

[TheGraytCat]

I checked this PR while looking at Groma for a Gray Cat video. Two security gaps still remain at 37337a01. I used temporary projects and dummy files only.

1. Other computers can still reach the server. [Both reviewers]

   The server still has no local-only listener. The new check trusts the Host header, which the client can choose.

   I connected through my computer's non-local network address, sent Host: localhost:<port>, and got a 200 response containing the dummy source file. A client that can reach the port can make the same request. The edit and scanner routes also have no sign-in, and requests without an Origin pass the new check.

   Please bind the server to 127.0.0.1 by default and use the matching browser address. Keep the browser request checks too.

2. Files outside the project still reach the viewer and exports. [Both reviewers; comparison check by Codex]

   The new path check checks the written path, not the real file location. I made src/orders.ts point to a dummy file outside the project. The source route returned its contents, and groma export copied them into snapshot.js. Filtering links out of a new scan does not protect references already saved in the architecture.

   There is also a separate route around the ../ check. With a saved reference to ../outside.txt, the direct source route correctly returned 404. However, /world.json?from=<commit> returned 200 and included that same dummy file in the comparison. Comparison reads happen without the new boundary check.

   Please check the real file location before every source read, including exports and revision comparisons. For a saved Git version, check against that version's temporary root. Files that were deleted still need to work in comparisons.

The existing web tests passed, including the foreign-host, foreign-origin and direct ../ checks. The scanner-listing test also passed. These checks improve the code, but they do not cover the cases above.

I ran the 11 changed test files on Bun 1.3.14: 32 tests passed, and two files failed to load because of parser errors in unchanged scanner files. I did not run the declared Bun 1.4.2 version. The two reviewers completed the source review; the live reproductions above were run by Codex. I would fix these gaps before relying on this PR for safe local use.

---

_{😸 Generated with The Gray Cat | Orchestrated by OpenAI Codex; reviewed by Codex + Claude in parallel}