fix(api): pin two authorization invariants, and say what these reads are scoped to

Nothing asserted that a secret list is narrowed to its caller's own rows.
The query is mocked, so its canned result proved nothing about what was
asked for, and removing the narrowing left the suite green. The same was
true of the refusal that keeps a workspace key — which acts for a workspace,
not a person — out of a personal secret list: with the policy flipped the
use case resolved and returned secrets. Both are pinned by the arguments
the query is built from.

Personal secrets remain invisible from a workspace the caller reaches
through inherited organization access. Their metadata has no canonical row,
only per-workspace mirrors written for a narrower set of workspaces than a
caller can authorize into, so a list scoped to one workspace cannot find
them while set and delete, which key on the person, work. Dropping the
scope would return one row per mirror and break pagination, and a mirror's
timestamps describe when it was written rather than when the secret was
made, so the sort has no meaning. Closing it is a storage change; the
divergence is recorded where the read happens and stated in the contract
rather than left implied.

Account-wide billing is requested by omitting the workspace, which is
indistinguishable from an ordinary request, so a workspace key asking for
it is answered with its own workspace rather than refused. The refusal
belongs where the intent exists, which is the client; the contract now says
what omission means for each kind of key, and that a workspace-scoped page
reports its members in aggregate and names none of them.

A run is addressed by an id that already names its workspace, so that read
takes no workspace and refuses one that is sent. Unlike its siblings,
nothing said so.

Author: waleedlatif1

apps/sim/lib/api/contracts/v2/billing.ts

@@ -42,7 +42,7 @@ export const v2BillingStatusQuerySchema = z
     workspaceId: workspaceIdSchema
       .optional()
       .describe(
-        'Workspace whose payer should be resolved. A workspace API key is pinned to its own workspace: any other id answers `404 Workspace not found`, which is also what an id that does not exist answers.'
+        'Workspace whose payer should be resolved. Omitting it selects account scope — the payer behind the calling *account*, across every workspace — and only a personal API key has an account to select. A workspace API key that omits it still resolves its own workspace, because omission is the same request as sending that key its own id; it is not a way to widen a workspace key. The response `workspaceId` reports which was resolved and is `null` only on account scope. A workspace API key is pinned to its own workspace: any other id answers `404 Workspace not found`, which is also what an id that does not exist answers.'
       ),
   })
   .strict()
@@ -172,7 +172,7 @@ export const v2BillingLogsQuerySchema = z
     workspaceId: workspaceIdSchema
       .optional()
       .describe(
-        "Narrow the ledger to usage events attributed to one workspace. It does not change whose events are reported — a personal API key always reports the usage of the person holding it, and a workspace API key always reports its own workspace's complete ledger across every member. The response `scope` field says which of the two you received. A workspace API key is pinned to its own workspace: any other id answers `404 Workspace not found`, which is also what an id that does not exist answers."
+        "Narrow the ledger to usage events attributed to one workspace. It does not change whose events are reported — a personal API key always reports the usage of the person holding it, and a workspace API key always reports its own workspace's complete ledger across every member. The response `scope` field says which of the two you received. Omitting it does not widen a workspace API key: that key has no account behind it, so an omitted id is the same request as its own id and the page still covers exactly one workspace, reported as `scope: workspace`. A ledger spanning every workspace an account touches requires a personal API key. A workspace API key is pinned to its own workspace: any other id answers `404 Workspace not found`, which is also what an id that does not exist answers."
       ),
     period: usageLogPeriodSchema
       .optional()
@@ -278,7 +278,8 @@ export const v2BillingLogEntrySchema = z
   .meta({
     id: 'V2BillingLogEntry',
     title: 'Billing log entry',
-    description: 'One credit-consuming usage event in the billing ledger.',
+    description:
+      'One credit-consuming usage event in the billing ledger. No field identifies the member who incurred it: on `workspace` scope a page is every member’s usage in aggregate, not a per-member breakdown, so it reconciles a workspace’s spend without publishing who spent it.',
   })
 export type V2BillingLogEntry = z.output<typeof v2BillingLogEntrySchema>
 
@@ -292,7 +293,7 @@ export type V2BillingLogEntry = z.output<typeof v2BillingLogEntrySchema>
 export const v2BillingLogsScopeSchema = z
   .enum(['user', 'workspace'])
   .describe(
-    "Whose usage this page reports. `user` — the events of the person whose personal API key made the request, narrowed by `workspaceId` when one was given; this omits other members' usage. `workspace` — every member's events for the workspace a workspace API key is pinned to."
+    "Whose usage this page reports. `user` — the events of the person whose personal API key made the request, narrowed by `workspaceId` when one was given; this omits other members' usage. `workspace` — every member's events for the workspace a workspace API key is pinned to, in aggregate: no entry names the member it belongs to."
   )
 
 export const v2ListBillingLogsContract = defineRouteContract({

apps/sim/lib/api/contracts/v2/logs.ts

@@ -307,7 +307,9 @@ export const v2LogDetailSchema = z
 export type V2LogDetail = z.output<typeof v2LogDetailSchema>
 
 export const v2LogParamsSchema = z.object({
-  runId: runIdSchema.describe('Unique workflow run identifier.'),
+  runId: runIdSchema.describe(
+    'Unique workflow run identifier. A run is addressed globally by this id: unlike the list and statistics routes, this route takes no workspace. The run carries its own workspace and the caller is authorized against that one, so a run the caller cannot reach is concealed as a 404 rather than filtered out.'
+  ),
 })
 
 /**
@@ -679,6 +681,18 @@ export const v2ListLogsContract = defineRouteContract({
   },
 })
 
+/**
+ * Deliberately without a workspace input, unlike {@link v2ListLogsContract} and
+ * `v2GetLogStatsContract`, which both require one.
+ *
+ * A run id is globally unique, so the run names its own workspace and the caller
+ * is authorized against that canonical scope. Accepting a workspace here would
+ * offer a filter that can only ever agree with the scope already derived, or
+ * disagree with it — and a caller who read the asymmetry as "this route ignores
+ * the workspace I asked for" is reading a flag that was never part of the
+ * request. `query: noInputSchema` is what makes that visible: a stray
+ * `workspaceId` is refused as an unrecognized key rather than dropped.
+ */
 export const v2GetLogContract = defineRouteContract({
   method: 'GET',
   path: '/api/v2/logs/[runId]',

apps/sim/lib/api/contracts/v2/secrets.ts

@@ -96,7 +96,11 @@ export type V2SecretSortBy = (typeof v2SecretSortFields)[number]
 export const v2ListSecretsQuerySchema = z
   .object({
     workspaceId: workspaceIdSchema.describe('Workspace whose secret metadata should be listed.'),
-    scope: v2SecretScopeSchema.optional().describe('Restrict results to one ownership scope.'),
+    scope: v2SecretScopeSchema
+      .optional()
+      .describe(
+        "Restrict results to one ownership scope. Personal results are not the caller's full personal set: this list reads the per-workspace credential mirrors of a personal secret, and a mirror exists only for workspaces the caller holds an explicit membership or ownership of. A personal secret is therefore omitted here when the caller reaches this workspace through inherited organization access, even though the same secret can be set and deleted from it. Prefer listing from a workspace the caller is an explicit member of until the mirrors are replaced by canonical personal-secret metadata."
+      ),
     search: v2SearchSchema.describe('Case-insensitive substring match against the secret name.'),
     ...v2SortFields(v2SecretSortFields, { sortBy: 'name', sortOrder: 'asc' }),
     ...v2PaginationFields({ description: 'Maximum secrets to return per page.' }),

apps/sim/lib/billing/application/authorized-billing-read-use-case.ts

@@ -47,6 +47,27 @@ function requireBillingReadPrincipal(
   }
 }
 
+/**
+ * Account scope is reachable only by a personal API key, and that is a property
+ * of what the two kinds of key are rather than a check performed here.
+ *
+ * Omitting `workspaceId` is how a caller asks for account scope, and it is also
+ * the plainest request anyone makes — `GET /api/v2/billing/status` with no query
+ * at all. The two are byte-identical on the wire, so a workspace API key that
+ * omits the id cannot be told apart from one that never had an opinion, and this
+ * function pins it to `principal.workspaceId` in both cases. It deliberately
+ * does not refuse the omission: a workspace key has no account to widen to, so
+ * there is no wider answer being withheld, and refusing would `403` every
+ * unparameterised billing read a workspace key makes.
+ *
+ * The consequence is that a caller cannot *assert* account scope over this
+ * surface, only imply it. A client that means "account-wide" must therefore
+ * establish that it holds a personal key — `GET /api/v2/meta` reports `keyType`
+ * — and refuse locally, rather than relying on a refusal that this layer has no
+ * signal to produce. The answer still says which scope it resolved:
+ * `workspaceId` is `null` only on the account branch, and the ledger reports
+ * `scope`.
+ */
 async function resolveBillingReadScope(
   principal: BillingReadPrincipal,
   operation: BillingReadOperation,

apps/sim/lib/logs/application/public-log-use-cases.test.ts

@@ -324,6 +324,60 @@ describe('public log application use cases', () => {
     )
   })
 
+  /**
+   * The run id is the whole of the request — the route takes no workspace — so
+   * the only thing standing between a personal key and any run in the system is
+   * that the permission is resolved against the run's own workspace. Resolving
+   * it against anything the caller supplied, or skipping it once the key
+   * authenticates, turns a globally unique id into a global read.
+   */
+  it('resolves the personal-key permission against the run workspace', async () => {
+    await getPublicLog.execute({
+      principal: { kind: 'personal_api_key', userId: 'user-9', keyId: 'key-9' },
+      input: { runId: 'run-1' },
+    })
+
+    expect(mocks.resolvePermission).toHaveBeenCalledWith(
+      'user-9',
+      'workspace-1',
+      null,
+      undefined,
+      expect.anything()
+    )
+  })
+
+  it('rejects a personal key with no permission on the run workspace', async () => {
+    mocks.resolvePermission.mockResolvedValueOnce(null)
+
+    await expect(
+      getPublicLog.execute({
+        principal: { kind: 'personal_api_key', userId: 'user-9', keyId: 'key-9' },
+        input: { runId: 'run-1' },
+      })
+    ).rejects.toMatchObject({ code: 'forbidden' })
+
+    expect(mocks.getLog).not.toHaveBeenCalled()
+    expect(mocks.materialize).not.toHaveBeenCalled()
+  })
+
+  /**
+   * A workspace that disallows personal keys must refuse them here too: the
+   * detail read is the one log route a personal key can reach without naming a
+   * workspace, so a gap in this branch is not covered by the list route's.
+   */
+  it('rejects a personal key in a workspace that disallows them', async () => {
+    mocks.loadWorkspace.mockResolvedValueOnce({ ...workspaceContext, allowPersonalApiKeys: false })
+
+    await expect(
+      getPublicLog.execute({
+        principal: { kind: 'personal_api_key', userId: 'user-9', keyId: 'key-9' },
+        input: { runId: 'run-1' },
+      })
+    ).rejects.toMatchObject({ code: 'forbidden' })
+
+    expect(mocks.getLog).not.toHaveBeenCalled()
+  })
+
   it('rejects a workspace key outside the run workspace before materialization', async () => {
     await expect(
       getPublicLog.execute({

apps/sim/lib/secrets/application/use-cases.test.ts

@@ -5,6 +5,7 @@ import type { Principal } from '@sim/auth/principal'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import type {
   DeleteSecretInput,
+  ListSecretsInput,
   ListSecretUsageInput,
   SetSecretInput,
 } from '@/lib/secrets/application/use-cases'
@@ -193,6 +194,64 @@ describe('secret application use cases', () => {
     expect(result.values).toEqual({})
   })
 
+  /**
+   * The list's cross-user boundary is one SQL predicate — `ownedEnvSecretsOnly` — and nothing
+   * else keeps another user's `env_personal` row out of the page. Dropping it would leave every
+   * other assertion in this file green, because the query is mocked and its canned rows prove
+   * nothing about what was asked for. So the pin is on the composed argument, per scope.
+   */
+  it.each([
+    ['personal' as const, ['env_personal']],
+    ['workspace' as const, ['env_workspace']],
+    [undefined, ['env_workspace', 'env_personal']],
+  ])("restricts the list to the caller's own secrets for scope %s", async (scope, types) => {
+    await listSecretsUseCase.execute({
+      principal: session,
+      input: {
+        workspaceId: workspace.workspaceId,
+        scope,
+        sortBy: 'name',
+        sortOrder: 'asc',
+        limit: 50,
+      },
+    })
+
+    expect(mocks.listCredentials).toHaveBeenCalledTimes(1)
+    const [args] = mocks.listCredentials.mock.calls[0]
+    expect(args.ownedEnvSecretsOnly).toBe(true)
+    expect(args.types).toEqual(types)
+    /** Taken from the principal, never from caller-supplied input. */
+    expect(args.userId).toBe(session.userId)
+    expect(args.workspaceId).toBe(workspace.workspaceId)
+  })
+
+  it('never reads secret metadata for a workspace API key, which has no personal identity', async () => {
+    const execute = listSecretsUseCase.execute as (args: {
+      principal: Principal
+      input: ListSecretsInput
+    }) => Promise<unknown>
+
+    await expect(
+      execute({
+        principal: {
+          kind: 'workspace_api_key',
+          workspaceId: workspace.workspaceId,
+          keyId: 'workspace-key-1',
+        },
+        input: {
+          workspaceId: workspace.workspaceId,
+          scope: 'personal',
+          sortBy: 'name',
+          sortOrder: 'asc',
+          limit: 50,
+        },
+      })
+    ).rejects.toMatchObject({ code: 'forbidden' })
+
+    expect(mocks.loadContext).not.toHaveBeenCalled()
+    expect(mocks.listCredentials).not.toHaveBeenCalled()
+  })
+
   it('rejects workspace keys before resolving or reading secret state', async () => {
     const execute = setSecretUseCase.execute as (args: {
       principal: Principal

apps/sim/lib/secrets/application/use-cases.ts

@@ -66,6 +66,24 @@ function credentialTypes(scope?: SecretScope) {
  * The secret's public `name` is stored as the credential `displayName`, so the
  * caller-facing `name` sort aliases to that column here. The alias must not
  * escape into the cursor's sort stamp — see {@link listSecretsUseCase}.
+ *
+ * KNOWN DIVERGENCE — personal scope is workspace-bound here and nowhere else.
+ * A personal secret's value is user-global (`environment.variables`, keyed by
+ * user alone), so {@link setSecretUseCase} and {@link deleteSecretUseCase} both
+ * act on it without reference to a workspace. This read cannot: `credential`
+ * carries a NOT NULL `workspaceId`, so a personal secret has no canonical
+ * metadata row — only per-workspace mirrors, written for the workspaces
+ * `getUserWorkspaceIds` returns (explicit `permissions` rows plus owned
+ * workspaces). That set is NARROWER than the set of workspaces a caller can
+ * authorize into, which also includes inherited organization access. In such a
+ * workspace there is no mirror, so this returns nothing while `set` and
+ * `delete` still work — the caller can create a secret they cannot list.
+ *
+ * {@link getPersonalSecretMetadata} already works around the same hole for the
+ * write's response projection. Closing it for reads is a storage change, not a
+ * scoping change: personal-secret metadata needs one canonical home (a nullable
+ * `credential.workspaceId`, or a dedicated table) before this query can drop the
+ * workspace predicate without double-counting mirrors or inventing timestamps.
  */
 async function listSecretMetadata(params: {
   workspaceId: string