Skip to content

security(api): reject non-text controls in multiline SQL requests - #854

Closed
seonghobae wants to merge 1 commit into
mainfrom
security/reject-sql-control-characters
Closed

security(api): reject non-text controls in multiline SQL requests#854
seonghobae wants to merge 1 commit into
mainfrom
security/reject-sql-control-characters

Conversation

@seonghobae

Copy link
Copy Markdown
Collaborator

Outcome

Reject non-text C0/C1 control characters in multiline ApplySqlIn.sql requests before the API handler or DDL validator runs. Horizontal tab, LF, and CR remain valid multiline text whitespace.

Closes #764.

Security boundary

  • Applies at the Pydantic request-model boundary.
  • Rejects C0 controls other than \\t, \\n, and \\r.
  • Rejects DEL and the C1 range (U+007F..U+009F).
  • Does not expand the accepted SQL grammar or execution authority.

Evidence

  • RED: eight representative C0/C1 inputs were accepted before the validator (8 expected failures); CR/LF/tab passed.
  • GREEN: 22 passed in tests/test_api_apply_sql.py.
  • Full backend: 388 passed, 1 skipped (proxy variables removed because the sandbox injects a SOCKS proxy while the hash-locked CI environment does not).
  • Mypy: clean across 68 source files.
  • Docstring contract: 1 passed.
  • Exact owned coverage: app/schemas.py 150/150 statements and 4/4 branches (100%).
  • git diff --check: clean.

Documentation

CHANGELOG.md and docs/api-security-checklist.md record the implemented request boundary and point to its code/tests.

Local tooling limitation

Repository guidance requests CodeGraph initialization/sync, but the executable is unavailable in this environment; direct rg symbol and caller search was used instead.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 46 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: bb8e7558-4c94-440e-a016-8375e2f5dd31

📥 Commits

Reviewing files that changed from the base of the PR and between 72afe6d and c00edbc.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • backend/app/schemas.py
  • backend/tests/test_api_apply_sql.py
  • docs/api-security-checklist.md

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Collaborator Author

Independent exact-head review requested for c00edbcffa28f59f32cc6aeab8375b062d56a04c.

@opencode-agent @cwl-noema-review Please review the request-model security boundary, the C0/C1 allow/deny classification, tests, and documentation. Do not mutate the branch or merge. The current CI, Security Scan, and SAST Semgrep runs are queued and are not being treated as passing; CodeRabbit's automatic comment is rate-limited and is not a review.

Copy link
Copy Markdown
Collaborator Author

Superseded by canonical PR #832. The unique C1-range coverage from this duplicate has been ported test-first to #832 exact head 5bf6cc56ca4e80d89fed852362a6ae4f8717be98, which also retains the broader raw-validation-response redaction and transport documentation. Closing this PR to preserve a single writer lane for issue #764; no merge or branch-protection action is performed.

@seonghobae seonghobae closed this Aug 11, 2026
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.

security(api): reject non-text controls in multiline SQL requests

1 participant