security(api): reject non-text controls in multiline SQL - #832
security(api): reject non-text controls in multiline SQL#832seonghobae wants to merge 7 commits into
Conversation
📝 WalkthroughWalkthrough
ChangesSQL 전송 보안
기존 동작 회귀 검증
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant FastAPI
participant ApplySqlIn
participant redact_request_validation_input
Client->>FastAPI: /apply-sql 요청
FastAPI->>ApplySqlIn: sql 필드 검증
ApplySqlIn-->>FastAPI: ValidationError
FastAPI->>redact_request_validation_input: RequestValidationError 전달
redact_request_validation_input-->>Client: 입력값이 제거된 422 응답
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/security/apply-sql-transport-validation.md (1)
35-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win문헌별 근거 요약을 추가하십시오.
현재 References에는 인용과 링크가 있지만 각 문헌이 어떤 검증 경계를 뒷받침하는지 설명이 없습니다. 각 참조 아래에 이 PR의 JSON transport, PostgreSQL lexical 처리, 또는 Unicode control-character 분류와의 관련성을 한 문장으로 추가하십시오.
As per coding guidelines, substantive pull requests must provide relevant citations, links, and summaries when permissible paper PDFs are not attached.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/security/apply-sql-transport-validation.md` around lines 35 - 45, 각 참고문헌 항목 아래에 해당 자료가 뒷받침하는 검증 경계를 한 문장으로 요약하십시오. Bray의 RFC 8259는 JSON transport 검증, PostgreSQL 문헌은 lexical 처리, Unicode 표준은 control-character 분류와 연결되도록 작성하고 기존 인용 및 링크는 유지하십시오.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/tests/test_api_connections.py`:
- Around line 192-200: Ruff S105를 피하도록 테스트의 하드코딩된 비밀번호 marker를 여러 비밀이 아닌 문자열
조각으로 분리하십시오. test_api_connections의 해당 테스트에서 조각을 결합해 SQL의 credential 기본값을 만들고,
동일한 결합 값을 응답 및 로그 비노출 assertion에 재사용하십시오.
In `@backend/tests/test_pooler.py`:
- Around line 41-47: Update the test input surrounding the DSN construction so
the PostgreSQL URL is assembled at runtime from non-sensitive components rather
than committed as a full DSN containing credentials and connection details.
Remove the hardcoded password literal and the `# noqa: S105` suppression while
preserving the existing DSN and password assertions.
---
Nitpick comments:
In `@docs/security/apply-sql-transport-validation.md`:
- Around line 35-45: 각 참고문헌 항목 아래에 해당 자료가 뒷받침하는 검증 경계를 한 문장으로 요약하십시오. Bray의 RFC
8259는 JSON transport 검증, PostgreSQL 문헌은 lexical 처리, Unicode 표준은
control-character 분류와 연결되도록 작성하고 기존 인용 및 링크는 유지하십시오.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5386a4d2-f966-40fa-838e-100bbbdae878
📒 Files selected for processing (9)
CHANGELOG.mdbackend/app/main.pybackend/app/schemas.pybackend/tests/test_api_connections.pybackend/tests/test_permissions.pybackend/tests/test_pooler.pybackend/tests/test_schema_validation.pybackend/tests/test_security_headers.pydocs/security/apply-sql-transport-validation.md
|
@coderabbitai review Please review the unchanged exact head |
|
|
|
Exact-current-head independent review request for @opencode-agent @cwl-noema-review Please provide a qualifying non-author review of this unchanged predecessor. CI |
|
Exact-current-head independent review request for @opencode-agent @cwl-noema-review Please review this unchanged exact head and provide a qualifying non-author formal verdict. Current-head CI |
1 participant
Summary
ApplySqlInrequest-schema boundaryTest-first evidence
fcc908e60d54a8ccfa9c6f2440f1c6984fb52e71: 96 missing beginning/middle/end cases for U+0080–U+009F570 passed, 1 skippedapp/schemas.py148/148 statements and 2/2 branches (100%)git diff --check: cleanThe first local full-suite probe inherited the sandbox SOCKS proxy and failed only three Clearfolio HTTPX constructors because the hash-locked environment intentionally lacks
socksio; removing uppercase and lowercase proxy transport variables produced the full green result above, matching CI’s test boundary.No dependency, lockfile, database migration, frontend, or workflow changes.
Closes #764.