fix(cli): review round 2 — body-cursor paging, timestamp sanitization

## `tables rows query` printed nothing (Cursor, High)

`isCursorList` only looked for `cursor` on the query slot, but `queryRows` is a
POST whose whole filter — cursor included — is in the body. It therefore took
the single-request path, which handed an array of rows to `printRecord` and
printed an empty record, and it never auto-paged past the first page.

Replaced with `cursorSlot`, which checks both slots and tells the pager where to
put the cursor back. Added a defensive branch so an array reaching the
single-resource path renders as a list with inferred columns rather than
silently printing nothing.

## Invalid timestamps bypassed sanitization (Greptile, P1 security)

`timestamp()` echoes an unparseable value verbatim, and that value is still
server-supplied — so the branch was a way past every other formatter for the
control sequences round 1 closed. Now sanitized on that path too. Audited the
remaining formatters: no other path returns a server value unsanitized.

Both fixes have tests that fail against the previous code.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EsThUPqZXjwuuyRjBbmVkj

Author: TheodoreSpeaks

packages/sim-cli/src/output/render.test.ts

@@ -9,6 +9,7 @@ import {
   printRecord,
   sanitize,
   text,
+  timestamp,
   visibleWidth,
 } from './render.js'
 
@@ -237,4 +238,14 @@ describe('sanitize', () => {
   it('is applied to values passing through text()', () => {
     expect(text(`${ESC}]0;x\u0007safe`)).toBe('safe')
   })
+
+  it('is applied to an unparseable timestamp, which is echoed verbatim', () => {
+    // The invalid-date branch returns the server's own string, so it was a way
+    // past every other formatter.
+    expect(timestamp(`${ESC}]0;pwned\u0007not-a-date`)).toBe('not-a-date')
+  })
+
+  it('still formats a valid timestamp normally', () => {
+    expect(timestamp('2026-07-31T09:14:22.500Z')).toBe('2026-07-31 09:14:22')
+  })
 })

packages/sim-cli/src/output/render.ts

@@ -63,7 +63,10 @@ export function text(value: unknown): string {
 export function timestamp(value: string | null | undefined): string {
   if (!value) return EMPTY
   const date = new Date(value)
-  if (Number.isNaN(date.getTime())) return String(value)
+  // Sanitized on the way out: an unparseable value is echoed verbatim, and it is
+  // still server-supplied, so this branch was a way to smuggle control sequences
+  // past every other formatter.
+  if (Number.isNaN(date.getTime())) return sanitize(String(value))
   return date.toISOString().replace('T', ' ').slice(0, 19)
 }
 

packages/sim-cli/src/runtime/build.test.ts

@@ -108,3 +108,39 @@ describe('commands parsed through commander', () => {
     expect(mockRequest).not.toHaveBeenCalled()
   })
 })
+
+describe('pagination slot', () => {
+  it('pages a body-cursor operation and renders its rows', async () => {
+    // `queryRows` is a POST whose cursor is in the body, not the query. Reading
+    // only the query made it take the single-request path and print nothing.
+    mockRequest.mockReset()
+    mockRequest
+      .mockResolvedValueOnce({ data: [{ id: 'r1' }], nextCursor: 'c1' })
+      .mockResolvedValueOnce({ data: [{ id: 'r2' }], nextCursor: null })
+    const lines: string[] = []
+    vi.spyOn(console, 'log').mockImplementation((line: string) => {
+      lines.push(line)
+    })
+
+    await program().parseAsync(['node', 'sim', 'tables', 'rows', 'query', 'tbl_1'])
+
+    expect(mockRequest).toHaveBeenCalledTimes(2)
+    // Second call resumes from the cursor — in the body, where the contract puts it.
+    expect(mockRequest.mock.calls[1][1].body).toMatchObject({ cursor: 'c1' })
+    expect(mockRequest.mock.calls[1][1].query).not.toHaveProperty('cursor')
+    // And the rows actually render rather than printing an empty record.
+    expect(JSON.parse(lines[0])).toEqual([{ id: 'r1' }, { id: 'r2' }])
+  })
+
+  it('keeps a query-cursor operation on the query slot', async () => {
+    mockRequest.mockReset()
+    mockRequest
+      .mockResolvedValueOnce({ data: [{ id: 'a' }], nextCursor: 'c1' })
+      .mockResolvedValueOnce({ data: [{ id: 'b' }], nextCursor: null })
+    vi.spyOn(console, 'log').mockImplementation(() => {})
+
+    await program().parseAsync(['node', 'sim', 'logs', 'list'])
+
+    expect(mockRequest.mock.calls[1][1].query).toMatchObject({ cursor: 'c1' })
+  })
+})

packages/sim-cli/src/runtime/build.ts

@@ -96,10 +96,24 @@ function summaryFor(operation: V2OperationName): string | undefined {
   return (V2_OPERATIONS[operation] as { summary?: string }).summary
 }
 
-/** Whether the operation answers with the `{ data, nextCursor }` list envelope. */
-function isCursorList(operation: V2OperationName): boolean {
-  const spec = V2_OPERATIONS[operation] as { query?: Record<string, FieldSpec> }
-  return Boolean(spec.query && 'cursor' in spec.query)
+/**
+ * Which request slot carries the pagination cursor, or null for a non-list
+ * operation.
+ *
+ * Both slots have to be checked: most lists take `cursor` as a query param, but
+ * `queryRows` is a POST whose whole filter — cursor included — is in the body.
+ * Looking only at the query made it fall through to the single-request path,
+ * which then rendered its array of rows through `printRecord` and printed
+ * nothing at all, and never auto-paged.
+ */
+function cursorSlot(operation: V2OperationName): 'query' | 'body' | null {
+  const spec = V2_OPERATIONS[operation] as {
+    query?: Record<string, FieldSpec>
+    body?: Record<string, FieldSpec>
+  }
+  if (spec.query && 'cursor' in spec.query) return 'query'
+  if (spec.body && 'cursor' in spec.body) return 'body'
+  return null
 }
 
 /** Adds the flags a field needs, or nothing when the contract omits it. */
@@ -199,7 +213,8 @@ function buildLeaf(operation: V2OperationName, spec: CommandSpec, leafName: stri
     const { client, profile } = clientFrom(host)
     const request = buildRequest(operation, positional, flags, profile.workspaceId)
 
-    if (isCursorList(operation)) {
+    const paging = cursorSlot(operation)
+    if (paging) {
       const rawLimit = Number.parseInt(String(flags.limit ?? DEFAULT_LIMIT), 10)
       if (Number.isNaN(rawLimit) || rawLimit < 0) {
         throw new SimApiError('--limit must be a non-negative number', 0)
@@ -210,10 +225,14 @@ function buildLeaf(operation: V2OperationName, spec: CommandSpec, leafName: stri
       const rows: unknown[] = []
       let cursor: string | null = null
       do {
+        // The cursor goes back in whichever slot the contract declared it.
         const page: V2Page<unknown> = await client.request(request.path, {
           method: operationSpec.method as 'GET' | 'POST',
-          query: { ...request.query, cursor },
-          body: request.body,
+          query: paging === 'query' ? { ...request.query, cursor } : request.query,
+          body:
+            paging === 'body'
+              ? { ...(request.body ?? {}), ...(cursor ? { cursor } : {}) }
+              : request.body,
         })
         rows.push(...page.data)
         cursor = page.nextCursor
@@ -231,8 +250,10 @@ function buildLeaf(operation: V2OperationName, spec: CommandSpec, leafName: stri
     })
     const data = result?.data ?? result
 
-    if (spec.columns && Array.isArray(data)) {
-      printList(profile.output, data, columnsFrom(spec.columns))
+    if (Array.isArray(data)) {
+      // Reached when a non-paginated operation answers with a collection.
+      // `printRecord` would silently print nothing for an array.
+      printList(profile.output, data, spec.columns ? columnsFrom(spec.columns) : inferColumns(data))
       return
     }