Implement P2 security hardening and P3 performance items - #747
Merged
Conversation
P2 (security hardening): - Hash refresh tokens at rest (SHA-256 via ITokenService.HashRefreshToken); Login/RefreshToken handlers look up by hash, never store the raw token. Rotation also sweeps the user's other revoked/expired rows. - Security headers at nginx: X-Content-Type-Options, Referrer-Policy, X-Frame-Options, a minimal CSP (frame-ancestors 'self'), and HSTS on every response. - SSE no longer carries any credential in its query string: POST /api/events/ticket (header-authenticated, including X-Household-Id) mints a short-lived single-use ticket (SseTicketService); GET /api/events redeems it instead of accepting a JWT via ?token=. - CSPRNG invite/recipe-share tokens (SecureTokenGenerator.GenerateHexToken, RandomNumberGenerator) replacing Guid.NewGuid(). P3 (performance): - Every Queries/ handler's .Query() call now chains .AsNoTracking() (58 handler files + SearchIndexService) — confirmed safe: no Queries/ handler saves, and none use EF Include (this codebase joins in C#), so there's no tracked/no-tracking identity-map risk. AsNoTracking is also a documented no-op against the unit tests' in-memory IQueryable mock. - MinioStorageService.GetFileStream streams through a bounded Pipe (64 KB backpressure threshold) instead of buffering the whole object into a MemoryStream first. - SSE broadcasts are household-scoped: SseConnectionManager registers each connection under the household id resolved at ticket issuance and only broadcasts within it; IRealtimeNotifier.Notify takes an explicit householdId parameter (all 14 call sites updated). - Composite-index and middleware-caching items investigated: both already covered (EF's FK-convention indexes on every HouseholdId column; a unique compound index already covers HouseholdMiddleware's own query) — no speculative changes made without profiling evidence. Frontend piece still open: useRealtimeSync.ts needs to call the new ticket endpoint before opening the EventSource, blocked on update-api-client regenerating the Kiota client for the new endpoint. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MoQrUqEN5AoByRCVYD6Qrt
Frontend half of P2-8: useRealtimeSync now mints a fresh, single-use ticket (POST /api/events/ticket via apiClient) on every connect and reconnect instead of reading the raw access token from localStorage. EventSource's own auto-retry is suppressed (onerror explicitly closes and schedules our own reconnect) since it would otherwise keep retrying the same URL with an already-spent ticket. Every item in SECURITY-PERFORMANCE-PLAN.md (P1-P3) has now landed and its durable guidance is folded into the relevant agent.md files, so the plan file is deleted per its own stated convention. Also fixes one bullet in Anything.Application/agent.md that was left describing the streaming fix as still-to-do, and documents the P3-13 index finding (EF's FK-convention index already covers every HouseholdId column) in Anything.Database/agent.md. Verified: frontend lint, full Jest suite (111 suites / 1164 tests), and production build all pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MoQrUqEN5AoByRCVYD6Qrt
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
P2 (security hardening):
Login/RefreshToken handlers look up by hash, never store the raw token.
Rotation also sweeps the user's other revoked/expired rows.
X-Frame-Options, a minimal CSP (frame-ancestors 'self'), and HSTS on
every response.
/api/events/ticket (header-authenticated, including X-Household-Id)
mints a short-lived single-use ticket (SseTicketService); GET
/api/events redeems it instead of accepting a JWT via ?token=.
RandomNumberGenerator) replacing Guid.NewGuid().
P3 (performance):
handler files + SearchIndexService) — confirmed safe: no Queries/
handler saves, and none use EF Include (this codebase joins in C#), so
there's no tracked/no-tracking identity-map risk. AsNoTracking is also
a documented no-op against the unit tests' in-memory IQueryable mock.
(64 KB backpressure threshold) instead of buffering the whole object
into a MemoryStream first.
each connection under the household id resolved at ticket issuance and
only broadcasts within it; IRealtimeNotifier.Notify takes an explicit
householdId parameter (all 14 call sites updated).
already covered (EF's FK-convention indexes on every HouseholdId
column; a unique compound index already covers HouseholdMiddleware's
own query) — no speculative changes made without profiling evidence.
Frontend piece still open: useRealtimeSync.ts needs to call the new
ticket endpoint before opening the EventSource, blocked on
update-api-client regenerating the Kiota client for the new endpoint.
Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01MoQrUqEN5AoByRCVYD6Qrt