feat(sast): detect bearer DNS validation TOCTOU

[seonghobae]

Security objective

Preserve the DNS-rebinding flaw fixed by security issue #892 / PR #898 as an executable AppGuardrail detector obligation instead of treating the merged runtime repair as the end of the incident.

Source authority

The source-backed vulnerable shape is protected-history predecessor develop@42a1ae16a8352c727b2a4a34f8a74eef32cff49c, where _push_findings first called _is_safe_url(url), then derived an endpoint, created an urllib.request.Request carrying Authorization: Bearer ..., and finally dispatched that request through urllib. The validation and connection therefore made separate DNS decisions. PR #898 (e8e4631f363da5a30b691f42261624db945088e0, protected merge c395459ed20cf47207cf5f8842f3c78a3a4c1298) replaced that preflight-only boundary with post_json_pinned_https, which connects to the validated address set while preserving TLS hostname verification.

Detector

Adds a HIGH CWE-367/CWE-918 detector family with unique packaged rule identities:

• python-bearer-preflight-dns-toctou covers Bearer authorization supplied directly by the tracked urllib.request.Request.
• python-bearer-preflight-dns-toctou-header-mutation covers the same validated-destination race when Bearer authorization is added or restored on that live request after construction.
• python-bearer-preflight-dns-toctou-multiline-constructor covers the corresponding reviewed line-wrapped constructor credential syntax.
• python-bearer-preflight-dns-toctou-multiline-header-mutation covers reviewed line-wrapped post-construction credential mutations, including flows where the tracked Request constructor itself is also line-wrapped.
• python-bearer-preflight-dns-toctou-dynamic-bearer-replacement covers a bounded post-construction case where a local replacement value is itself provably derived from a Bearer expression before being applied to the live Request.
• python-bearer-preflight-dns-toctou-unredirected-header-persistence covers the urllib-specific case where add_unredirected_header stores Bearer Authorization in unredirected_hdrs, so ordinary mutations of req.headers do not actually remove the live credential.

None of these rules match issue text. Together they bind, inside one Python function, the reusable failure path:

1. a fail-closed _is_safe_url(url) preflight;
2. an endpoint derived from that same URL;
3. an urllib.request.Request whose actual destination remains that endpoint;
4. Bearer credential provenance from either the direct headers= argument or a later supported request-header mutation; and
5. later urllib/reviewed-opener dispatch of that live request, which can resolve the hostname again after the check.

The credential-source subrules are mutually exclusive for the reviewed overlap. If the Request is already directly Bearer-authenticated, a simple later Bearer update remains a single primary-rule defect; the mutation subrule becomes authoritative for that Request only after a supported explicit Authorization removal followed by Bearer restoration. Opaque Authorization replacement, whole-header-map replacement/clearing, and unrelated Request.full_url mutation terminate stale credential or destination provenance rather than inheriting constructor state forever when those operations actually affect the active credential store. If a replacement variable is locally provable as Bearer ..., the bounded dynamic-replacement companion preserves detection. An initially unauthenticated Request that later receives Bearer authorization remains mutation-rule positive. For add_unredirected_header, ordinary req.headers.clear(), req.headers = {}, req.headers.pop("Authorization", ...), or regular-map Authorization overwrite do not remove the separate unredirected credential store; remove_header("Authorization") remains the supported universal removal boundary. The multiline companions preserve the same destination, request-identity, credential-state, and reachability barriers instead of treating wrapped formatting as a separate defect class. This preserves one family finding for one path without suppressing remove→restore or mutation-only flaws.

Current-head review also exposed two bounded exhaustive-control-flow false positives in the direct-Bearer primary rule. A direct two-arm if/else immediately before the outer urlopen(req) is now treated as sanitized only when both arms explicitly call req.remove_header("Authorization"); one-sided removal remains positive because a Bearer-bearing path still reaches the sink. Likewise, a direct two-arm if/else where both arms terminate with return/raise makes the following outer sink unreachable; if either arm can fall through, the finding remains. These are deliberately narrow proofs, not claims of general Python path analysis.

The family is deliberately bounded to the reviewed Python urllib shapes. Other HTTP libraries, cross-function request construction, non-_is_safe_url validators, custom transports that independently enforce pinning, and substantially different helper-mediated flow remain separate obligations rather than speculative HIGH findings.

Regression corpus

• tests/fixtures/security_corpus/appguardrail_bearer_dns_toctou_vulnerable.py preserves the historical POST preflight-then-second-resolution credential path.
• tests/fixtures/security_corpus/appguardrail_bearer_dns_toctou_fixed.py preserves the reviewed pinned-HTTPS repair.
• tests/test_bearer_dns_toctou_rule.py executes the production _scan_file path and fixes false-positive boundaries for unauthenticated urllib delivery, validation without dispatch, sibling-function evidence donation, formatting variants, reassignment, and reviewed dispatch forms.
• tests/test_bearer_dns_toctou_review_boundaries.py preserves destination/header provenance boundaries: request-body Authorization text is not header evidence; the tracked endpoint must be the actual Request URL; unrelated validated-URL replacement breaks provenance; and same-branch credential removal does not sanitize an opposite branch.
• tests/test_bearer_dns_toctou_restored_credentials.py covers direct post-construction Bearer mutations, remove/restore flows, non-Bearer negatives, and replacement/reachability barriers.
• tests/test_bearer_dns_toctou_latest_review_regressions.py requires unique packaged IDs, keeps data=/method= POST arguments visible before direct headers=, rejects nested fixed-destination Bearer request replacement, and preserves a nested replacement that still targets the validated endpoint.
• tests/test_bearer_dns_toctou_family_dedup.py executes the production scanner and requires exactly one family finding for initially Bearer-authenticated Requests followed by add_header, add_unredirected_header, or direct Authorization replacement, while preserving mutation-only and remove→restore positives.
• tests/test_bearer_dns_toctou_multiline_regressions.py executes the multiline companion rules through production _scan_file, with paired vulnerable/sanitized cases for endpoint and request replacement, credential removal/non-Bearer replacement, unreachable dispatch, self-derived provenance, remove→restore, and fully multiline Request-constructor plus add_header/add_unredirected_header/direct-header-assignment flows.
• tests/test_bearer_dns_toctou_request_state_mutations.py executes production _scan_file for opaque credential replacement, whole-header replacement/clearing, fixed versus self-derived full_url, provable Bearer-valued replacement variables, and packaged dynamic-rule identity.
• tests/test_bearer_dns_toctou_unredirected_persistence.py executes production _scan_file for Bearer credentials installed in unredirected_hdrs, proving ordinary-header clear/replacement/pop/overwrite do not sanitize that store while remove_header("Authorization") terminates it.
• tests/test_bearer_dns_toctou_exhaustive_branch_regressions.py executes production _scan_file for paired exhaustive-vs-partial credential-removal and branch-termination cases, requiring no family finding only when every direct two-arm route sanitizes or terminates before the outer sink.
• docs/TRACEABILITY.md records prevention versus detector maturity plus the paired false-positive/false-negative boundaries, including that endpoint replacement breaks destination provenance only before the Request is bound and that urllib's normal and unredirected header stores require distinct credential-state handling.
• CHANGELOG.d/892-bearer-dns-toctou-detector.md records the detector, post-construction request-state provenance, dynamic Bearer replacement, and unredirected-header persistence boundaries as security capabilities.

Exact candidate

• Current exact head: 678c77d6cfb7e8f40ce2b53f1de5b07b839bd95c.
• Review-derived RED regression commit: 865b0e1a236e04aabfb51b8c0551fb22c5561deb.
• Exhaustive-branch production repair: 678c77d6cfb7e8f40ce2b53f1de5b07b839bd95c.
• Fresh exact-head repository workflows and current-head review are authoritative; predecessor results are not counted as passing.

Merge boundary

Do not merge from predecessor checks. Only the exact current head is authoritative. The repository Security Process workflow installs the pinned CodeGraph CLI and runs python3 scanner/cli/appguardrail.py scan --codegraph .; its exact-head result is therefore the CodeGraph conditional-gate evidence for this PR. Merge or auto-merge only after required checks are terminal-success, current-head review findings are reconciled, qualifying independent approval exists, and normal protected-branch policy accepts the PR. No force-push, protection bypass, self-approval, warning suppression, or detector waiver is authorized.

---

Summary by CodeRabbit

• 보안

  • Python urllib Bearer 인증 요청에서 URL 사전 검증과 DNS 확인 사이의 TOCTOU 취약점을 탐지합니다.
  • 인증 헤더 변경·복원 및 다양한 요청 형식에 대한 탐지 정확도와 범위를 개선했습니다.
  • DNS 고정 HTTPS를 사용하는 보호된 요청은 안전한 사례로 구분합니다.

• 테스트

  • 취약·보호 사례, 경계 조건, 중복 탐지 방지를 포함한 회귀 테스트를 추가했습니다.
  • Python 3.12 환경을 테스트 범위에 추가했습니다.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Python urllib의 Bearer 인증 요청에서 URL 사전 검증 후 DNS를 다시 조회하는 TOCTOU 흐름을 탐지하는 HIGH 규칙을 확장했습니다. 요청 provenance, Authorization 헤더 변형, 다양한 urllib 호출 형태와 경계 조건을 회귀 테스트로 검증했습니다.

Changes

Bearer DNS TOCTOU 탐지

Layer / File(s)	Summary
탐지 규칙과 회귀 fixture scanner/rules/ssrf_bearer_restore.yml, scanner/rules/ssrf.yml, tests/fixtures/security_corpus/*bearer_dns_toctou_*.py	Bearer 헤더의 초기 설정·복원, URL provenance, urlopen 및 안전 리다이렉트 opener 호출을 탐지하도록 규칙을 갱신했습니다. 취약한 흐름과 DNS 고정 HTTPS 수정 흐름을 fixture로 추가했습니다.
탐지 계약과 기본 회귀 테스트 tests/test_bearer_dns_toctou_rule.py	HIGH severity, 규칙 식별자, CWE 매핑, 양성·음성 urllib 흐름과 기본 sink 형태를 검증합니다.
Provenance와 실행 경계 회귀 테스트 tests/test_bearer_dns_toctou_current_review_regressions.py, tests/test_bearer_dns_toctou_flow_regression.py, tests/test_bearer_dns_toctou_latest_review_regressions.py	Request URL 인자, endpoint와 request 재할당, 조건부 dispatch, 도달 불가 sink와 규칙 중복 로딩 경계를 검증합니다.
자격 증명과 패턴 경계 회귀 테스트 tests/test_bearer_dns_toctou_restored_credentials.py, tests/test_bearer_dns_toctou_review_boundaries.py, tests/test_bearer_dns_toctou_family_dedup.py, tests/test_bearer_dns_toctou_mutation_paths.py, tests/test_bearer_dns_toctou_one_line_request_args.py	Bearer 헤더 추가·복원, 인증 제거, 비-Bearer 인증, 중첩 데이터, 분기별 요청 대체, mutation 경로, 인자 순서와 family 중복 제거를 검증합니다.
추적성 및 변경 기록 docs/TRACEABILITY.md, CHANGELOG.d/892-bearer-dns-toctou-detector.md, .github/workflows/tests.yml	탐지기 family의 규칙 정체성, 적용 범위, provenance 경계, 승격 규칙과 Security 변경 내용을 기록합니다. 테스트 매트릭스에 Python 3.12를 추가했습니다.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: 🟡 Moderate · up to 4564f

The detector can currently misclassify vulnerable Python urllib flows by either connecting unrelated control-flow branches or missing Requests whose Bearer header follows other supported arguments. That could reduce security finding accuracy, so the PR is not merge-ready until these matching boundaries are corrected and covered by regression tests.

Sequence Diagram(s)

sequenceDiagram
  participant push_scan
  participant URLValidator
  participant urllibRequest
  participant urllibOpener
  participant DNSResolver
  push_scan->>URLValidator: URL 사전 검증
  URLValidator-->>push_scan: 검증 결과
  push_scan->>urllibRequest: Bearer Authorization 요청 생성
  push_scan->>urllibOpener: 요청 전송
  urllibOpener->>DNSResolver: 호스트명 재조회
  DNSResolver-->>urllibOpener: 연결 주소 반환

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 38.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 99 functions across 12 files. (3 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	제목은 Bearer DNS 검증 TOCTOU 탐지기 추가라는 주요 변경 사항을 정확하고 간결하게 설명합니다.

Full details: Docstring Coverage

Explanation

Docstring coverage is 38.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 99 functions across 12 files. (3 skipped: 3 unsupported.)

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch sentinel/detect-bearer-dns-toctou-892

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[devin-ai-integration]

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 5 new potential issues.

[devin-ai-integration]

📝 Info: Fixture imports are unnecessary

The regression path scans fixture text without importing it. Undefined helper and module names do not invalidate these source oracles.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 4 new potential issues.

[devin-ai-integration]

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 1 new potential issue.

[devin-ai-integration]

📝 Info: Inverse guards model exclusive paths

Credential insertion under enabled cannot reach dispatch under not enabled. Moving dispatch outside the guard creates a reachable credential-bearing path.

---

Was this helpful? React with 👍 or 👎 to provide feedback.