🛡️ Sentinel: [security improvement] subprocess에 명시적 shell=False 추가

[seonghobae]

보안 정책에 따라 sandboxed_web_e2e.py 파일 내부의 subprocess.run 및 subprocess.Popen 호출에 명시적으로 shell=False 파라미터를 추가하여 쉘 인젝션 등의 보안 취약점을 예방합니다.

---

PR created automatically by Jules for task 18171121124709112021 started by @seonghobae

Summary by CodeRabbit

• Refactor
  • 하위 호환성과 동작에 영향을 주지 않는 내부 코드 정리를 적용했습니다.

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 725d6de4-cff4-46cc-8a1a-11f2ccd84f4e

📥 Commits

Reviewing files that changed from the base of the PR and between fb17ef5 and daf58de.

📒 Files selected for processing (1)

• scripts/ci/sandboxed_web_e2e.py

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

---

📝 Walkthrough

Walkthrough

세 개의 subprocess 호출에서 키워드 인자 순서를 정리했습니다. shell=False를 각 호출의 앞쪽으로 이동했으며 실행 동작은 변경되지 않았습니다.

Changes

subprocess 호출 인자 순서 정리

Layer / File(s)	Summary
subprocess 키워드 인자 순서 변경 scripts/ci/sandboxed_web_e2e.py	격리 기능 확인, 서비스 시작, 셸 실행에 사용하는 subprocess 호출에서 shell=False의 위치를 변경했습니다. 실행 동작은 동일합니다.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~2 minutes

Change: Refactor

Suggested reviewers: cursoragent

Merge Risk: ⚪ Minimal · up to 1b57b

This change preserves subprocess behavior while making the shell-disabled configuration explicit, with no identified merge-blocking risk.

🚥 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	제목은 subprocess 호출에 명시적 shell=False를 추가하는 보안 개선이라는 PR의 주요 목적을 구체적으로 설명합니다.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files.
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 sentinel-explicit-shell-false-18171121124709112021

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[cwl-noema-review]

Noema LLM review

The change moves and duplicates the explicit shell=False argument in three subprocess call sites, making the security posture more visible without altering behavior. Since all removed lines are on the LEFT side, the final argument sets are unchanged; each call still uses shell=False. No regression or security issue was found. The earlier claim of an unchanged AST fingerprint was unsupported and is not relied upon.

Reviewed changed lines

• scripts/ci/sandboxed_web_e2e.py:243 (RIGHT): Added explicit shell=False to the bubblewrap capability probe. The original call already relied on Python's default, so this is declarative and behavior-preserving.
• scripts/ci/sandboxed_web_e2e.py:456 (RIGHT): Added shell=False to the subprocess.Popen call in start_service, moved earlier in the argument list. The effective arguments remain identical to the previous version, preserving shell-free process spawning.
• scripts/ci/sandboxed_web_e2e.py:461 (LEFT): Removed the original shell=False argument from start_service. This removal is paired with the addition at RIGHT 456, so the final call still contains exactly one shell=False.
• scripts/ci/sandboxed_web_e2e.py:597 (RIGHT): Added shell=False to the subprocess.run call in run_shell, ensuring the final code has exactly one explicit shell=False.
• scripts/ci/sandboxed_web_e2e.py:603 (LEFT): Removed the original shell=False argument from run_shell. Alongside the addition at RIGHT 597, this keeps the effective behavior unchanged.

Adversarial validation

• scripts/ci/sandboxed_web_e2e.py:597 (RIGHT) falsified: The final run_shell call contains exactly one shell=False; moving it does not change shell execution semantics. — The diff shows shell=False added at RIGHT 597 and removed at LEFT 603. The final call has exactly one explicit shell=False, matching prior behavior.
• scripts/ci/sandboxed_web_e2e.py:243 (RIGHT) falsified: Adding explicit shell=False to the bubblewrap probe preserves prior behavior (Python default). — The original call already relied on the default shell=False; the addition is declarative and does not alter other parameters or logic.
• scripts/ci/sandboxed_web_e2e.py:456 (RIGHT) falsified: start_service retains shell=False after the move, keeping shell-free spawning. — The added shell=False at RIGHT 456 mirrors the removed LEFT 461. The final subprocess.Popen call remains unchanged in effective arguments.
• Residual risk: No residual risk identified. The changes are purely declarative; Python defaults to shell=False and the explicit keyword matches prior behavior. All subprocess calls remain shell-free.

Findings

• No blocking findings.

• Result: APPROVE
• Head SHA: 008f3915606051adc32708e349fe32619256ae52
• Reviewer credential: noema-review-github-app-refresh
• Actor: cwl-noema-review[bot]

[opencode-agent]

Pull request overview

OpenCode reviewed the current-head product diff. Coverage is a separate gate.

Changed files

• scripts/ci/sandboxed_web_e2e.py — review and security gate shell path

Changed behavior

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["CI script: sandboxed_web_e2e.py"]
  S1 --> I1["review and security gate shell path"]
  I1 --> R1["Review risk: CI script: sandboxed_web_e2e.py"]
  R1 --> V1["bash -n plus Strix self-test"]

Loading

Findings

No source-backed product finding is synthesized from the coverage gate. A coverage miss belongs in the status comment.

• Head SHA: 1b57bc4fbbb31b16461dc805fb60d2d58fdf4382
• Workflow run: 35329633360
• Workflow attempt: 1
• Coverage gate: failure

Review outcome

Coverage is a gate, not the review. This body reviews the changed product files.

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["CI script: sandboxed_web_e2e.py"]
  S1 --> I1["review and security gate shell path"]
  I1 --> R1["Review risk: CI script: sandboxed_web_e2e.py"]
  R1 --> V1["bash -n plus Strix self-test"]

Loading

[opencode-agent]

OpenCode Review Overview

• Head SHA: 1b57bc4fbbb31b16461dc805fb60d2d58fdf4382
• Workflow run: 35329633360
• Workflow attempt: 1
• Gate result: COVERAGE_BLOCKED
• Coverage gate: failure
• Model pool: unknown
• Verdict: COVERAGE_BLOCKED
• Formal review: 🛡️ Sentinel: [security improvement] subprocess에 명시적 shell=False 추가 #2134 (review)

Coverage evidence did not pass, so approval is blocked. The formal pull-request review is the source-backed diff review, not this status comment.

[seonghobae]

Exact-head admission audit: 1b57bc4fbbb31b16461dc805fb60d2d58fdf4382 (base main@64aa08d7fa487deacd41c761c36277ca68cab6c9, 4 ahead / 0 behind).

현재 blocker: 활성 CHANGES_REQUESTED 1건; terminal workflow: SAST Semgrep:failure, Python Security:failure, CodeQL PR:cancelled.

유효 commit·diff·review evidence를 보존한 채 Draft/Proposed로 교정합니다. Base 이동이나 queue 대기만을 이유로 Close하지 않으며, Force Push·synthetic status/approval·manual rerun·bypass는 사용하지 않습니다. Blocker 수리 후 새 exact head에서 Checks와 review admission을 다시 받아야 합니다.

[seonghobae]

Closing as a duplicate of the semantic no-op already decided in #1390 (and #2129): subprocess.run/Popen default to shell=False, and _probe_isolation_capability passes an argv list, so adding the keyword changes no behavior or security boundary. start_service/run_shell on main already pass shell=False explicitly. Branch and commits remain recoverable; reopen if a reproduced shell-execution path is found.