Skip to content

security: backport v0.8.3 hardening to main#111

Merged
brandonhon merged 15 commits into
mainfrom
security/prod-backport
Jun 5, 2026
Merged

security: backport v0.8.3 hardening to main#111
brandonhon merged 15 commits into
mainfrom
security/prod-backport

Conversation

@brandonhon
Copy link
Copy Markdown
Owner

Security hardening backport for the v0.8.3 release. All changes target code that ships on main; develop-only features (email inbox, web push) are excluded.

High

  • Server-side HTML sanitization (bluemonday) at every ingest point + http(s)-only guard on rendered article/image URLs; SafeHTTPURL also requires a host.

Medium

  • Login timing equalized on unknown username (no enumeration oracle).
  • /api/users no longer fetches password_hash/fever_token into memory.
  • TT-RSS API import no longer leaks DNS/TLS/endpoint detail to the client.
  • Discover() validates the discovered feed link (SSRF).
  • TT-RSS API redirect guard honors the request context.
  • Malformed FTS5 search returns 400 instead of 500.
  • Ollama model names validated against an allowlist (admin endpoints).
  • Filters: per-user count cap (200) + regex length cap (512) — bounds the compiled-regex cache (memory DoS).
  • DNS-rebinding closed: IP-pinning dialer re-resolves and rejects private resolved IPs on the feed fetcher + readability client.

Low / hardening

  • Bootstrap admin password 8-char minimum.
  • SSRF redirect guard baked into NewFetcher (safe default is the only option).
  • ExtractFromURL requires a guarded HTTP client.

Verified: build, vet, full test suite, gofmt all green on main's base.

brandonhon and others added 15 commits June 5, 2026 14:51
Feed/article HTML is rendered via {@html}, with the CSP as the only
defense against stored XSS. Add server-side sanitization so the CSP
and sanitization are defense-in-depth rather than a single control.

- feed.SanitizeHTML (bluemonday UGCPolicy): strips <script>, inline
  event handlers, <style>, and javascript:/data: URLs.
- feed.SafeHTTPURL: promoted from the TT-RSS importer so every ingest
  path shares one http(s)-only URL guard.
- Apply at all ingest points: feed parse (body + article href),
  readability extraction, TT-RSS import, and the LLM cleaned_html.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A missing username returned in ~1ms while a real one took ~100ms
(argon2), leaking account existence via a timing side channel. Run a
throwaway argon2id derivation with the live cost params on the
user-not-found path so both paths cost the same.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
handleListUsers projected away secrets in Go but ListUsers still
SELECTed password_hash and fever_token into memory. Add ListUsersPublic
(id, username, email, is_admin, created_at) so those columns never
leave the database for a request that only needs identity and role.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The TT-RSS API import returned raw err.Error(), exposing the resolved
endpoint, internal hostnames, and DNS/TLS detail. Log the full error
server-side and return a generic message, mirroring the SMTP-test
handler.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Discover() returned the alternate <link> URL without running it through
the SSRF validator, while DiscoverAll already validates each discovered
link. Validate before returning; on reject/unresolvable, fall through to
the fallback probes (which validate each) rather than handing back an
unchecked URL.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
apiClient built its redirect SSRF guard with context.Background(), so
the per-redirect validation ignored the import request's cancellation
and deadline. Pass the call's ctx through instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A syntactically-invalid search (unbalanced quote, bare boolean operator,
bad column filter) surfaced as a 500. Detect the FTS5 MATCH-syntax error
classes, return store.ErrInvalidQuery, and map it to 400 — it's client
input, not a server fault.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
BootstrapAdmin accepted any non-empty EMBER_ADMIN_PASSWORD while the
create-user and change-password paths enforce an 8-char floor. Apply
the same minimum so the first-run admin can't be seeded weak.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
NewFetcher built an unguarded client and relied on every caller to set
CheckRedirect afterward. Make validate a constructor parameter and wire
RedirectGuard in at construction so the safe default is the only option
— a caller can't accidentally build a fetcher that follows redirects to
private addresses.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Promotes microcosm-cc/bluemonday (+ douceur, gorilla/css) to direct deps
for the feed.SanitizeHTML sanitizer introduced in this backport.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fuzzing surfaced that a scheme-only value like "HTTP:" (or an opaque
"https:foo") passed the guard and was returned as a "safe" URL despite
having no host to link/fetch. Not a javascript:/data: bypass, but a
sloppy result for a value rendered as an href/src. Require u.Host != "".
Relative links are resolved to absolute before this call, so legitimate
URLs are unaffected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The admin LLM endpoints passed the model name verbatim to the Ollama
API. Constrain it to the documented [registry/][namespace/]name[:tag]
shape so a rogue/compromised admin can't coax Ollama into pulling from
an arbitrary registry or dereferencing path-traversal components.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The nil-client fallback built an unguarded client (no SSRF redirect/dial
guard). It's never used in production (the poller passes a guarded
client), so make nil an explicit error rather than a silent footgun.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ding

urlcheck.Check validated the hostname, but the HTTP stack resolved DNS
again at dial time — an attacker controlling DNS could answer public to
the check and private to the dial (TOCTOU rebind). Add
urlcheck.DialContext / GuardedTransport, which re-resolves, rejects any
private resolved IP, and dials the pinned address. Wire it into the feed
fetcher and the poller's readability client (alongside the existing
redirect guards).

(Backport of develop 57a9db1; the push-notifier wiring is omitted —
that feature isn't on main.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Each filter's regex is compiled and cached for the process lifetime
(reCache, no eviction), so an authenticated user creating unbounded
filters/patterns is a slow-burn memory DoS. Cap at 200 filters per user
and 512 chars per regex. RE2 already rules out backtracking ReDoS.

(main-targeted equivalent of develop cad70d0, which doesn't cherry-pick
cleanly because develop's filters is the enhanced PR #78 version.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@brandonhon brandonhon enabled auto-merge (squash) June 5, 2026 21:55
@brandonhon brandonhon merged commit 9156008 into main Jun 5, 2026
8 checks passed
@brandonhon brandonhon deleted the security/prod-backport branch June 5, 2026 21:58
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