fix(w6): close a head-authorization hole, a TZ leak, and five tests that could not fail

Six risks an adversarial read of this week's diff raised, verified one at a
time. Two of the six were already correct and are reported as such rather than
changed.

`v2HeadAuthorizationResponse` optional-called the use case's authorization
phase, so a use case without one would have answered the bodiless 200 that
`headSafe: false` exists to prevent. The definition-time guard does cover both
builders that reach it — they are its only callers — but an optional call turns
a missing phase into that leak silently, so the responder now refuses instead
of skipping.

`packages/db/timestamps.test.ts` assigned `process.env.TZ` at module scope and
never restored it. `TZ` is process state: a worker running files back to back
carried Asia/Tokyo into every file that followed, and only when the ordering put
it after this one. The zone is now set and restored around the file, with both
properties the suite depends on intact.

Upload publication moved its staging area out of the destination's own
directory into a shared `.staging` root, which makes the publishing `link` a
cross-subtree one. A volume mounted under part of the uploads tree puts the two
on different devices and `link` answers `EXDEV`, which the same-directory link
could not. Publication now copies onto the destination's device and links from
there, keeping the create-or-fail step that stops a replay from overwriting a
stored object.

Five tests that passed regardless of the code:

- `resolveFolderPathFilter` was only ever exercised through hand-written
  reimplementations in the suites that mock it out, so widening a miss to
  unfiltered — every filtered list answering with the whole workspace — left
  them all green. The real helper is now tested where it lives.
- The only measurement of `generateWorkspaceFileKey` asserted the key's last
  component against `NAME_MAX` rather than the component plus the sidecar
  written beside it, so it passed with the sidecar reservation removed.
- `GET /logs` asserted only that a rejected cursor does NOT name `sortBy`,
  which almost any wording satisfies, including one saying nothing at all.
- The skills lifecycle test asserted that the four writes agree on a
  workspace-key policy, which a lifecycle uniformly allowing one also
  satisfies; it now pins the policy they agree on and the kinds they admit.
- The v2 skills create test lost `expect(capture).not.toHaveBeenCalled()` when
  the create path moved to a personal key. The behaviour it pinned is gone —
  the workspace-key create is refused now — so it is re-homed as the refusal
  reaching the caller as a 403 with no analytics behind it.

Two claims did not hold. `CURSOR_VERSION` is correctly left at 1: the filter
stamp is additive, a pre-stamp token still decodes, an unfiltered read still
resumes, and only a filtered replay fails — with a conflict that names the
filter, where a version bump would answer a generic unreadable-cursor 400 to
every in-flight token. Tests pin all three, plus the minted version itself. And
the upload-session key-budget cases do exercise the real shared budget through
the real segment builder; only the workspace-key prefix is the stub's, which is
now stated where the stub is declared.

Author: waleedlatif1

apps/sim/app/api/v2/logs/route.test.ts

@@ -24,6 +24,7 @@ vi.mock('@/lib/logs/application/list-public-logs', () => ({
   listPublicLogs: { operation: { id: 'logs.list' }, execute: mocks.execute },
 }))
 
+import { UNREADABLE_CURSOR_MESSAGE } from '@/lib/api/cursor-binding'
 import { OrchestrationError } from '@/lib/core/orchestration/types'
 import { cursorFilterScope, encodeScopedCursor } from '@/app/api/v2/lib/response'
 import { GET } from '@/app/api/v2/logs/route'
@@ -216,7 +217,14 @@ describe('GET /api/v2/logs', () => {
     expect(mocks.execute).not.toHaveBeenCalled()
   })
 
-  /** Neither param exists on this operation, so naming them sends the caller nowhere. */
+  /**
+   * An undecodable token says nothing about which param changed, and this
+   * operation declares neither `sortBy` nor `sortOrder` under a `.strict()`
+   * query schema — so the sort-mismatch message would answer one 400 with
+   * advice that earns a second. The message is asserted exactly rather than by
+   * absence: "does not say sortBy" is satisfied by almost any wording, including
+   * one that tells the caller nothing at all.
+   */
   it('names the params a rejected cursor is actually bound to', async () => {
     const response = await GET(
       new NextRequest(
@@ -225,6 +233,8 @@ describe('GET /api/v2/logs', () => {
     )
 
     const body = await response.json()
+    expect(body.error.message).toBe(UNREADABLE_CURSOR_MESSAGE)
+    expect(body.error.message).toContain('Restart pagination without a cursor')
     expect(body.error.message).not.toContain('sortBy')
     expect(body.error.message).not.toContain('sortOrder')
   })

apps/sim/app/api/v2/skills/route.test.ts

@@ -50,6 +50,7 @@ vi.mock('@/lib/skills/application/use-cases', () => ({
   createSkillUseCase: { operation: { id: 'skills.create' }, execute: mocks.create },
 }))
 
+import { PrincipalKindAuthorizationError } from '@/lib/core/application'
 import { cursorFilterScope, cursorSortKey, encodeOffsetCursor } from '@/app/api/v2/lib/response'
 import { GET, POST } from '@/app/api/v2/skills/route'
 
@@ -311,6 +312,33 @@ describe('/api/v2/skills', () => {
     )
   })
 
+  /**
+   * The workspace-key create used to be the case this file pinned analytics
+   * against: it succeeded, and the assertion was that no `skill_created` event
+   * was attributed to a principal with no human subject. `skills.create` now
+   * denies the key outright, so what needs pinning here is the surface's half of
+   * that — the refusal reaches the caller as the operation's own 403, and a
+   * create that never happened emits nothing.
+   */
+  it('refuses a workspace-key create and records no analytics for it', async () => {
+    mocks.create.mockRejectedValueOnce(
+      new PrincipalKindAuthorizationError('workspace_api_key', 'skills.create')
+    )
+
+    const response = await POST(
+      request('POST', '/api/v2/skills', {
+        workspaceId: WORKSPACE_ID,
+        name: skill.name,
+        description: skill.description,
+        content: skill.content,
+      })
+    )
+
+    expect(response.status).toBe(403)
+    expect(mocks.create).toHaveBeenCalledWith(expect.objectContaining({ principal: PRINCIPAL }))
+    expect(mocks.capture).not.toHaveBeenCalled()
+  })
+
   it('authenticates before parsing skill input', async () => {
     mocks.authenticate.mockRejectedValueOnce(new MockV2ApiKeyUnauthenticatedError())
 

apps/sim/lib/api/server/routes/v2-json-route.test.ts

@@ -35,6 +35,7 @@ import {
   defineV2JsonRoute,
   type V2ErrorPolicy,
   v2ApiKeyAuth,
+  v2HeadAuthorizationResponse,
   v2OrchestrationErrorPolicy,
   v2RateLimits,
 } from '@/lib/api/server/routes/v2-json-route'
@@ -623,4 +624,23 @@ describe('defineV2JsonRoute HEAD on a route that is not head-safe', () => {
   it('refuses at definition time to build the route when the use case cannot authorize', () => {
     expect(() => createHeadHandler({ omitAuthorize: true })).toThrow(/authorize/)
   })
+
+  /**
+   * The definition-time guard is what a route hits, and it covers both builders
+   * that answer a `HEAD` this way. This pins the responder's own behaviour if it
+   * is ever reached another way: a missing authorization phase has to fail,
+   * because skipping it hands back the bodiless 200 for a resource nothing
+   * authorized — the leak the guard exists to prevent, restored.
+   */
+  it('refuses to answer 200 when the authorization phase is missing', async () => {
+    await expect(
+      v2HeadAuthorizationResponse({
+        useCase: { authorize: undefined },
+        principal,
+        input: { widgetId: 'widget-1', workspaceId: 'workspace-1' },
+        request: headRequest(),
+        errorPolicy: v2OrchestrationErrorPolicy,
+      })
+    ).rejects.toThrow(/authorize/)
+  })
 })

apps/sim/lib/api/server/routes/v2-json-route.ts

@@ -195,6 +195,14 @@ export function requireHeadAuthorizableUseCase(
  * produced — 400, 401, 403, 404, 429 — and only an authorized caller reaches the
  * 200. What a `HEAD` never reaches is the use case's business phase, so the
  * outbound connection, the row write, and the audit event stay unfired.
+ *
+ * A use case with no `authorize` is refused here rather than skipped. Both
+ * builders that call this already refuse such a route at module load through
+ * {@link requireHeadAuthorizableUseCase}, and they are its only callers, so the
+ * refusal is unreachable through them. It is not written as a comment because
+ * the alternative — an optional call — degrades a missing phase into exactly the
+ * bodiless 200 this function exists to stop, and it does so silently. Failing
+ * closed makes an authorization that actually ran the only route to that 200.
  */
 export async function v2HeadAuthorizationResponse(args: {
   useCase: Pick<OperationUseCase<ApplicationOperation, unknown, unknown>, 'authorize'>
@@ -203,8 +211,14 @@ export async function v2HeadAuthorizationResponse(args: {
   request: NextRequest
   errorPolicy: V2ErrorPolicy
 }): Promise<NextResponse> {
+  const { authorize } = args.useCase
+  if (typeof authorize !== 'function') {
+    throw new Error(
+      'HEAD on a route that is not head-safe reached a use case with no authorize(); answering 200 would leak the existence of a resource the GET never authorized.'
+    )
+  }
   try {
-    await args.useCase.authorize?.({
+    await authorize({
       principal: args.principal,
       input: args.input,
       request: args.request,

apps/sim/lib/folders/queries.test.ts

@@ -17,6 +17,7 @@ import {
   listActiveFolderRows,
   listFoldersForWorkspace,
   loadActiveFolderPathIndex,
+  resolveFolderPathFilter,
   resolveRestoredFolderId,
   toFolderApi,
   wouldCreateFolderCycle,
@@ -318,6 +319,40 @@ describe('folder queries', () => {
     })
   })
 
+  /**
+   * The one place the real helper is exercised. Every list use case that filters
+   * by `folderPath` mocks this module out and stands a reimplementation in for
+   * it, so a defect here — a miss widening to unfiltered, a root path that stops
+   * resolving — would leave all of those suites green while every filtered list
+   * answered with the wrong rows.
+   */
+  describe('resolveFolderPathFilter', () => {
+    const index = {
+      pathById: new Map([['f-1', 'Reports']]),
+      idByPath: new Map([['Reports', 'f-1']]),
+    }
+
+    it('treats an omitted path as no filter at all', () => {
+      expect(resolveFolderPathFilter(index, undefined)).toEqual({ kind: 'unfiltered' })
+    })
+
+    it('resolves the root path to the workspace root rather than to a folder id', () => {
+      expect(resolveFolderPathFilter(index, '/')).toEqual({ kind: 'folder', folderId: null })
+    })
+
+    it('resolves a named path to its folder id', () => {
+      expect(resolveFolderPathFilter(index, 'Reports')).toEqual({ kind: 'folder', folderId: 'f-1' })
+    })
+
+    /**
+     * A path naming no active folder narrows the list to nothing. Widening it to
+     * `unfiltered` would answer a scoped read with every row in the workspace.
+     */
+    it('narrows to nothing for a path that names no active folder', () => {
+      expect(resolveFolderPathFilter(index, 'Archive')).toEqual({ kind: 'noMatch' })
+    })
+  })
+
   describe('toFolderApi', () => {
     it('serializes timestamps to ISO strings and preserves a null deletedAt', () => {
       expect(toFolderApi(ROW)).toMatchObject({

apps/sim/lib/skills/application/operations.test.ts

@@ -27,6 +27,13 @@ describe('skill operation registry', () => {
    * The invariant the create/delete split violated. A principal kind that can
    * create a skill must be able to remove it, or its only possible interaction
    * with the resource is to accumulate rows it can never reach again.
+   *
+   * Symmetry alone is not the property. A lifecycle that uniformly ALLOWED a
+   * workspace key is just as symmetric and reopens the hole, because the edit
+   * paths cannot resolve an acting subject for one. So both halves are pinned:
+   * the writes agree on a policy, and the policy they agree on is the one every
+   * edit path can honour. The principal kinds are compared directly rather than
+   * left to the test's own name.
    */
   it('admits the same principal kinds to every write in the lifecycle', () => {
     const writes = [
@@ -35,9 +42,11 @@ describe('skill operation registry', () => {
       skillOperations.upsert,
       skillOperations.delete,
     ]
-    const policies = writes.map((operation) => operation.workspaceApiKey)
 
-    expect(new Set(policies).size).toBe(1)
+    expect(new Set(writes.map((operation) => operation.workspaceApiKey))).toEqual(new Set(['deny']))
+    for (const operation of writes) {
+      expect(operation.principalKinds).toEqual(skillOperations.delete.principalKinds)
+    }
   })
 
   it('gates every edit path on a human subject rather than workspace role', () => {

apps/sim/lib/table/rows/cursor.test.ts

@@ -218,5 +218,27 @@ describe('tokens minted before the filter stamp', () => {
     expect(() => assertCursorQueryBinding(decoded, { predicate: ACTIVE })).toThrow(
       /Restart paging without the cursor/
     )
+    /**
+     * The code, not just the wording, is what a bumped `CURSOR_VERSION` would
+     * cost: every in-flight token would fail `INVALID_CURSOR` at decode instead,
+     * including the unfiltered ones that resume fine today.
+     */
+    try {
+      assertCursorQueryBinding(decoded, { predicate: ACTIVE })
+      expect.unreachable('a re-filtered replay must be refused')
+    } catch (e) {
+      expect((e as TableQueryValidationError).code).toBe('CURSOR_FILTER_CONFLICT')
+    }
+  })
+
+  /**
+   * The version a token minted today carries. Pinned so a bump is a deliberate
+   * edit here rather than a silent one that strands every cursor a running
+   * deploy already handed out.
+   */
+  it('mints tokens at the version the previous deploy could already read', () => {
+    const token = encodeCursor({ lastRow: ROW, keysetValid: true, nextOffset: 10 })
+
+    expect(JSON.parse(Buffer.from(token, 'base64url').toString('utf8')).v).toBe(1)
   })
 })

apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.test.ts

@@ -3,6 +3,7 @@
  */
 
 import { describe, expect, it } from 'vitest'
+import { LOCAL_UPLOAD_METADATA_SUFFIX } from '@/lib/uploads/core/storage-key'
 import {
   findWorkspaceFileRecord,
   generateWorkspaceFileKey,
@@ -92,12 +93,26 @@ describe('workspace file reference normalization', () => {
   })
 })
 
+/**
+ * The only place the real key builder is measured — the upload-session suites
+ * stand a stub in for it — so the budget is asserted here the way the filesystem
+ * enforces it. Local storage writes a metadata sidecar beside the object under
+ * the object's own name, so `NAME_MAX` bounds the key's last component PLUS that
+ * suffix, not the component alone. Measuring the component alone passes with the
+ * sidecar reservation removed, and the overflow returns as an `ENAMETOOLONG` 500
+ * on a name the contract already admitted.
+ */
 describe('workspace file storage keys', () => {
-  it('keeps the last key component within one path component for the longest admitted name', () => {
+  /** POSIX `NAME_MAX`, in bytes, for one path component. */
+  const NAME_MAX = 255
+
+  it('leaves the longest admitted name room for its local sidecar', () => {
     const key = generateWorkspaceFileKey('ws_123', `${'a'.repeat(251)}.txt`)
     const lastSegment = key.slice(key.lastIndexOf('/') + 1)
 
-    expect(Buffer.byteLength(lastSegment, 'utf-8')).toBeLessThanOrEqual(255)
+    expect(
+      Buffer.byteLength(`${lastSegment}${LOCAL_UPLOAD_METADATA_SUFFIX}`, 'utf-8')
+    ).toBeLessThanOrEqual(NAME_MAX)
     expect(key.startsWith('workspace/ws_123/')).toBe(true)
     expect(lastSegment.endsWith('.txt')).toBe(true)
   })

apps/sim/lib/uploads/upload-session/provider.test.ts

@@ -1,9 +1,16 @@
 /**
  * @vitest-environment node
  */
-import { mkdir, readdir, readFile, rm, stat } from 'node:fs/promises'
+import { link, mkdir, readdir, readFile, rm, stat } from 'node:fs/promises'
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 
+/**
+ * Spied rather than replaced: every assertion in this file reads the real
+ * filesystem, and the only behaviour worth faking is a single `link` answering
+ * `EXDEV`, which no temporary directory can be made to produce on its own.
+ */
+vi.mock('node:fs/promises', { spy: true })
+
 const { testUploadDirectory, mockS3Presign, mockS3PartUrls } = vi.hoisted(() => ({
   testUploadDirectory: `/tmp/sim-upload-session-provider-${process.pid}`,
   mockS3Presign: vi.fn(),
@@ -117,6 +124,60 @@ describe('local upload-session provider', () => {
     })
   })
 
+  /**
+   * Staging moved out of the destination's own directory into one shared
+   * `.staging` root, which is what makes this reachable: a volume mounted under
+   * part of the uploads tree puts the staged object and its destination on
+   * different devices, and a hard link cannot span them. Publication has to
+   * survive that without giving up the create-or-fail the link provides.
+   */
+  it('publishes across a filesystem boundary a hard link cannot span', async () => {
+    vi.mocked(link).mockRejectedValueOnce(
+      Object.assign(new Error('EXDEV: cross-device link'), { code: 'EXDEV' })
+    )
+
+    await writeLocalPutObject({
+      uploadId: 'upload-1',
+      key: 'workspace/workspace-1/file.bin',
+      body: byteStream('ab', 'cd'),
+      expectedSize: 4,
+      contentType: 'application/octet-stream',
+      metadata: METADATA,
+    })
+
+    await expect(readFile(localPath('workspace/workspace-1/file.bin'), 'utf8')).resolves.toBe(
+      'abcd'
+    )
+    await expect(
+      headProviderObject({
+        provider: 'local',
+        key: 'workspace/workspace-1/file.bin',
+        context: CONTEXT,
+      })
+    ).resolves.toMatchObject({ size: 4, uploadId: 'upload-1' })
+    expect(await temporaryFiles('workspace/workspace-1')).toEqual([])
+    expect(await allEntries('.staging')).toEqual([])
+  })
+
+  it('still refuses to overwrite an existing object when the link cannot span devices', async () => {
+    const params = {
+      uploadId: 'upload-1',
+      key: 'workspace/workspace-1/file.bin',
+      expectedSize: 3,
+      contentType: 'application/octet-stream',
+      metadata: METADATA,
+    }
+    await writeLocalPutObject({ ...params, body: byteStream('one') })
+    vi.mocked(link).mockRejectedValueOnce(
+      Object.assign(new Error('EXDEV: cross-device link'), { code: 'EXDEV' })
+    )
+
+    await expect(writeLocalPutObject({ ...params, body: byteStream('two') })).rejects.toThrow()
+
+    await expect(readFile(localPath(params.key), 'utf8')).resolves.toBe('one')
+    expect(await temporaryFiles('workspace/workspace-1')).toEqual([])
+  })
+
   it('does not let a replayed PUT overwrite the final object', async () => {
     const params = {
       uploadId: 'upload-1',