Skip to content

๐Ÿ›ก๏ธ Sentinel: [HIGH] Fix credential leak due to unhandled DSN URL schemes#412

Open
seonghobae wants to merge 1 commit into
mainfrom
security/fix-dsn-redaction-urlsplit-bypass-3749945389775870191
Open

๐Ÿ›ก๏ธ Sentinel: [HIGH] Fix credential leak due to unhandled DSN URL schemes#412
seonghobae wants to merge 1 commit into
mainfrom
security/fix-dsn-redaction-urlsplit-bypass-3749945389775870191

Conversation

@seonghobae

Copy link
Copy Markdown
Collaborator

๐Ÿšจ Severity: HIGH
๐Ÿ’ก Vulnerability: urllib.parse.urlsplit์ด ์–ธ๋”์Šค์ฝ”์–ด๊ฐ€ ํฌํ•จ๋œ URL ์Šคํ‚ด(์˜ˆ: snowflake_invalid://)์„ ๋นˆ ๋ฌธ์ž์—ด๋กœ ์ฒ˜๋ฆฌํ•˜์—ฌ, ์ž˜๋ชป๋œ ํ˜•์‹์˜ DSN ์—ฐ๊ฒฐ ์‹คํŒจ ์‹œ ์—๋Ÿฌ ๋ฉ”์‹œ์ง€์— ํ‰๋ฌธ ์•”ํ˜ธ๊ฐ€ ๊ทธ๋Œ€๋กœ ๋…ธ์ถœ๋˜๋Š” ์ž๊ฒฉ ์ฆ๋ช… ์œ ์ถœ(Credential Leak) ์ทจ์•ฝ์ ์ž…๋‹ˆ๋‹ค.
๐ŸŽฏ Impact: ๊ณต๊ฒฉ์ž ํ˜น์€ ๋‚ด๋ถ€ ๋กœ๊ทธ ์ ‘๊ทผ์ž๊ฐ€ ๋ฐ์ดํ„ฐ๋ฒ ์ด์Šค ์ž๊ฒฉ ์ฆ๋ช…์„ ํš๋“ํ•  ์ˆ˜ ์žˆ์Šต๋‹ˆ๋‹ค.
๐Ÿ”ง Fix: ๋น„๋ฐ€๋ฒˆํ˜ธ ๋งˆ์Šคํ‚น์„ ์œ„ํ•œ DSN ํŒŒ์‹ฑ ์‹œ, ์•Œ ์ˆ˜ ์—†๋Š” ์Šคํ‚ด ๋ถ€๋ถ„์„ ์ž„์‹œ๋กœ http://๋กœ ์น˜ํ™˜ํ•˜์—ฌ urlsplit์ด ์‚ฌ์šฉ์ž ์ •๋ณด๋ฅผ ์ •์ƒ์ ์œผ๋กœ ํŒŒ์‹ฑํ•˜๋„๋ก ๊ฐ•์ œ ์šฐํšŒํ–ˆ์Šต๋‹ˆ๋‹ค.
โœ… Verification: backend/tests/test_dsn_redaction.py ์œ ๋‹› ํ…Œ์ŠคํŠธ๋ฅผ ์ถ”๊ฐ€ํ•˜์—ฌ ์ •๊ทœํ™” ๋ฐ ๋งˆ์Šคํ‚น ์„ฑ๊ณต์„ ๊ฒ€์ฆํ–ˆ์Šต๋‹ˆ๋‹ค. (cd backend && uv run pytest tests/test_dsn_redaction.py)


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

โ€ฆL schemes)

Python์˜ `urllib.parse.urlsplit` ํ•จ์ˆ˜๊ฐ€ ์–ธ๋”์Šค์ฝ”์–ด(`_`)๊ฐ€ ํฌํ•จ๋œ URL ์Šคํ‚ด(์˜ˆ: `snowflake_invalid://`)์„ ์˜ฌ๋ฐ”๋ฅด๊ฒŒ ํŒŒ์‹ฑํ•˜์ง€ ๋ชปํ•˜๊ณ  ๋นˆ(empty) ์Šคํ‚ด์œผ๋กœ ํ‰๊ฐ€ํ•˜๋Š” ๋ฌธ์ œ๊ฐ€ ์žˆ์—ˆ์Šต๋‹ˆ๋‹ค.
์ด๋กœ ์ธํ•ด DSN ์—๋Ÿฌ ๋ฉ”์‹œ์ง€์— ํฌํ•จ๋œ ๋น„๋ฐ€๋ฒˆํ˜ธ๋ฅผ ์ถ”์ถœํ•˜์—ฌ ๋งˆ์Šคํ‚น(redaction)ํ•˜๋Š” ๋กœ์ง์ด ๋™์ž‘ํ•˜์ง€ ์•Š์•„, ๋กœ๊ทธ ๋ฐ ํด๋ผ์ด์–ธํŠธ ์—๋Ÿฌ ์‘๋‹ต์— ๋ฐ์ดํ„ฐ๋ฒ ์ด์Šค ์ž๊ฒฉ ์ฆ๋ช…์ด ๋…ธ์ถœ๋˜๋Š” ์ทจ์•ฝ์ ์ด ์กด์žฌํ–ˆ์Šต๋‹ˆ๋‹ค.

ํŒŒ์‹ฑ ์ „ ์ •๊ทœํ‘œํ˜„์‹์„ ํ†ตํ•ด ์Šคํ‚ด ๋ถ€๋ถ„์„ ์ž„์‹œ๋กœ ์•Œ๋ ค์ง„ ์˜ฌ๋ฐ”๋ฅธ ํ˜•ํƒœ(์˜ˆ: `http://`)๋กœ ๋ณ€๊ฒฝํ•œ ๋’ค `urlsplit`์„ ์ˆ˜ํ–‰ํ•˜๋„๋ก ์šฐํšŒ ์ฝ”๋“œ๋ฅผ ์ถ”๊ฐ€ํ•˜์—ฌ, ์•Œ ์ˆ˜ ์—†๊ฑฐ๋‚˜ ์ž˜๋ชป๋œ ์Šคํ‚ด์„ ๊ฐ€์ง„ DSN ๋ฌธ์ž์—ด์—์„œ๋„ ์˜ฌ๋ฐ”๋ฅด๊ฒŒ ๋น„๋ฐ€๋ฒˆํ˜ธ ํ›„๋ณด๊ตฐ์„ ์ถ”์ถœํ•˜๊ณ  ๋งˆ์Šคํ‚นํ•  ์ˆ˜ ์žˆ๋„๋ก ์ˆ˜์ •ํ–ˆ์Šต๋‹ˆ๋‹ค.

๋˜ํ•œ ํ•ด๋‹น ์ด์Šˆ์— ๋Œ€ํ•œ ์ž๋™ํ™”๋œ ํšŒ๊ท€ ํ…Œ์ŠคํŠธ(regression test)๋ฅผ ์ถ”๊ฐ€ํ•˜๊ณ , ๊ด€๋ จ ๋‚ด์šฉ์„ `.jules/sentinel.md` ์— ๊ธฐ๋กํ•˜์˜€์Šต๋‹ˆ๋‹ค.
Copilot AI review requested due to automatic review settings July 1, 2026 21:40
@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 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

This PR addresses a credential leak risk in backend error redaction when DSNs use non-standard URL schemes (notably schemes containing underscores) that urllib.parse.urlsplit fails to parse correctly, potentially preventing password extraction/redaction.

Changes:

  • Add a fallback parsing path in DSN password-candidate extraction by temporarily substituting an invalid/unknown scheme with http:// when urlsplit yields an empty scheme.
  • Add unit tests to verify secret candidate extraction and error-message redaction for DSNs with unsupported schemes and for standard PostgreSQL DSNs.
  • Document the vulnerability/learning/prevention guidance in .jules/sentinel.md.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
backend/app/dsn_redaction.py Adds scheme-substitution fallback so urlsplit can extract userinfo/query secrets for redaction even when the original scheme is malformed (e.g., contains underscores).
backend/tests/test_dsn_redaction.py Adds focused tests validating secret candidate extraction and redaction behavior for malformed-scheme and standard DSNs.
.jules/sentinel.md Records the vulnerability pattern and mitigation guidance for future prevention.

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

@github-actions github-actions 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 exhausted the configured model pool without a usable current-head review conclusion. This is not approval evidence, so the PR is blocked until a source-backed review can establish approval sufficiency or identify concrete fixes.

Findings

1. HIGH .jules/sentinel.md:1 - OpenCode could not establish approval sufficiency

  • Problem: every configured model path failed to produce a usable current-head control block.
  • Root cause: model execution, timeout, export, normalization, or approval-gate validation did not complete after exponential retry across the configured model pool.
  • Impact: approving from deterministic check state alone would miss PR-intent mismatches, missing files, edge-case bugs, robustness gaps, UX/DX regressions, security issues, and CodeGraph-backed base/head flow changes.
  • Fix: rerun OpenCode after model availability recovers, or update the PR with the missing files, tests, docs, generated artifacts, and verification evidence needed for a source-backed review conclusion.
  • Regression test: keep the approval gate posting REQUEST_CHANGES, not APPROVE or check-only failure, when no model produces a valid current-head review.

Summary

  • Result: REQUEST_CHANGES
  • Reason: coverage-evidence passed and peer GitHub Checks completed without failures, but no model produced a valid review control block.
  • Deterministic evidence checked but not used for approval: current-head changed-file evidence (.jules/sentinel.md, backend/app/dsn_redaction.py, backend/tests/test_dsn_redaction.py); coverage-evidence result success; peer checks from statusCheckRollup excluding this OpenCode check.
  • Model outcome: model_pool=exhausted; selected_model=none.
  • Head SHA: 3962109996437937e24b2b79bfb2e9abc1398221
  • Workflow run: 28549565437
  • Workflow attempt: 1

No PR approval was posted because model-output failure is not evidence that the PR has no blockers.

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file: sentinel.md"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file: sentinel.md"]
  R1 --> V1["required checks"]
  Evidence --> S2["Backend (2 files)"]
  S2 --> I2["API and service runtime"]
  I2 --> R2["Review risk: Backend (2 files)"]
  R2 --> V2["backend tests"]
Loading

Comment thread .jules/sentinel.md
**Vulnerability:** Database driver exceptions can echo DSN fragments, query parameters, or assignment-style secrets after connection failures, leaking plaintext passwords through snapshot error messages and queue logs.
**Learning:** Redacting only the literal DSN is not enough. Error messages may contain decoded, percent-encoded, query-string, or `password=`/`api_key=` style forms of the same secret.
**Prevention:** Sanitize snapshot job errors before persisting or re-raising them, and raise sanitized exceptions with `from None` so Python exception chaining does not reattach the original secret-bearing exception.

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.

HIGH OpenCode could not establish approval sufficiency

  • Problem: the model pool exhausted without a valid current-head review control block, so this changed line cannot be approved from deterministic check state alone.
  • Impact: PR-intent mismatches, missing files, robustness bugs, UX/DX regressions, and CodeGraph-backed flow changes could be missed.
  • Fix: rerun OpenCode after model availability recovers, or add the missing source/test/docs/generated verification evidence needed for a source-backed approval.
  • Verification: rerun the OpenCode Review workflow and confirm it emits APPROVE or source-backed REQUEST_CHANGES for this head SHA.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

  • Head SHA: 3962109996437937e24b2b79bfb2e9abc1398221
  • Workflow run: 28549565437
  • Workflow attempt: 1
  • Gate result: REQUEST_CHANGES (approval step)

Pull request overview

OpenCode exhausted the configured model pool without a usable current-head review conclusion. This is not approval evidence, so the PR is blocked until a source-backed review can establish approval sufficiency or identify concrete fixes.

Findings

1. HIGH .jules/sentinel.md:1 - OpenCode could not establish approval sufficiency

  • Problem: every configured model path failed to produce a usable current-head control block.
  • Root cause: model execution, timeout, export, normalization, or approval-gate validation did not complete after exponential retry across the configured model pool.
  • Impact: approving from deterministic check state alone would miss PR-intent mismatches, missing files, edge-case bugs, robustness gaps, UX/DX regressions, security issues, and CodeGraph-backed base/head flow changes.
  • Fix: rerun OpenCode after model availability recovers, or update the PR with the missing files, tests, docs, generated artifacts, and verification evidence needed for a source-backed review conclusion.
  • Regression test: keep the approval gate posting REQUEST_CHANGES, not APPROVE or check-only failure, when no model produces a valid current-head review.

Summary

  • Result: REQUEST_CHANGES
  • Reason: coverage-evidence passed and peer GitHub Checks completed without failures, but no model produced a valid review control block.
  • Deterministic evidence checked but not used for approval: current-head changed-file evidence (.jules/sentinel.md, backend/app/dsn_redaction.py, backend/tests/test_dsn_redaction.py); coverage-evidence result success; peer checks from statusCheckRollup excluding this OpenCode check.
  • Model outcome: model_pool=exhausted; selected_model=none.
  • Head SHA: 3962109996437937e24b2b79bfb2e9abc1398221
  • Workflow run: 28549565437
  • Workflow attempt: 1

No PR approval was posted because model-output failure is not evidence that the PR has no blockers.

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file: sentinel.md"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file: sentinel.md"]
  R1 --> V1["required checks"]
  Evidence --> S2["Backend (2 files)"]
  S2 --> I2["API and service runtime"]
  I2 --> R2["Review risk: Backend (2 files)"]
  R2 --> V2["backend tests"]
Loading

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