fix(cli): review round 5 — header sanitization, auth ordering, stale suggestion

## Table headers stayed executable (Greptile, P1 security)

Round 1 sanitized cell *values* but not the column *names*, and a table's
columns are user-defined — so the same control sequences were still executable
one row higher, in the header. Sanitizing is now done inside `renderTable`
rather than at each call site, so a future column source cannot reopen it, with
the two key-derived column builders covered as well.

## Fresh install was told the wrong first step (Cursor, Low)

Generated commands read `profile.workspaceId` directly, bypassing
`requireWorkspace()` — which checks the key first precisely so a new user is
told to log in rather than to set a workspace they cannot use yet. That ordering
was fixed for the hand-written commands earlier and reintroduced by the runtime.
`sim tables list` on an empty profile now says "Not logged in" again.

## A stale suggestion shadowed the fallback (Cursor, Medium)

The picker took `selected ?? suggestedWorkspaceId ?? lastActiveWorkspaceId`. The
suggestion comes from a profile the CLI wrote earlier, so it can name a
workspace the user has since left — and merely being truthy, it blocked the
last-active fallback and left the card on "no workspace" with a perfectly good
one available. It now counts only when it resolves against the loaded list.

Two of the three have tests that fail against the previous code; the third is
verified end-to-end (`sim tables list` on an empty profile).

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

Author: TheodoreSpeaks

apps/sim/app/cli/auth/cli-auth-view.tsx

@@ -71,11 +71,21 @@ export function CliAuthView() {
    */
   const loadingWorkspaces = isPlatform && workspaces.isPending
 
-  // The terminal's suggestion, then the user's last active workspace. Derived at
-  // render rather than synced into state through an effect, so the first paint
-  // after the list loads already shows the right row.
-  const workspaceId =
-    selected ?? request.suggestedWorkspaceId ?? workspaces.data?.lastActiveWorkspaceId ?? null
+  /**
+   * The terminal's suggestion, then the user's last active workspace. Derived at
+   * render rather than synced into state through an effect, so the first paint
+   * after the list loads already shows the right row.
+   *
+   * The suggestion only counts when it resolves to a workspace the user
+   * actually has. It comes from a profile the CLI wrote earlier, so it can name
+   * a workspace they have since left or one that no longer exists — and being
+   * merely truthy, it used to shadow the last-active fallback and leave the card
+   * on "no workspace" with a perfectly good one available.
+   */
+  const suggested = workspaces.data?.workspaces.some((w) => w.id === request.suggestedWorkspaceId)
+    ? request.suggestedWorkspaceId
+    : null
+  const workspaceId = selected ?? suggested ?? workspaces.data?.lastActiveWorkspaceId ?? null
   const chosen = workspaces.data?.workspaces.find((w) => w.id === workspaceId)
 
   // Only an admin can bind a key to a workspace. Anything less still gets a

packages/sim-cli/src/commands/hand-written.ts

@@ -96,7 +96,9 @@ function rowColumns(rows: Row[]): Column<Row>[] {
   return [
     { header: 'id', value: (row) => row.id },
     ...keys.map((key) => ({
-      header: key,
+      // A table's column names are user-defined, so the header is remote
+      // content just as much as the cell beneath it.
+      header: sanitize(key),
       value: (row: Row) => {
         const value = row.data[key]
         if (value === null || value === undefined) return text(null)

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

@@ -14,6 +14,7 @@ import {
 } from './render.js'
 
 const ESC = String.fromCharCode(27)
+const BEL = String.fromCharCode(7)
 
 /** Colour is stripped when not writing to a TTY, so force it on for these assertions. */
 const coloured = new Chalk({ level: 1 })
@@ -239,6 +240,15 @@ describe('sanitize', () => {
     expect(text(`${ESC}]0;x\u0007safe`)).toBe('safe')
   })
 
+  it('is applied to a table header, not only its cells', () => {
+    // A table's column names are user-defined, so the header is remote content
+    // too — sanitizing cells alone left the sequences executable one row up.
+    const hostile = `${ESC}]0;pwned${BEL}email`
+    printList('table', [{ v: 'a@b.co' }], [{ header: hostile, value: () => 'a@b.co' }])
+    expect(logged[0]).not.toContain(ESC)
+    expect(logged[0]).toContain('EMAIL')
+  })
+
   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.

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

@@ -134,13 +134,17 @@ function pad(value: string, width: number): string {
 function renderTable<T>(rows: T[], columns: Column<T>[]): string {
   if (rows.length === 0) return chalk.dim('No results.')
 
+  // A header can be a user-defined column name (a table's own columns), so it is
+  // remote content and gets the same treatment as a cell. Doing it here rather
+  // than only at each call site means a future column source cannot reopen this.
+  const headers = columns.map((column) => sanitize(column.header))
   const cells = rows.map((row) => columns.map((column) => column.value(row)))
-  const widths = columns.map((column, index) =>
-    Math.max(visibleWidth(column.header), ...cells.map((line) => visibleWidth(line[index])))
+  const widths = columns.map((_column, index) =>
+    Math.max(visibleWidth(headers[index]), ...cells.map((line) => visibleWidth(line[index])))
   )
 
-  const header = columns
-    .map((column, index) => chalk.dim(pad(column.header.toUpperCase(), widths[index])))
+  const header = headers
+    .map((label, index) => chalk.dim(pad(label.toUpperCase(), widths[index])))
     .join('  ')
     .trimEnd()
 

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

@@ -86,7 +86,10 @@ function inferColumns(rows: unknown[]): Column<unknown>[] {
   }
 
   return keys.map((key) => ({
-    header: key,
+    // The key itself is remote data when the rows are user-defined, and the
+    // header is printed just like a cell — sanitizing values but not headers
+    // left the same control sequences executable one row higher.
+    header: sanitize(key),
     value: (row: unknown) => renderCell(at(row, key), 'auto'),
   }))
 }
@@ -211,7 +214,19 @@ function buildLeaf(operation: V2OperationName, spec: CommandSpec, leafName: stri
     }
 
     const { client, profile } = clientFrom(host)
-    const request = buildRequest(operation, positional, flags, profile.workspaceId)
+    // `requireWorkspace` checks the key first on purpose, so a fresh install is
+    // told to log in rather than to set a workspace it cannot use yet. Reading
+    // `profile.workspaceId` directly skipped that ordering.
+    const needsWorkspace = Boolean(
+      (operationSpec.query && PROFILE_INJECTED_FIELD in operationSpec.query) ||
+        (operationSpec.body && PROFILE_INJECTED_FIELD in operationSpec.body)
+    )
+    const request = buildRequest(
+      operation,
+      positional,
+      flags,
+      needsWorkspace ? client.requireWorkspace() : profile.workspaceId
+    )
 
     const paging = cursorSlot(operation)
     if (paging) {