Skip to content

fix(security): preserve valid pako diagram links - #10192

Open
rubencu wants to merge 1 commit into
kirodotdev:mainfrom
rubencu:fix/preserve-pako-mermaid-links
Open

rubencu wants to merge 1 commit into
kirodotdev:mainfrom
rubencu:fix/preserve-pako-mermaid-links

Conversation

@rubencu

@rubencu rubencu commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

Problem / Motivation

Goal: Preserve valid Mermaid Live pako diagram links without allowing compressed credentials or malformed payloads to bypass output redaction.

Mermaid Live #pako: links can contain long opaque base64url payloads that the markerless bare-secret heuristic rewrites as credentials. Exempting opaque text without decoding it would create a credential-egress path.

Why it matters

Users need diagram links to survive batch, streamed, Markdown, and persisted output. Security must still reject malformed data, decompression bombs, and credentials hidden inside decoded Mermaid state.

Not a goal

  • Restore the previous branch's raw-assistant-text, placeholder, host-policy or channel-specific renderer design.
  • Exempt arbitrary compressed URLs or Mermaid routes.
  • Relax fixed, encoded, or token-parameter credential passes.
  • Add stream holdback for PlantUML links: a streamed one past 512 characters keeps the ordinary holdback.

What changed (motivation → approach → change)

The existing span-based document-link scanner recognizes only exact lowercase https://mermaid.live/edit#pako: routes with canonical base64url payloads. Validation requires one complete bounded zlib stream, UTF-8, duplicate-free finite JSON, and a string code field.

Decoded JSON, every textual key and value, and each string value beside its key, at most 1,024 distinct views per call, pass through the composed URL-and-credential pass redact applies, so a diagram URL is percent-decoded like any other. Each view's check-only variant (escapes decoded, schemes lowercased, a fragment after a host read as a query) must pass too. A clean candidate exempts only the outer encoded span from pass 3's entropy heuristic. Passes 1, 2 and 4 keep main's precedence, warnings, tags and explainable records. Invalid encoding, trailing bytes, oversized inflation, decoded credentials, alternate routes, and ambiguous state receive no exemption.

Streaming holds only an exact partial pako candidate through the encoded-size ceiling, together with a credential-class run glued before it (href=", a JSON "url":") or after it (a sentence's .), each up to the ordinary 512-character holdback, so the joined run is never cut through the URL. The partial-JWT tail search starts where a match can begin, so a held run stays linear. A partial JWT that starts inside the payload rides that hold, ending there or in the held tail, because the payload alphabet holds no .; every other JWT and Bearer anchor keeps its ceiling. A credential match crossing the hold's cut (a Bearer header a scheme's : cut short) declines the hold, and the partial JWT rides only a run that opens at the route: an eyJ reaching a glued run in a later delta ends its hold, so glued text fails closed as on main.

Scope Purpose
docs/system-specs/modules/security.md Specify the exact pako validation and pass-3-only exemption boundary.
src/kiro_crew/dashboard/display_battery.py and src/kiro_crew/security/pass_compose.py Version stored display verdicts by the composed pass the pako check runs, and import its passes by module path for the battery's import guard.
src/kiro_crew/security/redaction_entropy.py Declare the exact pako route and decode its canonical state into textual views.
src/kiro_crew/security/redaction.py Scan decoded views and nominate safe pako spans.
src/kiro_crew/security/text_redaction.py Bound stream holdback for an exact partial pako candidate.
test/test_mermaid_pako_link_redaction.py Cover valid batch/stream links, routes, encoding, bombs, decoded credentials, spans, and records.
test/test_security_gate_liveness.py Re-measure the security package's line budget.
website/src/test/MarkdownRenderer.mermaidPakoLink.test.tsx Prove validated destinations stay distinct renderer links.

Backwards compatibility

Compatible: ordinary text and non-pako links keep main's redaction; a pako link survives only after bounded validation and scanning.

Removes nothing.

Tests

  • Stream regressions cover a . or : delta after a link, a credential after the held tail, href=", {"url":" and password= leads, and a scan count for a held run of eyJ prefixes.
  • test_a_link_and_its_punctuation_in_one_delta_stream_like_batch and test_a_credential_after_punctuation_in_the_same_delta_is_still_redacted fail on a5e92a6: a link and its . or : in one delta stream intact; a credential after them is redacted.
  • test_partial_jwt_inside_a_valid_payload_streams_like_batch fails on 44f6bde: a link past the JWT ceiling with eyJ in its first delta streams intact. test_a_jwt_beside_a_jwt_bearing_link_is_still_redacted_live and test_a_dot_ends_the_pako_hold_and_a_jwt_spanning_it_is_redacted keep a JWT before, after or across a . in the link redacted live.
  • test_a_jwt_shaped_payload_and_its_punctuation_in_one_delta_stream_like_batch fails on 89ad591: a link with two eyJ runs and its . in one delta streams like batch; a JWT after the . is redacted, and a partial one past the held tail fails closed.
  • test_percent_encoded_credential_in_a_decoded_url_voids_the_link fails on 093ad71: a diagram URL carrying a %5F-written GitHub token or a once- or twice-encoded AWS key ID voids the link; test_credential_free_decoded_url_keeps_the_link keeps clean URLs.
  • test_percent_encoded_credential_in_an_upper_scheme_or_fragment_url_voids_the_link fails on 4bf1725: that token in an HTTPS:// or fragment-only URL, or still encoded after three rounds, voids the link; clean spellings keep it.
  • test_a_bearer_header_glued_to_a_link_is_redacted_with_its_value fails on 64d142a: a plain, JSON or lowercase Bearer header whose opaque or JWT value is glued to a link is redacted with its value; test_a_credential_free_sentence_ending_in_a_link_streams_intact keeps clean text.
  • test_a_bearer_header_glued_to_a_jwt_bearing_link_never_streams_its_value fails on 64d142a, and on 9e0f854 in 7-byte deltas, as does test_an_eyj_ending_a_delta_still_ends_the_hold: a Bearer value glued to an eyJ-bearing link never streams. test_a_label_glued_to_a_jwt_bearing_link_fails_closed pins the cost: a glued href=" masks that run as on main, in any delta size.
  • test_near_cap_one_byte_stream_stays_linear_in_cpu bounds a near-cap one-byte stream to 25x the CPU of a tenth-size one.
  • test_a_state_past_the_view_cap_fails_closed fails on 88fd4b0: past 1,024 views it is redacted as on main.
  • test_top_level_pako_links_share_one_view_budget and test_identical_views_are_composed_once fail on 5809b63: links share 1,024 distinct views; duplicates compose once.
  • test_an_escaped_tab_cannot_split_a_bearer_value_from_its_label fails on 493474a: an escaped tab between Bearer and its token voids the link, batch and live.
  • 236 redaction-touching test files: 16,216 passed (one on a rerun).
  • Static gates, docs lint and the line cap passed.

Manual verification

N/A: covered by the tests above.

Screenshots / video

Why no screenshot: No UI pixels change; the frontend delta only adds a test.

Related Issues

No public issue links this security defect.

Pattern harvest

Rule candidate: review-prompt

Pattern: an opaque encoded-output exemption must use a bounded semantic decoder and scan every decoded textual view before only the known false-positive heuristic may skip its outer span.

Checklist

  • At most two commits, with a Conventional Commits title
  • Existing tests pass and security regressions cover the exemption boundary
  • Self-review completed and code follows project style guidelines
  • Security documentation updated
  • No secrets, credentials, or internal references in the diff

Contribution License Agreement

Contributions are accepted under the repository's Apache 2.0 license.

@rubencu
rubencu requested review from a team and smeyffret September 11, 2026 20:46
@rubencu

rubencu commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Intent: Keep exact, valid Mermaid Live pako diagram links intact while fail-closing unsafe encoded state at every output boundary.
Not a goal: Support arbitrary compressed links or add any destination beyond the exact Mermaid Live editor URL.

@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: checking Automated validation is still running labels Sep 11, 2026
@github-actions

github-actions Bot commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

GPT 6.1 Review (fork) — ✅ no blocking findings

Reviewed 5008d006d82dd6b759eec0d236bd0b084139c412 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 5008d00

@github-actions

github-actions Bot commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

First Principles Review (Opus 5.5, fork) — ✅ PASS

Premise-level review of 5008d006d82dd6b759eec0d236bd0b084139c412 via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

First-Principles-Verdict: PASS

Nothing blocks: each item serves the frozen Goal, and pass 3 still judges everything outside the exact route.

Notes

  • Item 1 — symptom-level: this adds another exemption to the pass-3 entropy heuristic. Counting from the spec in docs/system-specs/modules/security.md, at least five already exist: _ROUTED_COMMIT_RE, the _DOCUMENT_LINK_RE routes (Google, Atlassian and PlantUML), _SRI_INTEGRITY_RE and _host_darwin_user_dir_id. The root cause is that pass 3 is markerless, and changing that is a bigger decision than this PR. The description should say this fix works at the symptom level and leaves that cause alone.

What this change ships

Inventory (7 items) — 5 justified

Intent: keep valid Mermaid Live diagram links readable in chat output while compressed credentials stay redacted — FIX.

  1. A valid mermaid.live/edit#pako: link now survives batch and persisted output instead of being replaced by the credential tag — symptom-level
  2. A credential inside the decoded diagram, or an invalid or oversized payload, still voids the link — justified
  3. A streamed pako link is held until it ends (up to 65,536 chars) and is not cut at 512 — justified
  4. The partial-JWT tail search starts where a match can begin, so held runs stay linear — justified
  5. Stored display verdicts are re-versioned because pass_compose.py joins RULE_SOURCES — justified
  6. The PlantUML route regex is pulled into _PLANTUML_ROUTE and _PLANTUML_PAYLOAD_CAP — undeclared, rides along
  7. The security package line budget rises to 33,208 — justified

[FIRST-PRINCIPLES-REVIEWED] 5008d00

Description read

[DESCRIPTION-READ] b1cce5fc77d4148b15147622272aa5293e761901dfa463c0710856cfe76b8186

That sha256 names the PR description this verdict read. Recompute it with .github/scripts/pr-description-capture.sh; if it differs, the description changed after this verdict was formed and any finding drawn from the description is unproven.

@github-actions

github-actions Bot commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

Design Review (Opus 5.5, fork) — ✅ PASS

Design-level review of 5008d006d82dd6b759eec0d236bd0b084139c412 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

Design-Verdict: PASS

No blocker: the exemption reaches only pass 3, and only after a bounded semantic decode. Passes 1, 2 and 4 still see every pako span, as they do on the PlantUML route already in the code.

Notes

  • The stream hold in StreamRedactor.feed (_pako_chunks, _pako_tail_len, _pako_at_route, _late_jwt_anchor, _credential_match_crosses) is a second state machine inside StreamRedactor. The test list shows about nine rounds of fixes to it. If another glued-run spelling turns up, narrow the hold: let a pako candidate ride only when it opens at the route. Don't add a tenth predicate.
  • The spec says "this weak route may withhold through the 65,536-character payload cap". That means a link that never ends shows nothing until the stream ends. This latency is something users will see and it isn't covered by a test. One test showing the hold releases at the cap would turn the stated bound into a checked one.

[DESIGN-REVIEWED] 5008d00

Description read

[DESCRIPTION-READ] 53cc5d78135da841c5cf07d67944c8162286e621c1df00067b0855305ae09a26

That sha256 names everything this verdict read: the PR description, plus the 1 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

@github-actions

github-actions Bot commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

Opus 5.5 Review (fork) — ✅ no blocking findings

Reviewed 5008d006d82dd6b759eec0d236bd0b084139c412 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 5008d00

@github-actions github-actions Bot added the merge conflict Branch has merge conflicts with its base — author must resolve before merge label Sep 11, 2026
@github-actions

github-actions Bot commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

Security Scope Review (fork) — ✅ PASS

Which legitimate operations does 5008d006d82dd6b759eec0d236bd0b084139c412 newly refuse? Model proposes, scripts/deny_diff.py decides — updated in place on each push. A script-confirmed regression blocks PR readiness.

adjudicated, zero confirmed regressions

Scope-Verdict: PASS

Reason: No row came back confirmed as newly refused. This change loosens credential redaction: pass 3 now skips a validated Mermaid pako link, and the change adds a stream hold. Neither of the three shell-deny checks gets stricter. The Mermaid exemption only reaches the shell exfil check through _credential_clean → redact_credentials (security/exfil.py:513). The rest of the change is output-side redaction (the StreamRedactor pako hold, the view and byte budgets), which the classifier cannot judge. Those are named under UNADJUDICATED.

TIGHTENING: _DOCUMENT_LINK_RE / _document_link_spans in redaction pass 3, plus the StreamRedactor.feed pako hold. The change mostly widens an exemption. The one fail-closed rule it adds is "the link that overruns it fails closed, and so does every link after it", from _mermaid_pako_verdict: if len(views) > view_limit: return False, spent, view_limit + 1. The single threat it targets is a credential carried inside a zlib-compressed mermaid.live/edit#pako: payload. The changed views are judged by compose(view)[0] != view and _pako_check_variant, and any change voids the exemption.

ADJUDICATED: 15 candidates, 0 confirmed newly-refused

PLATFORM COVERAGE: I wrote posix and windows rows. The diff adds no path, separator, interpreter or env-var assumption to any shell check. Its only new assumption is URL text: an exact lowercase https://mermaid.live/edit#pako: route with a base64url payload. That text is the same on both hosts, so I put it inside each host's own way of calling a command:

  • posix: gh pr comment, git commit -m with a trailing )., curl -sI, xdg-open
  • windows: gh from a Windows shell, curl.exe, PowerShell Start-Process

Around those I added rows for the alternatives the diff regrouped inside the same regex (PlantUML on both platforms, Google Docs, Confluence), a commit permalink, the view#pako: route one token off edit#pako:, and running the new test module with .venv\Scripts\python.exe on Windows. No row type was assumed and left without a candidate.

UNADJUDICATED:

  • Guard: StreamRedactor.feed / flush in security/text_redaction.py: the new _pako_chunks hold, _pako_candidate_with_tail, _late_jwt_anchor, _credential_match_crosses, and the rewritten _partial_jwt_tail start search.
    • Call path: streamed assistant or tool output → StreamRedactor.feed(chunk) → hold or commit → _redact_window → broadcast. It is never on the tool-call gate.
    • Inputs I would have submitted:
      • a chat reply streamed one byte at a time that ends …#pako:<payload>. followed by a newline;
      • a JSON "url":"https://mermaid.live/edit#pako:…" split across deltas;
      • an Authorization: Bearer header followed by a pako link in the same delta;
      • prose that has eyJ after a held pako run;
      • a stream that ends (flush) while a pako hold is open.
    • Ordinary regression risk: text is withheld until the payload cap or a terminator arrives, which is latency, not loss.
  • Guard: _mermaid_pako_verdict and the shared view and byte budgets in _document_link_spans.
    • Call path: redact_credentials pass 3 → _document_link_spans → _mermaid_pako_views → pass_compose.compose, imported lazily.
    • Inputs I would have submitted:
      • a doc or chat message carrying several real Mermaid Live links, where an earlier large state uses up the 1,024-view or 65,536-byte budget so later links are judged as before;
      • a diagram whose code contains an ordinary https:// URL with a #fragment right after the host, which _pako_check_variant rewrites to ? before the composed pass judges it.
    • Ordinary regression risk: none. The worst case for an ordinary link is the old redaction, never a new one.
  • Guard: the import change in security/pass_compose.py (import kiro_crew.security.exfil as exfil) and the new early import kiro_crew.security.pass_compose in dashboard/display_battery.py. Changing import order could break the hook process at start-up, which would be a gate-wide failure rather than a single refused row. A string classifier can't judge that. The real classifier run at HEAD will only show it as a harness error, not as a row verdict.

[SCOPE-REVIEWED] 5008d00

Security scope review -- adjudicated candidates

  • macos-latest (platform posix): 10 classified, 0 newly refused, 10 still allowed.
  • ubuntu-latest (platform posix): 10 classified, 0 newly refused, 10 still allowed.
  • windows-latest (platform windows): 5 classified, 0 newly refused, 5 still allowed.

No candidate flipped from allowed to refused.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 11, 2026
@rubencu
rubencu force-pushed the fix/preserve-pako-mermaid-links branch from de0f833 to 852291b Compare September 12, 2026 01:42
@rubencu

rubencu commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

self-added: yes

  • Fixed circular-import rationale span=777d0760c77b

disposition: fixed
Added an explicit # circular import: comment at the function-local scan_exfiltration_urls import.
The comment explains that exfil imports the pako protection helpers from the security facade, so the deferred import is intentional.
Fixed in 852291ba2bda25bf0192452482a00d038e28df7d.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed merge conflict Branch has merge conflicts with its base — author must resolve before merge readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 12, 2026
@rubencu

rubencu commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

self-added: yes
mechanism: keep the platform-to-security dependency lazy at the redaction call boundary

  • Rebutted — span=a4c27d3e83c4

_redact_with_policy_findings is intentionally imported inside redact_via_context(): platform context is on bootstrap paths, while importing security initializes a large regex stack.
Moving this security import to module scope would add avoidable platform module-load work and increase import-cycle risk. The function-local import runs only when redaction is requested, and Python caches it thereafter.
Findings that require moving this deferred platform-to-security dependency to module scope are covered by this ruling. The lazy import is intentional dependency loading, not incorrect behavior.

@rubencu
rubencu force-pushed the fix/preserve-pako-mermaid-links branch from 852291b to 9c23fdb Compare September 12, 2026 04:58
@rubencu

rubencu commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author
  • Fixed: CJK boundary ownership release

Matching CJK closers and separator-plus-backtick boundaries now release outer-URL ownership through the same shared acceptance predicate in batch and streaming paths.
Red-before two-link tests cover both boundary classes at every stream split.
Fixed in 9c23fdb80af5bda09e83249d7d67993230ed73b5.

@rubencu

rubencu commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author
  • Fixed: terminal dashboard persistence policy

Normal segments, every terminal partial-output persistence arm, and Stop-hook egress now share one baseline-plus-active pako-aware assistant boundary.
A red-before provider-death regression proves companion-only decoded state cannot persist.
Fixed in 9c23fdb80af5bda09e83249d7d67993230ed73b5.

@rubencu

rubencu commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author
  • Fixed: channel thinking split across tool events

TurnDriver preserves thinking redactor state across tool/control frames because channel renderers concatenate those frames, then flushes at answer text, completion, or abnormal stream end.
A red-before tool-interleaving regression proves the joined companion credential is redacted.
Fixed in 9c23fdb80af5bda09e83249d7d67993230ed73b5.

@rubencu

rubencu commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author
  • Fixed: dashboard reasoning burst boundary

Dashboard thinking now flushes at text, tool-call, and completion because its browser starts a new reasoning block at each tool row; channel TurnDriver behavior remains intentionally different.
Backend boundary tests and 275 browser reducer tests prove separate benign bursts stay unchanged.
Fixed in 9c23fdb80af5bda09e83249d7d67993230ed73b5.

@github-actions github-actions Bot added the merge conflict Branch has merge conflicts with its base — author must resolve before merge label Sep 12, 2026
@rubencu
rubencu force-pushed the fix/preserve-pako-mermaid-links branch from 9c23fdb to 218301b Compare September 12, 2026 06:09
@rubencu

rubencu commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

self-added: yes
mechanism: preserve outer URL ownership through false Markdown destination markers

  • Fixed — span=50b2215ffa8d

Markdown destination state now requires not has_outer_url. Once an outer URL owns the token, ]( stays URL data and the following pako payload fails closed.
Batch and every-stream-split regressions were red before this fix; normal Markdown destination coverage remains green.
Fixed in 218301bf85cb34185c9b7bbeb864925b3bc4e9e3.

@rubencu

rubencu commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author
  • Rebutted: Item 4 renderer-valid CJK and quoted-href boundaries

Fail-closed here is safe from exfiltration but still corrupts a renderer-valid link, which is the user-visible defect this PR exists to stop.
CJK support was added only after exact dashboard renderer cases failed review; quoted href handling mirrors browser-trimmable URL bytes under a strict 1 KiB fail-closed ceiling.

@rubencu

rubencu commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Head a5e92a6 (one commit on main a932db7) answers the GPT review of 44f6bde.

Fixed (GPT, blocking): a partial-JWT match that starts inside a validated pako payload now rides the pako hold instead of the 4,096-char JWT ceiling. The payload alphabet holds no ., so no complete JWT fits inside it, and a . ends the candidate and sends the joined text through ordinary scanning. Every other JWT and Bearer anchor keeps its ceiling, and the scan before emission is unchanged. Tests in test/test_mermaid_pako_link_redaction.py:

  • test_partial_jwt_inside_a_valid_payload_streams_like_batch[4096|1000000]: a valid link longer than the JWT ceiling, whose first delta carries eyJ in its payload, streams intact. Both cases fail on 44f6bde, where live output replaced the link with the redaction tag.
  • test_a_jwt_beside_a_jwt_bearing_link_is_still_redacted_live[before|after, 7|4096]: a real JWT just before or just after that link is still redacted live.
  • test_a_dot_ends_the_pako_hold_and_a_jwt_spanning_it_is_redacted: a . in the candidate ends the hold, and a JWT spanning it is redacted.
    All three compare live output with redact, the composed batch pass the stream applies to each window; for a URL that carries a credential that pass redacts the whole URL, and live output matches it there too.

Dispositioned: the First Principles note (pass) about the no-op emitted variable is kept, since removing it is a refactor outside this fix. Cross-Platform Portability on 44f6bde was cancelled before it finished, and Sensitive Change Review is a same-repo lane.

Package budget re-measured at 32,336. 16,458 scoped tests (228 files), the static gates and the AWS key-value stream suite passed on 4c9d96a; the PR tests and the 2,000-line cap passed again on a932db7.

@rubencu

rubencu commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Head 093ad71 (one commit on main e9f86a7) answers the GPT review of a5e92a6.

Fixed (GPT, blocking): a link whose delta also carries its trailing punctuation (See <link>. in one chunk) is now held as a pako candidate. The partial route must end on a payload byte, so _pako_candidate_with_tail in security/text_redaction.py reads a credential-class tail of at most 512 characters off the buffer first and seeds the same tail hold a later . delta already gets. A partial JWT now rides the pako hold only when it lies wholly inside the payload, so a JWT-looking run that reaches the tail keeps its ceiling. Tests in test/test_mermaid_pako_link_redaction.py, all failing on a5e92a6: test_a_link_and_its_punctuation_in_one_delta_stream_like_batch (., : and ...:, with and without a later delta) and test_a_credential_after_punctuation_in_the_same_delta_is_still_redacted. Both compare live output with the composed batch pass.

The rebase also merges the package budget note from #22229; the budget is re-measured at 32,393. The Opus lane reported its review incomplete on a5e92a6, and Sensitive Change Review is a same-repo lane.

16,466 scoped tests (228 files) and the static gates passed on 8e80404; the PR tests, the AWS key-value stream suite and the 2,000-line cap passed again on e9f86a7.

@rubencu

rubencu commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Pushed dbfbdbb, rebased onto 961b568 (no conflict).

GPT (security-class, upheld on 093ad71): fixed.

  • _mermaid_pako_verdict now judges each decoded view with the composed URL-and-credential pass that redact() applies (pass_compose.compose). A credential that the URL pass finds only after percent-decoding now voids the exemption.
  • Diagram scanning stays non-nested (_IN_DIAGRAM_SCAN). Decode strictness, the decoded-byte budget, the JWT/Bearer ceilings and the r28 hold are unchanged.
  • display_battery.py lists pass_compose.py as a rule source, because redaction now imports it. pass_compose.py imports its two submodules directly, so the display-battery import guard stays exact.
  • Tests:
    • test_percent_encoded_credential_in_a_decoded_url_voids_the_link fails on 093ad71. It covers a GitHub token written with %5F, and an AWS key ID written with %49 and %2549, in batch and live output.
    • test_credential_free_decoded_url_keeps_the_link and test_a_pako_link_inside_a_decoded_view_is_ordinary_text pin what stays the same.

CI red on 093ad71 (Backend Tests shard 5): fixed.

  • test_near_cap_one_byte_stream_stays_within_cpu_budget read 2.43 s of thread CPU against a fixed 2.0 s budget under coverage tracing. On this host the stream costs the same on a5e92a6 and 093ad71.
  • It is replaced by test_near_cap_one_byte_stream_stays_linear_in_cpu, which follows the conftest scaling convention: the near-cap link may cost at most 25x (REDOS_SCALING_RATIO) a link a tenth its size. A slower traced runner stays green, and a quadratic stream still fails.

Not changed: the PlantUML verdict on main uses the same credential-only check, as the adjudication notes. It is outside this PR.

Tests

  • On e9f86a7: 16,842 scoped tests passed. The one failure is the boot-order pin with a built dist; both composition files pass dist-less (65). Static gates passed. The security package budget is re-measured at 32,399.
  • On 961b568: the PR tests, the display-battery guards, the flake ratchet, the dist-less composition run and the 2,000-line cap passed again.
  • Sensitive Change Review fails only because this is a fork PR.

@rubencu

rubencu commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Pushed 4bf1725, rebased onto 0ce632d.

GPT fenced finding on 89ad591: fixed inside r28. (text_redaction.py:536: sentence punctuation could discard a valid pako link.)

  • A partial-JWT match that starts inside a validated pako payload may now end in the held tail of the candidate. That tail is at most 512 bytes; in the finding it is the trailing . of a sentence.
  • The held run is still scanned in full when it terminates. No hold, ceiling or anchor changed.
  • Tests:
    • test_a_jwt_shaped_payload_and_its_punctuation_in_one_delta_stream_like_batch and test_a_jwt_after_a_jwt_shaped_link_and_its_punctuation_is_still_redacted fail on 89ad591 and pass here.
    • test_a_partial_jwt_past_the_held_tail_still_fails_closed pins the 512-byte bound.

Rebase: test/test_security_gate_liveness.py conflicted with the budget raise for the display host-set snapshot in #16544. Both notes are kept, and _PACKAGE_LINE_BUDGET is re-measured at 32,415.

CI on 89ad591: Frontend Tests shard 3 failed in CrewDynamicDashboard.remint.test.tsx. This PR does not touch that file, and the test passes here (9 of 9, run twice).

Tests

  • On 7d9673b: 16,853 scoped tests passed. The one failure is the boot-order pin with a built dist; dist-less, both composition files pass (65).
  • On 0ce632d: the pako, budget and display-battery tests and the 2,000-line cap passed again, as did the static gates.

@rubencu

rubencu commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Pushed 81be492, rebased onto 69b2e50.

  • GPT's decoded-URL-variant finding on 4bf1725 (redaction.py:105) is fixed inside the pako verdict only. Each decoded view must also pass compose as a check-only variant: percent-escapes decoded to a fixpoint (at most 3 rounds; text still encoded after that voids the link), HTTP(S) schemes lowercased, and a fragment right after the host read as a query. _URL_RE, compose and ordinary text outside a pako state are unchanged. New test_percent_encoded_credential_in_an_upper_scheme_or_fragment_url_voids_the_link covers uppercase HTTPS:// and fragment-only URLs, single and double encoding, and the round cap (10 cases fail on 4bf1725). Two credential-free cases in test_credential_free_decoded_url_keeps_the_link keep the link.
  • Conflict with fix(security): keep SRI integrity digests out of the bare-secret pass #19689: security.md keeps both paragraphs. _PACKAGE_LINE_BUDGET keeps both notes and is re-measured at 32,515. The pako verdict runs with main's SRI exemption in place, and test_sri_integrity_redaction.py passes.
  • 17,250 scoped tests pass. The one red is the boot-order pin that needs a built dist, and it is green dist-less (65). Gates and the 2,000-line cap pass.

@rubencu

rubencu commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Pushed 64d142a, rebased onto b2b87b0 to resolve a conflict with #23119 (the test_security.py split) in security.md. The bare-secret bullet keeps this PR's Mermaid pako text, and the ?token= bullet takes main's copy, which names test_security_stream_redaction.py. No code change. 1,746 PR, SRI and main-touched security tests pass, as do the dist-less composition run (65), gates, docs lint and the 2,000-line cap.

@rubencu

rubencu commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Pushed e9b7b6f, rebased onto 4ef2a92.

  • GPT's finding on 64d142a (text_redaction.py:561): the pako hold could separate a Bearer header from its value. A scheme's : ends the Authorization: Bearer anchor early, and the cut sent the header and the token to two batch calls. A credential match that starts in the commit and runs into the held run now declines the hold (_credential_match_crosses), so main's handling applies to that buffer. Ceilings, anchors, the 512 floor and the pre-emission scan are unchanged.
  • A related case found while testing this: a Bearer header before an eyJ-bearing link, in 2,048-4,096-char chunks. Main commits the header early there too, and saves the token only through the JWT ceiling. The partial-JWT ride now applies only to a run that opens at the route, so glued text keeps the JWT ceiling and fails closed. Accepted cost: in one large delta, an href=", {"url":" or password= glued before an eyJ-bearing link is masked as on main, while the bare link still streams intact.
  • New tests in test_mermaid_pako_link_redaction.py:
    • test_a_bearer_header_glued_to_a_link_is_redacted_with_its_value (18 cases, fail on 64d142a)
    • test_a_bearer_header_glued_to_a_jwt_bearing_link_never_streams_its_value (fails on 64d142a)
    • test_a_label_glued_to_a_jwt_bearing_link_in_one_large_delta_fails_closed (pins the cost)
    • test_a_credential_free_sentence_ending_in_a_link_streams_intact
  • Same on main and outside this PR's scope: the Bearer/link shape leaks an opaque token at 7-, 64- and 513-char chunks, a JWT value leaks its claims segment at 600-char chunks, and the : cuts the Bearer value short in _BEARER_ANCHOR_PARTIAL_RE.
  • 16,831 scoped tests pass (one red: the boot-order pin that needs a built dist; it passes dist-less, 65). The flake ratchet, gates and the 2,000-line cap pass. _PACKAGE_LINE_BUDGET is re-measured at 32,539.

@rubencu

rubencu commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

Pushed 543e867, rebased onto 988fad0. It resolves a conflict with #23192 (the test_security.py split, part 2) in security.md: the bare-secret bullet keeps this PR's Mermaid pako text, and the ?token= bullet takes main's copy, which names test_security_token_params.py. No code change. 970 PR, SRI, stream and main-touched security tests pass, as do the dist-less composition run (65), gates, docs lint and the 2,000-line cap.

@iamwhatever iamwhatever left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes on 543e867.

I found no defect in the pako exemption. _mermaid_pako_verdict decodes under a budget and runs the composed pass on every view, and the exemption covers pass 3 only.

Please address these items:

  1. Your comments name two known gaps outside this PR, and neither has a tracking issue. Open one issue per item and link both in the description:
    • On main, a Bearer header followed by a link leaks an opaque token at 7, 64 and 513-char chunks, and a JWT leaks its claims at 600-char chunks.
    • The PlantUML verdict still uses the credential-only decoded check.
      I checked #14126 and #20817; neither covers these.
  2. Reply to the two notes on this head:
    • Opus: the view traversal has no count cap, and each view runs compose up to twice.
    • First Principles: emitted is always empty.
  3. Rebase to resolve the conflict in test/test_security_gate_liveness.py.

After that, this still needs a human. The PR changes what is redacted, and Sensitive Change Review is red pending a maintainer's attestation.

— crew-pr-reviewer (r122)

@rubencu

rubencu commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

Pushed 88fd4b0, rebased onto 7796e2a.

  • Conflict with main's fix(security): deny kirocrew skills authority-retire from an agent shell #4533 (deny kirocrew skills authority-retire from an agent shell) in test/test_security_gate_liveness.py. Main raised the security package budget from 32,188 to 32,206. Both notes are kept, and the PR's budget was re-measured at 32,557 (main's 32,206 plus this PR's unchanged 351 lines). Every other PR line matches the published patch.
  • The runs on 543e867 finished green apart from Sensitive Change Review (fork infra). GPT and Opus had no blocking findings; First Principles, Design, UX and Security Scope passed.
  • On 1f03360: 3,313 PR and main-touched tests, the dist-less composition contract (65), the static gates and the 2,000-line cap passed. On the final base: 130 PR tests, the budget test and the cap passed again.

@rubencu

rubencu commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

Pushed c25a9cb, rebased onto d2417a4. This answers items 2 and 3 of review 5624890509.

  • Item 2, First Principles note: the always-empty emitted prefix is gone from StreamRedactor.feed, and its returns read as on main again. The PR's remaining diff in feed is only the pako hold. This is a pure subtraction, so no test can fail first; the 23 modules that use StreamRedactor pass (2,055 tests).
  • Item 2, Opus note: _mermaid_pako_views now stops at _MERMAID_PAKO_VIEW_CAP = 1_024 views. Past the cap the link earns no exemption and is redacted as on main, and the decoded bytes still count against the shared budget. A view-heavy state took 82-106 ms before and about 1-2 ms now; a realistic state (14 views) is unchanged at 0.1 ms and stays exempt.
    • test_mermaid_pako_link_redaction.py::test_a_state_past_the_view_cap_fails_closed[7|4096] fails on 88fd4b0.
    • test_a_state_at_the_view_cap_keeps_its_link guards the boundary.
    • security.md names the cap, and the budget was re-measured at 32,563.
  • Item 3, the test_security_gate_liveness.py conflict, was resolved in 88fd4b0.
  • Item 1, the two tracking issues, is with the author.
  • Checks: 12,846 scoped tests passed on 7796e2a. The 22 host-caused failures reproduce on a clean 7796e2a checkout, and one known flaky test passes alone. The dist-less composition contract (65), the static gates and the 2,000-line cap passed. On the final base: 372 PR and main-touched tests, the gates and the cap passed again.

@rubencu

rubencu commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

Pushed f14008a, rebased onto 02ae080.

  • Conflict with main's fix(security): block shell-expansion (arithmetic) evasion of the IMDS denial #22826 (shell-expansion IMDS gate) in test/test_security_gate_liveness.py. Main re-measured the security package budget from 32,206 to 32,438. Both notes are kept, and the PR's budget was re-measured at 32,795: main's 32,438 plus this PR's unchanged 357 lines. Every other PR line matches the published patch.
  • The runs on c25a9cb finished. GPT and Opus had no findings, and Design and UX passed. The reds were Sensitive Change Review (fork infra), Coverage Gate downstream, and Backend shard 7 test_remote_session.py::test_child_exit_flushes_output_and_finishes_the_link (assert 255 == 0). That test came with main's feat: add Coder remote session environments #19833, and this PR touches no remote-session code. It passes 5 of 5 alone on both this head and a clean d2417a4 checkout.
  • Item 1 of review 5624890509 (tracking issues) is still with the author.
  • 615 PR, budget, composition and main-touched tests passed, as did the static gates and the 2,000-line cap.

@rubencu

rubencu commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main 33cdb8f1. Main's #23335 raised the security package budget in test/test_security_gate_liveness.py from 32,438 to 32,447 (one paths_lists entry), so the rebase conflicted on _PACKAGE_LINE_BUDGET.

Resolution: both budget notes kept. This PR's note now starts from 32,447, and the budget is re-measured at 32,804 (main's 32,447 plus this PR's 357 lines, unchanged). No other file changed: the other 8 paths in the patch are identical to the previous head.

Checks on the rebase:

  • The PR's tests, the tests main changed since the previous base, and every redaction/security-related module (423 files): 29,126 passed. The 22 failures are test_design_tweak_backend.py and test_auto_improvement_members.py, which fail only on this host's temp directory and are not touched by this diff.
  • After main moved to 33cdb8f1: the PR's tests, the newly touched tests and the budget test passed (270).
  • Dist-less dashboard composition (65), static gates, the 2,000-line cap, docs lint and the PR body check all pass.

The UX Review check on the previous head stayed "in progress" because its Fork UX Review run was cancelled at 12:37Z; this push starts a new one.

@rubencu

rubencu commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main 48fb674c. Main's #23292 (the rm_floor backtick and glued-redirect reads) raised the security package budget in test/test_security_gate_liveness.py from 32,447 to 32,518, so the rebase conflicted on _PACKAGE_LINE_BUDGET again.

Resolution: both budget notes kept. This PR's note now starts from 32,518, and the budget is re-measured at 32,875 (main's 32,518 plus this PR's 357 lines, unchanged). No other file changed.

Checks on the rebase:

  • The PR's tests, the tests main changed since the previous base, and the redaction, shell-floor and security modules (581 files): 37,885 passed. The 24 failures are not from this diff:
    • test_design_tweak_backend.py (21) and test_auto_improvement_members.py fail on this host's temp directory.
    • The boot-order pin fails only with a built website/dist; the dist-less run passes (65).
    • test_find_ui.py (one case) failed once in the parallel run and passed alone (179 of 179).
  • After main moved to 48fb674c: the PR's tests, the newly touched tests and the budget test passed (290).
  • Static gates, the 2,000-line cap, docs lint and the PR body check pass.

@rubencu

rubencu commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main 177acfe6. Main's #23146 (plural-route commit permalinks in the bare-secret pass) rewrote the "Bare label-less secret-key detection" paragraph in docs/system-specs/modules/security.md. That is the same one-line paragraph this PR extends with its Mermaid pako section, so the rebase conflicted there.

Resolution: main's rewritten paragraph is kept, with this PR's pako section inserted unchanged at the same place. Main's edits are all earlier in the line, and the inserted text is byte-identical to the previous head's. Main's #23146 also changed security/redaction_entropy.py, which this PR edits too; it merged without conflict, and the measured package total is still 32,875, matching this PR's budget pin.

The other 8 paths in the patch are identical to the previous head.

Checks on the rebase:

  • The PR's tests, the tests main changed since the previous base, and the redaction, shell-floor and security modules (758 files): 46,362 passed. The 23 failures are not from this diff: 22 are this host's temp-directory reds (test_design_tweak_backend.py, test_auto_improvement_members.py), and the boot-order pin fails only with a built website/dist (the dist-less run passes, 65).
  • After main moved to 177acfe6: the PR's tests, the newly touched tests and the budget test passed (133).
  • Static gates, the 2,000-line cap, docs lint and the PR body check pass.

The previous head's reds were all outside this diff: a Backend shard 8 CI timeout in test_execution_context.py (passes locally in 15 s), the downstream Coverage Gate, and Sensitive Change Review.

@rubencu

rubencu commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto fada42f7b3; the new head is 5809b63de9. The only conflict was the security package line budget in test/test_security_gate_liveness.py. Main #23627 raised that budget to 32,526 for one _WRITE_PROTECTED_HOME_PATHS entry. Main's note and this PR's note are both kept. This PR's note now reads "Raised again, from 32,526". I re-measured the package on the rebased tree: 32,883 lines, which is main plus this PR's unchanged 357, and the pin is set to that. The other 8 paths carry the same diff as before.

Tests:

  • 48,968 scoped tests passed. There were 2 failures:
    • The boot-order pin. It needs a built website/dist and passes without one (65/65).
    • test_find_ui, a one-case parallel flake. The file passes alone (179/179).
  • After the final rebase onto fada42f7, main's newly touched tests and this PR's tests passed. Only the same boot-order pin failed.
  • The static gates, line cap, docs lint and check-body all pass.

@rubencu

rubencu commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

This addresses the Opus finding on 5809b63de9 (redaction_entropy.py:863). The new head is 41ba1eaa05, rebased onto d5b8c2099c. The view cap applied per pako link, and the per-call budget counted only decoded bytes. Many links in one text could therefore multiply the composed passes.

  • _document_link_spans now gives every top-level pako link in one call a shared budget of 1,024 distinct views, alongside the decoded-byte budget. It is _MERMAID_PAKO_VIEW_CAP, the same number as the per-state cap.
  • _mermaid_pako_verdict charges each link's distinct views to that budget, and judges identical views only once.
  • A state that needs more views than remain fails closed, before any compose. It spends the rest of the budget, so every later link fails closed too. This matches what the byte budget already did.
  • The comments and security.md now state the per-call bound. The package budget note names the view budget, and the package re-measures at 32,900 lines.

New regressions, both failing on 5809b63de9:

  • test_top_level_pako_links_share_one_view_budget: two 600-view links are each kept alone. Together, only the first is kept, and the composes stay within 2 × 1,024.
  • test_identical_views_are_composed_once: a state of 1,000 empty strings composed 1,004 times before the fix. It now composes at most 8 times and keeps its link.

Tests: an 815-file scoped selection passed 49,134 tests. Its one red is the boot-order pin, which needs a built website/dist and passes without one (65/65). Main's newly touched tests passed after the final rebase. The static gates, the line cap and docs lint pass.

@iamwhatever iamwhatever left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes on 41ba1eaa0545bc5cd4a8691a01f1459be973f007.

The lane notes and the conflict from our r122 review are answered: emitted is gone, views are capped per state and shared across links, and the Opus finding on 5809b63de9 is fixed with regression tests. No AI lane has an open finding on this head.

One item is still open: the two known gaps outside this PR have no tracking issue. Please open one issue per item and link both in the description:

  • On main, a Bearer header followed by a link leaks an opaque token at 7, 64 and 513-char chunks, and a JWT leaks its claims at 600-char chunks.
  • The PlantUML verdict still uses the credential-only decoded check.

After that this still needs a human. The PR changes what is redacted (security/redaction.py, redaction_entropy.py, text_redaction.py, pass_compose.py), and Sensitive Change Review is red pending a maintainer.

— crew-pr-reviewer (r124)

@rubencu

rubencu commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto 3503760e2e; the new head is 9cf78512f9. There was one conflict, the security package line budget in test/test_security_gate_liveness.py:

  • Main fix(security): keep member records in a gateway-only directory #23645 raised the budget by 1, from 32,526 to 32,527, for its member-records leaf in paths_lists. Main's note and this PR's note are both kept, and this PR's note now reads "Raised again, from 32,527". The package now measures 32,901 lines: main's 32,527 plus this PR's unchanged 374. The pin is now 32,901.

The rest of the patch merged without conflicts.

Tests, run on ec2619c6f3:

  • A 1,018-file scoped selection passed 60,622 tests. Two failed:
    • The boot-order pin, which needs a built website/dist and passes without one (65/65).
    • test_session_pool.py::TestConfigWiring::test_a_blank_pool_agent_is_the_default_template_not_a_same_named_alias, which is order-dependent under xdist. It passes alone, and its file passes 57/57.
  • The static gates, the line cap and docs lint passed. The description's test line now gives these counts.

Main then moved to 3503760e2e (#23867, #23842, #23852; no shared file). On that base, the tests those commits touched, plus this PR's tests, passed (405, and 65 without a built website/dist).

@rubencu

rubencu commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto badd8f5aa8; the new head is 6b936cac10. There was one conflict, the security package line budget in test/test_security_gate_liveness.py:

  • Main feat(notifications): Web Push for the notification bus #23333 (Web Push) raised the budget by 3, from 32,527 to 32,530, for its web-push leaf in paths_lists. Main's note and this PR's note are both kept, and this PR's note now reads "Raised again, from 32,530". The package now measures 32,904 lines: main's 32,530 plus this PR's unchanged 374. The pin is now 32,904.

The rest of the patch merged without conflicts.

Tests, run on 19877bc051:

  • A 1,269-file scoped selection passed 72,483 tests. It is the previous selection plus the tests main touched since the previous base. Six failed, all specific to this host:
    • The boot-order pin, which needs a built website/dist and passes without one (65/65).
    • Three test_cli.py::TestDoctor cases, which fail the same way without this PR.
    • Two signal-timing cases, test_push_guard_squash.py [SIGINT] and test_push_guard_diagnostics.py [SIGHUP], under load. Both files pass alone (94/94).
  • The security liveness file (10/10), the PR's vitest file (3/3), the static gates, the line cap, docs lint and the description check passed. Vitest ran with main's vite.config.ts minus the React Compiler plugin, because this host's npm registry token has expired, so ChatPaneNotices.tsx ran uncompiled here. The description's test line now gives these counts.

Main then moved to badd8f5aa8 (#24052, testing/harness.py only; no shared file). On that base, test/test_harness.py and this PR's tests passed (222, and 65 without a built website/dist).

@rubencu

rubencu commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Pushed ce6681eccb, rebased onto ce32c2f4db. It answers the Opus 5.5 blocking finding on 6b936cac10 (comment): the two new pako tests carried absolute thread-CPU budgets, which testing-conventions D7 does not allow for a new complexity guard. The change is test-only, in test/test_mermaid_pako_link_redaction.py:

  • test_near_cap_one_byte_stream_stays_linear_in_cpu no longer has the 3-second tenth-size ceiling. It keeps only the bound relative to the tenth-size stream it measures itself (REDOS_SCALING_RATIO per 10x, above REDOS_SCALING_FLOOR_SECONDS).
  • test_terminated_held_candidate_of_jwt_prefixes_stays_within_cpu_budget is replaced by test_terminated_held_candidate_of_jwt_prefixes_scans_only_its_tail. It wraps text_redaction._PARTIAL_JWT_TAIL_RE, counts the characters each search is handed, and requires a 4x run of eyJ prefixes to hand it the same count as a 1x run (4 characters each today). With the search started at the buffer start, the shape before the tail-search fix, it fails: "the tail search read 12070 characters of a 4x run, 3070 of 1x".

No production code changed: the other 8 paths have the same diff as on 6b936cac10.

Tests, on 8edd26d9ba: the pako file, the security liveness file, the facade mirror guards and the tests main touched since the previous base passed (343). The flake-pattern ratchet, the static gates, the line cap and the description check passed, and 65 passed without a built website/dist. Main then moved to ce32c2f4db (#24099, #24114, #24104; no shared file); on that base, the tests those commits touched and this PR's tests passed (200). The description's stream-regressions line now says "a scan count" instead of "a CPU bound".

@rubencu

rubencu commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto 9d31193afe; the new head is 7f665b8c56. There was one conflict, the security package line budget in test/test_security_gate_liveness.py:

  • Main perf(redaction): native single-pass credential gate, Python fallback #19628 (native single-pass credential gate) raised the budget by 71, from 32,530 to 32,601, for the optional kc_native pre-filter in redaction_patterns. Main's note and this PR's note are both kept, and this PR's note now reads "Raised again, from 32,601". The package now measures 32,975 lines: main's 32,601 plus this PR's unchanged 374. The pin is now 32,975.

The rest of the patch merged without conflicts, including docs/system-specs/modules/security.md beside main's #24000 edits: the other 8 paths have the same diff as on ce6681eccb.

Tests, run on 5c01f30ac0:

  • 359 redaction, security and main-touched test files passed 19,221 tests, with no failures. They include main's native-gate fallback and pre-filter suites, which run the Python gate here because kc_native is not built on this host.
  • The security liveness file, the PR's vitest file (3/3), the flake-pattern ratchet, the static gates, the line cap, docs lint and the description check passed, and 65 passed without a built website/dist.

Main then moved to 9d31193afe (#23719, #23223, #23717, #23977, #23795, #23925, #23968, #24000). On that base, the 54 test files those commits touched and this PR's tests passed (2,854), docs lint and the line cap passed, and 65 passed without a built website/dist. The description's test line now gives the 359-file run.

@rubencu

rubencu commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto 2356a1ba1c; the new head is c88e386669. There was one conflict, the security package line budget in test/test_security_gate_liveness.py:

  • Main fix(security): extend native config write tier #23935 (native config write tier) raised the budget by 12, from 32,601 to 32,613, for the crewmate folder's state files in paths_lists. Main's note and this PR's note are both kept, and this PR's note now reads "Raised again, from 32,613". The package now measures 32,987 lines: main's 32,613 plus this PR's unchanged 374. The pin is now 32,987.

The rest of the patch merged without conflicts, including docs/system-specs/modules/security.md beside main's edits: the other 8 paths have the same diff as on 7dd99ebcb5.

Tests, run on 2356a1ba1c:

@rubencu

rubencu commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto 6b1b65d to clear a conflict with #24392 in test/test_security_gate_liveness.py. Main raised _PACKAGE_LINE_BUDGET from 32,613 to 32,616 for the enterprise install-tree write fence, and this PR raised it for the pako branch. The resolution keeps both notes, restarts this PR's note from 32,616 and pins the measured total, 32,990 lines (this PR's 374 on top of main's 32,616). No code changed, and the rest of the patch is identical.

@rubencu

rubencu commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

Pushed 9e0f854 on 63c0cfc. It fixes GPT's fenced finding on 493474a (redaction_entropy.py:848). Both JSON views re-escaped a tab inside a decoded value, so Bearer and its token never appeared apart. The value checked on its own had also lost its Authorization label.

_mermaid_pako_views now also checks each string value beside its decoded key with nothing escaped ("key":"value"), inside the same 1,024-view cap. test_an_escaped_tab_cannot_split_a_bearer_value_from_its_label fails on 493474a and passes here, in batch and when streamed. The view-cap fixtures count the new view, and the security spec names it.

The rebase also resolved the _PACKAGE_LINE_BUDGET conflict with #23840. Main's note stays first, this PR's note now starts from 32,635, and the measured total is 33,015.

@rubencu

rubencu commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main to resolve a conflict in test/test_security_gate_liveness.py: since the last push, #24896, #20487, #25175 and #25445 raised the package budget from 32,635 to 32,798.

This push also fixes the GPT review finding on text_redaction.py. A Bearer value glued ahead of a pako route (Authorization: Bearer <opaque>/https://mermaid.live/edit#pako:…) streamed raw when the payload's eyJ arrived after the hold began, for example in 7-byte deltas. Main fails closed on that input.

  • StreamRedactor.feed now ends the hold of a run glued ahead of its route when an eyJ with a byte after it reaches the run (_late_jwt_anchor). Normal scanning then applies the 4,096-byte JWT ceiling and the fail-closed discard, as on main. A run that opens at the route keeps its ride.
  • A glued run is not held while its route, payload or tail holds an eyJ, so it is not held again after that release.
  • test_a_bearer_header_glued_to_a_jwt_bearing_link_never_streams_its_value now also runs in 7-byte deltas, with the eyJ early in the payload (inside main's 512-byte floor) and late. test_an_eyj_ending_a_delta_still_ends_the_hold ends a delta right after eyJ. test_a_label_glued_to_a_jwt_bearing_link_fails_closed, renamed from …_in_one_large_delta_fails_closed, also runs in 7-byte deltas. All five new cases fail on 9e0f854.
  • The package budget is 33,204, which is main's 32,798 plus this PR's 406 lines, 26 of them for this fix. The measured total is 33,204 lines.

@rubencu

rubencu commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

On review 5624890509 (crew-pr-reviewer):

  • Items 2 and 3 are answered in this comment. Since then the view cap became one budget shared by every pako link in a call (comment), and the item-3 budget conflict has been re-resolved on each rebase, most recently in acefe14.
  • Item 1 asks for two public tracking issues for the main-side gaps it lists. SECURITY.md asks that a potential security issue is reported privately rather than opened as a public GitHub issue. Maintainers, which channel do you want: public issues as the review asks, or a private report per SECURITY.md? Nothing will be opened, and no further detail added here, until you choose.

The First Principles lane could not run on acefe14 (GitHub API rate limit), and Sensitive Change Review still needs a maintainer's attestation.

@rubencu

rubencu commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Pushed 3e4b528, rebased onto main 3cdcfe893c. The rebase resolved one conflict in test/test_security_gate_liveness.py: main raised _PACKAGE_LINE_BUDGET to 32,799, so this PR's budget is now 33,205 (main's value plus this PR's 406 lines), which equals the measured package total. The rest of the patch is unchanged.

On 9fb1d4aa52, the PR's tests, the tests main changed since 4efafcf60f, the composition contracts and the static gates passed (7,382 tests); the only failures also fail on main on this host. After the final re-sync onto 3cdcfe893c, main's newly changed test file and the composition contracts pass again.

Validate Mermaid Live pako state before exempting its opaque payload from bare-secret redaction. Keep streamed candidates bounded and reassemble terminating bytes so credentials cannot cross an emission boundary.
@rubencu

rubencu commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main 203a9ce3 (head 5008d006). One conflict, in test/test_security_gate_liveness.py: main raised the security package line budget to 32,802 (#23855). This PR's own increment is unchanged (406 lines), so the pin is now 33,208, the measured total. Main's budget paragraphs stay whole and this PR's paragraph follows them. No code change.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor) merge conflict Branch has merge conflicts with its base — author must resolve before merge needs-author-decision PR blocked on author input readiness: passed Eligible automated validation passed for the current revision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants