fix(security): redact sandboxed CI evidence without secret-bearing history - #711
fix(security): redact sandboxed CI evidence without secret-bearing history#711seonghobae wants to merge 7 commits into
Conversation
|
Warning Review limit reached
Next review available in: 17 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough두 샌드박스 스크립트가 실행 증거와 subprocess 출력을 Changes샌드박스 출력 마스킹
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review Please review the exact current head. Focus on fail-closed redactor import, subprocess output and timeout redaction, service-log and evidence-note redaction, and scanner-safe regression fixtures. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== CURRENT HEAD ==='
git rev-parse HEAD
git status --short
echo '=== PR DIFF STAT ==='
git diff --stat origin/main...HEAD
echo '=== CHANGED FILES ==='
git diff --name-status origin/main...HEAD
echo '=== CHANGED PYTHON/TEST OUTLINES ==='
git diff --name-only origin/main...HEAD \
| rg '\.(py|pyi)$' \
| while IFS= read -r file; do
echo "--- $file"
ast-grep outline "$file" --items all || true
done
echo '=== SECURITY-RELATED DIFF ==='
git diff --unified=80 origin/main...HEAD -- \
'*.py' '*test*' '*Test*'Length of output: 32516 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== SHARED REDACTOR OUTLINE ==='
ast-grep outline scripts/ci/redact_sensitive_log.py --items all
echo '=== SHARED REDACTOR SOURCE ==='
cat -n scripts/ci/redact_sensitive_log.py
echo '=== ALL OUTPUT AND RAISE SITES IN WRAPPERS ==='
rg -n -C 4 \
'(?:\bprint\s*\(|\braise\s+|except\b|json\.dumps|subprocess\.(?:run|Popen)|read_text|open\()' \
scripts/ci/sandboxed_verify.py scripts/ci/sandboxed_web_e2e.py
echo '=== COMPLETE CHANGED WRAPPER SOURCE WITH LINE NUMBERS ==='
cat -n scripts/ci/sandboxed_verify.py
cat -n scripts/ci/sandboxed_web_e2e.py
echo '=== RELATED REDACTION TESTS AND FIXTURE VALUES ==='
rg -n -C 3 \
'(?:redact_text|timeout|TimeoutExpired|SANDBOXED_(?:VERIFY|WEB_E2E)_RESULT|mock_(?:token|session|password)|api_key|session_key|password)' \
tests scripts/ciLength of output: 201165 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== SHARED REDACTOR ==='
cat -n scripts/ci/redact_sensitive_log.py
echo '=== VERIFY TIMEOUT TESTS ==='
rg -n -A45 -B8 \
'def test_(?:main_reports_stubbed.*timeout|sandboxed_verify.*timeout|.*timeout.*redact)' \
tests/test_sandboxed_verify.py tests/test_sandboxed_verify_redaction.py 2>/dev/null || true
echo '=== WEB E2E TIMEOUT TESTS ==='
rg -n -A55 -B8 \
'def test_(?:main_reports_stubbed.*timeout|sandboxed_web_e2e_reports_e2e_timeout|.*timeout.*redact)' \
tests/test_sandboxed_web_e2e.py tests/test_sandboxed_web_e2e_redaction.py 2>/dev/null || true
echo '=== DIRECT SCRIPT/WORKFLOW CALLERS ==='
rg -n -C 5 \
'(?:sandboxed_verify\.py|sandboxed_web_e2e\.py|sandboxed_verify|sandboxed_web_e2e)' \
.github scripts tests \
-g '!scripts/ci/sandboxed_verify.py' \
-g '!scripts/ci/sandboxed_web_e2e.py'
echo '=== URL REDACTION PATTERN CHECK ==='
rg -n -C 4 \
'(?:URL|url|Bearer|token|secret|password|api[_-]?key|assignment|PATTERN|re\.compile)' \
scripts/ci/redact_sensitive_log.pyLength of output: 78615
P1 — Test the actual timeout publication paths
Add a focused test for each wrapper that raises The fail-closed import is correctly ordered after repository-root setup. The inspected normal subprocess output, evidence fields, and service-log tail routes apply the shared redactor. ✅ Action performedReview finished.
|
|
Addressed the current-head blocking regression-coverage finding in 1048987. Both wrappers now have focused tests that drive their actual |
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head3920b2356c78d6ded4f757bcb3af5e9693d95a64. -
Head SHA:
3920b2356c78d6ded4f757bcb3af5e9693d95a64 -
Workflow run: 30801773674
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["CI script (2 files)"]
S1 --> I1["review and security gate shell path"]
I1 --> R1["Review risk: CI script (2 files)"]
R1 --> V1["bash -n plus Strix self-test"]
Evidence --> S2["Test (2 files)"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test (2 files)"]
R2 --> V2["targeted test run"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage Decision
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["CI script (2 files)"]
S1 --> I1["review and security gate shell path"]
I1 --> R1["Review risk: CI script (2 files)"]
R1 --> V1["bash -n plus Strix self-test"]
Evidence --> S2["Test (2 files)"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test (2 files)"]
R2 --> V2["targeted test run"]
|
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head3920b2356c78d6ded4f757bcb3af5e9693d95a64. -
Head SHA:
3920b2356c78d6ded4f757bcb3af5e9693d95a64 -
Workflow run: 30801773674
-
Workflow attempt: 2
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["CI script (2 files)"]
S1 --> I1["review and security gate shell path"]
I1 --> R1["Review risk: CI script (2 files)"]
R1 --> V1["bash -n plus Strix self-test"]
Evidence --> S2["Test (2 files)"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test (2 files)"]
R2 --> V2["targeted test run"]
3920b23 to
07e3e0a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_sandboxed_web_e2e_redaction.py`:
- Around line 23-52: Update test_emit_result_redacts_payload_fields so
FakeArgs.evidence_note contains a sensitive fixture value such as api_key, then
assert that the original value is absent from captured.out while the redaction
marker remains present. Keep the existing command, path, and other
sensitive-value assertions intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 79d1f630-8b66-44b0-9151-ae43e6545cc6
📒 Files selected for processing (4)
scripts/ci/sandboxed_verify.pyscripts/ci/sandboxed_web_e2e.pytests/test_sandboxed_verify_redaction.pytests/test_sandboxed_web_e2e_redaction.py
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head8d84bf6b4a8c15e0321de6821ab3a661bc45b718. -
Head SHA:
8d84bf6b4a8c15e0321de6821ab3a661bc45b718 -
Workflow run: 30874658917
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["CI script (2 files)"]
S1 --> I1["review and security gate shell path"]
I1 --> R1["Review risk: CI script (2 files)"]
R1 --> V1["bash -n plus Strix self-test"]
Evidence --> S2["Test (2 files)"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test (2 files)"]
R2 --> V2["targeted test run"]
|
Closing this pre-baseline branch to reduce CI pressure. The sandbox evidence redaction fix remains in the consolidated post-#731 replay queue and should be replayed from current main with scanner-safe fixtures, fail-closed imports, and exact 100% coverage/docstring evidence. |
Security fix
sys.path.Replacement rationale
This supersedes #631. The prior branch contained credential-shaped fixtures in discarded commits; GitHub Advanced Security correctly continued to flag those commits even after the working tree was cleaned. This branch was rebuilt from current
mainand contains only scanner-safe fixtures, so the security gate can validate the actual final change without retaining the obsolete secret-bearing history.Validation
Current-head GitHub Actions are authoritative. The focused regression tests are included in this PR and no policy or security gate is bypassed.
Summary by CodeRabbit
보안 개선
[REDACTED]처리됩니다.테스트