fix(analytics): accept kh_ API keys on the four session-only analytics routes

[subheeksh5599]

Fixes #1932 (accepted, maintainer plan: build against the comment, not the original proposal).

What

The four analytics routes that still used session-only requireOrganization — networks, stream, summary, time-series — now use resolveOrganizationId and gate on SCOPE_MCP_READ, matching the sibling runs and spend-cap routes (per suisuss accepted comment on #1932). No new write surface.

Test

Adds tests/unit/analytics-summary-route-auth.test.ts: kh_ key resolves org (200), scoped key lacking mcp:read denied 403 insufficient_scope, OAuth JWT still resolves org, no credential 401. Verified locally: 4 passed.

[github-actions]

About the build check on this pull request

This pull request comes from a fork, so GitHub does not pass it the credentials build normally uses for our image registry cache and staging build configuration. The build still runs and still compiles the image, so a red build here is real; it just takes longer than on team branches.

Every workflow run on a pull request from a fork also waits for a maintainer to approve it, so checks can sit at "awaiting approval" for a while after each push. Nothing is needed from you for either of these.

[joelorzet]

The pattern matches app/api/analytics/runs/route.ts and the GET in spend-cap/route.ts, and all seven analytics routes now use one resolver.

Session auth is unaffected. resolveOrganizationId tries OAuth, then the API key, then the session cookie. Session callers carry no scope, and scopeSatisfies admits an undefined scope, so a cookie caller cannot reach the 403. These are GETs, so the CSRF origin check does not apply.

Two notes on the helper, neither a blocker here.

requireOrganization rejected anonymous sessions with a 401 and resolveOrganizationId does not. Every account gets an organization, anonymous ones included, so an anonymous session now reads its own org's analytics instead of getting a 401. That org is empty, so no other org's data is exposed. The open part is the cost of the aggregation queries. The gap already affects runs, facets, and spend-cap.

withPolicyGate returns early unless the context carries both userId and organizationId. OrganizationAuthContext has no userId, so the gate is inert for every caller of this helper. Nothing changes for these routes, because the gate matches mutations only. It matters once a governed route adopts the helper.

[joelorzet]

Two gaps open up on these four routes when requireOrganization is dropped. Both already affect runs, facets and spend-cap, so the fix belongs in resolveOrganizationId, but this PR takes the count from three routes to seven and should not widen it further first.

Anonymous sessions are no longer rejected. requireOrganization returned 401 on context.isAnonymous. resolveOrganizationId has no such check on its session branch. Every account gets an organization, anonymous ones included (lib/auth.ts:832), so an anonymous session now resolves an org and gets a 200 from all four routes. The org is empty, so no other tenant's data is exposed, but the aggregation queries run for an unauthenticated visitor.

withPolicyGate is inert for this helper. It returns early unless the context carries both userId and organizationId. OrganizationAuthContext has no userId field, and no return inside resolveOrganizationIdInner sets one. So the gate never runs for any caller of resolveOrganizationId. Nothing changes for these routes today, since the gate matches mutations only, but a governed route that adopts this helper would skip it in silence.

The rest is correct. The pattern matches runs/route.ts and the GET in spend-cap/route.ts, all seven analytics routes now share one resolver, and session auth is unaffected: session callers carry no scope and scopeSatisfies admits an undefined scope, so the 403 is unreachable from the dashboard. These are GETs, so the CSRF origin check does not apply.

[subheeksh5599]

Fixed the first gap in the helper itself, as you suggested.

resolveOrganizationId's session branch now rejects anonymous sessions the same way requireOrganization did: after confirming a session user exists, it checks isAnonymousUserShape(session.user) and returns 401 Authentication required before any org resolution. The throwaway-org provision no longer lets an unauthenticated visitor run the aggregation queries.

This fixes all seven routes at once (the four in this PR plus the existing runs/facets/spend-cap users of the helper), so adopting it can't widen the anonymous gap further — it closes it. Added a test case: OAuth and API-key auth both fail, session resolves an Anonymous/temp-* user → 401, and getAnalyticsSummary is never called.

On the withPolicyGate note — agreed it's inert because OrganizationAuthContext carries no userId, and agreed it's not a blocker here (mutation-only gate, GET routes). I've left it alone since the fix belongs in a broader change to the context type that would touch every return of both resolvers, and you've flagged it already affects the existing helper users. Happy to do that as a separate follow-up if you want it.

[joelorzet]

db4daae0 rejects the anonymous session, and the test covers it. The behaviour is right. The placement is the problem.

The check went into resolveOrganizationId, which 26 routes call. This PR needs it on four analytics routes. It now applies to /api/features, /api/web3/fetch-abi, /api/gas/estimate, /api/onboarding/status, /api/tags, /api/projects, the three /api/user/wallet/* reads, and the rest. Anonymous sessions exist so a visitor can explore before signing up, so several of those are on a path that used to work and now returns 401.

Two that need checking before this lands:

• /api/features calls resolveOrganizationId and returns its error. hooks/use-features.ts fetches it on mount and, per its own comment, polls and holds the UI in a loading state until it responds. A 401 there affects every plan-gated lock state an anonymous visitor sees.
• /api/web3/fetch-abi backs the ABI auto-fetch field in the workflow builder (components/workflow/config/abi-with-auto-fetch-field.tsx). workflow-toolbar.tsx gates save and deploy behind isAnonymousUser rather than the builder itself, so an anonymous visitor reaches that field.

Either move the rejection to the four routes this PR owns, or walk all 26 callers and say in the description which ones intentionally lose anonymous access. The first is the smaller change and keeps this PR's blast radius equal to its title.

The policy gate is still inert. withPolicyGate returns early unless the context carries both userId and organizationId. OrganizationAuthContext has no userId, and no return inside resolveOrganizationIdInner sets one, so the gate never runs for any caller of this helper. It matches mutations only, so these GETs are unaffected today, but a governed route that adopts the helper would skip it in silence. Adding userId to the session and API-key branches closes it.

[subheeksh5599]

You're right on both counts. The rejection has been moved out of the shared helper and onto the four routes this PR owns.

What changed:

• resolveOrganizationId no longer rejects anonymous sessions. All 26 callers keep their previous behaviour — /api/features, /api/web3/fetch-abi and the rest still serve anonymous visitors exactly as before.
• The four analytics routes now reject anonymous sessions at route level (401 "Authentication required"), matching the requireOrganization behaviour they replace. The check reads an isAnonymous flag the resolver now sets on its session-branch result — no second session fetch, and routes that don't check it are unaffected.

On the policy gate: the helper change is now purely additive so the gate can run without changing any caller's outcome: OrganizationAuthContext gains userId on all three branches (OAuth sub, API-key creator id, session user id) and isAnonymous on the session branch. withPolicyGate can now match for a governed route that adopts the helper, and no existing caller changes behaviour.

Test: the anonymous case now exercises the route-level check (OAuth + API-key fail, anonymous session resolves with an org, route returns 401, aggregation query never runs). 5/5 pass locally; typecheck clean for the changed files.

[subheeksh5599]

All items addressed. The branch is also rebased onto current staging (which now includes the merged #2300) so it is clean to merge.

userId removed. Dropped from the OrganizationAuthContext type and all three branches of resolveOrganizationId, along with the comment citing withPolicyGate. Nothing in the PR reads it, and the premise it came from is retracted. isAnonymous stays on the session branch - the route-level rejection is kept, but on the sidebar-parity grounds you gave, not on the requireOrganization premise (the test comment now says exactly that).

The org-less 400 is handled in the client, which also fixes the merged routes. use-analytics's processSection now treats a 400 with body "No active organization" as ORG_REQUIRED, alongside the existing 401/403 mapping. I chose the client over the route because /runs, /facets and /spend-cap already merged with the same 400-on-no-org behaviour, so a route-only fix would have left them broken while my four routes behaved differently. One place now maps the state for all seven, and a member with no org sees the join-org UI instead of "fetch failed: 400" on every analytics section they load together.

The scope test now pins the real case. scope: "" is swapped for scope: undefined - the actual unscoped-key shape (parseScopeInput never returns an empty string; api-key-auth maps a null column to undefined). The test asserts the unscoped key is admitted (200), documenting that scopeSatisfies treats undefined as full access for legacy keys, so a future change to scopeSatisfies breaks this test rather than silently narrowing every unscoped key.

Rebase note: my local clone's "upstream" remote was accidentally pointing at my fork, so earlier diffs compared against a stale staging. That is fixed - the branch is now rebased onto KeeperHub/keeperhub staging proper (20 commits ahead, including the #2300 merge) with no conflicts, and the diff is exactly the eight intended files.

[suisuss]

Previously raised, one line each: move the anonymous rejection off the shared helper onto the four routes - addressed, lib/middleware/auth-helpers.ts:475-491 no longer rejects and each route checks it. Drop userId and its withPolicyGate comment - addressed. The org-less 400 the dashboard does not handle - addressed, and my original finding was wrong; see below. Swap the impossible scope: "" test for scope: undefined - addressed, and it matches scopeSatisfies at lib/mcp/oauth-scopes.ts:258-263. The anonymous-test comment - addressed. CI had not run - addressed, all green at ae37e905.

Two corrections of mine to put on the record. My org-less 400 finding was wrong: the branch you mapped at components/analytics/use-analytics.ts:89-96 is unreachable, because fetchData returns early at :136 when activeOrgId is null, and the 400 only arises when the user has no membership at all - the same case. And you were right that requireOrganization never rejected anonymous accounts: lib/middleware/org-context.ts:55-63 sets isAnonymous: true only when there is no session, and :70-77 hard-codes false for any session user.

Blocking

• app/api/analytics/stream/route.ts:15-33 - the SSE endpoint is now reachable by a kh_ key with no rate limit and no connection cap. One request opens a connection that runs getAnalyticsChecksum every 5 seconds for its lifetime (lib/analytics/stream-start.ts:7,124), and lib/analytics/queries.ts:2185-2192 says in its own comment that the query "runs every poll interval for every connected viewer". checkRateLimit exists only under app/api/execute/* and the workflow simulate route - there is none on any analytics route. -> Previously the only client that could reach this was a browser EventSource bound to a session; a key holder can now hold N connections indefinitely. -> Cap concurrent stream connections per apiKeyId or per org, or leave stream session-only and key-enable the three JSON routes.

  This matters more because of the cache bypass beside it: lib/analytics/queries.ts:597-605 makes isCacheableRange false whenever customStart/customEnd are present, and the routes forward those params unvalidated (summary/route.ts:38-40 and siblings), so ?range=custom&customStart=1970-01-01&customEnd=<now> skips the 30-second cache on every call. Bounded by the org's own row count, so a load concern rather than a break - but it is why the missing limit is worth fixing now. Same shape already merged on /runs and /facets, so this is an extension rather than something you introduced.

Mechanical - actionable as-is

• The anonymous check is on four of the six analytics routes - runs/route.ts:11-24 and facets/route.ts:17-30 do not have it. use-analytics.ts fires all five in one fan-out, so an anonymous session gets 401 on three and 200 on two; onAbort at :79-83 means the 401 wins and nothing visibly breaks, but the surface is inconsistent. Apply one rule to all six.

• components/analytics/use-analytics.ts:79-83 - now that the 400 path produces ORG_REQUIRED, the 403 mapping to the same code is dead for session callers (session scope is undefined, so requireScope never denies, and checkSessionOrigin only fires on state-changing methods per auth-helpers.ts:50-53). The two mappings now disagree about what a 403 means: a key caller denied on scope would be labelled "no organization".

• The 400 branch at use-analytics.ts:89-96 is inert as written. Either drop it, or make it earn its place by also mapping 404 - auth-helpers.ts:143-149 returns that for a deactivated active org, and it currently falls through to the raw throw new Error("... fetch failed: 404") at :97-99.

With the team

• Whether analytics should be key-readable at all. Key callers bypass the MFA and country-trust gates by construction - proxy.ts:253-258 passes the gate when there is no session cookie - so key access is a genuinely different trust path from the session one, even with the org scoping correct. I'm weighing that against the docs already listing analytics as key-accessible and the fact that agents reading their own run history is the case this exists for. Not blocking, and nothing here is on you. I'll come back with a verdict.

Verdict

Changes requested - on the stream connection cap alone.

I re-verified the cross-tenant question this round rather than carrying the last one forward, because it is the thing that would matter most. There is no path: the API-key org comes only from the key row (lib/api-key-auth.ts:214), X-Organization-Id is read solely inside resolveSessionOrg and membership-joined (auth-helpers.ts:158-180), every analytics query pairs the client's projectId with the org predicate (queries.ts:739-741, :818-820, :919-921, and scopedLogs at :274-291), and cache keys are org-prefixed. The scope gate is the correct read constant on all four routes, and the error shapes leak nothing - 403 is insufficient_scope with no org data in the body.

Your claim that the client fix also covers the merged /runs and /facets checks out mechanically, one processSection, though it is inert for the same activeOrgId reason as above. The rebase is clean: eight files, exactly the intended set.

[subheeksh5599]

Addressed this round (squashed to one commit cc6c62257 on top of current staging):

Blocker - stream connection cap. The stream route is back to session-only (requireOrganization), so the PR no longer key-enables it. Rationale, checked against the code: the stream is an SSE live view whose only consumer is a browser EventSource (use-analytics.ts:340), and EventSource cannot send an Authorization header, so a kh_ key has no legitimate way to use it - key-enabling would only let a key holder hold N long-lived connections each running the 5s checksum poll. No MCP tool or lib reads the route. The revert also restores the getTimeRangeStart bound on the checksum that the key-enabled rewrite had dropped. The three JSON routes (networks, summary, time-series) stay key-enabled, which is what #1932 actually needs (agents reading run history over REST).

Mechanical - one rule for all six. Dropped the route-level anonymous rejection from the three JSON routes. An anonymous session now resolves to its own (empty) org and gets 200, matching the merged runs/facets behaviour exactly.

Mechanical - 403 mapping. processSection now maps only 401 -> AUTH_REQUIRED and 400/404 (with the specific error bodies) -> ORG_REQUIRED. A 403 is deliberately not mapped to ORG_REQUIRED anymore: for a session caller it cannot occur (scope undefined is admitted by scopeSatisfies), and for a key caller it means insufficient scope, which labelling as "no organization" would misrepresent.

Test updated to pin the new anonymous behaviour (anon session -> 200 on its own org). 36 tests pass across the two auth suites; biome and type-check clean.

[suisuss]

The blocker is gone by removing it from scope: app/api/analytics/stream/route.ts is no longer in the diff, staging's copy keeps requireOrganization at :10,12, and the PR is now the three JSON routes. That is the right call - EventSource cannot set an Authorization header, so the only consumer at use-analytics.ts:345 could never have used a key anyway.

The other three landed. The anonymous rejection is off all three routes and summary/route.ts:10-23 now matches the guard shape of the already-merged runs, facets and spend-cap routes exactly. use-analytics.ts:79-82 maps 401 only, with the reasoning stated - correct, since a session caller carries no scope so requireScope cannot deny it. And :91-102 now maps 400 and 404, with 404 being the reachable one.

One claim does not check out, though it costs nothing: the restored getTimeRangeStart bound was not something your branch had dropped. It arrived on staging via #2312 after your previous head was cut, so staging moved under you rather than the rewrite losing it.

You are also right that this is not a widening for anonymous accounts - lib/middleware/org-context.ts:70-77 hard-codes isAnonymous: false for any session user, so requireOrganization already served these three routes to them before this PR.

With the team - settled

Analytics stays key-readable. The question was whether key callers bypassing the MFA and country-trust gates by construction makes this a different trust path, and it does - but the surface is now three cached JSON reads rather than a long-lived stream, and /runs, /facets and /spend-cap are already merged in exactly this shape. Splitting the same surface across two trust models would be worse than either answer. Removing decision-needed.

Mechanical - actionable as-is

• tests/unit/analytics-summary-route-auth.test.ts - none of the five cases asserts the scope gate ever denies. The gate at summary/route.ts:18 is the one behaviour this PR adds, and all five cases expect 200 or 401, so removing requireScope entirely would break no test. Add a key with a non-satisfying scope expecting 403 insufficient_scope.

• lib/middleware/auth-helpers.ts:410-414 and :489 - isAnonymous? on OrganizationAuthContext is now dead. Nothing reads it: the only isAnonymous readers repo-wide are require-org.ts:10 on OrgContext and app/executions/[executionId]/page.tsx:63 on DualAuthContext, both untouched. Its own comment describes a caller that no longer exists. Deleting both hunks leaves this PR touching no shared auth code at all, which is worth having.

• docs/api/analytics.md has no auth section and is untouched, so the endpoints an agent is now meant to call over REST still have no documented way to authenticate. The merged routes set the same precedent, so this is a request rather than a blocker.

Verdict

Changes requested - on the missing scope-denial test, which is the only coverage for the gate this PR exists to add.

I re-derived the org-scoping question from scratch rather than carrying last round's verdict forward. Still clean: OAuth org comes from the token, API-key org only from the key row with no header override, X-Organization-Id is read solely inside resolveSessionOrg where it is membership-joined, and every SQL leg reachable from these three routes ANDs projectId with an org predicate. All six auth primitives are byte-identical to the versions I audited last round.

[subheeksh5599]

Round 4 addressed (17cd04d8d, rebased onto current staging):

Blocking - missing scope-denial test. Added "denies 403 insufficient_scope when the OAuth token lacks mcp:read" - an OAuth token that resolves an org but carries a scope that does not satisfy mcp:read gets 403 insufficient_scope before the query runs, mirroring the merged runs-route test. The gate at summary/route.ts:18 is now covered in both directions (deny + the existing admits).

Dead isAnonymous removed. Deleted the isAnonymous?: boolean field from OrganizationAuthContext and its session-branch setter in resolveOrganizationId. Nothing read it (the only isAnonymous readers repo-wide were require-org.ts on OrgContext and the executions page on DualAuthContext). lib/middleware/auth-helpers.ts is now byte-identical to staging - this PR touches no shared auth code. The dual-auth-context test's exact-shape expectation was updated to match (it was pinning the deleted field).

On the docs item (analytics.md auth section): agreed it is a request rather than a blocker and the merged routes set the same precedent - leaving it for a separate docs PR rather than widening this one.

[joelorzet]

Everything from the last round is closed. The scope-denial case is in at analytics-summary-route-auth.test.ts:164, so the gate is covered in both directions, and lib/middleware/auth-helpers.ts is byte-identical to staging by blob sha, so this PR touches no shared auth code at all. 14 checks pass.

One item left, and it is not a late addition. It comes from the issue.

#1932's Fix section ends with:

Docs: state which route groups accept API keys vs sessions.

So the docs are inside the accepted scope of the issue this PR closes. I know the argument for deferring was that the merged routes set the same precedent, and that precedent is real. But those routes shipped undocumented, which does not override an explicit line in the accepted issue.

It also matters more here than it looks. #1932's complaint is that an agent can execute and poll over REST but cannot read its own run history, which it calls backwards for an automation platform. Changing the routes while docs/api/analytics.md stays silent means an agent still has no way to discover the capability exists. The page has no auth heading and no Authorization mention at all today.

The good news is that one paragraph covers all six routes at once, since they share the page. Roughly:

• which of the analytics routes accept Authorization: Bearer kh_... and which stay session-only, naming /stream as the session-only one and the reason, since EventSource cannot send the header
• that a key needs mcp:read, and that an unscoped legacy key is admitted
• that a session caller carries no scope and is unaffected

That closes the issue's last line without widening the PR into anything else.