fix(control ui): accept paired device token for assistant media auth

[Patrick-Erichsen]

Fixes #70505 (partially)

• accept paired device-token auth on assistant-media, including the existing ?token= Control UI path
• keep the current media URL behavior for now

Follow-up:

• control ui: replace assistant-media ?token auth with scoped media tickets #70830

Tests:

• pnpm test src/gateway/control-ui.http.test.ts
• pnpm test ui/src/ui/chat/grouped-render.test.ts

[aisle-research-bot]

🔒 Aisle Security Analysis

We found 3 potential security issue(s) in this PR:

#	Severity	Title
1	🟡 Medium	Authentication tokens accepted via query string on Control UI assistant-media endpoint
2	🟡 Medium	Rate-limit bypass: successful device-token auth resets shared-secret auth throttling for same IP
3	🟡 Medium	Potential DoS from linear scan of paired devices on each Control UI request

1. 🟡 Authentication tokens accepted via query string on Control UI assistant-media endpoint

Property	Value
Severity	Medium
CWE	CWE-598
Location	src/gateway/control-ui.ts:413-447

Description

The Control UI assistant-media endpoint explicitly enables query-string authentication (?token=) by passing allowQueryToken: true into authorizeControlUiReadRequest(). This allows secrets to appear in URLs.

Why this is risky:

• Token leakage via URL surfaces: browser history, bookmarks, copy/paste, server/proxy access logs, CDN logs, analytics, and crash reports.
• Token exfiltration via referrer in some contexts: although Referrer-Policy: no-referrer is set, it is not a complete defense against URL-based leakage (e.g., intermediary logs, client-side logging).
• Increased CSRF-style exposure: URL-based bearer tokens make it easier for an attacker to trigger authenticated GETs if they can cause a victim to visit a crafted URL containing the token (or if the token is otherwise disclosed).

Vulnerable code paths include:

• resolveAssistantMediaAuthToken() reads token from query parameters.
• handleControlUiAssistantMediaRequest() enables allowQueryToken: true, allowing ?token= to authenticate assistant-media reads.

Recommendation

Prefer Authorization headers (or cookies with proper CSRF defenses) and disable query-string tokens for authenticated requests.

Fix option A (recommended): remove query token support

​// Do not allow query tokens
await authorizeControlUiReadRequest(req, res, {
  ...,
  allowQueryToken: false,
});

function resolveControlUiReadAuthToken(req: IncomingMessage): string | undefined {
  return getBearerToken(req) ?? undefined;
}

Fix option B (if query token is unavoidable for some clients):

• Use short-lived, single-use tokens scoped only to this resource.
• Immediately exchange the query token for a cookie/header-based session and redirect to a URL without the token.
• Ensure request logging middleware never logs full URLs (or at least strips token).

2. 🟡 Rate-limit bypass: successful device-token auth resets shared-secret auth throttling for same IP

Property	Value
Severity	Medium
CWE	CWE-307
Location	src/gateway/control-ui.ts:289-293

Description

In authorizeControlUiReadRequest, when shared-secret authentication fails but the provided token is a valid device token, the code resets both rate-limiter scopes:

• AUTH_RATE_LIMIT_SCOPE_DEVICE_TOKEN (expected)
• AUTH_RATE_LIMIT_SCOPE_SHARED_SECRET (problematic)

This allows an attacker who possesses any valid device token (operator role) to repeatedly clear the shared-secret failure counter for the same clientIp, enabling high-rate online guessing of the shared secret by alternating attempts:

1. Attempt shared-secret auth with a guessed token/password → failure increments shared-secret scope (inside authorizeHttpGatewayConnect).
2. Present valid device token → shared-secret scope is reset.
3. Repeat.

Because the limiter key is {scope, clientIp} and reset() deletes the entry, shared-secret lockouts can be continuously avoided from that IP, weakening protection of shared-secret credentials.

Recommendation

Do not reset the shared-secret scope on successful device-token authentication.

Instead, only reset the scope corresponding to the credential type that was successfully validated:

if (deviceTokenOk) {
  opts.rateLimiter?.reset(clientIp, AUTH_RATE_LIMIT_SCOPE_DEVICE_TOKEN);
  ​// Do not reset shared-secret scope here.
  resolvedAuthResult = { ok: true, method: "device-token" };
}

If you want a “global reset” behavior after any successful auth, ensure the shared secret cannot be brute-forced from the same endpoint (e.g., separate endpoints/scopes, require explicit shared-secret auth to reset shared-secret counters, or add an additional per-account/per-credential identifier so a device-token holder cannot clear another credential’s failures).

3. 🟡 Potential DoS from linear scan of paired devices on each Control UI request

Property	Value
Severity	Medium
CWE	CWE-400
Location	src/gateway/control-ui.ts:327-348

Description

authorizeControlUiDeviceReadToken performs an O(N) scan over all paired devices for every request that presents a token but fails shared-secret auth.

• Input: attacker-controlled bearer/query token from the HTTP request
• Work per request: listDevicePairing() loads pairing state from disk and sorts lists; then the code iterates over every paired device and performs a constant-time token comparison (verifyPairingToken) until a match is found.
• If opts.rateLimiter is not configured (it is optional), or if an attacker can distribute traffic across many source IPs, this can be abused to cause CPU and disk I/O amplification as the number of paired devices grows.

Vulnerable code:

const pairing = await listDevicePairing();
for (const device of pairing.paired) {
  const operatorToken = device.tokens?.[CONTROL_UI_OPERATOR_ROLE];
  ...
  if (!verifyPairingToken(token, operatorToken.token)) continue;
  const verified = await verifyDeviceToken({ deviceId: device.deviceId, token, ... });
  if (verified.ok) return true;
}

Recommendation

Avoid per-request linear scans and repeated state loads.

Mitigations (pick one or combine):

1. Add an index for token→device in the persisted pairing state (or store a keyed hash of the token) so verification can be O(1).
2. Cache pairing state in memory with an update mechanism, so listDevicePairing() does not hit disk on every auth attempt.
3. Require rate limiting to be enabled (do not treat rateLimiter as optional for this endpoint), and ensure the limiter check happens before any expensive disk/CPU work.

Example (conceptual):

​// Persist an index of tokenHash -> { deviceId, role }
const tokenHash = sha256base64url(token);
const hit = pairing.tokenIndex[tokenHash];
if (!hit) return false;
return (await verifyDeviceToken({ deviceId: hit.deviceId, token, role: hit.role, scopes: [...] })).ok;

---

Analyzed PR: #70741 at commit 4a1e8ff

_{Last updated on: 2026-04-23T23:19:16Z}

[greptile-apps]

Greptile Summary

This PR moves assistant-media auth from a shared-secret query token to an Authorization: Bearer header carrying a paired device token, and switches the client from a lightweight ?meta=1 availability probe to a full content fetch that produces a blob URL for rendering.

The gateway-side change (authorizeControlUiDeviceReadToken) is well-structured: it guards with a device-scoped rate limiter, short-circuits on a fast verifyPairingToken hash check before calling the heavier verifyDeviceToken, and only trusts the result when the operator.read scope is confirmed. The UI-side changes correctly move the token out of URLs (security improvement) and add proper blob URL lifecycle management via setAssistantAttachmentAvailabilityCache.

Confidence Score: 5/5

Safe to merge; all findings are non-blocking style and quality observations.

Both inline findings are P2. The blob URL accumulation only manifests on token rotation (uncommon in practice) and the eager-download tradeoff is an intentional design decision. No correctness, security, or data-integrity defects were found.

ui/src/ui/chat/grouped-render.ts — blob URL lifecycle when the auth token rotates

Comments Outside Diff (1)

1. ui/src/ui/chat/grouped-render.ts, line 931-932 (link)

   Orphaned blob URLs when auth token rotates

   The cache key embeds the auth token (${basePath ?? ""}::${normalizedAuthToken}::${source}), so when the token changes a brand-new entry is created for the same source. setAssistantAttachmentAvailabilityCache only revokes the old blob URL when it replaces an entry under the same key; it never touches the old key's entry. Those blob URLs accumulate for the lifetime of the session.

   In a long-running session where the token refreshes even once per source, every previously-fetched blob URL is leaked. resetAssistantAttachmentAvailabilityCacheForTest handles this correctly (iterates all entries before clearing), but that helper is test-only.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: ui/src/ui/chat/grouped-render.ts
   Line: 931-932
   
   Comment:
   **Orphaned blob URLs when auth token rotates**
   
   The cache key embeds the auth token (`${basePath ?? ""}::${normalizedAuthToken}::${source}`), so when the token changes a brand-new entry is created for the same source. `setAssistantAttachmentAvailabilityCache` only revokes the old blob URL when it replaces an entry under the **same key**; it never touches the old key's entry. Those blob URLs accumulate for the lifetime of the session.
   
   In a long-running session where the token refreshes even once per source, every previously-fetched blob URL is leaked. `resetAssistantAttachmentAvailabilityCacheForTest` handles this correctly (iterates all entries before clearing), but that helper is test-only.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: ui/src/ui/chat/grouped-render.ts
Line: 931-932

Comment:
**Orphaned blob URLs when auth token rotates**

The cache key embeds the auth token (`${basePath ?? ""}::${normalizedAuthToken}::${source}`), so when the token changes a brand-new entry is created for the same source. `setAssistantAttachmentAvailabilityCache` only revokes the old blob URL when it replaces an entry under the **same key**; it never touches the old key's entry. Those blob URLs accumulate for the lifetime of the session.

In a long-running session where the token refreshes even once per source, every previously-fetched blob URL is leaked. `resetAssistantAttachmentAvailabilityCacheForTest` handles this correctly (iterates all entries before clearing), but that helper is test-only.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: ui/src/ui/chat/grouped-render.ts
Line: 945-960

Comment:
**Eager full-file download replaces lightweight meta check**

The previous implementation issued a `?meta=1` request (JSON, tiny payload) to check availability, then only downloaded the actual bytes when the image or document was rendered. The new approach fetches the full file bytes upfront to create a blob URL. For large PDFs or high-res images in previews, this means the complete file is downloaded into memory during the availability check — even if the user never scrolls to or opens the attachment. Consider whether a two-phase approach (meta check first, then blob fetch on first render) would be worth the added complexity for your expected file sizes.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "test control ui device token read routes" | Re-trigger Greptile}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6e1e7cf6c8

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f2ad888fa7

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ca60f713fd

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".