🛡️ Sentinel: [HIGH] Fix DSN credential leak via urlsplit bypass#573
Open
seonghobae wants to merge 4 commits into
Open
🛡️ Sentinel: [HIGH] Fix DSN credential leak via urlsplit bypass#573seonghobae wants to merge 4 commits into
seonghobae wants to merge 4 commits into
Conversation
🚨 Severity: HIGH 💡 Vulnerability: `urllib.parse.urlsplit` failed to parse `netloc` for malformed DSNs (e.g. missing `://`), placing credentials in the `path`. This allowed passwords in these specific connection strings to bypass the backend's redaction logic and potentially leak into error messages. 🎯 Impact: Passwords could be exposed in user-facing error messages or backend logs if an invalid or obscurely-formatted DSN was provided. 🔧 Fix: Enhanced `_password_candidates_from_dsn` to detect when `urlsplit` fails to extract `netloc`. It now falls back to splitting on `:` and injecting a `http://` scheme to ensure secrets are correctly parsed and masked. Added a test case. ✅ Verification: Ran `pytest tests/test_dsn_redaction.py` to confirm the new `test_redacts_dsn_without_slashes` test passes and 100% test coverage is maintained.
|
👋 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. |
🚨 Severity: HIGH 💡 Vulnerability: `urllib.parse.urlsplit` failed to parse `netloc` for malformed DSNs (e.g. missing `://`), placing credentials in the `path`. This allowed passwords in these specific connection strings to bypass the backend's redaction logic and potentially leak into error messages. 🎯 Impact: Passwords could be exposed in user-facing error messages or backend logs if an invalid or obscurely-formatted DSN was provided. 🔧 Fix: Enhanced `_password_candidates_from_dsn` to detect when `urlsplit` fails to extract `netloc`. It now falls back to splitting on `:` and injecting a `http://` scheme to ensure secrets are correctly parsed and masked. Added a test case. ✅ Verification: Ran `pytest tests/test_dsn_redaction.py` to confirm the new `test_redacts_dsn_without_slashes` test passes and 100% test coverage is maintained.
🚨 Severity: HIGH 💡 Vulnerability: `urllib.parse.urlsplit` 함수가 `://`가 없거나(ex. postgres:user:pass@host) 언더스코어가 포함된 비표준 DSN 문자열을 처리할 때, 인증 정보를 `netloc`이 아닌 `path`로 파싱하는 문제가 있었습니다. 이로 인해 해당 문자열들이 백엔드 DSN 마스킹(redaction) 로직을 우회하여 사용자 에러 메시지나 로그에 비밀번호가 노출될 위험이 존재했습니다. 🎯 Impact: 잘못된 형식의 DSN 연결 시도 시, 비밀번호 등 민감한 인증 정보가 로그나 에러 응답을 통해 유출될 수 있었습니다. 🔧 Fix: `backend/app/dsn_redaction.py`의 `_password_candidates_from_dsn` 함수에 폴백(fallback) 로직을 추가했습니다. `urlsplit`이 `netloc`을 파싱하지 못한 경우, `:`을 기준으로 문자열을 분리하고 임의의 `http://` 스키마를 주입하여 인증 정보를 올바르게 추출 및 마스킹하도록 수정했습니다. ✅ Verification: `pytest tests/test_dsn_redaction.py`를 실행하여 100% 테스트 커버리지를 통과함을 확인했습니다. CHANGELOG를 한국어로 갱신했습니다.
🚨 Severity: HIGH 💡 Vulnerability: `urllib.parse.urlsplit` 함수가 `://`가 없거나(ex. postgres:user:pass@host) 언더스코어가 포함된 비표준 DSN 문자열을 처리할 때, 인증 정보를 `netloc`이 아닌 `path`로 파싱하는 문제가 있었습니다. 이로 인해 해당 문자열들이 백엔드 DSN 마스킹(redaction) 로직을 우회하여 사용자 에러 메시지나 로그에 비밀번호가 노출될 위험이 존재했습니다. 🎯 Impact: 잘못된 형식의 DSN 연결 시도 시, 비밀번호 등 민감한 인증 정보가 로그나 에러 응답을 통해 유출될 수 있었습니다. 🔧 Fix: `backend/app/dsn_redaction.py`의 `_password_candidates_from_dsn` 함수에 폴백(fallback) 로직을 추가했습니다. `urlsplit`이 `netloc`을 파싱하지 못한 경우, `:`을 기준으로 문자열을 분리하고 임의의 `http://` 스키마를 주입하여 인증 정보를 올바르게 추출 및 마스킹하도록 수정했습니다. ✅ Verification: `pytest tests/test_dsn_redaction.py`를 실행하여 100% 테스트 커버리지를 통과함을 확인했습니다. CHANGELOG를 한국어로 갱신했습니다.
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.
Findings
1. HIGH Current-head GitHub Checks - Fix failed required checks before approval
- Problem: Failed same-head checks remain for
820caedc000e81e3d64c147f94116e3f4915b43c. - Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
- Fix: Read and fix the failed check logs below, then rerun the current-head checks.
- Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.
Failed checks:
- Security Scan/dependency-review: FAILURE (https://github.com/ContextualWisdomLab/pg-erd-cloud/actions/runs/29458201401/job/87495940828)
- Security Scan/osv-scan: FAILURE (https://github.com/ContextualWisdomLab/pg-erd-cloud/actions/runs/29458201401/job/87495940850)
- dependency-review check run: failure (https://github.com/ContextualWisdomLab/pg-erd-cloud/actions/runs/29458201401/job/87495940828)
- osv-scan check run: failure (https://github.com/ContextualWisdomLab/pg-erd-cloud/actions/runs/29458201401/job/87495940850)
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (2 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (2 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Backend (3 files)"]
S2 --> I2["API and service runtime"]
I2 --> R2["Review risk: Backend (3 files)"]
R2 --> V2["backend tests"]
Contributor
OpenCode Review Overview
Pull request overviewOpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed. Findings1. HIGH Current-head GitHub Checks - Fix failed required checks before approval
Failed checks:
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (2 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (2 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Backend (3 files)"]
S2 --> I2["API and service runtime"]
I2 --> R2["Review risk: Backend (3 files)"]
R2 --> V2["backend tests"]
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🛡️ Sentinel: [HIGH] Fix DSN credential leak via urlsplit bypass
🚨 Severity: HIGH
💡 Vulnerability:
urllib.parse.urlsplitfailed to parsenetlocfor malformed DSNs (e.g. missing://), placing credentials in thepath. This allowed passwords in these specific connection strings to bypass the backend's redaction logic and potentially leak into error messages.🎯 Impact: Passwords could be exposed in user-facing error messages or backend logs if an invalid or obscurely-formatted DSN was provided.
🔧 Fix: Enhanced
_password_candidates_from_dsnto detect whenurlsplitfails to extractnetloc. It now falls back to splitting on:and injecting ahttp://scheme to ensure secrets are correctly parsed and masked. Added a test case.✅ Verification: Ran
pytest tests/test_dsn_redaction.pyto confirm the newtest_redacts_dsn_without_slashestest passes and 100% test coverage is maintained.PR created automatically by Jules for task 16334902666858344870 started by @seonghobae