Skip to content

Harden the security layer (DNS pinning, rate-limit key, body cap, scan budget) and add a test suite - #2

Merged
tinkthemaker merged 3 commits into
mainfrom
devin/1785763781-harden-security-layer
Aug 3, 2026
Merged

Harden the security layer (DNS pinning, rate-limit key, body cap, scan budget) and add a test suite#2
tinkthemaker merged 3 commits into
mainfrom
devin/1785763781-harden-security-layer

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes the gap between what guardUrl() validates and what the scanners actually connect to, plus a handful of smaller correctness fixes, and adds the repo's first tests.

DNS pinning (the main one). guardUrl() resolved a hostname and checked every answer, then handed the hostname to fetch() / tls.connect(), which resolved it again — so a second DNS answer could point anywhere (classic rebinding TOCTOU). guardUrl now returns every validated address and there's a shared lookup that replays only those:

export function pinnedLookup(addresses: PinnedAddress[]): LookupFunction
// honours options.family / options.all, ENOTFOUND when no validated address matches

safeFetch had to move off fetch() to node:http/node:https request() to accept lookup. The public contract is unchanged (SafeFetchResult, manual redirects with the guard re-applied per hop, Headers with every set-cookie appended separately); only the init type narrowed from RequestInit to a local SafeFetchInit. No accept-encoding is sent, so nothing needs decompressing. lib/tls/scan.ts keeps host/servername as the hostname and passes the same pinned lookup to both the strict and lenient handshakes, which also stops the two sockets from landing on different endpoints.

IPv6 literals. url.hostname returns IPv6 literals bracketed, so net.isIP("[::1]") === 0 and every IPv6-literal target fell through to the hostname path and died in dns.lookup — fail-closed, but also unusable. Brackets are now stripped before literal detection, ::ffff: mapped addresses are decoded from their normalised hex form (::ffff:127.0.0.1::ffff:7f00:1) before the IPv4 blocklist is applied, and 2001:db8::/32 joins the blocklist for parity with the IPv4 documentation ranges.

Rate-limit key. The leftmost x-forwarded-for entry is client-controlled, so it was trivially spoofable. New precedence: x-vercel-forwarded-forx-real-iprightmost XFF entry → "unknown". Expired buckets are now swept (time-based, or when the map exceeds 1000 entries) so the map can't grow unbounded.

Body cap. readLimitedText appended a chunk then compared against 256 KiB, so one large chunk overshot the cap. It now slices the final chunk so the cap is a hard limit.

Per-scan time budget. The 6s timeout was per hop while misconfig/CORS make 6–9 requests against 15s/20s maxDuration. safeFetch takes an optional absolute deadlineMs and clamps each hop's timeout to the remaining budget (recomputed after DNS); misconfig and CORS set one deadline per scan (13s / 18s) and thread it through the initial fetch and every probe.

Smaller fixes

  • lib/tls/der.ts: real bounds validation (truncated input, indefinite lengths, over-long length bytes, lengths past the buffer, OID ending on a continuation bit) and correct OID first-arc decoding — first = min(floor(b/40), 2), so 0x55 04 03 decodes as 2.5.4.3, not 1.45.4.3.
  • lib/cors/scan.ts: the "suffix bypass" probe generated https://attacker<host-with-dots-stripped>.example, which tests nothing the label claims. Suffix now ends with the target host (example.comhttps://notexample.com, catching naive endsWith); prefix stays https://example.com.attacker.example (naive startsWith). Vary: Origin is checked on every reflected-origin probe instead of only the arbitrary-origin one.
  • lib/tls/types.ts: dropped the ed25519 / ed448 / dsa keyType variants — detectKey() cannot produce them, so the Edwards-key finding was dead code.
  • components/ToolCard.tsx: removed Wrapper: any in favour of explicit Link / div branches.

Tests

vitest (only new dep, dev-only) with npm test wired into CI. 39 tests over the pure logic that carries the risk: the SSRF blocklists (v4, v6, mapped, documentation ranges) and DNS paths (mocked node:dns/promises), the DER reader incl. every new malformed case, JWT parse/analyze finding ids and severities, the misconfig header/cookie/disclosure graders, rate-limit key precedence and the 12/minute trip, and the CORS probe construction + reflection-with-credentials grading.

The IPv6 literal bug above was caught by these tests on the first CI run, not by hand.

Network paths of safeFetch and tls/scan.ts are not covered — worth a follow-up with a local server.

npm test, npm run lint, npm run typecheck and npm run build all pass.

Link to Devin session: https://app.devin.ai/sessions/c13b64ef84ed489f8e564ba16c6ffe96
Requested by: @tinkthemaker

@tinkthemaker tinkthemaker self-assigned this Aug 3, 2026
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 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. Add '(aside)' to your comment to have me ignore it.
  • 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

@vercel

vercel Bot commented Aug 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cyber-toolbox Ready Ready Preview Aug 3, 2026 2:06pm

@tinkthemaker
tinkthemaker merged commit 19344f4 into main Aug 3, 2026
3 checks passed
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.

1 participant