fix(platform): feedback reach, OAuth renewals, unique slots, one 429 - #3197
Merged
Conversation
Squash of the wt-3 batch for rebase onto main; the PR body carries the per-finding record.
…harness After the rebase onto #3189 and the renumbering to 0076, two probes drifted: the metadata-forgery probe voted on ids no message row carries, which the new reach gate refuses before any write, and the slot-migration probe still read the 0074 file. Both now use the lane's own message fixtures and the renumbered file.
larryro
force-pushed
the
fix/followup-authz-consistency
branch
from
September 4, 2026 04:10
f55c516 to
312186d
Compare
larryro
marked this pull request as ready for review
September 4, 2026 04:10
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
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.
Five verified follow-up defects from the backend review, each fixed at its root with a colocated unit test and a real-Postgres probe. Base:
origin/main@1ea4cc9e9.Per-finding outcome
POST /feedbacknever verifies messageId/threadId belong to the caller's orgloadOwnedThread/loadProjectSharedThread, the chat surface's own read grants); anything else is one opaque404 MESSAGE_NOT_FOUND. Red on base: itest probe answered200and stored the rival's row (items=2).storage_failedname: displayName)storeOauth2Grant: a workspace the team route already maps to this org RENEWS its credential (tokens rotated,active, detail cleared); a no-workspace connector renews its one/default oauth2 credential; a new workspace gets a unique label —Slack (Second Workspace)from the vendor'steam.name(now parsed), orSlack (2). Red on base: itestreconnecting … → error/storage_failed,second workspace → error/storage_failed.app.messageslacksUNIQUE (thread_id, order, step_order)CREATE UNIQUE INDEX messages_thread_slot, drop the redundant plain index. Appenders useINSERT … ON CONFLICT DO NOTHING RETURNING+ bounded re-claim. Red on base: 12 concurrent appends → 5 distinct orders (ties). Branch: 12/12 distinct, contiguous.RateLimitExceededError → 429shapesrateLimitedResponseis NOT on base — its file is added here verbatim as a superset)lib/rate-limit-response.ts:rateLimitedResponse(Hono),rateLimitedPlainResponse(Slack webhook, SSE auth),rateLimitExceededCause(documents/TTS wrappers carry the refusal ascause). Every refusal is429 {error:'RATE_LIMITED', data:{retryAfterMs}}+ whole-secondRetry-After≥ 1. Red on base: 5/6 doors had noRetry-After, the 6th the wrong body..env.exampledescribes the 0.5 backend as an opt-in parallel build with Convex presentBACKEND_UPSTREAM= proxy upstream override only,TALE_CONTROL_TOKENsemantics), removedPOSTGRES_URL/INSTANCE_NAME(read nowhere), correctedINSTANCE_SECRET's real role (WebDAV HMAC key derivation),/metrics/backend, theTALE_PROVIDER_KEY_name rule (1–64 chars, no 40-char env-sync cap), "convex image" → platform image.generateDevComposeemitsdepends_on: object-storewith no such servicetale devneeds blobs (the backend seeds the default blob connection at boot and refuses uploads without it;DEV_VOLUME_NAMESalready carriedobject-store-data) → the dev stack now definesobject-storevia the sharedcreateObjectStorageService. Red on base: new generator test fails withbackend-api depends on object-store, which the dev compose does not define.Intentionally untouched
RateLimitExceededErrorsites (protocol adapters, not app 429 shapes):auth/auth.ts(Better AuthAPIError TOO_MANY_REQUESTS),webdav/handlers.ts(WebDAVAppErrorconsumed by the DAV lib),documents/service.ts#retryRagIndexingForDocument(an RPC{success:false}soft result).Messages unique key + dedup
Key:
(thread_id, "order", step_order)— exactly the ordering readers sort by; every appender writesstep_order = 0atmax(order)+1, branch/arena copies preserve both columns into a fresh thread, and nothing legitimately shares a slot. Dedup (same migration, before the index): within every(thread_id, "order")group holding a tie, rows are renumbered0..n-1by(step_order, created_at_ms, id)— the order readers already observe, deterministic, no row deleted, untied groups untouched (replayed on a scratch table in the itest). Rolling-safe: the old image's appenders carry noON CONFLICT, so a lost race during the roll answers an error for one send instead of writing a tie. Under SERIALIZABLE,ON CONFLICTraises 40001 andtransactSerializablereruns the transaction; under READ COMMITTED the bounded loop re-claims the next slot.Tests
feedback/access.test.ts(4),feedback/routes.access.test.ts(1),connectors/oauth.test.ts(8),threads/store.test.ts(3),chat/append-message.test.ts(3),lib/rate-limit-response.test.ts(7);token_exchange.test.tsexpectation extended forteamName.generate-dev-compose.test.ts+2 (red on base: 2 fail).Verification (observed)
services/platform:tsc --noEmitclean;oxlint --type-awareclean; vitest server project 5580 pass.tools/cli: tsc clean, oxlint clean,bun test313 pass / 18 skip.backend:integration, throwaway tale-db + MinIO per run: branch 187 PASS / 7 FAIL, base 171 PASS / 7 FAIL — the 7 are identical base-environment failures (see discoveries), and both runs abort at the same point. The batch's own 16 probes: 16/16 on branch; feedback/slots/429/OAuth probes red on base code.depends_ontarget defined;object-storemounts the declaredobject-store-datavolume).Cross-class discoveries (not fixed here)
checkTwoFactor's third probe (fix(platform): gate conversation writes and audit 2FA successes #3187) runstwo-factor/enable→disableon the suite's MAIN user; Better Auth drops that session on disable (auth.api.getSession→ null →requireSession401), so every later probe fails (chat memories/composer/deferredERR/401, automations trigger mint →"404 Not Found"parsed as JSON → fatal abort). Reproduced standalone:GET /api/app/chat/memories200 → enable → 200 → disable → 401. Fix belongs to that probe (re-sign-in, refreshctx.cookie) — it currently hides ~200 later checks, includingcheckConnectorOauth(verified here by temporarily reordering, not committed).POST /api/app/conversations/composenow fires before fix(platform): blob refs grant nothing; upload and import lanes gated #3160's probe expects403 attachment_not_owned(compose → 403 FORBIDDEN).app/routes/*.test.tsxfail to load (Denied ID …/tale/node_modules/@fontsource/...woff2) whenevernode_modulesis a symlink to another clone; identical on base.docs/en/self-hosted/configuration/environment-reference.md:45still saysINSTANCE_SECRETderives "the deploy admin key" (Convex-era); needs the same correction as.env.examplein all three locales.Merge notes (open siblings)
fix/low-severity-authz-hardening) touchesfeedback/service.ts(dropsmetadata) and restructures the samecompleteOauth2block (atomic store+claim). Resolution: keep both — my gate runs before the insert; callstoreOauth2Grantfrom inside fix(platform): close five low-severity authz and tenant-isolation gaps #3189's transaction (updateCredential/createCredentialtake theSql|TransactionSqlthey are given).lib/rate-limit-response.ts+ test — add/add: take this branch's copy (a superset that contains fix(platform): map edge refusals to 4xx, lock skill writers, dedup audio #3186's two functions verbatim).chat/store.ts#beginTurnaroundappendMessageRow— my change is confined to the INSERT insideappendMessageRow; both apply.