Skip to content

fix(security): outbound-fetch allow-list, scrape budget, AutoPilot input, auth/XML hardening - #19

Merged
fleizean merged 9 commits into
developfrom
gsd/quick-261027-security-hardening
Oct 26, 2026
Merged

fleizean merged 9 commits into
developfrom
gsd/quick-261027-security-hardening

Conversation

@fleizean

Copy link
Copy Markdown
Member

Summary

Harden the outbound-fetch, analysis and auth surfaces from an internal defensive security review. Eight small, self-contained backend fixes; no user-facing behaviour change beyond tightening what the server will fetch.

Why

Closes #17. An internal review found a handful of hardening gaps on the scraping/outbound-fetch, analysis and auth code. No working exploit is shipped here and the specifics are kept out of the public issue on purpose (see SECURITY.md); this PR is the fix.

Changes

  • feat(security): external release-note fetches are constrained to a documentation-host allow-list (new DocumentationHostAllowList, exact-label/domain-suffix match — never substring). ReleaseNotesUrlResolver skips non-allowed hosts; the existing private-IP SsrfGuard is unchanged and GitHub handling is untouched.
  • feat(security): the analyze endpoint's background scrape now goes through the same per-IP + global scrape budget the metadata endpoint already enforces (new IScrapeBudget Application port + ScrapeBudget API adapter over IRateLimitCounterStore, registered alongside the IProgressReporter precedent). The 12h per-package dedup is kept; the System refresh job bypasses the budget.
  • feat(security): AutoPilot release notes are fenced as untrusted data in the LLM prompt, and the quality gate rejects drafts whose fenced code blocks pipe a URL into a shell. Output stays draft-only (IsPublished=false).
  • fix(auth): pin JWT validation to HS256 (ValidAlgorithms).
  • fix(auth): unify the JWT lifetime behind one 8h constant (was an 8h token with a 24h reported expiry).
  • fix(nuget): harden the two bare .nuspec XDocument.Parse sites against XXE (DtdProcessing.Prohibit, XmlResolver=null, MaxCharactersFromEntities=0), matching the manifest/POM pattern already in the codebase.
  • fix(analysis): make the per-IP analysis concurrency cap atomic (check-and-increment, single release).
  • fix(suggestions): clamp the suggestions limit to [1,50].

Notes for reviewers

  • Backend only. No schema changes, no new dependencies.
  • One existing Postgres test (PostgresQueryTests.cs) gained a single null! for the new IScrapeBudget constructor parameter to keep the build green — no test logic was added.
  • Two stale claims in .claude/CLAUDE.md were corrected locally, but .claude is git-ignored, so that correction is not part of this diff.

Checklist

  • The pull request targets develop and is about one thing
  • Commit messages follow Conventional Commits (feat(scope): ..., fix(scope): ...)
  • CI is green (backend tests, frontend build and tests, dependency gate)
  • Schema changes are EF Core migrations generated by the tool, not hand-edited (none here)
  • A changed backend DTO has its TypeScript twin updated (no DTO changed)
  • New dependencies are justified and their lock files are updated in the same commit (none)
  • No secrets, tokens or personal data in code, logs or this description
  • Code comments are kept to a minimum; the reasoning lives in this description

Enes Yağız and others added 8 commits October 27, 2026 01:20
…ation host allow-list

- Add DocumentationHostAllowList with exact-label/suffix matching (no substring bypass)
- Skip ExternalPage fetch when the host is not on the curated allow-list

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…scrape budget

- Add IScrapeBudget port (Application) + ScrapeBudget adapter (API) over IRateLimitCounterStore
- Thread the analysing IP into TriggerBackgroundScrapingAsync so enqueue reserves the shared per-IP(30/hr)+global(600/hr) budget
- Keep the 12h per-package dedup before the reserve; System refresh bypasses the budget
- Reduce PackageController.MayQueueScrapeAsync to the shared port

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…eject shell-pipe code blocks

- Wrap scraped release notes in a labelled untrusted-data delimiter block with an ignore-instructions guard
- Reject drafts whose fenced code blocks pipe a downloaded URL into a shell (curl/wget | sh, bash -c piped URL, iex/Invoke-Expression download)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Set ValidAlgorithms so only HS256-signed tokens are accepted (blocks algorithm confusion)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Derive both the token expiry and LoginResponse.ExpiresAt from TokenLifetimeHours (8h), fixing the 24h/8h mismatch

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Replace XDocument.Parse with a DtdProcessing.Prohibit, resolver-null, entity-capped XmlReader feeding XDocument.Load

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Increment then check-and-release so concurrent requests from one IP cannot exceed the cap; release the slot exactly once via an acquired flag

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Use Math.Clamp so non-positive values become 1 and oversized values are capped at 50

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@fleizean
fleizean merged commit c5ae986 into develop Oct 26, 2026
5 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.

Security hardening batch (outbound-fetch allow-list, scrape budget, AutoPilot input, auth/XML hardening)

1 participant