Uh oh!
There was an error while loading. Please reload this page.
🛡️ Sentinel: [CRITICAL/HIGH] Fix Server-Side Request Forgery (SSRF) risk in Noema LLM API URL - #294
Conversation
…isk in Noema LLM API URL
👋 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
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR hardens the Noema LLM review gate by validating the configured LLM endpoint URL scheme to prevent non-HTTP(S) schemes (e.g., file://) from being used, and adds a regression test to ensure invalid schemes are rejected.
Changes:
- Added URL scheme validation in
call_llmto only permithttp://andhttps://. - Added a pytest case asserting a
ValueErroris raised forfile://URLs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| scripts/ci/noema_review_gate.py | Rejects non-HTTP(S) endpoint URLs before making the request. |
| tests/test_noema_review_gate.py | Adds a regression test for invalid URL schemes (e.g., file://). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…isk in Noema LLM API URL
OpenCode Review Overview
Pull request overviewOpenCode reviewed the current-head bounded evidence and found no blocking issues. FindingsNo blocking findings. SummaryApproval sufficiency: bounded evidence supplied affirmative approval evidence for changed files, coverage/docstring posture, risk surfaces, and current-head verification; approval is not based merely on the absence of known blockers.
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["CI script: noema_review_gate.py"]
S1 --> I1["review and security gate shell path"]
I1 --> R1["Review risk: CI script: noema_review_gate.py"]
R1 --> V1["bash -n plus Strix self-test"]
Evidence --> S2["Test: test_noema_review_gate.py"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: test_noema_review_gate.py"]
R2 --> V2["targeted test run"]
|
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head bounded evidence and found no blocking issues.
Findings
No blocking findings.
Summary
Approval sufficiency: bounded evidence supplied affirmative approval evidence for changed files, coverage/docstring posture, risk surfaces, and current-head verification; approval is not based merely on the absence of known blockers.
Verification posture: CodeGraph evidence was initialized and bounded current-head evidence reviewed for changed-file evidence including scripts/ci/noema_review_gate.py, tests/test_noema_review_gate.py.
Linter/static: workflow/static review evidence is bounded by the current-head GitHub Checks gate and changed-file evidence.
TDD/regression: coverage execution evidence and focused changed hunks were reviewed from bounded-review-evidence.md.
Coverage: coverage execution evidence reports supported repository test suites passed.
Docstring coverage: coverage execution evidence reports configured repository docstring gates passed or docstring coverage was advisory.
DAG: CodeGraph/source-backed behavior map connects scripts/ci/noema_review_gate.py to the affected review, runtime, or workflow path and required checks.
PoC/execution: coverage-evidence job executed on the current head and reported PASS.
DDD/domain: workflow and repository-governance invariants were reviewed against changed files in bounded evidence.
CDD/context: CodeGraph evidence, changed-file history, and focused hunks were reviewed from bounded-review-evidence.md.
Similar issues: changed-file history evidence was reviewed for comparable local precedents.
Claim/concept check: bounded evidence, repository source, current-head workflow evidence, and, where numeric, scientific, statistical, or literature-backed claims are affected, original-paper/formula evidence and parameter-recovery expectations were used for claims.
Standards search: standards and external-source checks are delegated to configured OpenCode web_search/Context7/DeepWiki sources when applicable; no evidence-backed standards blocker is present in bounded evidence.
Compatibility/convention: changed workflow/script conventions, object naming, and reserved-word safety for schema/API/config/code surfaces were checked in bounded evidence.
Breaking-change/backcompat: deployment evidence and changed-file history were checked for backward-compatibility risk.
Performance: changed surfaces were checked for performance risk in bounded evidence.
Developer experience: changed automation, review, test, setup, and maintenance surfaces were checked for helpful or obstructive DX impact in bounded evidence.
User experience: connected user, operator, API, CLI, documentation, review-comment, status-check, rendering, and workflow-reader behavior was checked for contradictions against code, docs, and tests in bounded evidence.
Visual/DOM: Playwright visual, DOM locator, ARIA snapshot, console, and responsive evidence were checked when a web UI surface was present; for non-web surfaces, API/CLI/log/docs/workflow interaction evidence was reviewed instead.
Accessibility/i18n: accessibility, localization, and human-readable text surfaces were checked where UI, CLI, API message, docs, logs, or review text changed.
Supply-chain/license: dependency, package, model, container, and external-tool changes were checked in bounded evidence.
Packaging: package, build, test, lint, and security contracts were checked in bounded evidence.
Security/privacy: workflow-token, review-gate, and repository-automation security/privacy boundaries were checked in bounded evidence.
- Result: APPROVE
- Reason: Security fix properly implements URL scheme validation
- Head SHA:
74d5b17caa2cc2e6acf1da59c7a63a9d61d87398 - Workflow run: 28646384109
- Workflow attempt: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["CI script: noema_review_gate.py"]
S1 --> I1["review and security gate shell path"]
I1 --> R1["Review risk: CI script: noema_review_gate.py"]
R1 --> V1["bash -n plus Strix self-test"]
Evidence --> S2["Test: test_noema_review_gate.py"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: test_noema_review_gate.py"]
R2 --> V2["targeted test run"]
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head bounded evidence and found no blocking issues.
Findings
No blocking findings.
Summary
Approval sufficiency: bounded evidence supplied affirmative approval evidence for changed files, coverage/docstring posture, risk surfaces, and current-head verification; approval is not based merely on the absence of known blockers.
Verification posture: CodeGraph evidence was initialized and bounded current-head evidence reviewed for changed-file evidence including scripts/ci/noema_review_gate.py, tests/test_noema_review_gate.py.
Linter/static: workflow/static review evidence is bounded by the current-head GitHub Checks gate and changed-file evidence.
TDD/regression: coverage execution evidence and focused changed hunks were reviewed from bounded-review-evidence.md.
Coverage: coverage execution evidence reports supported repository test suites passed.
Docstring coverage: coverage execution evidence reports configured repository docstring gates passed or docstring coverage was advisory.
DAG: CodeGraph/source-backed behavior map connects scripts/ci/noema_review_gate.py to the affected review, runtime, or workflow path and required checks.
PoC/execution: coverage-evidence job executed on the current head and reported PASS.
DDD/domain: workflow and repository-governance invariants were reviewed against changed files in bounded evidence.
CDD/context: CodeGraph evidence, changed-file history, and focused hunks were reviewed from bounded-review-evidence.md.
Similar issues: changed-file history evidence was reviewed for comparable local precedents.
Claim/concept check: bounded evidence, repository source, current-head workflow evidence, and, where numeric, scientific, statistical, or literature-backed claims are affected, original-paper/formula evidence and parameter-recovery expectations were used for claims.
Standards search: standards and external-source checks are delegated to configured OpenCode web_search/Context7/DeepWiki sources when applicable; no evidence-backed standards blocker is present in bounded evidence.
Compatibility/convention: changed workflow/script conventions, object naming, and reserved-word safety for schema/API/config/code surfaces were checked in bounded evidence.
Breaking-change/backcompat: deployment evidence and changed-file history were checked for backward-compatibility risk.
Performance: changed surfaces were checked for performance risk in bounded evidence.
Developer experience: changed automation, review, test, setup, and maintenance surfaces were checked for helpful or obstructive DX impact in bounded evidence.
User experience: connected user, operator, API, CLI, documentation, review-comment, status-check, rendering, and workflow-reader behavior was checked for contradictions against code, docs, and tests in bounded evidence.
Visual/DOM: Playwright visual, DOM locator, ARIA snapshot, console, and responsive evidence were checked when a web UI surface was present; for non-web surfaces, API/CLI/log/docs/workflow interaction evidence was reviewed instead.
Accessibility/i18n: accessibility, localization, and human-readable text surfaces were checked where UI, CLI, API message, docs, logs, or review text changed.
Supply-chain/license: dependency, package, model, container, and external-tool changes were checked in bounded evidence.
Packaging: package, build, test, lint, and security contracts were checked in bounded evidence.
Security/privacy: workflow-token, review-gate, and repository-automation security/privacy boundaries were checked in bounded evidence.
- Result: APPROVE
- Reason: All tests and docstring checks passed; no unresolved review threads or failed GitHub checks found.
- Head SHA:
aec078931c0b9c07686544b6b4951955860d2189 - Workflow run: 28708369811
- Workflow attempt: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["CI script: noema_review_gate.py"]
S1 --> I1["review and security gate shell path"]
I1 --> R1["Review risk: CI script: noema_review_gate.py"]
R1 --> V1["bash -n plus Strix self-test"]
Evidence --> S2["Test: test_noema_review_gate.py"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: test_noema_review_gate.py"]
R2 --> V2["targeted test run"]
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head bounded evidence and found no blocking issues.
Findings
No blocking findings.
Summary
Approval sufficiency: bounded evidence supplied affirmative approval evidence for changed files, coverage/docstring posture, risk surfaces, and current-head verification; approval is not based merely on the absence of known blockers.
Verification posture: CodeGraph evidence was initialized and bounded current-head evidence reviewed for changed-file evidence including scripts/ci/noema_review_gate.py, tests/test_noema_review_gate.py.
Linter/static: workflow/static review evidence is bounded by the current-head GitHub Checks gate and changed-file evidence.
TDD/regression: coverage execution evidence and focused changed hunks were reviewed from bounded-review-evidence.md.
Coverage: coverage execution evidence reports supported repository test suites passed.
Docstring coverage: coverage execution evidence reports configured repository docstring gates passed or docstring coverage was advisory.
DAG: CodeGraph/source-backed behavior map connects scripts/ci/noema_review_gate.py to the affected review, runtime, or workflow path and required checks.
PoC/execution: coverage-evidence job executed on the current head and reported PASS.
DDD/domain: workflow and repository-governance invariants were reviewed against changed files in bounded evidence.
CDD/context: CodeGraph evidence, changed-file history, and focused hunks were reviewed from bounded-review-evidence.md.
Similar issues: changed-file history evidence was reviewed for comparable local precedents.
Claim/concept check: bounded evidence, repository source, current-head workflow evidence, and, where numeric, scientific, statistical, or literature-backed claims are affected, original-paper/formula evidence and parameter-recovery expectations were used for claims.
Standards search: standards and external-source checks are delegated to configured OpenCode web_search/Context7/DeepWiki sources when applicable; no evidence-backed standards blocker is present in bounded evidence.
Compatibility/convention: changed workflow/script conventions, object naming, and reserved-word safety for schema/API/config/code surfaces were checked in bounded evidence.
Breaking-change/backcompat: deployment evidence and changed-file history were checked for backward-compatibility risk.
Performance: changed surfaces were checked for performance risk in bounded evidence.
Developer experience: changed automation, review, test, setup, and maintenance surfaces were checked for helpful or obstructive DX impact in bounded evidence.
User experience: connected user, operator, API, CLI, documentation, review-comment, status-check, rendering, and workflow-reader behavior was checked for contradictions against code, docs, and tests in bounded evidence.
Visual/DOM: Playwright visual, DOM locator, ARIA snapshot, console, and responsive evidence were checked when a web UI surface was present; for non-web surfaces, API/CLI/log/docs/workflow interaction evidence was reviewed instead.
Accessibility/i18n: accessibility, localization, and human-readable text surfaces were checked where UI, CLI, API message, docs, logs, or review text changed.
Supply-chain/license: dependency, package, model, container, and external-tool changes were checked in bounded evidence.
Packaging: package, build, test, lint, and security contracts were checked in bounded evidence.
Security/privacy: workflow-token, review-gate, and repository-automation security/privacy boundaries were checked in bounded evidence.
- Result: APPROVE
- Reason: Security fix passes all verification gates
- Head SHA:
467b2c6e9fd500a0fc880b2ec39747a66ed4c9ce - Workflow run: 28727187608
- Workflow attempt: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["CI script: noema_review_gate.py"]
S1 --> I1["review and security gate shell path"]
I1 --> R1["Review risk: CI script: noema_review_gate.py"]
R1 --> V1["bash -n plus Strix self-test"]
Evidence --> S2["Test: test_noema_review_gate.py"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: test_noema_review_gate.py"]
R2 --> V2["targeted test run"]
Uh oh!
There was an error while loading. Please reload this page.
…redundant SSRF check Three tests broke the coverage-evidence gate because intentional changes landed without updating their assertions (or, for noema, added a redundant guard that shadowed an existing one): - test_call_llm_handles_configuration_and_verdicts: the redundant startswith() scheme guard (#297) was shadowed by the comprehensive, case-insensitive scheme check from #294, was case-sensitive (wrongly rejecting valid HTTPS:// URLs the suite exercises), and leaked the URL in its error (violating the "no original URL in error" intent). Removed the dead guard and its duplicate test block; file:// rejection stays covered by the existing assertion. Restores scripts/ci 100% coverage. - test_workflow_provisions_sandbox_tool_and_reviewer_agent: the opencode-review-target if: block was expanded to a multi-line form in a4df728 (cancel runs on PR close), growing the header past the regex's 240-char proximity window. Widened to 400; timeout-minutes: 360 is unique. - test_merge_scheduler_uses_escalating_mutation_credentials: #338 throttled review_dispatch_limit default from "-1" to "1"; updated the stale assertion. The "-1" empty-input fallback assertion is retained. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RTAMs4bpSZS77Xe3RQjv9P
…ertions Three tests failed on this branch (inherited red state from main), blocking the coverage-evidence job: 1. test_noema_review_gate: #297 re-added a case-sensitive `api_url.startswith("http://"/"https://")` guard AFTER #294's case-insensitive urlparse scheme check. It was redundant for rejecting file:// (the scheme check already raises "URL scheme must be http or https" first) and, worse, wrongly rejected valid uppercase-scheme URLs like HTTPS:// that #294 intentionally supports and tests. Removed the redundant/buggy check; SSRF protection is fully retained by the urlparse scheme + hostname + private-IP validation. Updated the stale assertion to expect the scheme-check message. 2. test_opencode_agent_contract (opencode-review-target timeout): the job grew a legitimate `if:` guard, pushing timeout-minutes:360 to 327 chars from the job key, past the 240-char proximity window. Widened the window to 360; still asserts the timeout belongs to that job. 3. test_opencode_agent_contract (scheduler default): #338 "Throttle central PR workflow queue" deliberately changed the review_dispatch_limit input default from "-1" to "1" (and shipped its own contract test), but this assertion was left stale. Updated it to "1". Full suite: 174 passed, coverage 100% (fail_under=100). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RTAMs4bpSZS77Xe3RQjv9P
* [codex] Fail OpenCode approve when review publish fails * [codex] Bound OpenCode model pool candidates * [codex] Guard OpenCode publish failure log groups * [codex] Cancel stale OpenCode PR review runs * Re-kick model pool once on OpenCode exhaustion * Log empty rekick outcome before fail-closed fallback * Simplify re-kick run invocation in approval gate * Harden re-kick logging for exhausted model pool * Retry exhausted OpenCode model pool with exponential backoff * Bound exhausted model re-kicks with exponential backoff window * fix(ci): unblock coverage-evidence by fixing three stale contract assertions Three tests failed on this branch (inherited red state from main), blocking the coverage-evidence job: 1. test_noema_review_gate: #297 re-added a case-sensitive `api_url.startswith("http://"/"https://")` guard AFTER #294's case-insensitive urlparse scheme check. It was redundant for rejecting file:// (the scheme check already raises "URL scheme must be http or https" first) and, worse, wrongly rejected valid uppercase-scheme URLs like HTTPS:// that #294 intentionally supports and tests. Removed the redundant/buggy check; SSRF protection is fully retained by the urlparse scheme + hostname + private-IP validation. Updated the stale assertion to expect the scheme-check message. 2. test_opencode_agent_contract (opencode-review-target timeout): the job grew a legitimate `if:` guard, pushing timeout-minutes:360 to 327 chars from the job key, past the 240-char proximity window. Widened the window to 360; still asserts the timeout belongs to that job. 3. test_opencode_agent_contract (scheduler default): #338 "Throttle central PR workflow queue" deliberately changed the review_dispatch_limit input default from "-1" to "1" (and shipped its own contract test), but this assertion was left stale. Updated it to "1". Full suite: 174 passed, coverage 100% (fail_under=100). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RTAMs4bpSZS77Xe3RQjv9P * test: cover noema parsed scheme guard --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Seongho Bae <seonghobae@me.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: opencode-agent[bot] <219766164+opencode-agent[bot]@users.noreply.github.com>
🚨 Severity: HIGH/MEDIUM
💡 Vulnerability: Noema LLM API 호출 시 URL 스키마 검증 누락으로 인한 Server-Side Request Forgery (SSRF) 및 Local File Inclusion 취약점.
🎯 Impact: 악의적인 사용자가 환경변수를 조작하여
file://과 같은 비정상적인 URL 스키마를 입력하면, CI 환경의 로컬 파일을 읽어 유출하거나 내부망을 탐색할 위험이 있습니다.🔧 Fix:
noema_review_gate.py의 URL 스키마가http://또는https://로 시작하는지 검증하도록 수정했습니다.✅ Verification: 유효하지 않은 스키마를 사용하는 경우
ValueError를 발생시키는지 확인하는 테스트 코드를 추가하였고, 테스트 커버리지 100%를 달성했습니다.PR created automatically by Jules for task 2765465087543431574 started by @seonghobae