Skip to content

fix(security): redact sandboxed CI evidence without secret-bearing history - #711

Closed
seonghobae wants to merge 7 commits into
mainfrom
automation/clean-ci-redaction-history
Closed

fix(security): redact sandboxed CI evidence without secret-bearing history#711
seonghobae wants to merge 7 commits into
mainfrom
automation/clean-ci-redaction-history

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Security fix

  • Redact subprocess stdout, stderr, timeout output, service-log tails, command arguments, paths, and evidence notes before publishing CI evidence.
  • Import the shared redactor fail-closed after establishing the repository root on sys.path.
  • Add regression coverage for both the generic verification wrapper and the web-E2E wrapper.

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 main and 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

  • 보안 개선

    • 샌드박스 검증 및 웹 E2E 실행 결과에서 API 키, 세션 키, 비밀번호 등 민감한 정보가 자동으로 [REDACTED] 처리됩니다.
    • 명령어, 경로, 환경 변수명, 표준 출력·오류, 서비스 로그 및 타임아웃 메시지에도 마스킹이 적용됩니다.
  • 테스트

    • 일반 실행, 타임아웃, subprocess 및 로그 출력 상황에서 민감정보가 노출되지 않는지 검증하는 회귀 테스트를 추가했습니다.

@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 17 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 20ec6281-b662-4f9f-b98e-a71a9a2c2152

📥 Commits

Reviewing files that changed from the base of the PR and between 07e3e0a and 8d84bf6.

📒 Files selected for processing (1)
  • tests/test_sandboxed_web_e2e_redaction.py
📝 Walkthrough

Walkthrough

두 샌드박스 스크립트가 실행 증거와 subprocess 출력을 redact_text로 마스킹합니다. 정상 실행, 타임아웃, 결과 페이로드, 서비스 로그에 대한 회귀 테스트를 추가했습니다.

Changes

샌드박스 출력 마스킹

Layer / File(s) Summary
검증 샌드박스 출력 마스킹
scripts/ci/sandboxed_verify.py, tests/test_sandboxed_verify_redaction.py
실행 명령, 경로, 메모, 환경 변수명, 표준 출력, 표준 오류, 타임아웃 출력을 마스킹합니다. 관련 테스트가 바이트·문자열 출력과 종료 코드 124를 검증합니다.
웹 E2E 출력 마스킹
scripts/ci/sandboxed_web_e2e.py, tests/test_sandboxed_web_e2e_redaction.py
결과 필드, 상태 메시지, E2E 출력, 타임아웃 출력, 서비스 로그 tail을 마스킹합니다. 관련 테스트가 API 키, 세션 키, 비밀번호의 원문 비노출을 검증합니다.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 샌드박스 CI 증거에서 민감정보를 제거하는 주요 변경 사항을 정확하고 간결하게 설명합니다.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch automation/clean-ci-redaction-history

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

Copy link
Copy Markdown
Contributor Author

@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.

@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown
🧩 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/ci

Length 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.py

Length of output: 78615


@seonghobae I found one blocking regression-coverage gap.

P1 — Test the actual timeout publication paths

tests/test_sandboxed_verify_redaction.py Line 16 tests timeout_output_text() and redact_text() as separate functions. tests/test_sandboxed_web_e2e_redaction.py has no sensitive subprocess.TimeoutExpired case. Therefore, both tests still pass if a later change removes redact_text(...) from the timeout handlers in scripts/ci/sandboxed_verify.py or scripts/ci/sandboxed_web_e2e.py.

Add a focused test for each wrapper that raises subprocess.TimeoutExpired with sensitive stdout and stderr. Assert that neither original value occurs in captured output. Use the existing fragment-built fixtures to keep the test sources scanner-safe.

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 performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor Author

Addressed the current-head blocking regression-coverage finding in 1048987. Both wrappers now have focused tests that drive their actual subprocess.TimeoutExpired handlers with scanner-safe sensitive stdout/stderr fixtures and assert the original values are absent from published output. Awaiting current-head required checks and independent review; no gate is being bypassed.

@opencode-agent opencode-agent 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.

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 success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before 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 head 3920b2356c78d6ded4f757bcb3af5e9693d95a64.

  • 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"]
Loading

@opencode-agent

opencode-agent Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

  • Head SHA: 8d84bf6b4a8c15e0321de6821ab3a661bc45b718
  • Workflow run: 30874658917
  • Workflow attempt: 1
  • Gate result: REQUEST_CHANGES (approval step)

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 success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before 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 head 8d84bf6b4a8c15e0321de6821ab3a661bc45b718.

  • 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"]
Loading

@opencode-agent
opencode-agent Bot disabled auto-merge August 3, 2026 09:37
@seonghobae seonghobae closed this Aug 3, 2026
@seonghobae seonghobae reopened this Aug 3, 2026
@seonghobae
seonghobae enabled auto-merge (squash) August 3, 2026 10:18
@opencode-agent
opencode-agent Bot disabled auto-merge August 3, 2026 10:21

@opencode-agent opencode-agent 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.

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 success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before 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 head 3920b2356c78d6ded4f757bcb3af5e9693d95a64.

  • 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"]
Loading

@opencode-agent
opencode-agent Bot force-pushed the automation/clean-ci-redaction-history branch from 3920b23 to 07e3e0a Compare August 3, 2026 19:57

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between 3f65dbe and 07e3e0a.

📒 Files selected for processing (4)
  • scripts/ci/sandboxed_verify.py
  • scripts/ci/sandboxed_web_e2e.py
  • tests/test_sandboxed_verify_redaction.py
  • tests/test_sandboxed_web_e2e_redaction.py

Comment thread tests/test_sandboxed_web_e2e_redaction.py

@opencode-agent opencode-agent 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.

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 success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before 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 head 8d84bf6b4a8c15e0321de6821ab3a661bc45b718.

  • 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"]
Loading

Copy link
Copy Markdown
Contributor Author

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.

@seonghobae seonghobae closed this Aug 4, 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