Breadway/bread
1
0
Fork 0
Code Issues 9 Pull requests 2 Projects Releases Packages Wiki Activity Actions

feat(scrub): bread-scrub, allowlist-first PII scrubber for breadbug #21

Merged
Breadway merged 4 commits from feature/breadbug-scrubber into main 2026-10-13 21:05:22 +08:00
Conversation 7 Commits 4 Files changed 6 +1685
Breadway commented 2026-10-09 09:13:29 +08:00
Owner
Copy link

Part of BREAD-15 and BREAD-37: the library half only.

Scope gate (coordinator, 2026-10-13): this PR delivers and is reviewed as the bread-scrub library. The cards' consumer criteria (scrub on breadd ingest and again at report time including notes, and markers staying distinguishable across restarts and multiple ingest scopes) stay open on BREAD-15 and BREAD-37. They are verified on main after the stacked store PR #22 (BREAD-14) and report PR #23 (BREAD-19) land. Merging this PR does not complete either card.

Part of epic ECO-EP1 (breadbug). Spec: ECO-66.

What

New workspace crate bread-scrub, used twice by breadbug: on ingest into the breadd store and again at report time (including the user's notes).

  • Free text is pattern-redacted: emails, IPv4/IPv6, MACs, UUIDs, long hex, known token formats (GitHub, GitLab, Slack, AWS, Google, JWT, Bearer/Basic, PEM private keys), key=value secrets, URL userinfo and query strings, home paths, user and host names, extra literals, SSID, serial, window titles, high-entropy strings.
  • Event payloads are allowlisted: dropped unless EventAllowlist names the fields; a dropped payload keeps only _dropped and safe-looking top-level key names.
  • Markers are stable within a scope (<IP:1>, <IP:2>), the table stores only salted hashes, capped at 4096 per category (<IP:+> beyond).
  • Idempotent: markers are never re-scrubbed, and scrub_text runs to a fixpoint.
  • Build-ids survive (context rule plus keep_build_id); legacy-mangled Rust symbols are parsed exactly so a _ZN prefix cannot exempt trailing text.
  • stats() gives per-category redaction counts for the preview.

Tests

  • 25 unit tests, plus a golden corpus with 25 seeded fake PII values (fails if any survives, checks debugging value is kept).
  • 2 x 20k-iteration fuzz tests: no panics, idempotence with same and fresh scrubber, seeded PII never survives when mixed with random fragments and invalid bytes.
  • cargo clippy --all-targets -D warnings, cargo fmt, xtask check-docs clean.
  • Release build: about 70 microseconds per random fuzz input including scrubber construction.

Known limits (documented in the crate docs)

  • Best effort on free text; it cannot know an arbitrary word is a name. That is why structured fields are allowlisted and the user previews the payload.
  • A numeric PII fragment glued to more digits or dots is treated as a different number.
  • Four-part dotted numbers glued to letters (e.g. v1.2.3.4) are treated as IPs; this errs toward redaction.

Not in this PR

Wiring into breadd's store (BREAD-14) and the report builder (BREAD-19).

Part of BREAD-15 and BREAD-37: the library half only. Scope gate (coordinator, 2026-10-13): this PR delivers and is reviewed as the bread-scrub library. The cards' consumer criteria (scrub on breadd ingest and again at report time including notes, and markers staying distinguishable across restarts and multiple ingest scopes) stay open on BREAD-15 and BREAD-37. They are verified on main after the stacked store PR #22 (BREAD-14) and report PR #23 (BREAD-19) land. Merging this PR does not complete either card. Part of epic ECO-EP1 (breadbug). Spec: ECO-66. ## What New workspace crate `bread-scrub`, used twice by breadbug: on ingest into the breadd store and again at report time (including the user's notes). - Free text is pattern-redacted: emails, IPv4/IPv6, MACs, UUIDs, long hex, known token formats (GitHub, GitLab, Slack, AWS, Google, JWT, Bearer/Basic, PEM private keys), key=value secrets, URL userinfo and query strings, home paths, user and host names, extra literals, SSID, serial, window titles, high-entropy strings. - Event payloads are allowlisted: dropped unless `EventAllowlist` names the fields; a dropped payload keeps only `_dropped` and safe-looking top-level key names. - Markers are stable within a scope (`<IP:1>`, `<IP:2>`), the table stores only salted hashes, capped at 4096 per category (`<IP:+>` beyond). - Idempotent: markers are never re-scrubbed, and `scrub_text` runs to a fixpoint. - Build-ids survive (context rule plus `keep_build_id`); legacy-mangled Rust symbols are parsed exactly so a `_ZN` prefix cannot exempt trailing text. - `stats()` gives per-category redaction counts for the preview. ## Tests - 25 unit tests, plus a golden corpus with 25 seeded fake PII values (fails if any survives, checks debugging value is kept). - 2 x 20k-iteration fuzz tests: no panics, idempotence with same and fresh scrubber, seeded PII never survives when mixed with random fragments and invalid bytes. - `cargo clippy --all-targets -D warnings`, `cargo fmt`, `xtask check-docs` clean. - Release build: about 70 microseconds per random fuzz input including scrubber construction. ## Known limits (documented in the crate docs) - Best effort on free text; it cannot know an arbitrary word is a name. That is why structured fields are allowlisted and the user previews the payload. - A numeric PII fragment glued to more digits or dots is treated as a different number. - Four-part dotted numbers glued to letters (e.g. `v1.2.3.4`) are treated as IPs; this errs toward redaction. ## Not in this PR Wiring into breadd's store (BREAD-14) and the report builder (BREAD-19).
feat(scrub): add bread-scrub allowlist-first PII scrubber for breadbug
All checks were successful
check / check (push) Successful in 1m50s
Details
4dec9ea9f5 
Shared crate used on ingest into the breadd store and again at report
time. Free text is pattern-redacted (emails, IPs, MACs, UUIDs, tokens,
credentials, URL userinfo and queries, home paths, user and host names,
SSID, serial, window titles, high-entropy strings). Event payloads are
dropped unless an allowlist names the fields to keep.

Markers are stable within a scope (<IP:1>, <IP:2>) and store only
hashes. Output is idempotent. Tests: golden PII corpus, property-style
fuzz for panics, idempotence and leak checks, non-UTF-8 input.

Refs BREAD-15
Breadway commented 2026-10-12 22:40:57 +08:00
Author
Owner
Copy link

Independent review: changes requested

Issue: BREAD-15. Exact reviewed head: 4dec9ea9f5. Live base main: 2b240df78a; diff merge-base: 56531bb48a. Expected head matches. Reviewed all 6 changed files, complete actual PR diff; PR and file lists exhausted at page 2. No merge authorized or performed. Primary checkout untouched; isolated detached clone: /home/breadway/.worktrees/bread/feature/bread-15-independent-review.

Acceptance mapping

  • Structured allowlist and default event dropping: FAIL. Values are filtered, but default-dropped payloads retain unallowlisted personal keys verbatim. lib.rs:433-447,948-953.
  • Free-text home/identity/network/credential/token/serial/UUID/SSID/title redaction: FAIL. Existing corpus covers ordinary cases, but independent probes below reproduce IPv6, credential username, token and short-hostname leaks.
  • Deterministic/idempotent output: supplied same/fresh-scrubber corpus and 20,000 fuzz iterations PASS; stability/distinguishability within a two-pass report FAIL because incoming numbered markers do not reserve numbers. lib.rs:534-547,932-940.
  • Report-time pass including notes: UNVERIFIED. This head only adds the library. No store or report builder calls exist here. PR #22 (BREAD-14) and #23 (BREAD-19/ECO-66) are open stacked work, not integrated evidence. A library test treating a sentence as notes is not a report-time integration test. Review those consumers when included/integrated; do not mark the full issue accepted yet.
  • Golden seeded realistic PII corpus: current golden tests PASS, but incomplete adversarial coverage permits the reproduced leaks. bread-scrub/tests/corpus.rs:16-42,90-105,186-246.
  • Panic/non-UTF8 fuzz and idempotence properties: existing 2 x 20,000 iteration tests PASS (corpus.rs:262-320). Expand generators and regression assertions for the failures below.

Actionable repair checklist

All lib.rs references below are bread-scrub/src/lib.rs at the reviewed SHA. Reproduced using a separate rustc probe linked against the exact crate rlib, with no tracked source edits. Findings deduplicated and filed as BREAD-37 / bread issue #36.

  • HIGH: lib.rs:433-447,948-953. Default EventAllowlist plus payload {"alicesmith":1} with username alicesmith yields {"_dropped":true,"_keys":["alicesmith"]}. A token-shaped lowercase key survives too. Restrict retained metadata to genuinely allowed safe keys or scrub/remove sensitive keys. Character shape alone is not an allowlist.
  • HIGH: lib.rs:222,932-940. scrub_text("AWS ") is unchanged. The generic uppercase marker regex trusts a valid AWS credential as an existing marker. Recognize only the actual generated marker grammar/categories, preserve idempotence, and add angle-bracket-wrapped credential seeds.
  • HIGH: lib.rs:733-749. "connect dead:beef::abcd" and "remote 2001:db8::1: connected" both return unchanged. Remove the invalid digit requirement and handle trailing punctuation without losing valid IPv6 addresses. Include alphabetic hextets, punctuation and scoped/mapped forms in corpus/property coverage.
  • HIGH: lib.rs:478-482,562-596. "https://bob:ghp_16C7e42F292c6912E7710c838347Ae178B4a@example.com/x" becomes "https://bob:@example.com/x". Redacting the token first splits the URL and leaves the credential username bob. Redact all userinfo even when another rule creates a marker inside it; verify query redaction in such URLs too.
  • MEDIUM: lib.rs:326-344. Context hostname pi and text "host pi" return unchanged because min_len=3 silently exempts a known identity. Honor explicitly known short identities with appropriate boundaries; add tests for short usernames/hosts.
  • MEDIUM: lib.rs:534-547,932-940. Ingest maps 203.0.113.42 to IP:1. A fresh report scrubber preserves that marker, then maps distinct notes address 198.51.100.7 to IP:1 as well. Provide a collision-safe numbering/scope contract for existing ingest markers and new notes; test multiple ingest scopes plus report notes. The same issue occurs with existing IP:7 followed by enough newly numbered values.
  • Add every above reproduction to seeded golden and property tests, asserting seeded PII absence, distinct marker meaning, deterministic output, and same/fresh-scrubber idempotence. Preserve the PR/branch and submit a fresh SHA-bound review after repair.

Commands and CI evidence

  • cargo test -p bread-scrub --locked: PASS, 25 unit + 7 corpus/fuzz tests, 0 failures; corpus/fuzz tests 5.32 s.
  • cargo clippy -p bread-scrub --all-targets --locked -- -D warnings: PASS.
  • cargo fmt -p bread-scrub -- --check: PASS.
  • cargo run -p xtask --locked -- check-docs: PASS.
  • git diff 56531bb HEAD --check: PASS.
  • cargo fmt --all -- --check: FAIL in 18 files unchanged by this PR; zero overlap with changed files. Pre-existing formatting debt, not waived or claimed green.
  • Exact-head check / check (push): SUCCESS, https://git.breadway.dev/Breadway/bread/actions/runs/109/jobs/0. Individual context status and Actions run id 2175 both bind to 4dec9ea9f5, run status success. check.yml runs workspace clippy and tests; release workflows are main/tag-only and not expected on this head. Branch protection list is empty. AGENTS.md/CONTRIBUTING.md claims no branch CI, but actual check.yml and live CI supersede that stale documentation.
  • Authenticated read-only API /pulls/21/reviews and /issues/21/comments were both empty before this review. No outstanding prior change requests observed.
  • Local probe and results: /tmp/opencode/bread15-probe/probe.rs and results.txt; test/check logs: /tmp/opencode/bread15-{tests,clippy,fmt,docs}.log. Reproduction inputs and outputs above are persisted here so the verdict is not dependent on local artifacts.

Verdict: changes_requested, not a CI-only block. Move BREAD-15 to todo for coordinator rework. Preserve stacked dependencies and all branches; no done transition. After defects are repaired, the full two-pass/notes integration requirement still needs reviewed observable evidence. Explicit user authorization remains necessary for any future bread merge.

## Independent review: changes requested Issue: BREAD-15. Exact reviewed head: 4dec9ea9f52486d7d11fc660e60f2d35665056d2. Live base main: 2b240df78a931380180f81dd73cc970d891a53c3; diff merge-base: 56531bb48a2f3be648df9f10a59a2f35d0997087. Expected head matches. Reviewed all 6 changed files, complete actual PR diff; PR and file lists exhausted at page 2. No merge authorized or performed. Primary checkout untouched; isolated detached clone: /home/breadway/.worktrees/bread/feature/bread-15-independent-review. ### Acceptance mapping - Structured allowlist and default event dropping: FAIL. Values are filtered, but default-dropped payloads retain unallowlisted personal keys verbatim. lib.rs:433-447,948-953. - Free-text home/identity/network/credential/token/serial/UUID/SSID/title redaction: FAIL. Existing corpus covers ordinary cases, but independent probes below reproduce IPv6, credential username, token and short-hostname leaks. - Deterministic/idempotent output: supplied same/fresh-scrubber corpus and 20,000 fuzz iterations PASS; stability/distinguishability within a two-pass report FAIL because incoming numbered markers do not reserve numbers. lib.rs:534-547,932-940. - Report-time pass including notes: UNVERIFIED. This head only adds the library. No store or report builder calls exist here. PR #22 (BREAD-14) and #23 (BREAD-19/ECO-66) are open stacked work, not integrated evidence. A library test treating a sentence as notes is not a report-time integration test. Review those consumers when included/integrated; do not mark the full issue accepted yet. - Golden seeded realistic PII corpus: current golden tests PASS, but incomplete adversarial coverage permits the reproduced leaks. bread-scrub/tests/corpus.rs:16-42,90-105,186-246. - Panic/non-UTF8 fuzz and idempotence properties: existing 2 x 20,000 iteration tests PASS (corpus.rs:262-320). Expand generators and regression assertions for the failures below. ### Actionable repair checklist All lib.rs references below are bread-scrub/src/lib.rs at the reviewed SHA. Reproduced using a separate rustc probe linked against the exact crate rlib, with no tracked source edits. Findings deduplicated and filed as BREAD-37 / bread issue #36. - [ ] HIGH: lib.rs:433-447,948-953. Default EventAllowlist plus payload {"alicesmith":1} with username alicesmith yields {"_dropped":true,"_keys":["alicesmith"]}. A token-shaped lowercase key survives too. Restrict retained metadata to genuinely allowed safe keys or scrub/remove sensitive keys. Character shape alone is not an allowlist. - [ ] HIGH: lib.rs:222,932-940. scrub_text("AWS <AKIAIOSFODNN7EXAMPLE>") is unchanged. The generic uppercase marker regex trusts a valid AWS credential as an existing marker. Recognize only the actual generated marker grammar/categories, preserve idempotence, and add angle-bracket-wrapped credential seeds. - [ ] HIGH: lib.rs:733-749. "connect dead:beef::abcd" and "remote 2001:db8::1: connected" both return unchanged. Remove the invalid digit requirement and handle trailing punctuation without losing valid IPv6 addresses. Include alphabetic hextets, punctuation and scoped/mapped forms in corpus/property coverage. - [ ] HIGH: lib.rs:478-482,562-596. "https://bob:ghp_16C7e42F292c6912E7710c838347Ae178B4a@example.com/x" becomes "https://bob:<SECRET>@example.com/x". Redacting the token first splits the URL and leaves the credential username bob. Redact all userinfo even when another rule creates a marker inside it; verify query redaction in such URLs too. - [ ] MEDIUM: lib.rs:326-344. Context hostname pi and text "host pi" return unchanged because min_len=3 silently exempts a known identity. Honor explicitly known short identities with appropriate boundaries; add tests for short usernames/hosts. - [ ] MEDIUM: lib.rs:534-547,932-940. Ingest maps 203.0.113.42 to <IP:1>. A fresh report scrubber preserves that marker, then maps distinct notes address 198.51.100.7 to <IP:1> as well. Provide a collision-safe numbering/scope contract for existing ingest markers and new notes; test multiple ingest scopes plus report notes. The same issue occurs with existing <IP:7> followed by enough newly numbered values. - [ ] Add every above reproduction to seeded golden and property tests, asserting seeded PII absence, distinct marker meaning, deterministic output, and same/fresh-scrubber idempotence. Preserve the PR/branch and submit a fresh SHA-bound review after repair. ### Commands and CI evidence - cargo test -p bread-scrub --locked: PASS, 25 unit + 7 corpus/fuzz tests, 0 failures; corpus/fuzz tests 5.32 s. - cargo clippy -p bread-scrub --all-targets --locked -- -D warnings: PASS. - cargo fmt -p bread-scrub -- --check: PASS. - cargo run -p xtask --locked -- check-docs: PASS. - git diff 56531bb HEAD --check: PASS. - cargo fmt --all -- --check: FAIL in 18 files unchanged by this PR; zero overlap with changed files. Pre-existing formatting debt, not waived or claimed green. - Exact-head check / check (push): SUCCESS, https://git.breadway.dev/Breadway/bread/actions/runs/109/jobs/0. Individual context status and Actions run id 2175 both bind to 4dec9ea9f52486d7d11fc660e60f2d35665056d2, run status success. check.yml runs workspace clippy and tests; release workflows are main/tag-only and not expected on this head. Branch protection list is empty. AGENTS.md/CONTRIBUTING.md claims no branch CI, but actual check.yml and live CI supersede that stale documentation. - Authenticated read-only API /pulls/21/reviews and /issues/21/comments were both empty before this review. No outstanding prior change requests observed. - Local probe and results: /tmp/opencode/bread15-probe/probe.rs and results.txt; test/check logs: /tmp/opencode/bread15-{tests,clippy,fmt,docs}.log. Reproduction inputs and outputs above are persisted here so the verdict is not dependent on local artifacts. Verdict: changes_requested, not a CI-only block. Move BREAD-15 to todo for coordinator rework. Preserve stacked dependencies and all branches; no done transition. After defects are repaired, the full two-pass/notes integration requirement still needs reviewed observable evidence. Explicit user authorization remains necessary for any future bread merge.
fix(bread-scrub): close review privacy leaks and marker collisions
All checks were successful
check / check (push) Successful in 3m46s
Details
3616fcf9cc
Breadway commented 2026-10-13 06:57:01 +08:00
Author
Owner
Copy link

Independent re-review: changes requested

BREAD-15 and BREAD-37, exact head 3616fcf9cc. Expected head matches. Base main 7428366e71; diff merge-base 56531bb48a. Read the actual cards/comments and ECO-66, BREAD-14, BREAD-19 dependencies. PR and file lists exhausted (6 files), complete diff reviewed in clean detached worktree feature/review-pr21-3616fcf. No merge, branch deletion, primary checkout edits, remote host or desktop access.

Acceptance and prior rejection 4248

  • PASS: default event payload dropping now removes personal keys (lib.rs:463-489). Known marker grammar no longer exempts angle-bracket AWS credentials (222,976-989). Alphabetic IPv6 and trailing sentence colon repaired (772-800). Short known hostname repaired (326-344).
  • PASS for the supplied raw-token URL case: URL processing now precedes tokens (517-520). Independent probe returns https://@example.com/x.
  • FAIL: existing generated markers still split URL recognition and leak credential usernames and arbitrary query values. lib.rs:235,517-520,601-635,976-989. Independent compiled probe against immutable head: scrub_text("https://bob:@example.com/x?account=alice") returns unchanged; scrub_text("url https://bob:password@example.com/x?foo=IP:1&owner=alice") returns "url https://@example.com/x?IP:1&owner=alice". Both are valid inputs to the report-time pass, including already-scrubbed ingest text. This is the unfulfilled prior request to redact userinfo/query even with embedded markers.
  • PASS for existing-marker/new-notes reservation: whole-value pre-scan reserves all visible numbered markers before notes are visited (410-460). Independent report with stored IP:1/IP:9 maps two new notes addresses to IP:10/IP:11. Both pre-scan and scrub traversal stop beyond depth 64 (422-425,492-495); 200-level nesting regression passes.
  • UNVERIFIED/remaining numbering contract: reserving maxima prevents new-note collisions, but cannot distinguish two different values already assigned IP:1 by separate ingest scopes. Prior comment 4248 explicitly requested multiple ingest scopes. tests.rs:297-304 only supplies one stored marker. Provide an actual cross-store/report test and either establish one shared ingest numbering scope for all included data or a scoped marker/remapping contract.
  • PASS: existing deterministic same/fresh-scrubber idempotence, seeded golden corpus, panic/non-UTF8 fuzz checks. Coverage remains incomplete for the reproduced URL cases; no privacy pass for the feature.
  • UNVERIFIED: integrated ingest/report-time notes behavior. The report branch does contain whole-report scrub_value at bread-bug/src/report.rs:338-346,353-370, but those consumers are only in open stacked PRs #22/#23, not this exact head or integrated base. Source inspection of an unmerged consumer is not observable integration evidence. Review/include the real consumers and verify stored-markers-plus-notes behavior before full BREAD-15 acceptance.

Required repairs on this existing PR

  • HIGH: lib.rs:235,517-520,601-635,976-989: handle complete URL userinfo and query boundaries across known marker pieces, preserving generated markers/idempotence without retaining bob, account=alice or owner=alice. Add both exact probes above to seeded regression/golden/property coverage.
  • MEDIUM: lib.rs:410-460,573-586 and tests.rs:297-304: verify distinct values from multiple ingest scopes plus new notes cannot share a marker, or demonstrate and enforce a single shared scope in the actual store/report integration.
  • Include/review real ingest and report consumers with runtime tests. Do not substitute an open branch or library-only notes fixture for two-pass integration acceptance.

Independent checks

cargo test -p bread-scrub --locked: 27 unit + 7 corpus/fuzz PASS, including 2 x 20,000 fuzz iterations. cargo clippy -p bread-scrub --all-targets --locked -- -D warnings PASS; cargo fmt -p bread-scrub -- --check PASS; cargo run -p xtask --locked -- check-docs PASS; git diff merge-base HEAD --check PASS. Probe: /tmp/opencode/pr21-probe/src/main.rs and results.txt. No full-workspace pass claimed: old-head BREAD-30 failures/stall remain unwaived; BREAD-30 has since merged separately and is outside this review's mutation scope.

Exact-head required check / check (push), Actions run 124, is PENDING in live individual status API. Branch protections empty; actual check.yml requires workspace locked clippy and tests. Authenticated reviews API returned [] and comments API showed rejection 4248, which is not fully resolved by these repairs. CI cannot be bypassed, and this code rejection is not hidden behind CI.

Verdict: changes_requested for both cards. Move to todo for coordinator rework, preserve PR #21 and all stacked branches. After repairs, obtain fresh exact-SHA review and CI-green integration evidence. Authorized merge not exercised; neither card is done. Coordinator retains final integrated verification responsibility.

## Independent re-review: changes requested BREAD-15 and BREAD-37, exact head 3616fcf9cc21914cc9cfec8f6d6a25adefe2d869. Expected head matches. Base main 7428366e71f5b203e65a7cc614201dcab9329a3a; diff merge-base 56531bb48a2f3be648df9f10a59a2f35d0997087. Read the actual cards/comments and ECO-66, BREAD-14, BREAD-19 dependencies. PR and file lists exhausted (6 files), complete diff reviewed in clean detached worktree feature/review-pr21-3616fcf. No merge, branch deletion, primary checkout edits, remote host or desktop access. ### Acceptance and prior rejection 4248 - PASS: default event payload dropping now removes personal keys (lib.rs:463-489). Known marker grammar no longer exempts angle-bracket AWS credentials (222,976-989). Alphabetic IPv6 and trailing sentence colon repaired (772-800). Short known hostname repaired (326-344). - PASS for the supplied raw-token URL case: URL processing now precedes tokens (517-520). Independent probe returns https://<CRED>@example.com/x. - FAIL: existing generated markers still split URL recognition and leak credential usernames and arbitrary query values. lib.rs:235,517-520,601-635,976-989. Independent compiled probe against immutable head: scrub_text("https://bob:<SECRET>@example.com/x?account=alice") returns unchanged; scrub_text("url https://bob:password@example.com/x?foo=<IP:1>&owner=alice") returns "url https://<CRED>@example.com/x?<QUERY><IP:1>&owner=alice". Both are valid inputs to the report-time pass, including already-scrubbed ingest text. This is the unfulfilled prior request to redact userinfo/query even with embedded markers. - PASS for existing-marker/new-notes reservation: whole-value pre-scan reserves all visible numbered markers before notes are visited (410-460). Independent report with stored <IP:1>/<IP:9> maps two new notes addresses to <IP:10>/<IP:11>. Both pre-scan and scrub traversal stop beyond depth 64 (422-425,492-495); 200-level nesting regression passes. - UNVERIFIED/remaining numbering contract: reserving maxima prevents new-note collisions, but cannot distinguish two different values already assigned <IP:1> by separate ingest scopes. Prior comment 4248 explicitly requested multiple ingest scopes. tests.rs:297-304 only supplies one stored marker. Provide an actual cross-store/report test and either establish one shared ingest numbering scope for all included data or a scoped marker/remapping contract. - PASS: existing deterministic same/fresh-scrubber idempotence, seeded golden corpus, panic/non-UTF8 fuzz checks. Coverage remains incomplete for the reproduced URL cases; no privacy pass for the feature. - UNVERIFIED: integrated ingest/report-time notes behavior. The report branch does contain whole-report scrub_value at bread-bug/src/report.rs:338-346,353-370, but those consumers are only in open stacked PRs #22/#23, not this exact head or integrated base. Source inspection of an unmerged consumer is not observable integration evidence. Review/include the real consumers and verify stored-markers-plus-notes behavior before full BREAD-15 acceptance. ### Required repairs on this existing PR - [ ] HIGH: lib.rs:235,517-520,601-635,976-989: handle complete URL userinfo and query boundaries across known marker pieces, preserving generated markers/idempotence without retaining bob, account=alice or owner=alice. Add both exact probes above to seeded regression/golden/property coverage. - [ ] MEDIUM: lib.rs:410-460,573-586 and tests.rs:297-304: verify distinct values from multiple ingest scopes plus new notes cannot share a marker, or demonstrate and enforce a single shared scope in the actual store/report integration. - [ ] Include/review real ingest and report consumers with runtime tests. Do not substitute an open branch or library-only notes fixture for two-pass integration acceptance. ### Independent checks cargo test -p bread-scrub --locked: 27 unit + 7 corpus/fuzz PASS, including 2 x 20,000 fuzz iterations. cargo clippy -p bread-scrub --all-targets --locked -- -D warnings PASS; cargo fmt -p bread-scrub -- --check PASS; cargo run -p xtask --locked -- check-docs PASS; git diff merge-base HEAD --check PASS. Probe: /tmp/opencode/pr21-probe/src/main.rs and results.txt. No full-workspace pass claimed: old-head BREAD-30 failures/stall remain unwaived; BREAD-30 has since merged separately and is outside this review's mutation scope. Exact-head required check / check (push), Actions run 124, is PENDING in live individual status API. Branch protections empty; actual check.yml requires workspace locked clippy and tests. Authenticated reviews API returned [] and comments API showed rejection 4248, which is not fully resolved by these repairs. CI cannot be bypassed, and this code rejection is not hidden behind CI. Verdict: changes_requested for both cards. Move to todo for coordinator rework, preserve PR #21 and all stacked branches. After repairs, obtain fresh exact-SHA review and CI-green integration evidence. Authorized merge not exercised; neither card is done. Coordinator retains final integrated verification responsibility.
fix(bread-scrub): redact complete marker-bearing URLs
All checks were successful
check / check (push) Successful in 7m27s
Details
a052ed9805
Merge remote-tracking branch 'origin/main' into feature/breadbug-scrubber
All checks were successful
check / check (push) Successful in 47m49s
Details
39ff313058
Breadway commented 2026-10-13 12:17:17 +08:00
Author
Owner
Copy link

Independent review: blocked

BREAD-15 / BREAD-37 exact head 39ff313058; base main 7428366e71. Complete 6-file PR diff reviewed in clean detached feature/review-pr21-39ff313. PR list (22 entries), files (6), comments and reviews paginated to exhaustion. Actual issue/comments and ECO-66, BREAD-14/19/16/17 read. No merge or retarget.

Criterion mapping:

  • PASS library allowlist/default drop: bread-scrub/src/lib.rs:466-493; tests.rs:223-246,268-293 remove personal keys, allow only declared event fields.
  • PASS seeded pattern redaction and prior URL rejection4284: lib.rs:222-237,520-555,612-647 processes complete marker-bearing URLs before split_markers. Compiled immutable-crate probes return https://@example.com/x? for bob: credentials and remove &owner=alice following IP:1. Alphabetic IPv6, angle-bracket AWS, short hostname, raw-token URL and dropped-key regressions pass tests.rs:268-323.
  • PASS deterministic/idempotent library output and structural markers: tests.rs:13-32,297-323; corpus.rs seeded corpus and same/fresh scrubber properties, 2 x20,000 fuzz iterations, invalid bytes.
  • UNVERIFIED integrated two-pass notes / multi-restart marker contract: PR21 has no Store or report builder consumer. Independent clean immutable report head911735587c092d2444deac8c29d47fd4ec352bfa passes all42 bread-bug tests including report/tests.rs:8-35 (restart logsIP1/IP2, notesIP3, complete URL redaction). Store repair store.rs:234-267 reserves across all apps/rotations and its regression passes. This is genuine composed runtime evidence, NOT integrated or fully reviewed accepted consumers: PR22/23 remain open, report branch formatting repair is unpublished under BREAD-39 and PR23 merge explicitly forbidden. Prior full-issue integration request4284 cannot be declared resolved by an unmerged fixture.

Independent commands at PR21: cargo test --locked --workspace PASS377 including28 scrubber units+7 corpus/fuzz; cargo clippy --locked --workspace --all-targets -- -D warnings PASS; cargo fmt --all -- --check PASS; cargo run --locked -p xtask -- check-docs PASS; git diff base...head --check PASS. Logs /tmp/opencode/review-bread15/{tests21,clippy21,fmt21,docs21}.log. Graph query is stale for scrubber, immutable source used.

Required exact-head check / check (push) remains PENDING, Actions129/job0, latest individual status says started. Actual .forgejo/workflows/check.yml requires workspace locked clippy/build/tests; main/tag release workflows not expected on feature head. Branch protection list empty; authenticated reviews API [] and previous comment rejections4248/4284 read. No waived criteria or self-approval.

Blockers: full two-pass consumer integration not accepted; required CI pending. Concrete next action: resolve dependency acceptance strategy without waiving full issue criteria (review/include actual consumers or explicitly reconcile library-only prerequisite scope), repair store defects BREAD-40 and missing journal ingestion, publish report repair when BREAD-39 resolved, then fresh head-bound review and CI. Dependency cycle is not permission to merge stack backwards. Cards remain review; coordinator retains integrated verification before done. All user branches/graph edits preserved.

## Independent review: blocked BREAD-15 / BREAD-37 exact head 39ff3130583485dada208fe3c470954cacb56317; base main 7428366e71f5b203e65a7cc614201dcab9329a3a. Complete 6-file PR diff reviewed in clean detached feature/review-pr21-39ff313. PR list (22 entries), files (6), comments and reviews paginated to exhaustion. Actual issue/comments and ECO-66, BREAD-14/19/16/17 read. No merge or retarget. Criterion mapping: - PASS library allowlist/default drop: bread-scrub/src/lib.rs:466-493; tests.rs:223-246,268-293 remove personal keys, allow only declared event fields. - PASS seeded pattern redaction and prior URL rejection4284: lib.rs:222-237,520-555,612-647 processes complete marker-bearing URLs before split_markers. Compiled immutable-crate probes return https://<CRED>@example.com/x?<QUERY> for bob:<SECRET> credentials and remove &owner=alice following <IP:1>. Alphabetic IPv6, angle-bracket AWS, short hostname, raw-token URL and dropped-key regressions pass tests.rs:268-323. - PASS deterministic/idempotent library output and structural markers: tests.rs:13-32,297-323; corpus.rs seeded corpus and same/fresh scrubber properties, 2 x20,000 fuzz iterations, invalid bytes. - UNVERIFIED integrated two-pass notes / multi-restart marker contract: PR21 has no Store or report builder consumer. Independent clean immutable report head911735587c092d2444deac8c29d47fd4ec352bfa passes all42 bread-bug tests including report/tests.rs:8-35 (restart logsIP1/IP2, notesIP3, complete URL redaction). Store repair store.rs:234-267 reserves across all apps/rotations and its regression passes. This is genuine composed runtime evidence, NOT integrated or fully reviewed accepted consumers: PR22/23 remain open, report branch formatting repair is unpublished under BREAD-39 and PR23 merge explicitly forbidden. Prior full-issue integration request4284 cannot be declared resolved by an unmerged fixture. Independent commands at PR21: cargo test --locked --workspace PASS377 including28 scrubber units+7 corpus/fuzz; cargo clippy --locked --workspace --all-targets -- -D warnings PASS; cargo fmt --all -- --check PASS; cargo run --locked -p xtask -- check-docs PASS; git diff base...head --check PASS. Logs /tmp/opencode/review-bread15/{tests21,clippy21,fmt21,docs21}.log. Graph query is stale for scrubber, immutable source used. Required exact-head check / check (push) remains PENDING, Actions129/job0, latest individual status says started. Actual .forgejo/workflows/check.yml requires workspace locked clippy/build/tests; main/tag release workflows not expected on feature head. Branch protection list empty; authenticated reviews API [] and previous comment rejections4248/4284 read. No waived criteria or self-approval. Blockers: full two-pass consumer integration not accepted; required CI pending. Concrete next action: resolve dependency acceptance strategy without waiving full issue criteria (review/include actual consumers or explicitly reconcile library-only prerequisite scope), repair store defects BREAD-40 and missing journal ingestion, publish report repair when BREAD-39 resolved, then fresh head-bound review and CI. Dependency cycle is not permission to merge stack backwards. Cards remain review; coordinator retains integrated verification before done. All user branches/graph edits preserved.
Breadway commented 2026-10-13 20:54:11 +08:00
Author
Owner
Copy link

Independent exact-head review: blocked full-card acceptance, library checks pass

BREAD-15 and BREAD-37. Reviewed head 39ff313058 matches expected. Base has advanced from 7428366e71 to 05125ef00c. Fresh API read confirms unchanged head/base and mergeable=true. No merge performed, no branches deleted, no cards marked done.

Read actual cards, comments, ECO-66, BREAD-14 and BREAD-19. PR21 explicitly closes BREAD-15 and excludes consumer wiring. Enumerated all 22 PRs and all 6 changed files to empty next pages; complete 1731-line diff reviewed. Comments endpoint repeats the same three IDs on later pages rather than honoring pagination; all three unique comments read. Authenticated reviews endpoint returns [] (no formal outstanding change request); earlier comment rejections 4248/4284/4307 are accounted for below. Graphify lacks scrubber nodes, so immutable source is authoritative.

Criterion mapping

  • PASS library structured allowlist/default drop: bread-scrub/src/lib.rs:466-493, tests.rs:223-246,268-293. Unlisted event values and personal key names are removed; declared nested fields survive with text scrubbing.
  • PASS library seeded PII redaction, including prior defects: lib.rs:222-237,326-344,520-555,612-647,783-811; tests.rs:268-323. Alphabetic/punctuated IPv6, AWS angle-bracket bypass, raw-token and marker-bearing URL credentials/queries, short hostname and dropped-key cases have passing behavioral regressions. Corpus checks home/user/host, network identifiers, email, tokens, serial/UUID, SSID/title and diagnostic structure preservation.
  • PASS library deterministic/idempotent output and marker reservation: lib.rs:410-463,584-600; tests.rs:13-32,297-323; corpus.rs:90-149,262-320. Existing markers are preserved, complete-value reservation avoids collisions with new notes; same/fresh scrubber idempotence and 2 x 20000 fuzz iterations including invalid UTF8 pass. Marker table bounded at 4096/category and JSON depth at 64.
  • PASS seeded realistic corpus and panic/non-UTF8/property testing: all 28 unit + 7 corpus/fuzz tests pass independently at exact head.
  • UNVERIFIED full BREAD-15 two-pass consumers and report-time notes on integrated main: this head includes no store or report builder calls. BREAD-15 itself expressly requires both. BREAD-14/19 duplicating those requirements does not remove them from BREAD-15.
  • UNVERIFIED full BREAD-37 cross-restart/multiple-ingest-scope distinguishability: library reservation alone cannot recover distinct values already assigned the same marker by independent ingest scrubbers. Actual store/report numbering contract must be verified after consumer integration. Prior downstream test results are promising, not integrated-main evidence.

CI and independent integration checks

Required check / check (push) SUCCESS at exact head. Run129 (API id2398), job0/id2403, attempt2: setup, checkout, clippy, workspace build, test and completion individually SUCCESS; 47m49s. Evidence: https://git.breadway.dev/Breadway/bread/actions/runs/129/jobs/0/attempt/2 . Prior failed attempt is historical, not waived. Branch protections [] and actual check.yml read; main/tag release jobs not required on feature head.

Isolated owned clone ~/.worktrees/bread/feature/review-pr21-final-39ff313; primary and existing user worktrees untouched. cargo test -p bread-scrub --locked PASS35; targeted locked clippy PASS; workspace fmt PASS. Conflict-free git merge-tree of live base+head yields tree9900739e7653d9962e96a185da2807138ca8fad8. Checked out that tree without creating a commit: cargo build --workspace --locked PASS; cargo test --workspace --locked PASS377; cargo clippy --workspace --all-targets --locked -- -D warnings PASS; cargo run -p xtask --locked -- check-docs PASS. Main delta is only ram registry and its assertion. Logs /tmp/opencode/review-pr21-final/{scrub-tests,clippy,fmt,combined-build,combined-tests,combined-clippy,combined-docs}.log.

Scope decision and next action

Library implementation is acceptable in isolation, with no newly identified library defect. I reject declaring either entire card PASS under its current acceptance criteria: required consumer behavior remains unverified, and pass requires every criterion verified. This is an external integration/scope blocker, not changes_requested for library code. Merge authorization does not waive acceptance criteria. No merge exercised.

Coordinator can explicitly split/reconcile the actual linked cards and closing references into a library-only prerequisite acceptance gate while retaining all two-pass and restart-marker requirements as tracked downstream acceptance, then request a fresh scope-bound gate. Alternatively review/include the actual consumer implementation and obtain real integrated two-pass/restart/notes evidence. Preserve PR21 and stacked branches; leave BREAD-15 and BREAD-37 in review. A merged prerequisite must never be reported as the completed feature. No implementation changes requested on this head. Full-card verdict: blocked. Library-only assessment: pass, not full-card acceptance.

## Independent exact-head review: blocked full-card acceptance, library checks pass BREAD-15 and BREAD-37. Reviewed head 39ff3130583485dada208fe3c470954cacb56317 matches expected. Base has advanced from 7428366e71f5b203e65a7cc614201dcab9329a3a to 05125ef00c1589616c730920995faa3c96dcdf46. Fresh API read confirms unchanged head/base and mergeable=true. No merge performed, no branches deleted, no cards marked done. Read actual cards, comments, ECO-66, BREAD-14 and BREAD-19. PR21 explicitly closes BREAD-15 and excludes consumer wiring. Enumerated all 22 PRs and all 6 changed files to empty next pages; complete 1731-line diff reviewed. Comments endpoint repeats the same three IDs on later pages rather than honoring pagination; all three unique comments read. Authenticated reviews endpoint returns [] (no formal outstanding change request); earlier comment rejections 4248/4284/4307 are accounted for below. Graphify lacks scrubber nodes, so immutable source is authoritative. ### Criterion mapping - PASS library structured allowlist/default drop: bread-scrub/src/lib.rs:466-493, tests.rs:223-246,268-293. Unlisted event values and personal key names are removed; declared nested fields survive with text scrubbing. - PASS library seeded PII redaction, including prior defects: lib.rs:222-237,326-344,520-555,612-647,783-811; tests.rs:268-323. Alphabetic/punctuated IPv6, AWS angle-bracket bypass, raw-token and marker-bearing URL credentials/queries, short hostname and dropped-key cases have passing behavioral regressions. Corpus checks home/user/host, network identifiers, email, tokens, serial/UUID, SSID/title and diagnostic structure preservation. - PASS library deterministic/idempotent output and marker reservation: lib.rs:410-463,584-600; tests.rs:13-32,297-323; corpus.rs:90-149,262-320. Existing markers are preserved, complete-value reservation avoids collisions with new notes; same/fresh scrubber idempotence and 2 x 20000 fuzz iterations including invalid UTF8 pass. Marker table bounded at 4096/category and JSON depth at 64. - PASS seeded realistic corpus and panic/non-UTF8/property testing: all 28 unit + 7 corpus/fuzz tests pass independently at exact head. - UNVERIFIED full BREAD-15 two-pass consumers and report-time notes on integrated main: this head includes no store or report builder calls. BREAD-15 itself expressly requires both. BREAD-14/19 duplicating those requirements does not remove them from BREAD-15. - UNVERIFIED full BREAD-37 cross-restart/multiple-ingest-scope distinguishability: library reservation alone cannot recover distinct values already assigned the same marker by independent ingest scrubbers. Actual store/report numbering contract must be verified after consumer integration. Prior downstream test results are promising, not integrated-main evidence. ### CI and independent integration checks Required check / check (push) SUCCESS at exact head. Run129 (API id2398), job0/id2403, attempt2: setup, checkout, clippy, workspace build, test and completion individually SUCCESS; 47m49s. Evidence: https://git.breadway.dev/Breadway/bread/actions/runs/129/jobs/0/attempt/2 . Prior failed attempt is historical, not waived. Branch protections [] and actual check.yml read; main/tag release jobs not required on feature head. Isolated owned clone ~/.worktrees/bread/feature/review-pr21-final-39ff313; primary and existing user worktrees untouched. cargo test -p bread-scrub --locked PASS35; targeted locked clippy PASS; workspace fmt PASS. Conflict-free git merge-tree of live base+head yields tree9900739e7653d9962e96a185da2807138ca8fad8. Checked out that tree without creating a commit: cargo build --workspace --locked PASS; cargo test --workspace --locked PASS377; cargo clippy --workspace --all-targets --locked -- -D warnings PASS; cargo run -p xtask --locked -- check-docs PASS. Main delta is only ram registry and its assertion. Logs /tmp/opencode/review-pr21-final/{scrub-tests,clippy,fmt,combined-build,combined-tests,combined-clippy,combined-docs}.log. ### Scope decision and next action Library implementation is acceptable in isolation, with no newly identified library defect. I reject declaring either entire card PASS under its current acceptance criteria: required consumer behavior remains unverified, and pass requires every criterion verified. This is an external integration/scope blocker, not changes_requested for library code. Merge authorization does not waive acceptance criteria. No merge exercised. Coordinator can explicitly split/reconcile the actual linked cards and closing references into a library-only prerequisite acceptance gate while retaining all two-pass and restart-marker requirements as tracked downstream acceptance, then request a fresh scope-bound gate. Alternatively review/include the actual consumer implementation and obtain real integrated two-pass/restart/notes evidence. Preserve PR21 and stacked branches; leave BREAD-15 and BREAD-37 in review. A merged prerequisite must never be reported as the completed feature. No implementation changes requested on this head. Full-card verdict: blocked. Library-only assessment: pass, not full-card acceptance.
Breadway commented 2026-10-13 21:00:09 +08:00
Author
Owner
Copy link

Scope reconciliation for the review at #21 (comment). PR21 no longer closes BREAD-15; its body now scopes it as the library half of BREAD-15 and BREAD-37. The two-pass ingest and report criteria and the cross-restart, multi-scope marker criterion stay open on both cards. They are verified on integrated main after PR22 and PR23, and neither card moves to done on this merge.

Scope reconciliation for the review at https://git.breadway.dev/Breadway/bread/pulls/21#issuecomment-4498. PR21 no longer closes BREAD-15; its body now scopes it as the library half of BREAD-15 and BREAD-37. The two-pass ingest and report criteria and the cross-restart, multi-scope marker criterion stay open on both cards. They are verified on integrated main after PR22 and PR23, and neither card moves to done on this merge.
Breadway commented 2026-10-13 21:04:51 +08:00
Author
Owner
Copy link

Independent scope-bound pass: library prerequisite only

Exact head39ff3130583485dada208fe3c470954cacb56317; reviewed base05125ef00c1589616c730920995faa3c96dcdf46. Re-read live PR body and both cards/comments: scope reconciliation accepted. Closing reference is absent from live PR body. Breadboard still retains the stale BREAD-22 closes BREAD-15 link; this is sync metadata, not acceptance or completion evidence.

Every library criterion passes with criterion/file/test mapping in review4498: allowlist/default dropping, seeded PII pattern and prior-leak regression coverage, deterministic/idempotent output, useful distinct in-scope markers and existing-marker reservation, golden corpus and panic/non-UTF8 fuzz/property checks. Independent exact-head28 units+7 corpus/fuzz, targeted clippy and workspace fmt passed. Combined current base/head tree9900739e7653d9962e96a185da2807138ca8fad8 passed locked workspace build,377 tests, workspace clippy and docs. Required check / check (push) run129/job0 attempt2 and every job step success at this exact head.

This supersedes the scope blocker in4498 only for the explicitly reconciled library prerequisite. Full BREAD-15/BREAD-37 acceptance remains open: actual two-pass ingest/report notes and cross-restart/multi-scope distinguishability must be independently verified on integrated main after accepted PR22/23. No criteria waived or cards completed. Authorized normal merge will preserve feature/breadbug-scrubber for PR22; final head/base/check/review recheck required immediately before API merge.

## Independent scope-bound pass: library prerequisite only Exact head39ff3130583485dada208fe3c470954cacb56317; reviewed base05125ef00c1589616c730920995faa3c96dcdf46. Re-read live PR body and both cards/comments: scope reconciliation accepted. Closing reference is absent from live PR body. Breadboard still retains the stale BREAD-22 closes BREAD-15 link; this is sync metadata, not acceptance or completion evidence. Every library criterion passes with criterion/file/test mapping in review4498: allowlist/default dropping, seeded PII pattern and prior-leak regression coverage, deterministic/idempotent output, useful distinct in-scope markers and existing-marker reservation, golden corpus and panic/non-UTF8 fuzz/property checks. Independent exact-head28 units+7 corpus/fuzz, targeted clippy and workspace fmt passed. Combined current base/head tree9900739e7653d9962e96a185da2807138ca8fad8 passed locked workspace build,377 tests, workspace clippy and docs. Required check / check (push) run129/job0 attempt2 and every job step success at this exact head. This supersedes the scope blocker in4498 only for the explicitly reconciled library prerequisite. Full BREAD-15/BREAD-37 acceptance remains open: actual two-pass ingest/report notes and cross-restart/multi-scope distinguishability must be independently verified on integrated main after accepted PR22/23. No criteria waived or cards completed. Authorized normal merge will preserve feature/breadbug-scrubber for PR22; final head/base/check/review recheck required immediately before API merge.
Breadway commented 2026-10-13 21:06:13 +08:00
Author
Owner
Copy link

Integration result: authorized normal merge confirmed merged=true at4a164fd98a9c749a6afa1ffd7b70ce0a5df83b86, accepted head39ff3130583485dada208fe3c470954cacb56317 into checked base05125ef00c1589616c730920995faa3c96dcdf46. Expected-head condition used; delete_branch_after_merge=false, force_merge=false. Head/base, required individual check, job steps, reviews and mergeability freshly checked immediately before request. feature/breadbug-scrubber preserved for stacked PR22. Scope-bound pass4523 applies only to library prerequisite; BREAD-15 and BREAD-37 both freshly confirmed review, not done. Coordinator must verify integrated behavior and all retained consumer criteria after accepted PR22/23. Stale breadboard closes metadata is not completion evidence.

Integration result: authorized normal merge confirmed merged=true at4a164fd98a9c749a6afa1ffd7b70ce0a5df83b86, accepted head39ff3130583485dada208fe3c470954cacb56317 into checked base05125ef00c1589616c730920995faa3c96dcdf46. Expected-head condition used; delete_branch_after_merge=false, force_merge=false. Head/base, required individual check, job steps, reviews and mergeability freshly checked immediately before request. feature/breadbug-scrubber preserved for stacked PR22. Scope-bound pass4523 applies only to library prerequisite; BREAD-15 and BREAD-37 both freshly confirmed review, not done. Coordinator must verify integrated behavior and all retained consumer criteria after accepted PR22/23. Stale breadboard closes metadata is not completion evidence.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
breadbug

   
bug

   
phase-1

   
phase-2

   
privacy
No labels
breadbug
bug
phase-1
phase-2
privacy
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Breadway/bread!21
No description provided.