P0: close unauthenticated /api/operator/* + drop client-trusted approval - #33
P0: close unauthenticated /api/operator/* + drop client-trusted approval#33EpicStarAi wants to merge 2 commits into
Conversation
…usted approval P0: the entire /api/operator/* branch was outside the auth gate. Only qclaw/status checked a session; [...path] (verbatim backend proxy), command, confirm and schedule were reachable with no cookie, and command/schedule took accountId from the request body while schedule forwarded a hardcoded operatorApproved:true. - middleware matcher now covers /api/operator/:path* (no-store + surface). - Restore binding-backed resolveBoundAccount in telegramGuard (this backup branch had regressed it to a null stub, so the tree did not compile). - getPrincipal() -> 401 in [...path], command, confirm, schedule. - command/schedule resolve the account SERVER-SIDE via resolveBoundAccount; client accountId is ignored; no owner-matched binding -> 403. - confirm/schedule enforce telegramMutationsEnabled() -> 403 when false. - schedule's operatorApproved is server-set and only reachable AFTER the auth+mutation+binding gate (mirrors /api/telegram/binding/send); never taken from the body. - OperatorOffice: drop the fictional NOVIKOVA / authorizationStateReady / TDLib 1.8.64 identity; render honest "account not connected" from live qclaw/status data. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ator matcher The prod release (qclaw-release-20260717-1130) carries a public-host redirect (PUBLIC_REDIRECT_HOSTS / clonePublicRedirectUrl) that the backup base branch lacked. Rebase the middleware on the live prod version and add ONLY the /api/operator/:path* matcher entry, so the deploy does not regress the /client -> epic-gram.com login redirect. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f8fc938d6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export async function resolveBoundAccount(principal: Principal): Promise<BoundResolution> { | ||
| let binding; | ||
| try { | ||
| binding = await bindingsDb.getByWorkspace(principal.workspaceId); |
There was a problem hiding this comment.
In production with DATABASE_URL set, this lookup does not actually fail closed: telegramBindingsDb.getByWorkspace() catches Postgres errors and falls through to the filesystem store (apps/web/lib/telegramBindingsDb.ts:116-125), so a DB outage can authorize /api/telegram/* and the new operator gates from a stale .telegram-bindings.json row instead of denying as this guard's catch block implies. For the authz gate, use a DB-only lookup or make the DB adapter propagate errors when DATABASE_URL is configured.
Useful? React with 👍 / 👎.
| const bound = await resolveBoundAccount(principal); | ||
| if (bound.kind !== "ok") { | ||
| return NextResponse.json( | ||
| { ok: false, ownerMatched: bound.kind !== "mismatch", executed: false, message: "К вашему профилю не привязан owner-matched Telegram-аккаунт." }, | ||
| { status: 403, headers: H } |
There was a problem hiding this comment.
When TELEGRAM_MUTATION=true, this route verifies that the caller has some ready binding but then forwards the original body unchanged; the backend confirm path reads the account from accountId || action.accountId in services/api/src/operator-agent.mjs:161-164. An authenticated user with any binding can therefore confirm a payload that names another slot in deployments where confirms are executable, bypassing the server-resolved account check. Rebuild the forwarded JSON with bound.accountId or reject mismatched IDs before proxying.
Useful? React with 👍 / 👎.
| const id = binding.tdlibAccountId; | ||
| // Never serve the shared legacy NOVIKOVA / main slot behind a login. | ||
| if (!id || isForbiddenAccountId(id)) return { kind: "mismatch" }; |
There was a problem hiding this comment.
This guard compares the raw DB value to the forbidden IDs without trimming, while the backend normalizes account IDs with String(accountId || "main").trim() before selecting the TDLib slot (services/api/src/telegram-runtime.mjs:45-48). If a legacy/imported binding stores tdlibAccountId as "main " or "\t7369372055", this check returns ok and later operator schedule/confirm sends are normalized back onto the forbidden shared slot. Trim/canonicalize the id before isForbiddenAccountId() and before returning it.
Useful? React with 👍 / 👎.
| export async function GET(req: NextRequest, ctx: { params: { path: string[] } }) { | ||
| if (!(await getPrincipal())) return unauthorized(); | ||
| return forward(req, ctx.params.path, "GET"); |
There was a problem hiding this comment.
When this catch-all now rejects requests without an epic_session, it also rejects the admin health check at apps/web/app/admin/page.tsx:47-52; /api/admin/login only returns {ok} and does not set that cookie (apps/web/app/api/admin/login/route.ts:36-40). An operator who successfully enters the admin password but has not separately gone through referral login will therefore always see the backend status as down. Either issue the expected session/admin cookie or use a status endpoint that matches the admin auth flow.
Useful? React with 👍 / 👎.
1 participant
P0 security fix — operator bridge auth gate
The entire
/api/operator/*branch was outside the auth gate. Onlyqclaw/statuschecked a session;[...path](verbatim backend proxy),command,confirm, andschedulewere reachable with no cookie.command/scheduletookaccountIdfrom the request body, andscheduleforwarded a hardcodedoperatorApproved: true.Changes
/api/operator/:path*(no-store + surface coverage).resolveBoundAccount— this backup branch had regressed it to a null stub, so the tree did not compile.getPrincipal()→ 401 on GET+POST (covers/status,/production/*,/ops/*,/accounts/*, … all served by this catch-all).resolveBoundAccount; clientaccountIdignored; no owner-matched binding → 403.telegramMutationsEnabled()→ 403 when false.operatorApprovedis server-set and only reachable AFTER the auth+mutation+binding gate (mirrors/api/telegram/binding/send); never from the body.NOVIKOVA 💋/authorizationStateReady/TDLib 1.8.64; renders honest "account not connected" from live data.Verification
/login→ 200; no-store applied to operator responses.Owner-isolation gate
resolveBoundAccountverified line-by-line: DB-only by principal workspace,authState=readyonly, client accountId ignored, forbidden ids (main/novikova/7369372055) rejected, workspace mismatch → mismatch.🤖 Generated with Claude Code