π‘οΈ Sentinel: [MEDIUM] Fix μ 보 μ μΆ μ·¨μ½μ (Information Leakage in Exceptions)#490
π‘οΈ Sentinel: [MEDIUM] Fix μ 보 μ μΆ μ·¨μ½μ (Information Leakage in Exceptions)#490seonghobae wants to merge 2 commits into
Conversation
νμ΄μ¬ μλΉμ€μ μμΈ μ²λ¦¬(Exception handling) λΆλΆμμ λ΄λΆ νμΌ κ²½λ‘ λ±μ λ―Όκ°ν μ λ³΄κ° μ¬μ©μμκ² λ°νλ μ μλ μ μ¬μ μΈ μ 보 μ μΆ μ·¨μ½μ μ μμ νμ΅λλ€. μμΈ λ°μ μ μλ¬μ λ΄μ©μ κ·Έλλ‘ λ ΈμΆνμ§ μκ³ , μ¬μ©μ μΉνμ μ΄λ©΄μ μμ ν μΌλ°μ μΈ μλ΄ λ©μμ§λ‘ μλ΅νλλ‘ νμμ΅λλ€. - `services/analysis-engine/src/bandscope_analysis/api.py`: 843λ²μ§Έ μ€ μμΈ μ²λ¦¬ λ©μμ§ μμ - `services/analysis-engine/src/bandscope_analysis/cli.py`: 49~50λ²μ§Έ μ€ μμΈ μ²λ¦¬ λ©μμ§ μμ
|
π 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 New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
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
Exceptionpayload 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.
| except Exception: | ||
| result_queue.put(("runtime_error", "An unexpected error occurred during stem separation.")) |
| 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 Review Overview
Pull request overviewOpenCode reviewed the current-head evidence but found unresolved human review threads before approval. Findings1. HIGH .github/workflows/opencode-review.yml:1 - Unresolved human review thread blocks automated approval
Review thread evidenceLatest unresolved human review thread evidence
|
There was a problem hiding this comment.
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_updatesstill 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"]
μ΄ν΄νμ΅λλ€. μμ λ΄μ©μ΄ #497 λ° #411 PRλ‘ λλμ΄ λ체λμμΌλ―λ‘, ν΄λΉ λ΄μ©μ λμνλ©° λ³Έ μμ μ μ¬κΈ°μ μ’ λ£νλλ‘ νκ² μ΅λλ€. |
π¨ 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