block 1: critical ship-blockers (upload cap, auth hardening, bounded workq, scoped search) by aksOps · Pull Request #60 · RandomCodeSpace/docsiq

aksOps · 2026-04-23T19:00:18Z

Closes all five Block 1 items from the production-polish roadmap (docs/superpowers/specs/2026-04-23-production-polish-roadmap-design.md). Ships via 16 commits organized as 8 tasks per the Block 1 plan (docs/superpowers/plans/2026-04-23-block1-critical-plan.md), each with its own spec-compliance + code-quality review gates.

Summary

1.2 Upload bomb — http.MaxBytesReader + Content-Length fast-path caps multipart uploads; configurable via server.max_upload_bytes (default 256 MiB).
1.4 Insecure default — cmd/serve.go refuses to start when server.api_key is empty AND host binds non-loopback. Uses net.ParseIP().IsLoopback() so [::1], 127.0.0.0/8, and ::ffff:127.0.0.1 are all covered.
1.3 Fire-and-forget indexing — new internal/workq bounded pool (Workers + QueueDepth capacity, send-on-closed-channel race guarded by sync.RWMutex); upload handler submits through the pool, returns 503 + Retry-After: 30 when full; server shutdown drains with a 30s deadline after srv.Shutdown.
1.1 API key in HTML — new POST /api/session exchanges bearer for docsiq_session httpOnly; Secure; SameSite=Strict cookie (Max-Age 30d); DELETE /api/session clears it; auth middleware accepts header OR cookie; SPA handler no longer injects the <meta name=\"docsiq-api-key\"> tag; UI apiFetch and useMCP.rpc now send credentials: 'include' and never attach Authorization.
1.5 Full-scan entity fetch — new Store.EntitiesForDocs(ctx, docIDs) JOINs entities → relationships with chunked IN-lists (chunk size 900, below SQLite's 999 default); internal/search/local.go uses it instead of AllEntities.

Test plan

Full Go suite with race detector: CGO_ENABLED=1 go test -tags sqlite_fts5 -timeout 300s ./... → 541/541 pass across 23 packages
UI vitest: npm test -- --run → 54/54 pass across 18 files
UI typecheck: tsc --project tsconfig.app.json --noEmit → clean
Vite build bundle: 468 KB JS + 113 KB CSS, under the 640 KiB CI budget
New targeted tests: TestUploadMaxBytes{,_UnknownContentLength}, TestValidateServeSecurity (12 subtests), TestPool_* (5 subtests incl. TestPool_SubmitRaceDuringClose 50x32), TestUpload_ReturnsRetryOnFullQueue, TestSession_* (3), TestAuth_AcceptsValidCookie, TestAuth_RejectsMissingBothHeaderAndCookie, TestSpaHandler_DoesNotInjectAPIKey, TestEntitiesForDocs_* (2)
Manual smoke against a TLS-terminated deployment: POST /api/session returns 204 + Set-Cookie; subsequent /api/* calls succeed with cookie alone; DELETE /api/session clears it; restart with 503/Retry-After under load

Pre-merge review

Final integration review verdict: ⚠️ Merge with documented follow-ups. Per-task reviews (spec + quality) passed for all 8 tasks; the one Important integration finding (dead meta-tag read in useMCP.ts) was fixed inline in b7b1168.

Known follow-ups (non-blocking, file after merge)

docsiq login CLI is referenced in code comments / spec but doesn't exist yet. Operators on non-loopback hosts need curl -X POST /api/session -H \"Authorization: Bearer …\" to seed the cookie for a fresh browser.
Secure: true cookies are silently dropped by browsers over plain HTTP. go run . serve on HTTP localhost with an API key configured will leave the SPA unable to auth. Consider a server.cookie_secure config override (default true) for TLS-off dev.
Historical plan doc (docs/superpowers/plans/2026-04-18-ui-redesign.md) still describes the old meta-tag flow. Left as append-only history.

Commits (16)

`18a61bc` feat(api): cap multipart upload size via MaxBytesReader
`6a932ee` fix(api): address Task 1 code-review feedback
`8cbf5d0` feat(serve): refuse insecure default on non-loopback bind
`834b0d2` fix(serve): address Task 2 code-review feedback
`f12f785` feat(workq): bounded worker pool with graceful drain
`27fce09` fix(workq): guard Submit against send-on-closed-channel race
`358551f` feat(api): submit upload indexing through workq with graceful drain
`08e6aa8` fix(api): address Task 4 code-review follow-ups
`d5b7c70` chore(api): silence unused-parameter lint in upload_workq_test
`9339296` feat(api): session cookie path alongside bearer auth
`f6567f3` fix(api): defense-in-depth symmetry for session auth
`b885731` feat(ui): cookie-based auth; stop reading bearer from DOM
`45eb475` fix(ui): surface session-exchange failures, scrub meta tag first
`952fcdd` fix(api): stop injecting API key into served HTML
`c4ed56a` perf(search): scope entity fetch to top-hit docs in local search
`b7b1168` fix(ui): drop legacy meta-tag read in MCP hook, use cookie auth

21 files, +1,326 / −90.

Blocks 2–7 will ship as separate PRs per the roadmap cadence.

🤖 Generated with Claude Code

New server.max_upload_bytes config (default 100 MiB, env DOCSIQ_SERVER_MAX_UPLOAD_BYTES). Upload handler wraps request body with http.MaxBytesReader before ParseMultipartForm so oversized requests terminate at the transport with 413 instead of allocating memory or temp files. Closes the P2-1 TODO in handlers.go. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- Drop double WriteHeader in *http.MaxBytesError branch (MaxBytesReader already committed the 413) - Test asserts JSON contract + Content-Type header - New TestUploadMaxBytes_UnknownContentLength covers the chunked/unknown-length slow path - Extract writeTooLarge helper (DRY the 413 body) - Comment limit<=0 escape hatch on the config field - slog.Warn on oversize reject (matches repo convention) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

validateServeSecurity fails startup when server.api_key is empty AND server.host is not loopback. Loopback+empty emits a prominent slog warning instead. Closes the "auth-disabled-by-default" ship-blocker. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- Refuse empty server.host (binds all interfaces, not loopback) - Use net.ParseIP().IsLoopback() to accept [::1], 127.0.0.0/8, and ::ffff:127.0.0.1 as loopback - Table-driven tests cover new hosts + empty-string case - Assert DOCSIQ_SERVER_API_KEY is named in the refusal message - gofmt Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

New internal/workq package. Pool has fixed worker count and a bounded submission queue; Submit is non-blocking and returns ErrQueueFull when saturated. Close(ctx) cancels the pool context and waits for workers to drain within the caller's deadline. Preparing to replace fire-and-forget upload indexing goroutine. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

RWMutex guards the jobs-channel send in Submit against a concurrent Close that calls close(p.jobs). Previously a Submit that passed the p.closed check could be preempted and then send on an already-closed channel. Add a stress test (50 iterations x 32 concurrent Submits + Close) to exercise the race under `-race`. Modernize the iteration count in TestPool_CloseDrainsInflight to Go 1.22's for-range-N form. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Upload handler now submits the indexing closure to a bounded workq Pool instead of spawning a detached goroutine. When the queue is full the handler returns 503 with Retry-After: 30. On shutdown, cmd/serve calls pool.Close with a 30s deadline so in-flight jobs honour the cancelled context and partial writes are avoided. Config: server.workq_workers (default NumCPU), server.workq_depth (default 64). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- upload(): set progress to rejected state before writing 503 when workq.Submit returns ErrQueueFull or ErrClosed, so progress polling reflects the rejection instead of stale "queued" forever. - upload_workq_test.go: defer close(block) after defer pool.Close so drain completes even if the test panics. Also add started-channel sync and fill both channel slots (capacity = Workers+QueueDepth = 2) so saturation is deterministic under the race detector. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

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

New POST /api/session exchanges an Authorization: Bearer key for a docsiq_session httpOnly; Secure; SameSite=Strict cookie (30-day Max-Age). DELETE /api/session clears it. Auth middleware now accepts either the Authorization header or the cookie; /api/session itself is public (it is the auth boundary). Preparing to stop injecting the key into served HTML. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- bearerAuthMiddleware: explicitly reject when server.api_key is unset, matching newSessionHandler's guard. Previously correct only via the no_token branch firing first; fragile under refactor. - extractToken: trim whitespace from the session cookie value for parity with the Authorization header path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

apiFetch now sends credentials: 'include' on every request; the Authorization header is no longer attached. On boot, initAuth exchanges the legacy meta-tag bearer (if present) for a cookie via POST /api/session exactly once and scrubs the meta tag. Production installs that already ship cookies out-of-band (via `docsiq login`) skip the exchange entirely. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- Check res.ok on POST /api/session. Non-2xx rejects sessionReady so the UI sees a single clear auth-boundary failure instead of a cascade of generic 401s on subsequent fetches. - Scrub the legacy meta tag BEFORE awaiting the fetch. The bearer is already captured as a closure param; leaving the tag in the DOM during a slow/hung exchange widens the disclosure window. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The SPA handler no longer rewrites index.html to embed the bearer in a <meta name="docsiq-api-key"> tag. Browser auth now flows through the docsiq_session cookie set by POST /api/session. Closes the "API-key-in-HTML" ship-blocker. The stale spa_meta_test.go asserted the injection was present and is replaced by spa_test.go which asserts the tag is absent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Replaces the full-table AllEntities scan with EntitiesForDocs, which joins the entities → relationships tables and filters by relationships.doc_id with a chunked IN-list (chunk size 900, below SQLite's 999 variable limit). Local search now scales sub-linearly with corpus size. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

useMCP had its own getBearer() reading the docsiq-api-key meta tag and attaching Authorization to /mcp requests — a parallel path that Tasks 5-7 missed. Now the hook sends credentials: 'include' and relies on the docsiq_session cookie, matching apiFetch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

aksOps and others added 16 commits April 23, 2026 14:29

chore(api): silence unused-parameter lint in upload_workq_test

d5b7c70

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

aksOps enabled auto-merge (squash) April 23, 2026 19:00

aksOps merged commit e0445db into main Apr 23, 2026
12 checks passed

aksOps deleted the feat/block1-critical branch April 23, 2026 19:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

block 1: critical ship-blockers (upload cap, auth hardening, bounded workq, scoped search)#60

block 1: critical ship-blockers (upload cap, auth hardening, bounded workq, scoped search)#60
aksOps merged 16 commits intomainfrom
feat/block1-critical

aksOps commented Apr 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

aksOps commented Apr 23, 2026

Summary

Test plan

Pre-merge review

Known follow-ups (non-blocking, file after merge)

Commits (16)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant