Skip to content

perf(redaction): skip invalid key rescans without masking diagnostics - #1264

Open
seonghobae wants to merge 2 commits into
mainfrom
fix/redaction-diagnostic-fidelity-linear-scan
Open

perf(redaction): skip invalid key rescans without masking diagnostics#1264
seonghobae wants to merge 2 commits into
mainfrom
fix/redaction-diagnostic-fidelity-linear-scan

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

What

  • Reuse the existing sensitive-key grammar and assignment parser, but jump directly to regex candidates instead of invoking Python parsing at every character.
  • Advance past an invalid key after one bounded parse, so repeated sensitive substrings cannot trigger quadratic rescans.
  • Preserve benign diagnostics such as token expired at: 2026-01-01.

Why

PR #1154 accumulated seven files of conflicting scope and a deliberate diagnostic-fidelity regression. This clean protected-main replacement keeps only the source-backed redaction optimization and its regressions.

Verification

  • 1,394 passed, 1 skipped, 16 subtests passed
  • 8,361 statements and 3,224 branches at 100% coverage
  • docstrings: 100%
  • compileall and git diff --check: pass
  • local 2.24 MB ordinary-log sample: about 2.9 seconds on protected main and 0.6 seconds on this branch

Scope

Supersedes #1154. No scheduler, dependency, workflow, or documentation behavior is changed.


Open in Devin Review

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 31 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8ff1af9f-40ce-4d9c-9dc6-0dda3313836c

📥 Commits

Reviewing files that changed from the base of the PR and between 613a33e and a32e394.

📒 Files selected for processing (2)
  • scripts/ci/redact_sensitive_log.py
  • tests/test_opencode_security_boundaries.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

devin-ai-integration[bot]

This comment was marked as resolved.

seonghobae added a commit that referenced this pull request Aug 23, 2026
Record #1263 as the G-03 main-line Strix successor, add #1264, and
keep GitHub BLOCKED/CLEAN as snapshot metadata rather than merge
authorization. No `.github` PR merged this pass.

Copy link
Copy Markdown
Contributor Author

@opencode-agent @cwl-noema-review current-head review for 8f20ac3600d289e0bffb0a58cb6931cadf76e717.

Bounded redaction skip of invalid-key quadratic rescans without masking benign diagnostics. tests/test_opencode_security_boundaries.py passed on this head (37 tests). Independent current-head approval is required.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Independent current-head review request for exact HEAD cbc5852b25634cb333a32da1a89de9825cb24802.

G-03: required Strix fail-closes 0-vuln MODEL QUALITY WARNING until #1263 lands. Predecessor-head reviews are not merge evidence. Do not self-approve. COPILOT_GITHUB_TOKEN is not used.

@devin-ai-integration devin-ai-integration Bot 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.

Devin Review found 1 new potential issue.

Open in Devin Review

Comment thread scripts/ci/redact_sensitive_log.py
@seonghobae
seonghobae enabled auto-merge (squash) August 24, 2026 02:17
@seonghobae

Copy link
Copy Markdown
Contributor Author

@opencode-agent review

Review exact current head 98c5151 against protected main@0c6b9a6459c9dbdf5e23fb01df7a32a8a14964b3. Walk through redaction diagnostics, invalid-key handling, security boundaries, and regression coverage; publish a substantive Reviews API verdict on this SHA.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-head verification receipt

  • Exact head: 98c51519c7ffec7e083909bf834e847469ff4b35
  • Exact base: 0c6b9a6459c9dbdf5e23fb01df7a32a8a14964b3
  • Verified merge-result tree: 6478e04363666536f8bdc8c45e6f650fa250e8b9 (equal to exact head tree)
  • Focused security regression: 37 passed
  • Full suite: 1398 passed, 1 skipped, 16 subtests
  • Production statement and branch coverage: 100%
  • Public docstrings: 100%
  • Ruff, Python compile, diff check, and interrogate: passed
  • Hosted current evidence: CodeQL, coverage, dependency, OSV, Noema, scheduler, and Trivy checks passed; Strix failure is the known provider-only NIM/OpenAI fallback failure on the predecessor run, and current OpenCode review remains pending.
  • Decision: WAIT_AND_REMEDIATE until current-head required review/Strix gates are terminal; normal auto-merge remains enabled.

No source, workflow, ruleset, protected-branch, or bypass mutation was used.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-head formal review request for 98c5151. Inspect the complete current diff, especially linear redaction scan behavior and diagnostic preservation. Publish a substantive Reviews API verdict tied only to this exact SHA, with changed-file walkthrough, security/control-plane impact, test gaps, and residual uncertainty. Do not reuse predecessor evidence. @opencode-agent

@seonghobae

Copy link
Copy Markdown
Contributor Author

Strix on exact head 98c5151 failed closed after the scan emitted no authoritative report: the NVIDIA NIM path was classified as provider infrastructure failure and the configured openai-direct fallback returned HTTP 404. No vulnerability finding or source-test failure is present in the run log; all other required checks are green. I am rerunning the failed Strix job through the normal workflow path. This PR remains blocked until a current authoritative Strix result and the normal review gate are available.

@seonghobae
seonghobae force-pushed the fix/redaction-diagnostic-fidelity-linear-scan branch from 98c5151 to a32e394 Compare August 24, 2026 09:29
@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head review loop

  • Pull request: perf(redaction): skip invalid key rescans without masking diagnostics #1264
  • Exact head checked: a32e394af3effca5c93a759912ad9f112a50a079
  • The digit-prefixed sensitive-key finding is already fixed in the current tree: the bounded candidate scan advances past leading digits before parsing, and the regression suite covers both 9password=value redaction and 9safe=value preservation.
  • Current exact-head Checks are successful for the feature gate; no new source edit is warranted.

Please publish a fresh substantive current-head review and eligible approval. The PR remains unmerged until the protected review gate is satisfied.

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