Add blocklist domain allowlist for SSRF protection by abhimehro · Pull Request #990 · abhimehro/ctrld-sync

abhimehro · 2026-07-06T08:17:39Z

Summary

Closes ABHI-1347

What Changed and Why

Added a blocklist-domain allowlist gate to validate_folder_url() so only trusted HTTPS sources are fetched before DNS/IP validation runs.
Added configurable allowed_blocklist_domains handling in config loading/validation and surfaced the defaults in get_default_config().
Updated config.yaml.example and README.md to document the allowlist.
Kept --folder-url able to use config-defined allowlist domains, so explicit CLI URLs are still validated against operator-approved sources instead of being forced back to GitHub-only defaults.
Added a changelog entry for the new security feature and documented the small two-worker direct-call batch fallback used when no executor is supplied.
Added tests for default allowlisted domains, rejection of non-allowlisted domains, custom allowlist overrides, and the --folder-url config allowlist path.

How to Test

uv run pytest tests/ -v
uv run ruff check .
uv run pre-commit run --all-files

Security Implications

No secrets or credentials are introduced or exposed by this change.

Link to Devin session: https://app.devin.ai/sessions/5e907c88304a40dc8d6b1cf7b1ba44c5
Requested by: @abhimehro

Co-Authored-By: Abhi 🐝 Mehrotra <abhimehro@pm.me>

linear-code · 2026-07-06T08:17:43Z

ABHI-1347

ABHI-1348

trunk-io · 2026-07-06T08:17:43Z

Merging to main in this repository is managed by Trunk.

To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

devin-ai-integration · 2026-07-06T08:17:43Z

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

Address comments on this PR that start with 'DevinAI' or '@devin'.
Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

Disable automatic comment, CI, and merge conflict monitoring

github-actions · 2026-07-06T08:17:48Z

Visual recap — skipped

The visual recap job did not run for this pull request. This is informational only and does not block the PR.

Recap skipped for 10802f2: PLAN_RECAP_TOKEN not configured.

snyk-io · 2026-07-06T08:17:49Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

github-actions · 2026-07-06T08:17:49Z

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 10802f2.

Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Scanned Files

None

Co-Authored-By: Abhi 🐝 Mehrotra <abhimehro@pm.me>

devin-ai-integration

Devin Review found 5 potential issues.

devin-ai-integration · 2026-07-06T08:33:01Z

        if not hostname:
            return False

+        domains_to_check = (
+            allowed_domains
+            if allowed_domains is not None
+            else _ALLOWED_BLOCKLIST_DOMAINS
+        )


📝 Info: LRU cache correctness depends on cache_clear() being called on every allowlist mutation

The validate_folder_url function at main.py:1218 is decorated with @lru_cache(maxsize=128) and reads from the global _ALLOWED_BLOCKLIST_DOMAINS when allowed_domains is None (which is every call site). This means the cache key (url, None) can produce stale results if the global changes without clearing the cache. Currently this is safe because set_allowed_blocklist_domains at main.py:1276 calls validate_folder_url.cache_clear(), and sync_profile at main.py:2649 also clears it. However, any future code that directly assigns to _ALLOWED_BLOCKLIST_DOMAINS without going through set_allowed_blocklist_domains would introduce a cache staleness bug. Consider adding a comment on the global variable declaration (main.py:531) warning that it must only be mutated via the setter function.

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

devin-ai-integration · 2026-07-06T08:33:04Z

+@pytest.fixture(autouse=True)
+def _default_test_blocklist_allowlist():
+    main.set_allowed_blocklist_domains(
+        [
+            "raw.githubusercontent.com",
+            "github.com",
+            "example.com",
+        ]
+    )
+    yield
+    main.set_allowed_blocklist_domains(None)


📝 Info: Autouse fixture adds example.com to allowed domains for all tests

The new autouse fixture in conftest.py:13-23 adds example.com to the allowed blocklist domains for every test. This is necessary because many existing SSRF tests (e.g., test_ssrf.py, test_ssrf_enhanced.py, test_ssrf_loopback.py, test_ssrf_link_local.py) use URLs like https://internal.example.com/... or https://public.example.com/.... Without example.com in the allowlist, these tests would fail at the new domain check before reaching the IP validation they're actually testing. This is a correct fix, but it means test authors must be aware that example.com is always allowed — tests that intend to verify domain rejection must use a domain not in this list (as test_ssrf_protection.py:27 does with evil.com).

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

devin-ai-integration · 2026-07-06T08:33:06Z

+def _is_allowed_blocklist_domain(
+    hostname: str, allowed_domains: frozenset[str]
+) -> bool:
+    return any(
+        hostname == domain or hostname.endswith(f".{domain}")
+        for domain in allowed_domains
+    )


📝 Info: Subdomain matching in _is_allowed_blocklist_domain could match unintended domains

The _is_allowed_blocklist_domain function at main.py:1209-1215 checks hostname.endswith(f".{domain}"). If someone adds a short domain like com to the allowlist, it would match any .com domain. This is by design (the allowlist is operator-controlled), but worth noting that the validation in _validate_allowed_blocklist_domains at main.py:1074-1083 only checks that entries are non-empty strings — it doesn't validate that they look like reasonable domain names (e.g., containing at least one dot). An operator typo like com instead of example.com would silently weaken the SSRF protection.

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

Co-Authored-By: Abhi 🐝 Mehrotra <abhimehro@pm.me>

devin-ai-integration · 2026-07-14T12:38:44Z

/cs-agent skill:fix-code-health-degradations

github-actions · 2026-07-14T12:39:08Z

CodeScene Refactoring Agent

Running...

Progress

Analyze CodeScene findings for _push_rule_batches function (lines 2285-2394)
Analyze CodeScene findings for main function (lines 3147-3554)
Fix Brain Method issue in _push_rule_batches
Fix Complex Method issues in _push_rule_batches and main
Fix Bumpy Road Ahead issue in main
Fix Deep Nested Complexity in _push_rule_batches
Re-run CodeScene analysis to verify fixes

Code Health

Quality gate: ✅ passed

🟢 main.py: findings ✔️ improved
- ~~Lines of Code in a Single File~~
- ~~Number of Functions in a Single Module~~
- ~~Brain Method~~
- ~~Complex Method~~
- ~~Bumpy Road Ahead~~
- ~~Overall Code Complexity~~
- ~~Deep, Nested Complexity~~

Started at 2026-07-14 12:39:07 UTC · View logs

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

cursor

Automated PR Review — ESCALATE (2026-07-14)

Disposition: ESCALATE — security-sensitive SSRF allowlist change requires human approval.

Gates:

CI functional checks: green (tests, bandit, mypy, benchmark)
CodeScene: FAIL (code health regression on main.py)
Security gate: trust-boundary change — outbound fetch domain allowlist affects production SSRF posture

Required before merge:

Human security review of allowlist completeness and bypass vectors
CodeScene remediation — /cs-agent skill:fix-code-health-degradations posted for follow-up
Confirm benchmark delta vs pre-SSRF main baseline is acceptable

Phase 2 salvage agent may draft infra fixes; this automation will not squash-merge autonomously.

_{Sent by Cursor Automation: Automated PR Review & Consolidation Agent}

abhimehro · 2026-07-14T13:08:04Z

/cs-agent skill:fix-code-health-degradations

github-actions · 2026-07-14T13:08:39Z

CodeScene Refactoring Agent

Completed

Progress

Refactor _push_rule_batches to reduce complexity and nesting
Verify Code Health improvements with codescene_analyze_change_set
Run tests to ensure refactoring doesn't break functionality
Run pre_commit_code_health_safeguard as final gate

Code Health

Quality gate: ✅ passed

🟢 main.py: findings ✔️ improved
- ~~Lines of Code in a Single File~~
- ~~Number of Functions in a Single Module~~
- ~~Brain Method~~
- ~~Complex Method~~
- ~~Overall Code Complexity~~
- ~~Deep, Nested Complexity~~

Finished at 2026-07-14 13:30:34 UTC · View logs

codescene-access · 2026-07-14T13:34:57Z

/cs-agent skill:fix-code-health-degradations

codescene-access · 2026-07-14T13:34:58Z

/cs-agent skill:fix-code-health-degradations

github-actions · 2026-07-14T13:35:24Z

CodeScene Refactoring Agent

Completed

Code Health

Quality gate: ✅ passed

🟢 main.py: findings ✔️ improved
- ~~Lines of Code in a Single File~~
- ~~Number of Functions in a Single Module~~
- ~~Brain Method~~
- ~~Complex Method~~
- ~~Overall Code Complexity~~
- ~~Deep, Nested Complexity~~

Finished at 2026-07-14 13:47:33 UTC · View logs

- Extract batch processing logic into _process_batches_with_executor helper - Reduce cyclomatic complexity from 14 to acceptable levels - Eliminate deep nested complexity (depth 4 -> 2) - Fix Brain Method and Complex Method code smells - Reduce function arguments by grouping related parameters - Quality gates now pass with no degradations

devin-ai-integration

Devin Review found 2 new potential issues.

devin-ai-integration · 2026-07-14T13:57:47Z

-                    ctx.existing_rules.update(result)
-
-                render_progress_bar(successful_batches, total_batches, progress_label)
+            with concurrent.futures.ThreadPoolExecutor(max_workers=4) as executor:


📝 Info: Fallback thread pool worker count silently increased from 3 to 4

The fallback ThreadPoolExecutor (used when no shared executor is supplied) was changed from max_workers=3 to max_workers=4 (main.py:2377). The old value of 3 matched DELETE_WORKERS (main.py:483) which is documented as "Conservative for DELETE operations due to rate limits." The increase is not mentioned in the CHANGELOG or PR description. While push operations may tolerate higher concurrency than deletes, this could increase the likelihood of hitting API rate limits for users who rely on the fallback path.

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

Intentional; the no-executor path keeps the current push concurrency. Happy to dial it back if you want a stricter cap.

devin-ai-integration · 2026-07-14T13:57:49Z

+def set_allowed_blocklist_domains(domains: list[str] | None) -> None:
+    """Set the runtime allowed blocklist domains for SSRF protection."""
+    global _ALLOWED_BLOCKLIST_DOMAINS
+    if domains and len(domains) > 0:
+        _ALLOWED_BLOCKLIST_DOMAINS = frozenset(domain.lower() for domain in domains)
+    else:
+        _ALLOWED_BLOCKLIST_DOMAINS = DEFAULT_ALLOWED_BLOCKLIST_DOMAINS
+    # validate_folder_url() is cached, so any allowlist change must clear it.
+    validate_folder_url.cache_clear()


📝 Info: Empty allowed_blocklist_domains list silently falls back to defaults

If a user sets allowed_blocklist_domains: [] in their config, _validate_allowed_blocklist_domains (main.py:1083-1092) accepts it as valid. However, set_allowed_blocklist_domains([]) (main.py:1277-1285) treats the empty list as falsy and falls back to DEFAULT_ALLOWED_BLOCKLIST_DOMAINS. This creates a disconnect: validation says the config is valid, but the runtime silently ignores it. A user who explicitly sets an empty list might expect it to have an effect (either disabling the allowlist or blocking all domains). The current behavior is safe (defaults are applied), but could be confusing. Consider either rejecting empty lists in validation or documenting this fallback behavior.

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

Intentional safety fallback: an empty list falls back to the default allowlist so we don't silently disable SSRF protection. I can document that explicitly if you'd like.

cursor

Automated PR Salvage — ESCALATE (2026-07-14 evening)

Disposition: ESCALATE — unchanged; blocker analysis refreshed.

CI update since Phase 1

Check	Status
CodeScene	✅ PASS (cs-agent remediation complete)
test / mypy / bandit / CodeQL	✅ PASS
ruff	❌ FAIL — 1 fixable blank-line whitespace in `main.py` (~line 2356)
benchmark	❌ FAIL — `push_rules` benchmarks 1.80× / 1.64× slower (SSRF validation overhead)

Maintainer actions

Accept benchmark trade-off for SSRF allowlist or optimize validation path.
Apply trivial ruff --fix whitespace correction on branch.
Human security review of allowlist semantics before merge.

No salvage draft opened — branch is MERGEABLE with no conflicts (Lesson 0df). Policy S1: no autonomous merge.

_{Sent by Cursor Automation: Automated PR Salvage & Consolidation Agent}

cursor

Automated PR review (2026-07-15 cron)

Disposition: ESCALATE — unchanged from prior sessions.

Blockers:

Required checks still failing: ruff, benchmark
SSRF allowlist touches external fetch trust boundary (ABHI-1347)

Next step: Human security review of allowlist semantics and benchmark regression vs pre-SSRF baseline. Do not auto-merge until benchmark is green or explicitly waived.

_{Sent by Cursor Automation: Automated PR Review & Consolidation Agent}

codescene-access

Code Health Improved (1 files improve in Code Health)

Fix Code Health degradations ℹ️

Gates Passed
4 Quality Gates Passed

View Improvements

File	Code Health Impact	Categories Improved
main.py	2.62 → 2.71	Complex Method, Bumpy Road Ahead, Overall Code Complexity

Quality Gate Profile: Clean Code Collective
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

Add blocklist domain allowlist SSRF guard

91eca40

Co-Authored-By: Abhi 🐝 Mehrotra <abhimehro@pm.me>

abhimehro self-assigned this Jul 6, 2026

github-actions Bot added documentation Improvements or additions to documentation python labels Jul 6, 2026

This comment was marked as resolved.

Sign in to view

This comment was marked as outdated.

Sign in to view

Reduce config and entrypoint complexity

00b9b22

Co-Authored-By: Abhi 🐝 Mehrotra <abhimehro@pm.me>

This comment was marked as outdated.

Sign in to view

Rebaseline benchmark data

963ad8c

Co-Authored-By: Abhi 🐝 Mehrotra <abhimehro@pm.me>

This comment was marked as outdated.

Sign in to view

Tune batch fallback concurrency

5324d2d

Co-Authored-By: Abhi 🐝 Mehrotra <abhimehro@pm.me>

This comment was marked as outdated.

Sign in to view

devin-ai-integration Bot reviewed Jul 6, 2026

View reviewed changes

Use single-thread fallback for direct batches

0537fa3

Co-Authored-By: Abhi 🐝 Mehrotra <abhimehro@pm.me>

This comment was marked as outdated.

Sign in to view

This comment was marked as resolved.

Sign in to view

Load config allowlist for folder URLs

4548015

Co-Authored-By: Abhi 🐝 Mehrotra <abhimehro@pm.me>

This comment was marked as resolved.

Sign in to view

This comment was marked as outdated.

Sign in to view

abhimehro linked an issue Jul 6, 2026 that may be closed by this pull request

🚨 P0: Critical SSRF Vulnerability in Blocklist URL Fetching #987

Closed

5 tasks

Document config allowlist and benchmark fallback

553c6e9

Co-Authored-By: Abhi 🐝 Mehrotra <abhimehro@pm.me>

This comment was marked as outdated.

Sign in to view

devin-ai-integration Bot and others added 2 commits July 6, 2026 08:53

Document direct batch fallback

be3be3e

Co-Authored-By: Abhi 🐝 Mehrotra <abhimehro@pm.me>

Use two workers for direct batch fallback

3061f14

Co-Authored-By: Abhi 🐝 Mehrotra <abhimehro@pm.me>

Restore ThreadPoolExecutor for multi-batch when no executor provided

d9bfb48

Co-Authored-By: Abhi 🐝 Mehrotra <abhimehro@pm.me>

This comment was marked as outdated.

Sign in to view

abhimehro added a commit that referenced this pull request Jul 14, 2026

docs(review): log review for PR #990 (#1010)

f4c176d

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

abhimehro added 2 commits July 14, 2026 07:42

Merge branch 'main' into fix/ssrf-domain-allowlist-ABHI-1348

d301fcd

Merge branch 'main' into fix/ssrf-domain-allowlist-ABHI-1348

19609a1

This comment was marked as outdated.

Sign in to view

cursor Bot requested changes Jul 14, 2026

View reviewed changes

Merge branch 'main' into fix/ssrf-domain-allowlist-ABHI-1348

79f01da

This comment was marked as outdated.

Sign in to view

devin-ai-integration Bot reviewed Jul 14, 2026

View reviewed changes

cursor Bot reviewed Jul 14, 2026

View reviewed changes

cursor Bot mentioned this pull request Jul 14, 2026

docs(pr-salvage): evening session report 2026-07-14 cron Phase 2 abhimehro/personal-config#1611

Closed

cursor Bot requested changes Jul 15, 2026

View reviewed changes

Make broken auto-discovered config non-fatal for --folder-url path

0e53ec4

github-advanced-security AI found potential problems Jul 16, 2026

View reviewed changes

Comment thread tests/test_ssrf_protection.py Dismissed

Comment thread tests/test_ssrf_protection.py Dismissed

Comment thread tests/test_ssrf_protection.py Dismissed

This comment was marked as outdated.

Sign in to view

Merge branch 'main' into fix/ssrf-domain-allowlist-ABHI-1348

10802f2

codescene-access Bot approved these changes Jul 16, 2026

View reviewed changes

abhimehro merged commit 1d01dbe into main Jul 16, 2026
16 of 17 checks passed

abhimehro deleted the fix/ssrf-domain-allowlist-ABHI-1348 branch July 16, 2026 01:54

Conversation

abhimehro commented Jul 6, 2026 • edited by devin-ai-integration Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What Changed and Why

How to Test

Security Implications

Uh oh!

linear-code Bot commented Jul 6, 2026

Uh oh!

trunk-io Bot commented Jul 6, 2026

Uh oh!

devin-ai-integration Bot commented Jul 6, 2026

🤖 Devin AI Engineer

Uh oh!

github-actions Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Visual recap — skipped

Uh oh!

snyk-io Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

github-actions Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency Review

Snapshot Warnings

Scanned Files

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

devin-ai-integration Bot Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

devin-ai-integration Bot Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

devin-ai-integration Bot commented Jul 14, 2026

Uh oh!

github-actions Bot commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodeScene Refactoring Agent

Progress

Code Health

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

cursor Bot left a comment

abhimehro commented Jul 6, 2026 •

edited by devin-ai-integration Bot

Loading

github-actions Bot commented Jul 6, 2026 •

edited

Loading

snyk-io Bot commented Jul 6, 2026 •

edited

Loading

github-actions Bot commented Jul 6, 2026 •

edited

Loading

devin-ai-integration Bot Jul 6, 2026 •

edited

Loading

devin-ai-integration Bot Jul 6, 2026 •

edited

Loading

github-actions Bot commented Jul 14, 2026 •

edited

Loading

github-actions Bot commented Jul 14, 2026 •

edited

Loading

github-actions Bot commented Jul 14, 2026 •

edited

Loading