Skip to content

πŸ›‘οΈ Sentinel: [MEDIUM] Fix 정보 유좜 취약점 (Information Leakage in Exceptions)#490

Closed
seonghobae wants to merge 2 commits into
developfrom
sentinel-fix-exception-leak-4133249976080958092
Closed

πŸ›‘οΈ Sentinel: [MEDIUM] Fix 정보 유좜 취약점 (Information Leakage in Exceptions)#490
seonghobae wants to merge 2 commits into
developfrom
sentinel-fix-exception-leak-4133249976080958092

Conversation

@seonghobae

Copy link
Copy Markdown
Collaborator

🚨 Severity: MEDIUM
πŸ’‘ Vulnerability: API 및 CLI μ˜ˆμ™Έ 처리 λΈ”λ‘μ—μ„œ FileNotFoundError λ“±κ³Ό 같이 μ—λŸ¬ λ©”μ‹œμ§€λ₯Ό νŒŒμ‹±ν•˜μ—¬ μ‘λ‹΅μœΌλ‘œ 내렀쀄 λ•Œ μ„œλ²„μ˜ λ‚΄λΆ€ 경둜, 파일λͺ… λ“± μ‹œμŠ€ν…œ λ‚΄λΆ€μ˜ λ―Όκ°ν•œ ꡬ쑰가 외뢀에 κ·ΈλŒ€λ‘œ λ…ΈμΆœλ  μœ„ν—˜μ΄ μžˆμ—ˆμŠ΅λ‹ˆλ‹€.
🎯 Impact: κ³΅κ²©μžκ°€ μ„œλ²„ λ‚΄μ˜ 디렉토리 ꡬ쑰 및 운영 체제 ν™˜κ²½ λ“± μ€‘μš” 정보λ₯Ό νŒŒμ•…ν•  수 μžˆλŠ” 정보 유좜(Information Leakage)이 λ°œμƒν•  수 μžˆμŠ΅λ‹ˆλ‹€.
πŸ”§ Fix: μ˜ˆμ™Έκ°€ λ°œμƒν•  λ•Œ str(error)λ‚˜ μ˜ˆμ™Έ 객체 자체λ₯Ό 응닡에 μ‚½μž…ν•˜μ§€ μ•Šκ³ , μ‹œμŠ€ν…œ μƒνƒœλ₯Ό λ…ΈμΆœν•˜μ§€ μ•ŠλŠ” 일반적이고 μ•ˆμ „ν•œ λ©”μ‹œμ§€λ‘œ λ³€κ²½ν–ˆμŠ΅λ‹ˆλ‹€.
βœ… Verification: uv run pytest tests/λ₯Ό 톡해 λ³€κ²½λœ λ©”μ‹œμ§€ ꡬ쑰가 ν…ŒμŠ€νŠΈλ₯Ό ν†΅κ³Όν•˜λŠ”μ§€ ν™•μΈν–ˆμœΌλ©°, npm run check λͺ…λ ΉμœΌλ‘œ ν”„λ‘ νŠΈμ—”λ“œ 및 전체 λΉŒλ“œμ— μ•…μ˜ν–₯을 μ£Όμ§€ μ•Šμ•˜μŒμ„ ν™•μΈν–ˆμŠ΅λ‹ˆλ‹€.


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

파이썬 μ„œλΉ„μŠ€μ˜ μ˜ˆμ™Έ 처리(Exception handling) λΆ€λΆ„μ—μ„œ λ‚΄λΆ€ 파일 경둜 λ“±μ˜ λ―Όκ°ν•œ 정보가 μ‚¬μš©μžμ—κ²Œ λ°˜ν™˜λ  수 μžˆλŠ” 잠재적인 정보 유좜 취약점을 μˆ˜μ •ν–ˆμŠ΅λ‹ˆλ‹€. μ˜ˆμ™Έ λ°œμƒ μ‹œ μ—λŸ¬μ˜ λ‚΄μš©μ„ κ·ΈλŒ€λ‘œ λ…ΈμΆœν•˜μ§€ μ•Šκ³ , μ‚¬μš©μž μΉœν™”μ μ΄λ©΄μ„œ μ•ˆμ „ν•œ 일반적인 μ•ˆλ‚΄ λ©”μ‹œμ§€λ‘œ μ‘λ‹΅ν•˜λ„λ‘ ν•˜μ˜€μŠ΅λ‹ˆλ‹€.

- `services/analysis-engine/src/bandscope_analysis/api.py`: 843번째 쀄 μ˜ˆμ™Έ 처리 λ©”μ‹œμ§€ μˆ˜μ •
- `services/analysis-engine/src/bandscope_analysis/cli.py`: 49~50번째 쀄 μ˜ˆμ™Έ 처리 λ©”μ‹œμ§€ μˆ˜μ •
@google-labs-jules

Copy link
Copy Markdown

πŸ‘‹ 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.

Copilot AI review requested due to automatic review settings July 1, 2026 02:46

Copilot AI 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.

Pull request overview

Mitigates potential information leakage by avoiding raw exception-string interpolation in a couple of user-facing error paths within the analysis engine (CLI + stem-separation worker), and updates tests/docs to reflect the new behavior.

Changes:

  • Replace CLI β€œread job file” failure output to avoid including the original exception text in stdout JSON.
  • In the stem-separation child worker, replace the catch-all Exception payload with a generic safe message.
  • Update worker error-mapping tests and add a Sentinel learning note.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
services/analysis-engine/tests/test_api.py Updates stem-separation worker tests to assert the new generic message for catch-all exceptions.
services/analysis-engine/src/bandscope_analysis/cli.py Stops embedding exception text in the CLI failure response when reading a job file fails.
services/analysis-engine/src/bandscope_analysis/api.py Changes worker catch-all exception handling to return a generic safe message.
.Jules/sentinel.md Documents the information-leakage lesson and prevention guidance.

πŸ’‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +843 to +844
except Exception:
result_queue.put(("runtime_error", "An unexpected error occurred during stem separation."))
Comment on lines +49 to 55
except Exception:
msg = (
"Failed to read job file. "
"Please ensure the file path is correct and accessible."
)
json.dump(failed_cli_response(msg), sys.stdout)
return 1
@opencode-agent

opencode-agent Bot commented Jul 1, 2026

Copy link
Copy Markdown

OpenCode Review Overview

  • Head SHA: 210117bf8f511682d0a029fd1865487d11c7cd02
  • Workflow run: 28489965270
  • Workflow attempt: 1
  • Gate result: REQUEST_CHANGES (approval step)

Pull request overview

OpenCode reviewed the current-head evidence but found unresolved human review threads before approval.

Findings

1. HIGH .github/workflows/opencode-review.yml:1 - Unresolved human review thread blocks automated approval

  • Problem: OpenCode reached an APPROVE control result, but the approval step found unresolved, non-outdated human review thread evidence on the current pull request.
  • Root cause: Human review feedback can arrive after bounded model evidence is prepared, so the approval step must re-query GitHub immediately before publishing an approval.
  • Fix: Address or resolve the listed human review thread(s), then re-run OpenCode on the current head.
  • Regression test: Keep the approval gate querying reviewThreads(first: 100) after model output and before create_pull_review APPROVE.

Review thread evidence

Latest unresolved human review thread evidence

services/analysis-engine/src/bandscope_analysis/api.py line 844

services/analysis-engine/src/bandscope_analysis/cli.py line 55

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (3 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (3 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Test: test_api.py"]
  S2 --> I2["regression suite"]
  I2 --> R2["Review risk: Test: test_api.py"]
  R2 --> V2["targeted test run"]
Loading

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

Pull request overview

OpenCode reviewed the current-head evidence but found unresolved human review threads before approval.

Findings

1. HIGH .github/workflows/opencode-review.yml:1 - Unresolved human review thread blocks automated approval

  • Problem: OpenCode reached an APPROVE control result, but the approval step found unresolved, non-outdated human review thread evidence on the current pull request.
  • Root cause: Human review feedback can arrive after bounded model evidence is prepared, so the approval step must re-query GitHub immediately before publishing an approval.
  • Fix: Address or resolve the listed human review thread(s), then re-run OpenCode on the current head.
  • Regression test: Keep the approval gate querying reviewThreads(first: 100) after model output and before create_pull_review APPROVE.

Review thread evidence

Latest unresolved human review thread evidence

services/analysis-engine/src/bandscope_analysis/api.py line 844

  • Latest human comment: @copilot-pull-request-reviewer at 2026-07-01T02:56:15Z
  • Comment URL: #490 (comment)
  • Comment excerpt: The PR description says API responses should avoid embedding raw exception strings, but run_analysis_job_updates still returns an external error message that interpolates the caught exception ("Stem separation failed: {error}"). Even if current stem-separation errors are usually filename-scoped, this still reintroduces the same information-leak pattern when upstream exceptions include sensitive details (paths, env info). Consider returning a generic safe message (and/or an error code) and logging the detailed exception internally instead.

services/analysis-engine/src/bandscope_analysis/cli.py line 55

  • Latest human comment: @copilot-pull-request-reviewer at 2026-07-01T02:56:15Z

  • Comment URL: #490 (comment)

  • Comment excerpt: This except Exception: path now returns a safe user-facing message, but it also drops the underlying exception completely. To keep debuggability without leaking details to stdout, capture the exception and log it (e.g., logging.exception(...)) while still returning the generic CLI message.

  • Result: REQUEST_CHANGES

  • Reason: unresolved human review thread(s) were present before approval.

  • Head SHA: 210117bf8f511682d0a029fd1865487d11c7cd02

  • Workflow run: 28489965270

  • Workflow attempt: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (3 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (3 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Test: test_api.py"]
  S2 --> I2["regression suite"]
  I2 --> R2["Review risk: Test: test_api.py"]
  R2 --> V2["targeted test run"]
Loading

@seonghobae

Copy link
Copy Markdown
Collaborator Author

Superseded by focused replacements #497 (analysis API/worker sanitization) and #411 (CLI error sanitization). Splitting the surfaces keeps the security fixes reviewable and avoids mixed scope.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

Closing as superseded by #497 and #411.

@seonghobae seonghobae closed this Jul 2, 2026
@google-labs-jules

Copy link
Copy Markdown

Superseded by focused replacements #497 (analysis API/worker sanitization) and #411 (CLI error sanitization). Splitting the surfaces keeps the security fixes reviewable and avoids mixed scope.

μ΄ν•΄ν–ˆμŠ΅λ‹ˆλ‹€. μž‘μ—… λ‚΄μš©μ΄ #497 및 #411 PR둜 λ‚˜λ‰˜μ–΄ λŒ€μ²΄λ˜μ—ˆμœΌλ―€λ‘œ, ν•΄λ‹Ή λ‚΄μš©μ— λ™μ˜ν•˜λ©° λ³Έ μž‘μ—…μ€ μ—¬κΈ°μ„œ μ’…λ£Œν•˜λ„λ‘ ν•˜κ² μŠ΅λ‹ˆλ‹€.

@google-labs-jules

Copy link
Copy Markdown

Closing as superseded by #497 and #411.

μ•Œκ² μŠ΅λ‹ˆλ‹€. κΈ°μ‘΄ μž‘μ—…μ΄ λŒ€μ²΄λ˜μ–΄ λ³Έ PR이 μ’…λ£Œλ¨μ„ μΈμ§€ν•˜μ˜€μŠ΅λ‹ˆλ‹€. μž‘μ—…μ„ μ€‘λ‹¨ν•˜κ² μŠ΅λ‹ˆλ‹€.

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.

2 participants