Skip to content

feat(ci): add a bounded subprocess primitive - #1280

Open
seonghobae wants to merge 9 commits into
mainfrom
codex/pr931-bounded-subprocess-core-20260824
Open

feat(ci): add a bounded subprocess primitive#1280
seonghobae wants to merge 9 commits into
mainfrom
codex/pr931-bounded-subprocess-core-20260824

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Outcome

First clean current-main layer replacing the monolithic #931.

  • continuously drains stdout and stderr into fixed-size final-suffix buffers
  • isolates commands in a POSIX process group and kills on overflow or timeout
  • kills the group again after the direct child exits so same-group descendants cannot retain inherited pipes
  • bounds every reader join and preserves sibling finalization after an error
  • keeps decoded UTF-8 evidence inside the declared byte budget even when replacement characters expand
  • provides bounded file-suffix reads without changing repository artifact semantics

This layer intentionally does not change sandboxed_verify, sandboxed_web_e2e, result schemas, workspace symlink policy, or the pip-audit lock. Those consumers remain separate stack layers.

Evidence

  • TDD reproduced both delayed escaped-descendant sentinel execution and UTF-8 replacement expansion before the fix
  • 25 focused tests passed; bounded_subprocess has 226 statements and 52 branches at 100%
  • full suite: 1,422 passed, 1 skipped, 16 subtests passed
  • full production total: 8,572 statements and 3,266 branches at 100%
  • scripts/ci docstrings: 100%
  • compileall and git diff --check: pass
  • APA 7th doctoring cites CWE-770 v4.20, Python 3.14.6 subprocess guidance, NIST SP 800-218, and POSIX.1-2024 process-group definitions

Part 1 of the clean replacement for #931.


Open in Devin Review

Summary by CodeRabbit

  • 새로운 기능

    • 검증 명령과 웹 E2E 실행에 출력 크기 제한과 UTF-8 로그 처리를 추가했습니다.
    • 타임아웃 또는 출력 초과 시 프로세스와 하위 프로세스를 정리합니다.
    • 실행 결과에 출력 제한, 캡처 실패, 경로 경계 거부 상태를 명확히 표시합니다.
  • 버그 수정

    • 외부를 가리키는 심볼릭 링크, 잘못된 URL, 누락되거나 실행 불가능한 명령을 안전하게 처리합니다.
    • 오류 발생 시 민감한 호스트 경로와 상세 트레이스백 노출을 방지합니다.
  • 문서

    • 출력 리소스 제한 및 심볼릭 링크 검증 정책을 문서화했습니다.

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

Next included review available in 7 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: e2c8eb6c-8603-4b41-8073-acce7cebec02

📥 Commits

Reviewing files that changed from the base of the PR and between 39e30cb and e587c05.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • scripts/ci/sandboxed_verify.py
  • scripts/ci/sandboxed_web_e2e.py
  • tests/test_sandboxed_verify_output_limits.py
  • tests/test_sandboxed_web_e2e_branch_contract.py

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4cf0b0d7-5712-4957-b4d3-1acff38acdbe

📥 Commits

Reviewing files that changed from the base of the PR and between 93b9cbb and 39e30cb.

📒 Files selected for processing (18)
  • CHANGELOG.md
  • docs/doctoring/sandboxed-output-resource-bounds.md
  • docs/doctoring/sandboxed-verification-symlink-boundary.md
  • scripts/ci/bounded_subprocess.py
  • scripts/ci/sandboxed_verify.py
  • scripts/ci/sandboxed_web_e2e.py
  • tests/test_bounded_subprocess.py
  • tests/test_bounded_subprocess_capture_startup.py
  • tests/test_bounded_subprocess_contract.py
  • tests/test_repository_branch_coverage_execution_sandboxes.py
  • tests/test_sandboxed_entrypoint_and_cleanup_coverage.py
  • tests/test_sandboxed_service_capture_startup.py
  • tests/test_sandboxed_verify.py
  • tests/test_sandboxed_verify_output_limits.py
  • tests/test_sandboxed_verify_symlink_boundary.py
  • tests/test_sandboxed_web_e2e.py
  • tests/test_sandboxed_web_e2e_branch_contract.py
  • tests/test_sandboxed_web_e2e_output_limits.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

POSIX bounded subprocess 계층을 추가했다. 검증 및 웹 E2E 실행에 출력 한도, 프로세스 그룹 정리, UTF-8 suffix, 심볼릭 링크 경계, 실행 오류 분류를 적용했다. 관련 결과 필드와 회귀 테스트도 추가했다.

Changes

샌드박스 실행 경계

Layer / File(s) Summary
bounded subprocess 계층
scripts/ci/bounded_subprocess.py, tests/test_bounded_subprocess*.py, docs/doctoring/sandboxed-output-resource-bounds.md
stdout/stderr를 동시에 읽고 제한된 suffix를 보존한다. 출력 초과와 timeout에서 프로세스 그룹을 종료한다. UTF-8 디코딩, reader 오류, 캡처 초기화 및 정리 실패를 처리한다.
sandboxed_verify 경계와 결과 처리
scripts/ci/sandboxed_verify.py, tests/test_sandboxed_verify*.py, docs/doctoring/sandboxed-verification-symlink-boundary.md
검증 명령을 bounded subprocess로 실행한다. 외부 심볼릭 링크를 거부한다. 명령 누락, 실행 불가, 출력 제한, timeout을 별도 결과로 기록한다.
웹 E2E 서비스 실행과 readiness
scripts/ci/sandboxed_web_e2e.py, tests/test_sandboxed_web_e2e*.py, tests/test_sandboxed_entrypoint_and_cleanup_coverage.py, tests/test_sandboxed_service_capture_startup.py
서비스 로그와 E2E 출력을 제한한다. 비HTTP readiness URL과 실행 파일 오류를 사전에 분류한다. 서비스 로그 초과와 capture 정리 실패를 결과에 기록한다. 관련 테스트 모킹과 분기 검증을 갱신한다.

Estimated code review effort: 5 (Critical) | ~120 minutes

Merge Risk: ⚪ Minimal · up to 39e30

This change adds bounded subprocess and file-suffix handling with no actionable merge-blocking risk remaining after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant sandboxed_web_e2e
  participant BackendService
  participant FrontendService
  participant bounded_subprocess
  sandboxed_web_e2e->>BackendService: bounded 서비스 시작
  sandboxed_web_e2e->>FrontendService: bounded 서비스 시작
  sandboxed_web_e2e->>BackendService: readiness 확인
  sandboxed_web_e2e->>FrontendService: readiness 확인
  sandboxed_web_e2e->>bounded_subprocess: E2E 명령 실행
  bounded_subprocess-->>sandboxed_web_e2e: 제한된 stdout/stderr와 종료 상태
  sandboxed_web_e2e->>BackendService: 종료 및 capture 정리
  sandboxed_web_e2e->>FrontendService: 종료 및 capture 정리
Loading

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 제목은 PR의 핵심 변경 사항인 bounded subprocess primitive 추가를 간결하고 명확하게 설명합니다.
Docstring Coverage ✅ Passed Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
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 codex/pr931-bounded-subprocess-core-20260824

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.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@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 could not approve from deterministic current-head evidence because GitHub Checks have failed.

Findings

1. HIGH Current-head GitHub Checks - Fix failed required checks before approval

  • Problem: Failed same-head checks remain for 88f5fcc62671ca6e635be05a9ab583adf7399c7a.
  • Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
  • Fix: Read and fix the failed check logs below, then rerun the current-head checks.
  • Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.

Failed checks:

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file: CHANGELOG.md"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file: CHANGELOG.md"]
  R1 --> V1["required checks"]
  Evidence --> S2["Docs: sandboxed-output-resource-bounds.md"]
  S2 --> I2["operator or user guidance"]
  I2 --> R2["Review risk: Docs: sandboxed-output-resource-bounds.md"]
  R2 --> V2["docs review"]
  Evidence --> S3["CI script: bounded_subprocess.py"]
  S3 --> I3["review and security gate shell path"]
  I3 --> R3["Review risk: CI script: bounded_subprocess.py"]
  R3 --> V3["bash -n plus Strix self-test"]
  Evidence --> S4["Test (3 files)"]
  S4 --> I4["regression suite"]
  I4 --> R4["Review risk: Test (3 files)"]
  R4 --> V4["targeted test run"]
Loading

@opencode-agent

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

  • Head SHA: 88f5fcc62671ca6e635be05a9ab583adf7399c7a
  • Workflow run: 32664786582
  • Workflow attempt: 1
  • Gate result: REQUEST_CHANGES (approval step)

Pull request overview

OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.

Findings

1. HIGH Current-head GitHub Checks - Fix failed required checks before approval

  • Problem: Failed same-head checks remain for 88f5fcc62671ca6e635be05a9ab583adf7399c7a.
  • Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
  • Fix: Read and fix the failed check logs below, then rerun the current-head checks.
  • Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.

Failed checks:

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file: CHANGELOG.md"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file: CHANGELOG.md"]
  R1 --> V1["required checks"]
  Evidence --> S2["Docs: sandboxed-output-resource-bounds.md"]
  S2 --> I2["operator or user guidance"]
  I2 --> R2["Review risk: Docs: sandboxed-output-resource-bounds.md"]
  R2 --> V2["docs review"]
  Evidence --> S3["CI script: bounded_subprocess.py"]
  S3 --> I3["review and security gate shell path"]
  I3 --> R3["Review risk: CI script: bounded_subprocess.py"]
  R3 --> V3["bash -n plus Strix self-test"]
  Evidence --> S4["Test (3 files)"]
  S4 --> I4["regression suite"]
  I4 --> R4["Review risk: Test (3 files)"]
  R4 --> V4["targeted test run"]
Loading

@seonghobae
seonghobae enabled auto-merge (squash) August 24, 2026 02:55
* fix(sandbox): bound verification evidence

* fix(sandbox): classify missing verification executable

* fix(sandbox): classify non-executable commands
seonghobae and others added 2 commits August 23, 2026 20:05
* fix(sandbox): bound verification evidence

* fix(sandbox): bound web E2E evidence

* fix(sandbox): classify missing verification executable

* fix(sandbox): classify missing web executables

* fix(sandbox): classify non-executable commands

* fix(sandbox): reject invalid readiness URL schemes
devin-ai-integration[bot]

This comment was marked as resolved.

@opencode-agent
opencode-agent Bot disabled auto-merge August 24, 2026 03:19
@seonghobae
seonghobae enabled auto-merge (squash) August 24, 2026 03:33
@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-head local evidence for e587c05627a7fb2b45f65375c0c1bc752eb9bde2 after merging main@0c6b9a6459c9dbdf5e23fb01df7a32a8a14964b3: 1,480 tests passed, 1 skipped, 16 subtests; statement and branch coverage are 100%; production docstrings are 100%; ruff, compileall, and git diff --check passed. Hosted required checks and a fresh exact-head review remain pending; no predecessor approval is reused.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@opencode-agent review | Independent exact-current-head formal review requested for e587c05627a7fb2b45f65375c0c1bc752eb9bde2. Read the full current diff and current-head checks only; do not reuse predecessor approvals or hosted evidence. Report changed-file walkthrough, security/control-plane impact, test gaps, residual uncertainty, and a structured verdict.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-head RCA for e587c05: Strix run 32691970400 failed closed after repeated NVIDIA NIM HTTP 429 responses, then the distinct fallback returned HTTP 404; no vulnerability report artifact was produced. This is provider/backend infrastructure evidence, not a source finding in the bounded subprocess change. Keep the fail-closed gate and rerun the failed Strix job normally.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head re-review request\n\nCurrent exact head: e587c05\n\nThe previous REQUEST_CHANGES review is tied to an older Strix/provider-failure head. Current-head evidence must be re-established from this SHA; predecessor failures and approvals are not being reused. Please inspect the complete current diff and publish a substantive review after the current required Checks finish. No merge is requested until exact-head checks and independent approval are current.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-head verification receipt

  • Exact head: e587c05627a7fb2b45f65375c0c1bc752eb9bde2
  • Exact base: 0c6b9a6459c9dbdf5e23fb01df7a32a8a14964b3
  • Verified merge-result tree: 2df7dde15c121e3f6c5c77867c0d3249edb5991e (equal to exact head tree)
  • Focused bounded-subprocess contract: 25 passed
  • Full suite: 1480 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, and current OpenCode review remains pending.
  • Decision: WAIT_AND_REMEDIATE until current-head 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 e587c05. Please inspect the complete changed-file diff, with emphasis on bounded subprocess behavior, timeout/cancellation, and authority boundary. Publish a substantive Reviews API verdict tied only to this exact SHA, including security/control-plane impact, test gaps, and residual uncertainty. Do not reuse predecessor-head evidence or failed provider checks as source findings. @opencode-agent

@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-head e587c05627a7fb2b45f65375c0c1bc752eb9bde2 contains the previously reviewed bounded-output and reader-error fixes. The remaining terminal Strix failure is being rerun normally; no source finding is established from the provider gate. Current-head required Checks and an independent substantive approval remain required before merge.

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