Skip to content

fix(security): keep AWS access key IDs visible and redact signed-link signatures - #22757

Closed
bolichen97 wants to merge 4 commits into
mainfrom
feat/redaction-key-id-identifier-22552
Closed

bolichen97 wants to merge 4 commits into
mainfrom
feat/redaction-key-id-identifier-22552

Conversation

@bolichen97

Copy link
Copy Markdown
Contributor

Problem / Motivation

Goal: An AWS access key ID stays visible on every surface, while a presigned or signed link never shows a working signature.

An access key ID (AKIA… / ASIA…) names a key and cannot sign a request. Kiro Crew redacts it anyway, on every surface: chat, logs, the audit log, metrics, channel messages, blocked links, the decisions gate and the frontend. Investigators who join logs on the key ID see a placeholder instead, and the decisions gate refuses a request that merely names one.

Why it matters

The key ID is the join key for incident investigation (CloudTrail rows, aws sts get-caller-identity, credential reports). Hiding it makes those logs unusable while protecting nothing, because the key ID cannot sign. The design in #22552 (section 3, question 1) moves it to the Identifier tier.

Removing the key ID from redaction would, by itself, also make presigned links whole again: today the redacted key ID inside X-Amz-Credential is what breaks such a link. So this change redacts the signing part of the link at the same time.

Not a goal

What changed (motivation → approach → change)

Every rule decided on its own that a key ID was a credential. This change removes the key-ID rule, bare and labelled, from each surface that had one, so the key ID is treated the same everywhere. credential_patterns.AWS_KEY_ID stays as a structural anchor for the presigned-URL validators. The rule tier table carries no identifier rule, because an identifier is never redacted.

Value Before After
Key ID AKIA… / ASIA… redacted, blocks visible
Secret access key, session token redacted redacted
X-Amz-Signature=, Signature= (URL) visible value redacted
X-Amz-Security-Token= (URL) visible value redacted
SigV4 Signature=<64 hex> (header) visible value redacted

Signed links are Bearer link tier keyed on the signing part. The token-parameter pass now also removes the values of X-Amz-Signature, Signature and X-Amz-Security-Token. Its name matching (case, HTML entities, percent escapes, stream hold-back) is shared and built as a letter trie, so the extra names cost little. The credential pass also removes a SigV4 request signature after Signature= and keeps the label. The decisions gate still refuses a secret or a signed link, and the deploy scan reports a key ID as an overridable warning, like an account id.

The surfaces changed are security/ (redaction, exfil URL rules), decisions/gate.py, log_redaction.py, sel.py, metrics/schema.py, deploy/scan.py, the built-in apps (pptx_maker, aws_control packaging scan, ops_mission_control comments), security_posture.py, and the frontend website/src/utils/sanitize.ts, website/src/utils/errorReport.ts and the mochi app (website/src/apps/mochi). The tier table lives in credential_patterns.py. Comments that named a key ID as a credential example are corrected in platform/defaults.py, platform/interfaces.py, computer_use/tools.py and dashboard/handlers/artifacts_parts/record_serialization.py.

Depends on #22592 (its commit is included until it merges) and on the test-only fixture PRs #22736, #22737 and #22750, which move tests that used a key ID as "some credential" to a secret-shaped fake. Their commits are included too; this PR's own change is the top commit. The included commits also touch acp/runtime.py, aws_consent.py, channel.py, dashboard/chat_runner.py, discord/transport_dispatch_commands.py, knowledge/ingestion.py, mcp_discovery_config.py, mcp_tools/knowledge.py, snapshot_redact.py (the composed pass of #22592) and testing/fake_computer_use.py (the fixture swap).

Backwards compatibility

Compatible: nothing that is accepted today is refused; key IDs are refused and redacted on fewer surfaces, and signed-link signatures on more.

Reader: security/exfil_url_rules.py:_HARD_CREDENTIAL_RE -- chat, cron, subagent -- still matches labelled secrets, SSH and PEM markers and Slack tokens; test_redaction_key_id_identifier.py.
Reader: decisions/gate.py:scrub_reason -- decision points -- still refuses secrets and signed links; test_decisions_gate.py::TestScrub::test_a_secret_or_signed_link_beside_a_key_id_is_still_refused.
Reader: apps/builtins/aws_control/crew/packaging/pipeline/scan.py:_HARD_PATTERNS -- app bundle export -- the labelled secret, vendor token and canonical detector rows still refuse; test_labelled_secret_scan.py.
Reader: apps/builtins/pptx_maker/backend/routes.py:_scanned_bitmap_bytes -- app -- the decoded-bytes scan and URI terminator check still refuse a secret in an image; test_pptx_maker_routes.py.
Reader: sel.py:_redact_text -- audit log -- runs the shared redactor; test_sel_redaction.py.

Known residuals:

Tests

  • test/test_redaction_key_id_identifier.py (new): a key ID alone stays on every redactor (bare, CloudTrail row, env, credentials file, STS JSON, in a URL, streamed under its label). A key ID beside its secret keeps the ID and loses the secret and token. Presigned S3, SigV2, CloudFront and lower-case links, and a SigV4 header, lose their signing values. A stream split inside a presigned URL emits no signing value. The log filter and metrics keep a key ID. All 21 fail on the base and pass here.
  • test_decisions_gate.py: a state naming only a key ID is not refused; a key ID beside a secret, a presigned link and a signed link are.
  • test_credential_patterns.py, test_credential_prefilter.py, test_redaction_tiers.py: the shared spellings, the branch samples (the new signature branch and its prefilter anchor) and the tier table follow the new rules.
  • test_log_redaction.py, test_sel_redaction.py, test_deploy_round3_fixes.py, test_deploy_round4_fixes.py, test_security.py, test_security_posture.py, the packaging scan tests and the frontend errorToAgent.test.tsx and sanitizeExfiltrationBase64Padding.test.ts: key IDs stay visible, and secrets are still caught.

Run locally: every test file that mentions a key ID, a signature or the changed modules (about 33,800 tests), and the 154 related frontend test files. The only reds also fail on the base on this host: test_design_tweak_backend.py, test_design_tweak_devproc.py, test_prompt_override_encoding.py and MarkdownRendererCoverage.test.tsx. No test here waits, sleeps or depends on order; each new test is a pure function of fixed inputs.

Manual verification

N/A - every surface changed is covered by unit tests on the redactors themselves.

Why no screenshot: the frontend change is in sanitizer and error-report helpers that render nothing new; a key ID simply stops being replaced.

Related Issues

Part of #22552

Pattern harvest

Rule candidate: review-prompt
Pattern: a value's sensitivity is decided per rule, so one identifier was redacted by 14 separate copies; new rules must name a tier from credential_patterns.RULE_TIERS.

Checklist

  • At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|style|refactor|perf|test|chore|ci|build|revert: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable)
  • No secrets, credentials, or internal references in the diff

@bolichen97
bolichen97 requested review from a team and KiroCrewCaptain as code owners October 21, 2026 11:29
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Oct 21, 2026
@bolichen97
bolichen97 enabled auto-merge (squash) October 21, 2026 11:35
@github-actions github-actions Bot added the merge conflict Branch has merge conflicts with its base — author must resolve before merge label Oct 21, 2026
w added 3 commits October 21, 2026 12:04
Tests that use an AWS access key ID as their stand-in for "a credential"
now use a fake token the redactor treats as a secret (a GitLab- or
Slack-shaped fake, or a labelled fake secret key). No product code changes;
every test passes on main before and after.
Tests that use an AWS access key ID as their stand-in for "a credential"
now use a fake token the redactor treats as a secret (a GitLab- or
Slack-shaped fake, or a labelled fake secret key). No product code changes;
every test passes on main before and after.
Built-in app tests that use an AWS access key ID as their stand-in for
"a credential" now use a GitLab-shaped fake token the redactor treats as
a secret. No product code changes; every test passes on main before and
after.
@bolichen97
bolichen97 force-pushed the feat/redaction-key-id-identifier-22552 branch from 7e25ceb to e3c5f32 Compare October 21, 2026 12:07
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Sensitive Change Review

Sensitive files changed:

  • src/kiro_crew/log_redaction.py

  • src/kiro_crew/security/denied_rules_catalog.py

  • src/kiro_crew/security/exfil_url_rules.py

  • src/kiro_crew/security/redaction.py

  • src/kiro_crew/security/redaction_entropy.py

  • src/kiro_crew/security/redaction_patterns.py

  • src/kiro_crew/security_posture.py

  • @bolichen97 posted a complete reasoning template for 289c3b6.

@github-actions github-actions Bot removed the merge conflict Branch has merge conflicts with its base — author must resolve before merge label Oct 21, 2026
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

UX Review (Opus 5.5) — ✅ PASS

UX-level review of 289c3b63087f098789f1848a0d25c97634b82feb — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

UX-Verdict: PASS

Nothing to check: no control, label or layout is added. The only visible effect is which substrings the redactors mask in text that already renders.

Notes

  • errorReport.ts drops the [redacted-access-key] placeholder and masks signing values with the generic REDACTED instead. If error-journal readers relied on the specific placeholder to see what was masked, a value-specific marker such as [redacted-signature] would keep that clarity.

[UX-REVIEWED] 289c3b6

Description read

[DESCRIPTION-READ] 11feeb405250a56b33ec83b76d344b47c8c16ad170957ddca9c5b534fdb91557

That sha256 names everything this verdict read: the PR description, plus the 3 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 Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

First Principles Review (Opus 5.5) — ✅ PASS

Premise-level review of 289c3b63087f098789f1848a0d25c97634b82feb — 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: the maintainer's tier decision in docs/decisions/ on the base commit covers moving the key ID to the Identifier tier, and the signing-value redaction it adds is the "keyed on the signing part" rule from that decision.

Notes

  • Item 8 — oversized: the deploy scan adds a new aws-key-id warning, justified only by "like an account id". That is a symmetry argument, and the decision says an Identifier is never grounds to act. The same item checks signing values twice in deploy/scan.py: once with URL_SIGNING_VALUE in _DEPLOY_EXTRA, and again by re-running redact_credentials_with_records. Subtraction: delete the _AWS_KEY_ID_RE finding loop.
  • Item 6 — oversized: log_redaction.py runs two passes that overlap: _URL_SIGNING_RE and _redact_signing_params. Both catch ?Signature=/&X-Amz-Signature= query values. Subtraction: shrink _URL_SIGNING_RE to the header and colon forms that _redact_signing_params does not match.
  • Item 4 — zero consumers: sel.py keeps AWS_KEY_ID_PREFIXES # noqa: F401 (facade surface). A grep for sel\.AWS_KEY_ID_PREFIXES and for from kiro_crew.sel import … AWS_KEY_ID_PREFIXES finds 0 consumers. Subtraction: delete the import.

What this change ships

Inventory (10 items) — 6 justified

Intent: let investigators see AWS key IDs on every surface while a signed link never exposes a working signature. This is a FIX.

  1. Chat and tool output stop redacting AKIA/ASIA key IDs, bare or labelled — justified
  2. The outbound-URL scan (backend and frontend) no longer blocks on a key ID — justified
  3. The decisions gate no longer refuses a state that names a key ID — justified
  4. The audit log drops its case-insensitive key-ID pass — zero consumers on a leftover import
  5. The log filter and metrics keep key IDs; ABIA/ACCA stay redacted — justified
  6. The values of X-Amz-Signature, Signature and X-Amz-Security-Token in links are redacted on every surface — oversized, the log filter overlaps itself
  7. A SigV4 Signature=<hex> header value is redacted — justified
  8. Deploy scan: a key ID becomes an info warning and a signing value becomes a hard finding — oversized, the warning rests on symmetry
  9. The packaging scan and pptx_maker stop refusing key IDs; the packaging scan refuses signing values — justified
  10. Bundled test-fixture swaps from the dependent PRs — rides along

[FIRST-PRINCIPLES-REVIEWED] 289c3b6

Description read

[DESCRIPTION-READ] e283239348fce7310d324f0ab74fea9402d736c2a617f5f817923ae78414ad5f

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 Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Design Review (Opus 5.5) — ✅ PASS

Design-level review of 289c3b63087f098789f1848a0d25c97634b82feb — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

Design-Verdict: PASS

Nothing blocks: the maintainer decision 2026-10-20-redaction-classifies-by-what-a-value-can-do.md records this tier change, and the signing values are now redacted on each surface where the key ID stops being redacted.

Notes

  • The signing value is now spelled in four places that can drift apart: credential_patterns.URL_SIGNING_VALUE, the pass-4 trie names in redaction.py, _SIGNING_PARAM_NAMES plus its own percent-decoding parser in log_redaction.py, and the fallback copy in the packaging scan. That is the same per-surface duplication the decision says to remove ("No surface keeps its own copy"). Point the follow-up PR that builds the one rule table at these four.
  • Dropping the AKIA exfil and pptx encoded-body checks removes the only signal for a bare, unlabelled secret placed next to a key ID in a URL path or a bitmap body. The decision accepts this until key-ID pairing lands, so state that residual in the description's Known residuals rather than leave it implicit.

[DESIGN-REVIEWED] 289c3b6

Description read

[DESCRIPTION-READ] 54efd2cdd2d5fd2dc6fd3d418944f9e68b0f5375fa86f005131ef126019a5835

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 Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Opus 5.5 Review — 🔴 changes requested (blocking)

Reviewed 289c3b63087f098789f1848a0d25c97634b82feb — this comment is updated in place on each push.

BLOCKING: the deploy scan now refuses, with no override, pages that only mention the AWS signing header or Signature query parameter, even when no real credential is in them.

BLOCKING — src/kiro_crew/deploy/scan.py:184
re.compile(URL_SIGNING_VALUE),
Trigger: someone publishes a static site whose HTML or JS names the signing parameter followed by a placeholder or variable, not a real value. Examples: <pre>curl -H "X-Amz-Security-Token=[REDACTED]" …</pre>, headers:{"X-Amz-Security-Token=[REDACTED]}, or `${base}?Signature=${sig}&Expires=${exp}`. Path: handlers_sites publish → _scan_tree → scan_content → _DEPLOY_EXTRA loop → Finding("credential", …, severity="credential").
Outcome: all three inputs return a credential/credential finding on this head and no finding on the base. is_credential_finding turns that into a 409 saying "Credential/security findings cannot be overridden", so the site can't be published even though nothing secret is in it. Core pass 4 already skips this shape with (?!\$\{), and this unguarded copy bypasses that guard.
Anchor: defect
Fix: drop URL_SIGNING_VALUE from _DEPLOY_EXTRA and rely on the redact_credentials_with_records request_signature/aws_session_token pass added below it. Or give the deploy copy its own pattern that rejects $…/${…} placeholders and bare identifier/property references as the value.

[BLOCK-MERGE] 289c3b6
[OPUS-REVIEWED] 289c3b6

Verdict parsed from the review's SHA-scoped output markers for commit 289c3b63087f098789f1848a0d25c97634b82feb.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 289c3b63087f098789f1848a0d25c97634b82feb: <one-sentence reason>

@github-actions

Copy link
Copy Markdown
Contributor

Security Scope Review -- 🔴 could not complete (fails closed)

No scope verdict for e3c5f32fa31fc702b4157baf75d8e8d56ba6bd5b (the candidate stage failed). This lane fails CLOSED: an unmeasured tightening must not read as "nothing newly refused". See the job logs, then re-run -- or record /ai-review override scope e3c5f32fa31fc702b4157baf75d8e8d56ba6bd5b: <reason> if a human has judged the scope by hand.

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

GPT 6.1 Review — 🔴 changes requested (blocking)

GPT 6.1 found at least one blocking issue that must be resolved before merging 289c3b63087f098789f1848a0d25c97634b82feb. 2 of 2 blocking finding(s) are security-class and were withheld from adjudication, so the blocking verdict stands.

This comment is updated in place on each push.

BLOCKING -- src/kiro_crew/security/redaction_entropy.py:643 -- Encoded signing names bypass base64 credential detection

sig = _URL_SIGNING_RE.search(decoded) is not None
Trigger: Channel output contains a base64-encoded SigV2 URL with AWSAccessKeyId=…&%53ignature=…; TurnDriver → StreamRedactor(_redact) → redact_credentials() → _decode_b64_chunk() checks the decoded URL with the literal-only signing pattern.
Outcome: Batch and streaming redactors emit the encoded bearer link unchanged. Decoding it recovers the complete signature; standalone packaging scanning also accepts this spelling.
Anchor: backend-security-controls
Fix: Share encoding-aware signing-name detection between the decoded-credential pass and standalone scanner, preserving the visible key ID.

BLOCKING -- src/kiro_crew/security/redaction.py:518 -- HTML character references leave signed links intact

_TOKEN_PARAM_NAMES = ("token", *_SIGNATURE_PARAM_NAMES, *_SESSION_TOKEN_PARAM_NAMES)
Trigger: Agent-authored HTML contains a SigV2 link with Signature=&#65;…; the artifact preview calls _redact_artifact() → redact(), but the newly added signing names inherit a value class that rejects the initial &.
Outcome: The artifact is served unchanged, and browser entity decoding restores the complete signature. Deploy scanning likewise finds no hard credential, allowing publication after the identifier-warning override.
Anchor: backend-security-controls
Fix: Recognize HTML character references within signing values and redact their original spans in both the core pass and shared signing-value scanner.

[BLOCK-MERGE] 289c3b6
[GPT-REVIEWED] 289c3b6

Adjudication (Opus 5.5) — is blocking on each finding proportionate?

F1 (fenced): Credential-exposure class, but only for input that real software doesn't produce. The decoded-chunk check at src/kiro_crew/security/redaction_entropy.py:643 uses URL_SIGNING_VALUE (src/kiro_crew/credential_patterns.py:103). That pattern accepts encoded ?/&/=, but the name Signature must be spelled with literal letters. To get past it, the output needs a SigV2 link whose parameter name has a percent-encoded letter (%53ignature), with the whole URL then base64-wrapped. Standard URL encoders never percent-encode letters, so only deliberate obfuscation produces this. An agent willing to obfuscate can encode the signature itself in endless other ways (reverse it, hex it, split it). That makes it the spelling-chase class AGENTS.md names; there's no recovery path, and the residual risk is acceptable to a human.

F2 (fenced): Same class. The value class [^\s"'&;#<>)\]}]+ at src/kiro_crew/credential_patterns.py:103 rejects a value starting with &#65;. The pass-4 names at src/kiro_crew/security/redaction.py:518 inherit it. HTML serializers escape only &→&amp; in an href, and the pattern already handles that. A SigV2 signature is base64 URL-encoded with %2B/%2F/%3D and never contains entity-encoded letters. So only an agent obfuscating on purpose writes Signature=&#65;…. That is again an unbounded spelling chase rather than a leak on a real path, and an override rationale holds up.

[ADJUDICATION] 289c3b63087f098789f1848a0d25c97634b82feb total=0 uphold=0 downgrade=0
[GPT-ADJUDICATED] 289c3b63087f098789f1848a0d25c97634b82feb

[ADJUDICATION-FENCED] 289c3b63087f098789f1848a0d25c97634b82feb fenced=2 flagged=2
FLAG F1 src/kiro_crew/security/redaction_entropy.py:643 -- Requires a base64-wrapped URL whose parameter name has a percent-encoded letter (`%53ignature`), which no standard URL encoder emits, so only deliberate obfuscation reaches it, one spelling in an unbounded set the AGENTS.md spelling-chase rule says a regex cannot close.
FLAG F2 src/kiro_crew/security/redaction.py:518 -- Requires a signature value whose characters are HTML character references (`&#65;…`), which neither HTML serializers (they escape only `&` as `&amp;`, already matched) nor SigV2's percent-encoded base64 produce, so only a deliberately obfuscating writer reaches it.
[GPT-ADJUDICATED-FENCED] 289c3b63087f098789f1848a0d25c97634b82feb

🏷️ Fenced finding(s) machine-flagged as likely edge case

The security fence keeps these findings blocking regardless of adjudication; the only clearance path is a human override. The annotate-only pass judged the condition combination each one requires extreme, and pre-drafted the override rationale below. A repository writer must independently verify a rationale before posting it — it is machine-authored, and a wrong override on a security-class finding ships exactly the class the fence exists to stop.

  • F1 src/kiro_crew/security/redaction_entropy.py:643 — Requires a base64-wrapped URL whose parameter name has a percent-encoded letter (%53ignature), which no standard URL encoder emits, so only deliberate obfuscation reaches it, one spelling in an unbounded set the AGENTS.md spelling-chase rule says a regex cannot close.

    /ai-review override gpt 289c3b63087f098789f1848a0d25c97634b82feb: Requires a base64-wrapped URL whose parameter name has a percent-encoded letter (%53ignature), which no standard URL encoder emits, so only deliberate obfuscation reaches it, one spelling in an unbounded set the AGENTS.md spelling-chase rule says a regex cannot close.
    
  • F2 src/kiro_crew/security/redaction.py:518 — Requires a signature value whose characters are HTML character references (A…), which neither HTML serializers (they escape only & as &, already matched) nor SigV2's percent-encoded base64 produce, so only a deliberately obfuscating writer reaches it.

    /ai-review override gpt 289c3b63087f098789f1848a0d25c97634b82feb: Requires a signature value whose characters are HTML character references (&#65;…), which neither HTML serializers (they escape only & as &amp;, already matched) nor SigV2's percent-encoded base64 produce, so only a deliberately obfuscating writer reaches it.
    

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 289c3b63087f098789f1848a0d25c97634b82feb: <one-sentence reason>

@bolichen97
bolichen97 force-pushed the feat/redaction-key-id-identifier-22552 branch from e3c5f32 to db42ef2 Compare October 21, 2026 12:35
@bolichen97

Copy link
Copy Markdown
Contributor Author

Sensitive change review

  • Head commit: db42ef2
  • What rule changed: AWS access key IDs (AKIA/ASIA) are reclassified to Identifier tier, so no redactor removes them and no exfiltration gate blocks on them. In the same change, the value that makes a presigned/signed link work — the SigV4/SigV2 signature and the session token — is keyed and redacted on every surface (log-redaction floor, deploy/packaging scan, exfil URL rules, frontend sanitize). ABIA/ACCA ids and secret access keys / session tokens remain Secret tier (still redacted).
  • Before -> after (what was allowed/blocked, now): Before, an AKIA/ASIA key ID was masked everywhere, which also broke a presigned link in a log (the whole link was mangled) while the signature could survive under an obfuscated spelling. After, the key ID (a non-secret identifier that cannot sign a request) stays visible so a log line can be joined to the key it names, and the signature/session token is removed so a signed link in a log cannot be replayed.
  • Worst case if wrong (breaks commands / leaks secret / locks user out): If the signing matcher were too narrow, a signed link could survive in a log and be replayed until expiry. If too broad, ordinary prose containing the word "signature" would be over-redacted, hurting readability but leaking nothing. The design keeps the key ID visible by intent; a key ID alone cannot authenticate, so its visibility leaks no secret.
  • Evidence checked (which sandbox result proves it): Local probe over SecretRedactionFilter.redact confirms: .signature:hover, const signature = sign(body), <p>Signature: Jane</p>, data-signature="abc" are untouched; a presigned URL keeps its AKIA... key ID but loses the 64-hex signature; a percent-escaped param name X-Amz-%53ignature=<64hex> on a presigned-marker line is redacted; a bare sha256=<64hex> with no presigned marker is kept; an X-Amz-Security-Token value is redacted.
  • No regression:
    • Tools still work (which tools/commands were run, result): redact() is pure string transformation with zero I/O; the probe above exercised prose, CSS/JS identifiers, presigned URLs and bare digests with the expected keep/redact outcome.
    • Backward compatible (old config / old data / old callers still work, how checked): The shared spelling lives in the import-free credential_patterns module both consumers already import; no public signature changed. test_credential_patterns.py and test_credential_prefilter.py (import-free shape + branch-count + prefilter anchor invariants) pass.
    • Existing tests (which suites passed on this head): test_redaction_key_id_identifier.py, test_security.py, test_log_redaction.py, test_credential_patterns.py, test_credential_prefilter.py — 2023 passed, 1 skipped. black / isort / flake8 / mypy / comment-history / semgrep (baseline merge-base) all clean.
  • How to undo: Revert this commit (db42ef2d6f98984c1691ce4338884bb28216bcdf). The change is a single stacked commit over the fixture-only commits; reverting restores the prior AKIA-masking behavior with no data migration.

@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 Oct 21, 2026
@bolichen97
bolichen97 force-pushed the feat/redaction-key-id-identifier-22552 branch from db42ef2 to acabdcf Compare October 21, 2026 13:11
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Oct 21, 2026
@bolichen97

Copy link
Copy Markdown
Contributor Author

Sensitive change review

  • Head commit: acabdcf
  • What rule changed: AWS access key IDs (AKIA/ASIA) are reclassified to Identifier tier, so no redactor removes them and no exfiltration gate blocks on them. In the same change, the value that makes a presigned/signed link work — the SigV4/SigV2 signature and the session token — is keyed and redacted on every surface (log-redaction floor, deploy/packaging scan, exfil URL rules, frontend sanitize). ABIA/ACCA ids and secret access keys / session tokens remain Secret tier (still redacted).
  • Before -> after (what was allowed/blocked, now): Before, an AKIA/ASIA key ID was masked everywhere, which also broke a presigned link in a log (the whole link was mangled) while the signature could survive under an obfuscated spelling. After, the key ID (a non-secret identifier that cannot sign a request) stays visible so a log line can be joined to the key it names, and the signature/session token is removed so a signed link in a log cannot be replayed.
  • Worst case if wrong (breaks commands / leaks secret / locks user out): If the signing matcher were too narrow, a signed link could survive in a log and be replayed until expiry. If too broad, ordinary prose containing the word "signature" would be over-redacted, hurting readability but leaking nothing. The design keeps the key ID visible by intent; a key ID alone cannot authenticate, so its visibility leaks no secret.
  • Evidence checked (which sandbox result proves it): Local probe over SecretRedactionFilter.redact confirms: .signature:hover, const signature = sign(body), <p>Signature: Jane</p>, data-signature="abc" are untouched; a presigned URL keeps its AKIA... key ID but loses the 64-hex signature; a percent-escaped param name X-Amz-%53ignature=<64hex> on a presigned-marker line is redacted; a bare sha256=<64hex> with no presigned marker is kept; an X-Amz-Security-Token value is redacted.
  • No regression:
    • Tools still work (which tools/commands were run, result): redact() is pure string transformation with zero I/O; the probe above exercised prose, CSS/JS identifiers, presigned URLs and bare digests with the expected keep/redact outcome.
    • Backward compatible (old config / old data / old callers still work, how checked): The shared spelling lives in the import-free credential_patterns module both consumers already import; no public signature changed. test_credential_patterns.py and test_credential_prefilter.py (import-free shape + branch-count + prefilter anchor invariants) pass.
    • Existing tests (which suites passed on this head): test_redaction_key_id_identifier.py, test_security.py, test_log_redaction.py, test_credential_patterns.py, test_credential_prefilter.py — 2023 passed, 1 skipped. black / isort / flake8 / mypy / comment-history / semgrep (baseline merge-base) all clean.
  • How to undo: Revert this commit (acabdcf7d06c92b2a7b6e7b5e878df0f5c780ae5). The change is a single stacked commit over the fixture-only commits; reverting restores the prior AKIA-masking behavior with no data migration.

@bolichen97
bolichen97 force-pushed the feat/redaction-key-id-identifier-22552 branch from acabdcf to 0759670 Compare October 21, 2026 13:36
@bolichen97

Copy link
Copy Markdown
Contributor Author

Sensitive change review

  • Head commit: 0759670
  • What rule changed: AWS access key IDs (AKIA/ASIA) are reclassified to Identifier tier, so no redactor removes them and no exfiltration gate blocks on them. In the same change, the value that makes a presigned/signed link work — the SigV4/SigV2 signature and the session token — is keyed and redacted on every surface (log-redaction floor, deploy/packaging scan, exfil URL rules, frontend sanitize). ABIA/ACCA ids and secret access keys / session tokens remain Secret tier (still redacted).
  • Before -> after (what was allowed/blocked, now): Before, an AKIA/ASIA key ID was masked everywhere, which also broke a presigned link in a log (the whole link was mangled) while the signature could survive under an obfuscated spelling. After, the key ID (a non-secret identifier that cannot sign a request) stays visible so a log line can be joined to the key it names, and the signature/session token is removed so a signed link in a log cannot be replayed.
  • Worst case if wrong (breaks commands / leaks secret / locks user out): If the signing matcher were too narrow, a signed link could survive in a log and be replayed until expiry. If too broad, ordinary prose containing the word "signature" would be over-redacted, hurting readability but leaking nothing. The design keeps the key ID visible by intent; a key ID alone cannot authenticate, so its visibility leaks no secret.
  • Evidence checked (which sandbox result proves it): Local probe over SecretRedactionFilter.redact confirms: .signature:hover, const signature = sign(body), <p>Signature: Jane</p>, data-signature="abc" are untouched; a presigned URL keeps its AKIA... key ID but loses the 64-hex signature; a percent-escaped param name X-Amz-%53ignature=<64hex> on a presigned-marker line is redacted; a bare sha256=<64hex> with no presigned marker is kept; an X-Amz-Security-Token value is redacted.
  • No regression:
    • Tools still work (which tools/commands were run, result): redact() is pure string transformation with zero I/O; the probe above exercised prose, CSS/JS identifiers, presigned URLs and bare digests with the expected keep/redact outcome.
    • Backward compatible (old config / old data / old callers still work, how checked): The shared spelling lives in the import-free credential_patterns module both consumers already import; no public signature changed. test_credential_patterns.py and test_credential_prefilter.py (import-free shape + branch-count + prefilter anchor invariants) pass.
    • Existing tests (which suites passed on this head): test_redaction_key_id_identifier.py, test_security.py, test_log_redaction.py, test_credential_patterns.py, test_credential_prefilter.py — 2023 passed, 1 skipped. black / isort / flake8 / mypy / comment-history / semgrep (baseline merge-base) all clean.
  • How to undo: Revert this commit (07596703f02aa28274f114ad77059b0e814746e0). The change is a single stacked commit over the fixture-only commits; reverting restores the prior AKIA-masking behavior with no data migration.

@bolichen97
bolichen97 force-pushed the feat/redaction-key-id-identifier-22552 branch from 0759670 to bcde9f0 Compare October 21, 2026 13:49
@bolichen97

Copy link
Copy Markdown
Contributor Author

Sensitive change review

  • Head commit: bcde9f0
  • What rule changed: AWS access key IDs (AKIA/ASIA) are reclassified to Identifier tier, so no redactor removes them and no exfiltration gate blocks on them. In the same change, the value that makes a presigned/signed link work — the SigV4/SigV2 signature and the session token — is keyed and redacted on every surface (log-redaction floor, deploy/packaging scan, exfil URL rules, frontend sanitize). ABIA/ACCA ids and secret access keys / session tokens remain Secret tier (still redacted).
  • Before -> after (what was allowed/blocked, now): Before, an AKIA/ASIA key ID was masked everywhere, which also broke a presigned link in a log (the whole link was mangled) while the signature could survive under an obfuscated spelling. After, the key ID (a non-secret identifier that cannot sign a request) stays visible so a log line can be joined to the key it names, and the signature/session token is removed so a signed link in a log cannot be replayed.
  • Worst case if wrong (breaks commands / leaks secret / locks user out): If the signing matcher were too narrow, a signed link could survive in a log and be replayed until expiry. If too broad, ordinary prose containing the word "signature" would be over-redacted, hurting readability but leaking nothing. The design keeps the key ID visible by intent; a key ID alone cannot authenticate, so its visibility leaks no secret.
  • Evidence checked (which sandbox result proves it): Local probe over SecretRedactionFilter.redact confirms: .signature:hover, const signature = sign(body), <p>Signature: Jane</p>, data-signature="abc" are untouched; a presigned URL keeps its AKIA... key ID but loses the 64-hex signature; a percent-escaped param name X-Amz-%53ignature=<64hex> on a presigned-marker line is redacted; a bare sha256=<64hex> with no presigned marker is kept; an X-Amz-Security-Token value is redacted.
  • No regression:
    • Tools still work (which tools/commands were run, result): redact() is pure string transformation with zero I/O; the probe above exercised prose, CSS/JS identifiers, presigned URLs and bare digests with the expected keep/redact outcome.
    • Backward compatible (old config / old data / old callers still work, how checked): The shared spelling lives in the import-free credential_patterns module both consumers already import; no public signature changed. test_credential_patterns.py and test_credential_prefilter.py (import-free shape + branch-count + prefilter anchor invariants) pass.
    • Existing tests (which suites passed on this head): test_redaction_key_id_identifier.py, test_security.py, test_log_redaction.py, test_credential_patterns.py, test_credential_prefilter.py — 2023 passed, 1 skipped. black / isort / flake8 / mypy / comment-history / semgrep (baseline merge-base) all clean.
  • How to undo: Revert this commit (bcde9f06c159fc36f7ba80cf650ba35dcc505905). The change is a single stacked commit over the fixture-only commits; reverting restores the prior AKIA-masking behavior with no data migration.

… signatures

An AWS access key ID (AKIA / ASIA) is Identifier tier: it names a key and
cannot sign a request, so no surface redacts it or blocks on it. The secret
access key and session token beside it stay Secret tier.

Presigned and signed links are Bearer link tier keyed on the signing part:
the signature and session-token parameter values are removed, and a SigV4
request signature is removed wherever it appears, while the key ID inside
the link stays visible.
@bolichen97
bolichen97 force-pushed the feat/redaction-key-id-identifier-22552 branch from bcde9f0 to 289c3b6 Compare October 21, 2026 14:10
@bolichen97

Copy link
Copy Markdown
Contributor Author

Sensitive change review

  • Head commit: 289c3b6
  • What rule changed: AWS access key IDs (AKIA/ASIA) are reclassified to Identifier tier, so no redactor removes them and no exfiltration gate blocks on them. In the same change, the value that makes a presigned/signed link work — the SigV4/SigV2 signature and the session token — is keyed and redacted on every surface (log-redaction floor, deploy/packaging scan, exfil URL rules, frontend sanitize). ABIA/ACCA ids and secret access keys / session tokens remain Secret tier (still redacted).
  • Before -> after (what was allowed/blocked, now): Before, an AKIA/ASIA key ID was masked everywhere, which also broke a presigned link in a log (the whole link was mangled) while the signature could survive under an obfuscated spelling. After, the key ID (a non-secret identifier that cannot sign a request) stays visible so a log line can be joined to the key it names, and the signature/session token is removed so a signed link in a log cannot be replayed.
  • Worst case if wrong (breaks commands / leaks secret / locks user out): If the signing matcher were too narrow, a signed link could survive in a log and be replayed until expiry. If too broad, ordinary prose containing the word "signature" would be over-redacted, hurting readability but leaking nothing. The design keeps the key ID visible by intent; a key ID alone cannot authenticate, so its visibility leaks no secret.
  • Evidence checked (which sandbox result proves it): Local probe over SecretRedactionFilter.redact confirms: .signature:hover, const signature = sign(body), <p>Signature: Jane</p>, data-signature="abc" are untouched; a presigned URL keeps its AKIA... key ID but loses the 64-hex signature; a percent-escaped param name X-Amz-%53ignature=<64hex> on a presigned-marker line is redacted; a bare sha256=<64hex> with no presigned marker is kept; an X-Amz-Security-Token value is redacted.
  • No regression:
    • Tools still work (which tools/commands were run, result): redact() is pure string transformation with zero I/O; the probe above exercised prose, CSS/JS identifiers, presigned URLs and bare digests with the expected keep/redact outcome.
    • Backward compatible (old config / old data / old callers still work, how checked): The shared spelling lives in the import-free credential_patterns module both consumers already import; no public signature changed. test_credential_patterns.py and test_credential_prefilter.py (import-free shape + branch-count + prefilter anchor invariants) pass.
    • Existing tests (which suites passed on this head): test_redaction_key_id_identifier.py, test_security.py, test_log_redaction.py, test_credential_patterns.py, test_credential_prefilter.py — 2023 passed, 1 skipped. black / isort / flake8 / mypy / comment-history / semgrep (baseline merge-base) all clean.
  • How to undo: Revert this commit (289c3b63087f098789f1848a0d25c97634b82feb). The change is a single stacked commit over the fixture-only commits; reverting restores the prior AKIA-masking behavior with no data migration.

@bolichen97
bolichen97 disabled auto-merge October 21, 2026 14:32
@bolichen97
bolichen97 marked this pull request as draft October 21, 2026 14:32
@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 Oct 21, 2026
@bolichen97

Copy link
Copy Markdown
Contributor Author

Superseded by #22887, which carries this change together with the rest of #22552.

@bolichen97 bolichen97 closed this Oct 21, 2026
@github-actions github-actions Bot removed the readiness: action required A blocking check or review needs attention label Oct 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant