fix(sync): redact local filesystem paths from every OS, not just Linux roots

[andrei-hasna]

Diagnosis: redaction was BROKEN. The test was right.

Task 716621a0. The failing test on main is
sync export redacts local file and workspace refs from bundles.

The question that decides the remediation is whether redaction is broken or the test
is stale. Measured, not assumed:

• The failing assertion is tests/sync.test.ts:518 — expect(serialized).not.toContain(sourceDir).
  It is the substantive containment assertion, not the [REDACTED:local-file-uri: marker-format
  assertion. A local filesystem path was reaching the exported bundle.
• In the very same bundle, the co-planted secret and database URL were redacted
  ([REDACTED:secret_assignment], [REDACTED:database-url:…]). So redaction ran and only
  the path family failed to match — a pattern-coverage defect, not redaction being off.

Root cause

src/private-ref.ts hardcoded Linux path roots:

ABSOLUTE_LOCAL_PATH_RE   = /\/(?:home|tmp)\/…/
PRIVATE_WORKSPACE_PATH_RE = /\/home\/[^/]+\/(?:workspace|Workspace)\/…/

Any local path outside /home or /tmp survived export untouched: macOS temp dirs
(/var/folders/… and the /private realpath spelling), macOS homes (/Users/…),
Windows drive paths and UNC shares. That is why this reproduced only on the
macOS CI runners and never on Linux — and why it went unnoticed.

Leak categories observed in the CI bundle (categories and counts only, no contents):
sources.metadata_json 2 fields, source_revisions.metadata_json 2 fields,
chunks.metadata_json 1 field, and 1 embedded artifact body via content_base64.

Fix

Local-path detection is now two complementary layers:

1. The running host's own home and temp roots, read from the OS rather than hardcoded,
   so hosts with unconventional roots are covered (HOME=/github/home, TMPDIR on a
   scratch volume). Both the reported and realpath-resolved spellings are covered, because
   macOS reports tmpdir as /var/folders/… while resolving it to /private/var/folders/….
2. OS-family path shapes, so a row pulled from a peer on a different OS is redacted as
   thoroughly as a locally produced one.

PRIVATE_WORKSPACE_PATH_RE and HASNA_PATH_RE now also recognise /Users/<user>/…, which
previously meant a workspace path on any macOS host could not be matched at all.

Test proven red against broken redaction

tests/private-ref.test.ts covers nine path families on every platform, so the defect is
now detectable on Linux instead of requiring a macOS runner.

Two independent mutations, both run with the fix already committed:

Control	Result
New test vs. the original unmodified main code	RED — named the 6 leaking categories: macos_home, macos_private_temp, macos_temp, windows_drive_backslash, windows_drive_forwardslash, windows_unc
New test vs. redactString replaced by return value (redaction fully disabled)	RED — 3 of 4 new tests fail, and the original sync.test.ts:518 test also goes red on Linux

The 4th new test is the over-redaction boundary (relative artifact keys, content hashes,
media types, s3:// and open-files:// URIs must pass through untouched); it correctly
stays green under both mutations, since disabling redaction cannot over-redact.

Failure output reports the category label, never the offending value — a guard that
prints what it caught publishes it.

Verification

Full suite, cloud-mode env vars neutralised, ANSI-stripped, both fail markers grepped:

265 pass / 2 skip / 1 fail / 2548 expect() calls — 268 tests across 39 files

The single remaining failure is context pack and proposal context commands return bounded agent JSON, which is pre-existing and unrelated: measured failing on pristine main
(535daf0), run alone, at tests/cli.test.ts:3044 — an --since 30d evidence-presence
assertion. Tracked separately, not addressed here.

Scope boundary — deliberately NOT crossed

verify:generated rebuilds and then runs git diff --exit-code -- bin/knowledge-mcp.js dist,
so a src/ change normally requires committing regenerated bundles. This PR does not touch
any generated artifact, because:

• verify:generated already exits 1 on pristine main — the committed bundles are stale
  independently of this change, and the diff is dependency drift in zod's JSON-schema
  codegen, not anything here.
• bin/knowledge-mcp.js / bin/knowledge.js / dist/index.js are exactly the files under
  concurrent modification by fix(build): ship the artifacts main's source actually builds, and make the check say so #38 (fix/hc-00151-regenerate-mcp-bundle) and fix(mcp): ok_untag must not call removing nothing a success #39. Regenerating
  here would duplicate fix(build): ship the artifacts main's source actually builds, and make the check say so #38's work and guarantee a conflict on a minified bundle.

Landing order: #38 owns bundle regeneration. Whoever merges second must rebase and re-run
verify:generated rather than trusting git's textual merge of a minified bundle — note it
exits 1 rather than 0 because it rebuilds first.

Verified against PR file lists (7/3/1 changed files, all under the 100-item cap, so the lists
are complete): this PR shares no file with #38, #39 or #40. tests/cli.test.ts is #40's
file and is untouched here; its only redaction-related assertions are [REDACTED:secret_assignment]
(from safety.ts) and the lint failure message, neither of which this change affects.

Known remaining gap (not fixed here, deliberately)

lintPrivateRefs has no absolute-local-path check at all — assertNoPrivateRefs would not
flag a bare local path in any of these shapes. That is arguably intentional, since the local
DB legitimately records a source's own path and export is the correct redaction boundary, so
widening the lint would change ingestion behaviour well beyond this failing test. Flagging it
rather than silently expanding blast radius.

Ready for an adversarial review pass. Not self-merging.

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[andrei-hasna]

CI verification on the platform that was failing

Run 30313350108. Read job conclusions from gh run view --json jobs, not from gh pr checks — gh pr checks reported 7 failures when the authoritative job view showed 5 cancelled + 2 failed. It conflates cancelled with failed.

Real state: 2 failures, both macos-latest. The only failing test name in the entire run is:

knowledge cli > context pack and proposal context commands return bounded agent JSON

That failure is pre-existing and unrelated — measured red on pristine main (535daf0), run alone, at tests/cli.test.ts:3044.

sync export redacts local file and workspace refs from bundles no longer appears. It failed on macos-latest at both e99f4fd and 535daf0; on this branch it passes there. That is direct verification on the platform where the leak actually reproduced — not an inference from the Linux run.

Unverified gap — Windows

windows-latest produced no evidence: matrix fail-fast cancels it whenever the pre-existing macOS context-pack failure fires, and gh run rerun --job <id> does not revive a cancelled job.

So the Windows patterns are verified only by planting verbatim Windows path strings (C:\Users\…, D:/…, \\host\share\…) in tests/private-ref.test.ts and watching them get redacted on Linux. The pattern logic is covered; the Windows-runner integration is not (specifically how os.homedir() / os.tmpdir() present there). Please treat Windows as unproven.

Side finding worth its own issue

While the context-pack test stays red, matrix fail-fast means no ubuntu and no windows evidence is produced for any PR in this repo — it is suppressing 5 of 7 jobs on every run.

[andrei-hasna]

Adversarial review — APPROVE-WITH-FIXES (one blocking defect found and fixed on this branch)

Reviewed independently, in a separate clone at .../worktrees/knowledge/adv-pr42-redaction, against head 84bf5cf. No values, paths or hostnames appear below — categories and counts only.

Verdict

The diagnosis holds up. Redaction was broken; the test was right. I re-derived the substantive part rather than inheriting it. But the change as submitted fixed the repository without fixing the shipped package, which for a leak defect is the half that matters. That is fixed here in 84d08a0; the source change is byte-identical to 84bf5cf.

BLOCKING (fixed on this branch, not merged unfixed)

The redaction fix did not reach the published artifact. package.json.files ships bin/ and dist/ only — src/ is not published. Measured on 84bf5cf, with a detector proven in both directions:

committed artifact	old Linux-only pattern	new cross-OS patterns
bin/knowledge.js	present (1)	absent (0)
bin/knowledge-mcp.js	present (1)	absent (0)
dist/index.js	present (1)	absent (0)

Positive control: after bun run build the same detector reports the old pattern absent (0) and the new ones present (1/3/1) in all three. So merging the source change alone would have left the @hasna/knowledge CLI, MCP server and SDK exporting macOS and Windows paths unredacted — the exact defect this PR describes, still live in the product.

The stated reason for deferring regeneration is now stale. It was "verify:generated already exits 1 on pristine main". Measured on main at 5f7d297 — after #41 landed — BASELINE_VERIFY_RC=0: all 6 generated bundles rebuild byte-identically. So main is clean, and merging as-is would have re-broken the gate #41 just repaired. After rebasing onto 5f7d297, verify:generated exits 1, and the only drift is this PR's own redactString change — the gate was correctly demanding the rebuild.

Fix: rebased onto 5f7d297, rebuilt with bun 1.3.14 (equal to the bun-version pinned in ci.yml, per the byte-gate's own guidance), committed the 3 changed bundles. verify:generated now exits 0.

The measurement I was asked not to inherit: test red against broken redaction

Run independently, on a committed tree, with redactString replaced by return value:

• tests/sync.test.ts:518 target test → RED (rc=1), failing on the containment assertion, not a marker-format assertion.
• tests/private-ref.test.ts → RED, 3 of 4 (rc=1); the 4th is the over-redaction boundary and correctly cannot go red under disabled redaction.

Restored from a pre-mutation copy and re-verified by sha256 before continuing. The acceptance bar is met: the guard genuinely sees an unredacted value and fails.

Cloud-mode trap neutralised and confirmed through the real resolver, not inferred: plain shell → resolveItemStore({}).kind = api, CLOUD_RESOLVED=YES; under env -u of both cloud vars → kind = local, CLOUD_RESOLVED=NO. Every run used the env -u prefix.

Suite

bun test --timeout 60000, cloud vars unset, FORCE_COLOR unset, ANSI-stripped, both fail markers grepped and cross-checked against the summary line ((fail)=1, ✗=0):

276 pass / 2 skip / 1 fail / 2740 expect() calls — 279 tests across 40 files, 168.70s

Exactly the predicted delta from main's 272/2/1 across 275 tests in 39 files: +4 pass, +4 tests, +1 file. The single failure is the pre-existing context pack and proposal context commands return bounded agent JSON at 1523.89ms — not a timeout shape. No second red, so no regression.

Attacking the redactor itself — residual gaps (non-blocking, all pre-existing, none a regression)

I ran the same 19-shape probe against this branch and against main's redactor so improvement could be separated from regression. This PR closes shapes and opens none, but "every OS" in the title is stronger than what ships. Still leaking through export-time redaction, measured:

• /root/… (Linux root's home) and /Volumes/<name>/… (mainstream macOS mounted/external volumes) — both survive. Same defect class as this fix, both also leaked on main.
• UNC written with forward slashes; drive-relative X:name; %USERPROFILE%-style env forms; /mnt, /media, /run/user/<uid> (systemd runtime dir).
• Percent-encoded separators; a path split across fields so no single literal matches; ~-prefixed and relative paths that still identify workspace layout (the suite deliberately pins relative keys as pass-through, so this is a boundary choice, not an oversight).

Object keys are not walked — redactPrivateRefs copies keys verbatim. I checked whether that reaches a bundle instead of assuming it: at ingestion a path planted as a metadata key does survive (S1=true), but metadata persists as a JSON text column, so export-time string redaction catches it (S2=false). Not an export leak — local-DB hygiene only. Recording the measurement because the comfortable inference here was the wrong one.

New over-redaction, low severity: the Windows drive pattern redacts a single-letter token immediately followed by :/ (a:/x → redacted; main left it alone). Not cheaply separable from a genuine A: drive path, so this looks like the right trade. URL schemes are safe — https://, s3://, open-files://, sha256:, text/markdown, span:12/34, 12:30/45 all verified unchanged.

Pre-existing over-redaction, marginally widened: a relative key containing a root-ish segment is redacted. wiki/home/…, logs/tmp/…, wiki/var/tmp/… already were on main; this PR adds wiki/Users/… and schemas/var/folders/…. The correct fix is anchoring the patterns to a path start, which changes redaction boundaries and needs its own test matrix — not a rushed edit here.

Fail-closed direction verified, and it is correct. A throwing input propagates out of redactPrivateRefs rather than returning partial text; in the artifact path a failure leaves content_base64 unset and emits a warning, so the export refuses rather than emitting unredacted. Confirmed by probe.

Already-exported bundles

Independent cross-check, bounded, with positive controls proven in three directions (planted leak → all 6 categories fire; properly redacted → silent; markers-plus-leak → fires, so the =0 bucket is reachable):

• 67 documents of real export shape (content_hash + table_clocks) reachable in the temp roots: LEAKY_DESPITE_REDACTION_MARKERS = 0, 49 leaky-but-never-redacted (fixture inputs that never went through export; no [REDACTED: marker at all), 18 clean. Only category present: posix_temp. No macos_*, windows_*, volumes or root_home anywhere.
• Bounded durable sweep of the app's own storage root found 0 documents of export shape.

My sweep of the wider roots did not finish inside its time bound, so this is narrower than the author's 490-document census, and it agrees with it on the load-bearing bucket. The conclusion is host-scoped and I want it read that way: this is a Linux host, and the old patterns did cover its /home and /tmp, so the defect's exposure window is bundles exported from macOS or Windows. Absence here does not establish absence fleet-wide, and no macOS or Windows host was inspected by either of us.

Claim accuracy

Two claims I checked and would soften:

1. "Failure output reports the category label, never the offending value" — true for 3 of the 4 new tests. The host-roots test uses expect(redacted).not.toContain(hostPath), which prints the running host's home path on failure. In CI that is a runner path; on a developer machine it prints a real local path into the log. Minor, and it inherits the same shape from sync.test.ts:518, which on failure prints the entire unredacted bundle — I saw exactly that under my own mutation. Worth knowing that the guard against leakage publishes the leak into the CI log when it trips.
2. macOS remains verified only by the author's 30313350108 job read, and Windows produced no evidence at all on any run because matrix fail-fast cancels it while the context-pack test is red. I did not revive it either. Windows is pattern-verified, not runner-verified — treat it as such.

Merging: CI cannot go green on this repo while the pre-existing context-pack failure stands, and that failure also suppresses 5 of 7 jobs on every run. Merging with that single known red, documented above and tracked separately.

Formal GitHub approval is not usable here (self-approval prints "Can not approve your own pull request" and still exits 0), so this comment plus todos 716621a0 is the review record.