Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
219 changes: 213 additions & 6 deletions packages/runtime-node/src/relay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,20 @@
/**
* The relay route is necessarily public (browsers must reach it), so it
* ships with a per-IP fixed-window limit. Default 120 req/min; set
* `false` to disable (e.g. when a WAF already rate-limits).

Check warning on line 21 in packages/runtime-node/src/relay.ts

View check run for this annotation

Autter.dev / autter/review-gate

🟠 Medium · PR mixes refactor and behavior change

This PR mixes a no-behavior hardening/refactor of payload sanitization (bounding and cycle-safe copying of `context`) with behavior changes to request-size enforcement and relay trust/rate-limit behavior in the same transport-boundary module. `packages/runtime-node/src/relay.ts` and `packages/runtime-node/test/relay.test.mjs` are the files contributing most to the mix. Split the cleanup/hardening from behavior changes: one PR for the `sanitizeBrowserPayload`/`boundContext` refactor and its tests, and a separate PR for the byte-length enforcement and `trustProxy`/rate-limit behavior. Because `packages/runtime-node/src/relay.ts` is a high-risk exported relay path with cross-scope fan-in, keeping the API-shape/security hardening separate from runtime behavior changes would make review safer. Blast radius — if the refactor introduces a regression the behavior change masks it on: functions `sanitizeBrowserPayload`, `forward`, `IpWindow.allow`, `firstForwardedFor`, `respond`, `createBrowserRelayFetchHandler`, `createBrowserRelayHandler`, `RelayOptions`; scopes `@autter/runtime-node`; dependent files `node:http`. Suggested fix: Split this PR so refactors land separately from behavior changes. Pure-refactor PRs should preserve behavior (no test changes beyond renames). Behavior-change PRs should focus on a single new capability. Start by extracting `packages/runtime-node/src/relay.ts`'s refactor portion (or its behavior portion, whichever is smaller) into its own PR.
*/
perIpRateLimit?: number | false;
/**

Check warning on line 24 in packages/runtime-node/src/relay.ts

View check run for this annotation

Autter.dev / autter/review-gate

🟠 Medium · Rate limiting not detected

`RelayOptions.trustProxy` makes the public browser relay rate limit depend on `X-Forwarded-For` again: `createBrowserRelayFetchHandler`/`createBrowserRelayHandler` now key `IpWindow.allow` from a requester-controlled header when this flag is enabled. If a deployment flips it on without a trusted proxy that overwrites the header, an attacker can rotate the first forwarded IP per request and bypass the relay-side window, driving unbounded body parsing and authenticated forwards through `forward` under the server ingest key. Blast radius — abusing this cascades to the downstream usage that depends on this file: functions `sanitizeBrowserPayload`, `forward`, `IpWindow.allow`, `firstForwardedFor`, `respond`, `createBrowserRelayFetchHandler`, `createBrowserRelayHandler`, `RelayOptions`; scopes `@autter/runtime-node`; dependent files `node:http`. Suggested fix: Keep the relay keyed from a trusted client-IP source only. Remove the boolean opt-in, or require a trusted-proxy resolver that validates the immediate peer before honoring `X-Forwarded-For`. If trustworthy client IP cannot be established, continue using the shared bucket for fetch handlers and `req.socket.remoteAddress` for Node handlers, and add tests showing spoofed forwarded headers cannot create fresh rate-limit buckets. Blast radius — abusing this cascades to the downstream usage that depends on this file: functions `sanitizeBrowserPayload`, `forward`, `IpWindow.allow`, `firstForwardedFor`, `respond`, `createBrowserRelayFetchHandler`, `createBrowserRelayHandler`, `RelayOptions`; scopes `@autter/runtime-node`; dependent files `node:http`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Rate limiting not detected — Risk: 55/100

RelayOptions.trustProxy makes the public browser relay rate limit depend on X-Forwarded-For again: createBrowserRelayFetchHandler/createBrowserRelayHandler now key IpWindow.allow from a requester-controlled header when this flag is enabled. If a deployment flips it on without a trusted proxy that overwrites the header, an attacker can rotate the first forwarded IP per request and bypass the relay-side window, driving unbounded body parsing and authenticated forwards through forward under the server ingest key. Blast radius — abusing this cascades to the downstream usage that depends on this file: functions sanitizeBrowserPayload, forward, IpWindow.allow, firstForwardedFor, respond, createBrowserRelayFetchHandler, createBrowserRelayHandler, RelayOptions; scopes @autter/runtime-node; dependent files node:http.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Functions/symbols: sanitizeBrowserPayload, forward, IpWindow.allow, firstForwardedFor, respond, createBrowserRelayFetchHandler, createBrowserRelayHandler, RelayOptions
  • Dependent files: node:http
  • Scopes: @autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Keep the relay keyed from a trusted client-IP source only. Remove the boolean opt-in, or require a trusted-proxy resolver that validates the immediate peer before honoring `X-Forwarded-For`. If trustworthy client IP cannot be established, continue using the shared bucket for fetch handlers and `req.socket.remoteAddress` for Node handlers, and add tests showing spoofed forwarded headers cannot create fresh rate-limit buckets. Blast radius — abusing this cascades to the downstream usage that depends on this file: functions `sanitizeBrowserPayload`, `forward`, `IpWindow.allow`, `firstForwardedFor`, `respond`, `createBrowserRelayFetchHandler`, `createBrowserRelayHandler`, `RelayOptions`; scopes `@autter/runtime-node`; dependent files `node:http`.

Flagged by Autter security & observability checks.

* Trust the client-supplied `X-Forwarded-For` header when keying the per-IP
* rate limit. Off by default: the header is spoofable, so an attacker could
* rotate it to bypass the window and drive unbounded parsing/forwarding
* under the server's ingest key. Enable ONLY behind a proxy/CDN you control
* that overwrites the header. When off, the fetch handler keys a single
* shared bucket, and the Node handler keys the real socket peer address.
* Only a strict boolean `true` enables it — a truthy string such as the
* common `process.env.TRUST_PROXY === "false"` slip stays on the safe path.
*/
trustProxy?: boolean;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [ai] Non-boolean trustProxy values can enable spoofable forwarded-IP rate-limit keys — Risk: 35/100

Both relay handlers use opts.trustProxy ? ... : ..., so any truthy runtime value enables X-Forwarded-For handling. In particular, configuration code that passes process.env.TRUST_PROXY may pass "false", which is truthy and causes the public relay to key its limiter from an attacker-controlled header. Treat the setting as enabled only for opts.trustProxy === true (and optionally reject non-boolean option values) so malformed configuration retains the safe default.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Functions/symbols: sanitizeBrowserPayload, forward, IpWindow.allow, firstForwardedFor, respond, createBrowserRelayFetchHandler, createBrowserRelayHandler, RelayOptions
  • Dependent files: node:http
  • Scopes: @autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Validate relay configuration at construction time and document the trusted-proxy requirement next to `trustProxy`. If this option is meant to be user-facing, prefer a strict schema or explicit boolean parsing so only a deliberate `true` enables forwarded-header trust. Blast radius — if this is exploited it cascades to the downstream usage that depends on this file: functions `sanitizeBrowserPayload`, `forward`, `IpWindow.allow`, `firstForwardedFor`, `respond`, `createBrowserRelayFetchHandler`, `createBrowserRelayHandler`, `RelayOptions`; scopes `@autter/runtime-node`; dependent files `node:http`.

Flagged by Autter security & observability checks.

/** Called when the async forward fails (default: console.warn). */
onError?: (err: unknown) => void;
}
Expand Down Expand Up @@ -62,6 +73,184 @@

const SEVERITIES = new Set(["fatal", "error", "warning", "info"]);

// Bound a browser-supplied `context` object so it honors the sanitiser's
// guarantee that a client can't smuggle unbounded cookies/DOM/bodies through
// the relay: like every other field, context is capped — bounded depth, a
// total-node budget, per-string length, and array/key limits. Cycles and
// throwing/revoked Proxy traps fail open (that value is dropped, sanitising
// continues).
const CONTEXT_MAX_DEPTH = 6;
const CONTEXT_MAX_NODES = 256;
const CONTEXT_MAX_STRING = 4000;
const CONTEXT_MAX_ARRAY = 100;
const CONTEXT_MAX_KEYS = 100;

// Redaction — the relay attaches the server's private ingest key and forwards
// browser-supplied context into privileged telemetry, so context must never
// carry secrets. We redact on two axes, at every nesting level: by KEY NAME
// (authorization, cookie, token, password, *_secret, *_key, session, jwt, …)
// and by secret-shaped VALUE (Bearer/Basic auth strings, JWTs) even under a
// benign/custom key. Numeric/boolean values under a matched key are kept —
// they can never be a credential, and this preserves usage counts such as
// `input_tokens`.
const REDACTED = "[redacted]";
const SECRET_KEY_RE =

Check warning on line 97 in packages/runtime-node/src/relay.ts

View check run for this annotation

Autter.dev / autter/review-gate

🟠 Medium · Browser relay retains API-key-shaped values under benign context keys

`boundContext` redacts sensitive key names plus Bearer/Basic and JWT-shaped strings, but preserves other credential formats under benign keys. For example, browser-controlled context such as `{ note: "sk-..." }` or `{ debug: "AKIA..." }` passes through `sanitizeBrowserPayload` and is forwarded by the privileged relay. Apply value-pattern redaction equivalent to the runtime-node attribute redactor, or use a strict context allowlist, before forwarding. Suggested fix: Extend the relay context sanitizer to recursively drop or redact secret-shaped values, not just secret-looking keys. Reuse the existing `boundContext`/`sanitizeBrowserPayload` path so browser-supplied context cannot carry raw API-key literals into `forward`, and add tests covering benign-key nested API keys reaching both relay handlers. Blast radius — if this credential is exploited it cascades to the downstream usage that depends on this file: functions `sanitizeBrowserPayload`, `forward`, `IpWindow.allow`, `firstForwardedFor`, `respond`, `createBrowserRelayFetchHandler`, `createBrowserRelayHandler`, `RelayOptions`; scopes `@autter/runtime-node`; dependent files `node:http`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [ai] Browser relay retains API-key-shaped values under benign context keys — Risk: 57/100

boundContext redacts sensitive key names plus Bearer/Basic and JWT-shaped strings, but preserves other credential formats under benign keys. For example, browser-controlled context such as { note: "sk-..." } or { debug: "AKIA..." } passes through sanitizeBrowserPayload and is forwarded by the privileged relay. Apply value-pattern redaction equivalent to the runtime-node attribute redactor, or use a strict context allowlist, before forwarding.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Functions/symbols: sanitizeBrowserPayload, forward, IpWindow.allow, firstForwardedFor, respond, createBrowserRelayFetchHandler, createBrowserRelayHandler, RelayOptions
  • Dependent files: node:http
  • Scopes: @autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Extend the relay context sanitizer to recursively drop or redact secret-shaped values, not just secret-looking keys. Reuse the existing `boundContext`/`sanitizeBrowserPayload` path so browser-supplied context cannot carry raw API-key literals into `forward`, and add tests covering benign-key nested API keys reaching both relay handlers. Blast radius — if this credential is exploited it cascades to the downstream usage that depends on this file: functions `sanitizeBrowserPayload`, `forward`, `IpWindow.allow`, `firstForwardedFor`, `respond`, `createBrowserRelayFetchHandler`, `createBrowserRelayHandler`, `RelayOptions`; scopes `@autter/runtime-node`; dependent files `node:http`.

Flagged by Autter security & observability checks.

/(password|passwd|pwd|passphrase|passcode|secret|token|api[_-]?key|apikey|access[_-]?key|secret[_-]?key|private[_-]?key|authorization|cookie|session[_-]?id|sessionid|session|credentials?|bearer|jwt|otp|x-api-key|signature)/i;
const SECRET_VALUE_RE = /^\s*(bearer|basic)\s+\S+/i;
const JWT_RE = /\beyJ[A-Za-z0-9_-]{5,}\.eyJ[A-Za-z0-9_-]{5,}\.[A-Za-z0-9_-]+/;

/** Redact a string that looks like a credential (auth header value / JWT). */
function scrubSecretValue(s: string): string {
return SECRET_VALUE_RE.test(s) || JWT_RE.test(s) ? REDACTED : s;
}

/** UTF-8 byte length of a string (portable across edge runtimes). */
export function byteLength(text: string): number {
return new TextEncoder().encode(text).length;
}

/**
* Bounded, cycle-safe deep copy of an untrusted `context` value. Anything
* past a depth/node/length limit, a cycle, or a hostile/revoked Proxy (whose
* trap throws on classification, `length`, key enumeration, or element reads)
* is dropped. Never throws — returns a plain, bounded object.
*/
export function boundContext(value: unknown): unknown {
let nodes = 0;
const seen = new WeakSet<object>();
const walk = (v: unknown, depth: number): unknown => {
if (v === null) return null;
const t = typeof v;
if (t === "string") return scrubSecretValue((v as string).slice(0, CONTEXT_MAX_STRING));

Check failure on line 124 in packages/runtime-node/src/relay.ts

View check run for this annotation

Autter.dev / autter/review-gate

🔴 High · Browser context still forwards credential formats under benign keys

This public relay copies every string at a non-matching context key and only calls `scrubSecretValue`, which recognizes Bearer/Basic prefixes and JWT-shaped values. Consequently an unauthenticated caller can send a raw OpenAI key (`sk-...`), AWS access key (`AKIA...`), GitHub/Slack token, private-key block, URL credential, or email under a key such as `note`; it survives `boundContext`, is attached to the sanitized event, and is JSON-serialized in the authenticated forward. The server redactor already defines these formats, but this privileged browser-forwarding boundary does not use it, and the ingester's `scrubContext` only examines top-level keys and email values, so it does not close the nested path. Suggested fix: Apply the runtime-node redactor's value-format coverage recursively at the relay boundary (private key blocks, OpenAI/GitHub/AWS/Slack tokens, URL credentials, and email), or replace arbitrary browser context with an explicit allowlist. Add an end-to-end relay test using each representative secret under a benign nested key and assert the serialized forwarded payload contains no raw value.

Check failure on line 124 in packages/runtime-node/src/relay.ts

View check run for this annotation

Autter.dev / autter/review-gate

🔴 High · Public relay still forwards credential-shaped values under benign context keys

The relay route is deliberately unauthenticated, yet every accepted event is forwarded with the server ingest key. `boundContext` only treats Bearer/Basic-prefixed strings and JWT-shaped text as secret values. It therefore preserves raw OpenAI (`sk-...`), AWS (`AKIA...`/`ASIA...`), GitHub, Slack, PEM private-key, URL-credential, and email values when a browser caller puts them beneath an innocuous nested key such as `context.debug` or `context.request.note`. The forwarded payload is then accepted by `/v1/browser` under the server key, whose normalizer only redacts values by key and email-shaped strings, so these secrets can be persisted and sent to the sink through this privileged route. This cannot be proven safe for the public-to-private forwarding boundary. Suggested fix: Make browser relay context redaction cover the same credential value formats as runtime-node redact.ts at every nesting level (private-key blocks, OpenAI/GitHub/AWS/Slack tokens, URL credentials, and email), or replace arbitrary context with a strict allowlist. Add end-to-end tests through sanitizeBrowserPayload and forwarding asserting values under benign nested keys never survive.

Check failure on line 124 in packages/runtime-node/src/relay.ts

View check run for this annotation

Autter.dev / autter/review-gate

🔴 High · Relay leaves recognized credential formats under benign context keys

This path only redacts Bearer/Basic-prefixed strings and JWT-shaped values. A public relay caller can put an OpenAI key (`sk-...`), AWS access key (`AKIA...`), GitHub/Slack token, PEM private-key block, URL credentials, or email under an innocuous nested key; line 124 retains it and line 295 inserts it into the payload that `forward` serializes with the server ingest key. The downstream browser normalizer only examines top-level keys and email-shaped strings, so the nested or non-email value remains in stored telemetry. This is the same browser-controlled secret disclosure route the change is meant to close. Suggested fix: Apply the runtime-node redactor's value patterns recursively in boundContext (or use a strict browser-context allowlist), including private-key blocks, OpenAI/GitHub/AWS/Slack tokens, URL credentials, and email values. Add an end-to-end relay-to-normalizer assertion that a nested benign-key value in each supported credential format cannot reach the forwarded payload.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [ai] Secret values under benign context keys still reach privileged telemetry — Risk: 78/100

boundContext retains every string whose key does not match SECRET_KEY_RE, and scrubSecretValue only recognizes Bearer/Basic prefixes and JWT-shaped text. A public relay caller can therefore submit a credential under an innocuous key, for example {context:{note:"sk-<OpenAI key>"}}, {context:{debug:"AKIA..."}}, or a PEM/private connection string. That string is retained at line 122, incorporated into the sanitized event at line 293, and serialized into the authenticated fetch to the configured ingester at lines 312-320. The downstream browser normalizer only redacts values by key and email-shaped strings, so it also preserves this value. This remains a browser-controlled secret-disclosure path into telemetry despite the new recursive traversal.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: packages/runtime-node/src/relay.ts, packages/otlp-ingester/src/normalize-browser.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Use an explicit allowlist for browser context fields, or extend recursive value redaction to cover the credential formats already protected by `packages/runtime-node/src/redact.ts` (private-key blocks, OpenAI/GitHub/AWS/Slack tokens, URL credentials, and email as applicable). Add an end-to-end relay test proving a secret value under a benign nested key is absent or redacted in the forwarded payload.

Flagged by Autter security & observability checks.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 [ai] Browser context still forwards credential formats under benign keys — Risk: 88/100

This public relay copies every string at a non-matching context key and only calls scrubSecretValue, which recognizes Bearer/Basic prefixes and JWT-shaped values. Consequently an unauthenticated caller can send a raw OpenAI key (sk-...), AWS access key (AKIA...), GitHub/Slack token, private-key block, URL credential, or email under a key such as note; it survives boundContext, is attached to the sanitized event, and is JSON-serialized in the authenticated forward. The server redactor already defines these formats, but this privileged browser-forwarding boundary does not use it, and the ingester's scrubContext only examines top-level keys and email values, so it does not close the nested path.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: packages/runtime-node/src/relay.ts, packages/runtime-node/src/redact.ts, packages/otlp-ingester/src/normalize-browser.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Apply the runtime-node redactor's value-format coverage recursively at the relay boundary (private key blocks, OpenAI/GitHub/AWS/Slack tokens, URL credentials, and email), or replace arbitrary browser context with an explicit allowlist. Add an end-to-end relay test using each representative secret under a benign nested key and assert the serialized forwarded payload contains no raw value.

Flagged by Autter security & observability checks.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 [ai] Public relay still forwards credential-shaped values under benign context keys — Risk: 84/100

The relay route is deliberately unauthenticated, yet every accepted event is forwarded with the server ingest key. boundContext only treats Bearer/Basic-prefixed strings and JWT-shaped text as secret values. It therefore preserves raw OpenAI (sk-...), AWS (AKIA.../ASIA...), GitHub, Slack, PEM private-key, URL-credential, and email values when a browser caller puts them beneath an innocuous nested key such as context.debug or context.request.note. The forwarded payload is then accepted by /v1/browser under the server key, whose normalizer only redacts values by key and email-shaped strings, so these secrets can be persisted and sent to the sink through this privileged route. This cannot be proven safe for the public-to-private forwarding boundary.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: packages/runtime-node/src/relay.ts, packages/otlp-ingester/src/server.ts, packages/otlp-ingester/src/normalize-browser.ts, packages/runtime-node/src/redact.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Make browser relay context redaction cover the same credential value formats as runtime-node redact.ts at every nesting level (private-key blocks, OpenAI/GitHub/AWS/Slack tokens, URL credentials, and email), or replace arbitrary context with a strict allowlist. Add end-to-end tests through sanitizeBrowserPayload and forwarding asserting values under benign nested keys never survive.

Flagged by Autter security & observability checks.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [ai] Relay leaves recognized credential formats under benign context keys — Risk: 78/100

This path only redacts Bearer/Basic-prefixed strings and JWT-shaped values. A public relay caller can put an OpenAI key (sk-...), AWS access key (AKIA...), GitHub/Slack token, PEM private-key block, URL credentials, or email under an innocuous nested key; line 124 retains it and line 295 inserts it into the payload that forward serializes with the server ingest key. The downstream browser normalizer only examines top-level keys and email-shaped strings, so the nested or non-email value remains in stored telemetry. This is the same browser-controlled secret disclosure route the change is meant to close.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: packages/runtime-node/src/relay.ts, packages/otlp-ingester/src/normalize-browser.ts, packages/runtime-node/src/redact.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Apply the runtime-node redactor's value patterns recursively in boundContext (or use a strict browser-context allowlist), including private-key blocks, OpenAI/GitHub/AWS/Slack tokens, URL credentials, and email values. Add an end-to-end relay-to-normalizer assertion that a nested benign-key value in each supported credential format cannot reach the forwarded payload.

Flagged by Autter security & observability checks.

if (t === "number" || t === "boolean") return v;
if (t !== "object") return undefined;
if (depth >= CONTEXT_MAX_DEPTH || nodes >= CONTEXT_MAX_NODES) return undefined;
const obj = v as object;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [heuristic] Generic placeholder identifier in production logic — Risk: 45/100

Identifier obj is a generic placeholder; production logic should name the actual concept. Blast radius — if this AI-generated slop ships it cascades to the downstream usage that depends on this file: functions sanitizeBrowserPayload, forward, firstForwardedFor, IpWindow.allow, respond, createBrowserRelayFetchHandler, createBrowserRelayHandler; scopes @autter/runtime-node; dependent files node:http.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Functions/symbols: sanitizeBrowserPayload, forward, firstForwardedFor, IpWindow.allow, respond, createBrowserRelayFetchHandler, createBrowserRelayHandler
  • Dependent files: node:http
  • Scopes: @autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
In `packages/runtime-node/src/relay.ts` around line 98, this identifier uses a generic placeholder name (`data`/`result`/`handler`/`temp`/`item`) in production logic. Rename it to describe what it actually contains. Placeholder names are a hallmark of AI-generated scaffolding that was never edited for context. Blast radius — if this AI-generated slop ships it cascades to the downstream usage that depends on this file: functions `sanitizeBrowserPayload`, `forward`, `firstForwardedFor`, `IpWindow.allow`, `respond`, `createBrowserRelayFetchHandler`, `createBrowserRelayHandler`; scopes `@autter/runtime-node`; dependent files `node:http`.

Flagged by Autter security & observability checks.

if (seen.has(obj)) return undefined;
seen.add(obj);
// Array.isArray can throw on a revoked Proxy — guard the classification.
let isArr = false;
try {
isArr = Array.isArray(v);
} catch {
return undefined;
}
if (isArr) {
const arr = v as unknown[];
const out: unknown[] = [];
// `length` can be a throwing/hostile trap — guard the read.
let len = 0;
try {
len = arr.length;
} catch {
return out;
}
for (let i = 0; i < len && i < CONTEXT_MAX_ARRAY; i++) {
if (nodes >= CONTEXT_MAX_NODES) break;
nodes++;
let el: unknown;
try {
el = walk(arr[i], depth + 1);
} catch {
el = undefined;
}
if (el !== undefined) out.push(el);
}
return out;
}
let keys: string[];
try {
keys = Object.keys(obj);
} catch {
return undefined;
}
const out: Record<string, unknown> = {};
for (let i = 0; i < keys.length && i < CONTEXT_MAX_KEYS; i++) {
if (nodes >= CONTEXT_MAX_NODES) break;
const key = keys[i];
if (key === undefined) continue;
nodes++;
// Redact secret-bearing keys at any depth. Numeric/boolean values
// can't be credentials and are preserved (e.g. usage counts); any
// other value (string, nested object/array) is dropped entirely.
if (SECRET_KEY_RE.test(key)) {
let raw: unknown;
try {
raw = (obj as Record<string, unknown>)[key];
} catch {
out[key] = REDACTED;
continue;
}
const rt = typeof raw;
out[key] = rt === "number" || rt === "boolean" ? raw : REDACTED;

Check failure on line 185 in packages/runtime-node/src/relay.ts

View check run for this annotation

Autter.dev / autter/review-gate

🔴 High · Sensitive numeric context values are forwarded intact

For every key matched by `SECRET_KEY_RE`, this branch deliberately preserves numeric and boolean values. The public request path accepts arbitrary object-valued `event.context`, so `{context:{otp:123456}}` or `{context:{sessionId:987654}}` reaches this branch, is attached to the sanitized event, and is sent in the privileged ingest request. Numeric OTPs and session identifiers are credentials; preserving all numbers because the key might be a usage counter leaves a direct telemetry disclosure path. Suggested fix: Redact all values at sensitive keys by default. If usage counters must survive, allow only an exact set of canonical usage-counter keys and only finite non-negative numeric values, matching the server redactor's `USAGE_TOKEN_KEYS` behavior. Cover numeric OTP and numeric session ID through the forwarding path.

Check failure on line 185 in packages/runtime-node/src/relay.ts

View check run for this annotation

Autter.dev / autter/review-gate

🔴 High · Public relay preserves numeric credentials under sensitive context keys

The browser relay is public and attaches the private ingest key when it forwards accepted payloads, but the sensitive-key branch explicitly preserves all number and boolean values. An unauthenticated caller can submit a numeric OTP, session ID, recovery code, or passcode in context (for example `{context:{otp:123456}}` or `{context:{sessionId:987654}}`); those keys match `SECRET_KEY_RE`, but the raw credential is serialized to the authenticated downstream ingestion request. The claim that numbers cannot be credentials is false, so this weakens the required boundary that public browser input must not carry secrets into private telemetry. Suggested fix: Always redact sensitive context keys regardless of value type, except for a small exact allowlist of validated non-negative telemetry usage-count keys such as input_tokens, output_tokens, and total_tokens. Add tests that numeric otp, sessionId, passcode, and recovery code are redacted through both relay handlers.

Check failure on line 185 in packages/runtime-node/src/relay.ts

View check run for this annotation

Autter.dev / autter/review-gate

🔴 High · Sensitive numeric context values are explicitly forwarded

For every key matched by SECRET_KEY_RE, this branch preserves any numeric or boolean value rather than redacting it. The public handlers pass object-valued event.context to boundContext, so `{context:{otp:123456}}` and `{context:{sessionId:987654}}` are valid browser payloads and are serialized into authenticated telemetry unchanged. Numeric OTPs and session identifiers are credentials; treating all numbers as safe also contradicts the key-based redaction invariant. The existing test codifies the issue by expecting `sessions` and token-named fields to survive instead of restricting preservation to exact non-secret usage-count keys. Suggested fix: Redact all values under sensitive keys by default. If usage counters must survive, use an exact allowlist equivalent to redact.ts's USAGE_TOKEN_KEYS and require finite non-negative numeric values; add coverage for numeric otp and sessionId values through sanitizeBrowserPayload and a relay handler.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [ai] Numeric credentials are intentionally forwarded from browser context — Risk: 74/100

boundContext explicitly retains every number or boolean whose key matches SECRET_KEY_RE. This is reachable through the public relay: sanitizeBrowserPayload accepts an object-valued event context and places boundContext(e.context) in the forwarded payload, then both handlers call forward with that payload. A browser can therefore submit {context:{otp:123456}} or {context:{sessionId:987654}}; both key names match the redaction expression, but line 183 preserves the raw numeric value and it is serialized into privileged downstream telemetry. Numeric one-time passcodes and numeric session identifiers are credentials, so the stated invariant that context must never carry secrets is not met.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: packages/runtime-node/src/relay.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Do not exempt numeric values solely because they are numeric for sensitive key names. Drop or redact every value under a matched secret-bearing key, or replace the broad regex with an explicit non-secret usage-count allowlist (for example exact `input_tokens`, `output_tokens`, and `total_tokens`) before applying redaction. Add relay sanitizer coverage for numeric `otp`, `sessionId`, and other numeric identifiers.

Flagged by Autter security & observability checks.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [ai] Sensitive numeric context values are forwarded intact — Risk: 78/100

For every key matched by SECRET_KEY_RE, this branch deliberately preserves numeric and boolean values. The public request path accepts arbitrary object-valued event.context, so {context:{otp:123456}} or {context:{sessionId:987654}} reaches this branch, is attached to the sanitized event, and is sent in the privileged ingest request. Numeric OTPs and session identifiers are credentials; preserving all numbers because the key might be a usage counter leaves a direct telemetry disclosure path.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: packages/runtime-node/src/relay.ts, packages/runtime-node/src/redact.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Redact all values at sensitive keys by default. If usage counters must survive, allow only an exact set of canonical usage-counter keys and only finite non-negative numeric values, matching the server redactor's `USAGE_TOKEN_KEYS` behavior. Cover numeric OTP and numeric session ID through the forwarding path.

Flagged by Autter security & observability checks.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [ai] Public relay preserves numeric credentials under sensitive context keys — Risk: 77/100

The browser relay is public and attaches the private ingest key when it forwards accepted payloads, but the sensitive-key branch explicitly preserves all number and boolean values. An unauthenticated caller can submit a numeric OTP, session ID, recovery code, or passcode in context (for example {context:{otp:123456}} or {context:{sessionId:987654}}); those keys match SECRET_KEY_RE, but the raw credential is serialized to the authenticated downstream ingestion request. The claim that numbers cannot be credentials is false, so this weakens the required boundary that public browser input must not carry secrets into private telemetry.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: packages/runtime-node/src/relay.ts, packages/otlp-ingester/src/server.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Always redact sensitive context keys regardless of value type, except for a small exact allowlist of validated non-negative telemetry usage-count keys such as input_tokens, output_tokens, and total_tokens. Add tests that numeric otp, sessionId, passcode, and recovery code are redacted through both relay handlers.

Flagged by Autter security & observability checks.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [ai] Sensitive numeric context values are explicitly forwarded — Risk: 74/100

For every key matched by SECRET_KEY_RE, this branch preserves any numeric or boolean value rather than redacting it. The public handlers pass object-valued event.context to boundContext, so {context:{otp:123456}} and {context:{sessionId:987654}} are valid browser payloads and are serialized into authenticated telemetry unchanged. Numeric OTPs and session identifiers are credentials; treating all numbers as safe also contradicts the key-based redaction invariant. The existing test codifies the issue by expecting sessions and token-named fields to survive instead of restricting preservation to exact non-secret usage-count keys.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: packages/runtime-node/src/relay.ts, packages/runtime-node/src/redact.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Redact all values under sensitive keys by default. If usage counters must survive, use an exact allowlist equivalent to redact.ts's USAGE_TOKEN_KEYS and require finite non-negative numeric values; add coverage for numeric otp and sessionId values through sanitizeBrowserPayload and a relay handler.

Flagged by Autter security & observability checks.

continue;
}
let child: unknown;
try {
child = walk((obj as Record<string, unknown>)[key], depth + 1);
} catch {
child = undefined;
}
if (child !== undefined) out[key] = child;
}
return out;
};
let result: unknown;
try {
result = walk(value, 0);
} catch {
result = undefined;
}
return result && typeof result === "object" ? result : {};
}

/**
* Read a fetch `Request` body while enforcing `maxBody` as it is consumed,
* counting real UTF-8 bytes from the byte stream (so multibyte payloads are
* measured correctly). An oversized body is rejected as soon as the limit is
* crossed — the stream is cancelled instead of being fully buffered first.
*/
async function readBodyBounded(
request: Request,
maxBody: number,
): Promise<{ tooLarge: true } | { tooLarge: false; text: string }> {
const body = request.body;
if (!body) {
// No readable stream to meter — fall back to a buffered read + byte check.
const text = await request.text();
return byteLength(text) > maxBody
? { tooLarge: true }
: { tooLarge: false, text };
}
const reader = body.getReader();
const chunks: Uint8Array[] = [];
let total = 0;
for (;;) {
const { done, value } = await reader.read();
if (done) break;
if (!value) continue;
total += value.byteLength;
if (total > maxBody) {
// Cancel is fire-and-forget: awaiting a cancel() that throws,
// rejects, or never settles would hang the response (or drop it to
// a 400). The oversize decision is already made — detach the cancel
// and return 413 immediately.
void Promise.resolve()
.then(() => reader.cancel())
.catch(() => {});
return { tooLarge: true };
}
chunks.push(value);
}
const buf = new Uint8Array(total);
let offset = 0;
for (const chunk of chunks) {
buf.set(chunk, offset);
offset += chunk.byteLength;
}
return { tooLarge: false, text: new TextDecoder().decode(buf) };
}

// Whitelist sanitiser — anything not listed here is dropped, so a
// compromised or buggy client can't smuggle cookies/DOM/bodies through the
// relay. Returns null when the payload is structurally invalid.
Expand Down Expand Up @@ -103,7 +292,7 @@
? { route: e.route.split("?")[0]!.slice(0, 1000) }
: {}),
...(typeof e.context === "object" && e.context !== null
? { context: e.context }
? { context: boundContext(e.context) }

Check failure on line 295 in packages/runtime-node/src/relay.ts

View check run for this annotation

Autter.dev / autter/review-gate

🔴 High · Browser relay preserves opaque secret values under benign context keys

Although `boundContext` redacts sensitive key names recursively, its value-based redaction only detects Bearer/Basic strings and JWTs. A browser caller can place other credential formats—or opaque passwords or session secrets—under benign keys such as `note` or `debug`; those strings are retained in `context` and forwarded through the authenticated relay. Use a strict context allowlist or extend value redaction to cover the credential formats protected by the server-side redactor. Suggested fix: Change `boundContext` to recursively redact or drop secret-bearing values at every nesting level before they reach `sanitizeBrowserPayload`. Use an explicit allowlist or broaden value-based secret detection so raw tokens, cookies, auth headers, session IDs, and credential-like strings are removed even when they appear under non-secret keys. Add regression tests covering nested objects and arrays. Blast radius — exposing this data cascades to the downstream consumers that depend on this code: functions `sanitizeBrowserPayload`, `forward`, `IpWindow.allow`, `firstForwardedFor`, `respond`, `createBrowserRelayFetchHandler`, `createBrowserRelayHandler`, `RelayOptions`; scopes `@autter/runtime-node`; dependent files `node:http`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Browser relay forwards arbitrary secret-bearing context fields — Risk: 68/100

boundContext bounds context size and structure but does not apply a key/value allowlist or redact sensitive values. Because sanitizeBrowserPayload accepts any object-valued e.context, callers can submit cookies, authorization headers, passwords, tokens, or session identifiers in nested context and the relay forwards them to telemetry. Apply an explicit context schema/allowlist or redact sensitive keys and values before forwarding.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Functions/symbols: sanitizeBrowserPayload, forward, firstForwardedFor, IpWindow.allow, respond, createBrowserRelayFetchHandler, createBrowserRelayHandler
  • Dependent files: node:http
  • Scopes: @autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Tighten the browser relay context sanitizer so secret-bearing values are redacted or dropped before they can reach downstream telemetry/logging. Add explicit key-based redaction for common secret fields like password, token, sessionId, cookie, auth, and set-cookie, and ensure `sanitizeBrowserPayload` never retains raw secret strings in `context`.

Flagged by Autter security & observability checks.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Sensitive data in logs — Risk: 86/100

sanitizeBrowserPayload now forwards e.context through boundContext(...), but this sanitizer still preserves arbitrary user-supplied context values instead of redacting secret-bearing fields. That means downstream relay telemetry from createBrowserRelayHandler / createBrowserRelayFetchHandler can still carry passwords, tokens, session IDs, cookies, or auth headers in nested context objects. Blast radius — if this logging/observability gap is exploited it affects: functions sanitizeBrowserPayload, forward, firstForwardedFor, IpWindow.allow, respond, createBrowserRelayFetchHandler, createBrowserRelayHandler; scopes @autter/runtime-node; dependent files node:http.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Functions/symbols: sanitizeBrowserPayload, forward, firstForwardedFor, IpWindow.allow, respond, createBrowserRelayFetchHandler, createBrowserRelayHandler
  • Dependent files: node:http
  • Scopes: @autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Update `boundContext` in `packages/runtime-node/src/relay.ts` to explicitly drop or redact secret-like keys and values before they reach telemetry. Add a small allowlist or key-based redaction for fields such as `password`, `token`, `sessionId`, `cookie`, `authorization`, and `set-cookie`, and ensure nested objects/arrays cannot reintroduce raw secret strings. Add tests showing secret-bearing context values are removed from `sanitizeBrowserPayload` output.

Flagged by Autter security & observability checks.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Browser relay forwards unredacted client-supplied context — Risk: 41/100

The relay accepts arbitrary object-valued event.context and boundContext only bounds its shape and size. It does not redact or allowlist sensitive keys or values, so a public client can submit cookies, authorization headers, passwords, tokens, or session identifiers that are forwarded into telemetry. Apply explicit context allowlisting or redact/drop sensitive keys and values before forwarding.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Functions/symbols: sanitizeBrowserPayload, forward, firstForwardedFor, IpWindow.allow, respond, createBrowserRelayFetchHandler, createBrowserRelayHandler
  • Dependent files: node:http
  • Scopes: @autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Keep the public relay from ingesting secret-bearing context data. Either drop `context` entirely on this unauthenticated path or apply a strict allowlist/redaction step before persistence so cookies, auth tokens, session IDs, and similar values cannot be written downstream. Blast radius — if this auth regression ships it cascades to the downstream usage that depends on this guard: functions `sanitizeBrowserPayload`, `forward`, `firstForwardedFor`, `IpWindow.allow`, `respond`, `createBrowserRelayFetchHandler`, `createBrowserRelayHandler`; scopes `@autter/runtime-node`; dependent files `node:http`.

Flagged by Autter security & observability checks.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 [ai] Nested credentials in public relay context are forwarded without redaction — Risk: 82/100

The public same-origin relay accepts arbitrary object-valued event.context and the new boundContext copy preserves every key and string value at every permitted nesting level. An unauthenticated browser caller can therefore submit a nested credential such as {context:{request:{authorization:"Bearer victim-token"}}}; the relay attaches the server's private ingest key and forwards it. The receiving /v1/browser handler authenticates that key and stores the payload. Its scrubContext only tests each top-level context key, so it preserves the nested request.authorization value. This creates a credential disclosure path into telemetry storage and the downstream sink under the server key, rather than dropping or masking browser-supplied auth material before privileged forwarding.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: packages/runtime-node/src/relay.ts, packages/otlp-ingester/src/server.ts, packages/otlp-ingester/src/normalize-browser.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Redact or drop sensitive keys recursively in boundContext before returning the copied context. Cover case-insensitive password, token, sessionId, cookie, auth, authorization, bearer, credential, api-key, and set-cookie variants at every nesting level, and add an end-to-end relay-to-normalization test proving raw secret strings never persist or reach sink payloads.

Flagged by Autter security & observability checks.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Sensitive data in logs — Risk: 88/100

sanitizeBrowserPayload now forwards e.context through boundContext, but the new redaction only covers a key-name regex and a couple of string patterns. Nested browser-supplied context can still carry raw secret values under benign keys, and createBrowserRelayFetchHandler / createBrowserRelayHandler will persist them in downstream telemetry via the public sanitizeBrowserPayload path. Blast radius — if this logging/observability gap is exploited it affects: functions sanitizeBrowserPayload, forward, IpWindow.allow, firstForwardedFor, respond, createBrowserRelayFetchHandler, createBrowserRelayHandler, RelayOptions; scopes @autter/runtime-node; dependent files node:http.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Functions/symbols: sanitizeBrowserPayload, forward, IpWindow.allow, firstForwardedFor, respond, createBrowserRelayFetchHandler, createBrowserRelayHandler, RelayOptions
  • Dependent files: node:http
  • Scopes: @autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Strengthen `boundContext` so any browser-supplied `context` value that looks secret-bearing is removed or redacted before it reaches telemetry. Cover nested objects/arrays recursively, and add tests proving raw values for passwords, tokens, session IDs, cookies, authorization headers, bearer/basic auth strings, and JWT-like strings never survive through `sanitizeBrowserPayload` into the relay output.

Flagged by Autter security & observability checks.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 [ai] Browser relay preserves opaque secret values under benign context keys — Risk: 80/100

Although boundContext redacts sensitive key names recursively, its value-based redaction only detects Bearer/Basic strings and JWTs. A browser caller can place other credential formats—or opaque passwords or session secrets—under benign keys such as note or debug; those strings are retained in context and forwarded through the authenticated relay. Use a strict context allowlist or extend value redaction to cover the credential formats protected by the server-side redactor.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Functions/symbols: sanitizeBrowserPayload, forward, IpWindow.allow, firstForwardedFor, respond, createBrowserRelayFetchHandler, createBrowserRelayHandler, RelayOptions
  • Dependent files: node:http
  • Scopes: @autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Change `boundContext` to recursively redact or drop secret-bearing values at every nesting level before they reach `sanitizeBrowserPayload`. Use an explicit allowlist or broaden value-based secret detection so raw tokens, cookies, auth headers, session IDs, and credential-like strings are removed even when they appear under non-secret keys. Add regression tests covering nested objects and arrays. Blast radius — exposing this data cascades to the downstream consumers that depend on this code: functions `sanitizeBrowserPayload`, `forward`, `IpWindow.allow`, `firstForwardedFor`, `respond`, `createBrowserRelayFetchHandler`, `createBrowserRelayHandler`, `RelayOptions`; scopes `@autter/runtime-node`; dependent files `node:http`.

Flagged by Autter security & observability checks.

: {}),
});
}
Expand Down Expand Up @@ -157,23 +346,37 @@
return new Response(null, { status: 405 });
}
if (limiter) {
// Only honor X-Forwarded-For behind an explicitly trusted proxy —
// otherwise a caller could spoof a fresh IP per request to bypass
// the window. With no trusted peer source in a fetch runtime, fall
// back to one shared bucket (a conservative global limit).
const ip =
firstForwardedFor(request.headers.get("x-forwarded-for")) || "unknown";
opts.trustProxy === true

Check failure on line 354 in packages/runtime-node/src/relay.ts

View check run for this annotation

Autter.dev / autter/review-gate

🔴 High · trustProxy makes the public relay limiter key attacker-controlled

The intentionally public relay forwards accepted payloads with the server's private ingest key, so its local limiter is the guard before body parsing and privileged forwarding. Setting the newly introduced `trustProxy: true` keys that guard directly from the first request-supplied `X-Forwarded-For` value. The boolean neither identifies nor verifies the immediate peer as a controlled proxy, nor proves that the proxy overwrites the header. In a common proxy configuration that appends or preserves client headers, an unauthenticated caller can rotate the leading value to obtain unlimited fresh 120/min buckets and drive unbounded parsing and authenticated downstream requests. The downstream per-key ingester limit is shared by all relay clients and is only reached after this public endpoint has already accepted and initiated the work. Suggested fix: Replace the boolean trustProxy option with an adapter/deployment-supplied trusted client-IP resolver, or a trusted-proxy configuration that validates the immediate peer/proxy chain before consulting X-Forwarded-For. Retain socket peer identity (Node) or a conservative shared bucket (fetch) when trusted identity cannot be established, and add a test showing spoofed leading X-Forwarded-For values cannot create new buckets.

Check failure on line 354 in packages/runtime-node/src/relay.ts

View check run for this annotation

Autter.dev / autter/review-gate

🔴 High · trustProxy directly re-enables a requester-spoofable rate-limit key

Setting the new boolean to true makes both relay handlers derive the limiter key from the first X-Forwarded-For value without validating the immediate peer or proxy chain. Any deployment that enables the documented proxy mode behind a proxy which appends or preserves client headers permits an unauthenticated caller to rotate that leading value and acquire unlimited fresh 120/min buckets. The local relay still parses each body and initiates privileged forwards under the shared server ingest key before downstream limits apply, so this does not meet the intended abuse guard. Suggested fix: Do not accept a raw forwarded header based on a boolean. Accept a deployment-supplied trusted client-IP resolver, or verify a configured trusted-proxy boundary/chain before consuming X-Forwarded-For. Cover spoofed leading forwarded values in both fetch and Node handlers when proxy support is enabled.

Check failure on line 354 in packages/runtime-node/src/relay.ts

View check run for this annotation

Autter.dev / autter/review-gate

🔴 High · trustProxy enables request-controlled IP rotation on the public relay

The public relay uses this limiter before parsing and privileged forwarding, but setting `trustProxy: true` makes its bucket key the first value from the request-supplied `X-Forwarded-For` header. The option has no trusted proxy boundary, peer/proxy-chain verification, or adapter-provided client-IP resolver; the comment merely relies on deployment behavior. In a deployment where a proxy forwards or appends rather than overwrites the header, an unauthenticated caller can rotate the leading value to bypass the only relay-side request limit and drive arbitrary authenticated forwards using the shared server ingest key. The downstream server-key limiter is not equivalent because it is reached after this public route has accepted, parsed, and initiated each request. Suggested fix: Replace the boolean with a trusted client-IP resolver supplied by the deployment adapter, or validate the immediate peer/proxy chain before using X-Forwarded-For. Keep the socket address (Node) or a conservative shared bucket (fetch) when trusted client IP cannot be established, and test spoofed leading forwarded values cannot create new buckets.

Check warning on line 354 in packages/runtime-node/src/relay.ts

View check run for this annotation

Autter.dev / autter/review-gate

🟠 Medium · Rate limit fallback is still spoofable in fetch runtime

When `trustProxy` is false, the fetch relay now uses the shared bucket for all requests. That avoids trusting `X-Forwarded-For`, but it also removes per-client abuse control entirely for public deployments, so a single caller can still drive unbounded parsing and forwarding as long as the shared limit allows it. This is a material regression from the previous per-IP throttle and should be paired with a trusted client-IP source or a documented explicit global limit. Blast radius — if this issue ships it degrades the downstream usage that depends on this file: functions `sanitizeBrowserPayload`, `forward`, `IpWindow.allow`, `firstForwardedFor`, `respond`, `createBrowserRelayFetchHandler`, `createBrowserRelayHandler`, `RelayOptions`; scopes `@autter/runtime-node`; dependent files `node:http`. _Flagged by the Release Notes Curator agent._ Suggested fix: Change the fetch relay so it either derives a stable client key from a trusted adapter-provided IP source or, if no trusted source exists, enforces a conservative but explicit shared limiter separate from the per-IP path. Add tests that prove public requests cannot evade throttling by choosing their own `X-Forwarded-For` value and that the fallback still limits abusive traffic.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 [ai] trustProxy makes the public relay limiter key attacker-controlled — Risk: 82/100

The intentionally public relay forwards accepted payloads with the server's private ingest key, so its local limiter is the guard before body parsing and privileged forwarding. Setting the newly introduced trustProxy: true keys that guard directly from the first request-supplied X-Forwarded-For value. The boolean neither identifies nor verifies the immediate peer as a controlled proxy, nor proves that the proxy overwrites the header. In a common proxy configuration that appends or preserves client headers, an unauthenticated caller can rotate the leading value to obtain unlimited fresh 120/min buckets and drive unbounded parsing and authenticated downstream requests. The downstream per-key ingester limit is shared by all relay clients and is only reached after this public endpoint has already accepted and initiated the work.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: packages/runtime-node/src/relay.ts, packages/otlp-ingester/src/server.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Replace the boolean trustProxy option with an adapter/deployment-supplied trusted client-IP resolver, or a trusted-proxy configuration that validates the immediate peer/proxy chain before consulting X-Forwarded-For. Retain socket peer identity (Node) or a conservative shared bucket (fetch) when trusted identity cannot be established, and add a test showing spoofed leading X-Forwarded-For values cannot create new buckets.

Flagged by Autter security & observability checks.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 [ai] trustProxy directly re-enables a requester-spoofable rate-limit key — Risk: 82/100

Setting the new boolean to true makes both relay handlers derive the limiter key from the first X-Forwarded-For value without validating the immediate peer or proxy chain. Any deployment that enables the documented proxy mode behind a proxy which appends or preserves client headers permits an unauthenticated caller to rotate that leading value and acquire unlimited fresh 120/min buckets. The local relay still parses each body and initiates privileged forwards under the shared server ingest key before downstream limits apply, so this does not meet the intended abuse guard.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: packages/runtime-node/src/relay.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Do not accept a raw forwarded header based on a boolean. Accept a deployment-supplied trusted client-IP resolver, or verify a configured trusted-proxy boundary/chain before consuming X-Forwarded-For. Cover spoofed leading forwarded values in both fetch and Node handlers when proxy support is enabled.

Flagged by Autter security & observability checks.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [ai] trustProxy enables request-controlled IP rotation on the public relay — Risk: 70/100

The public relay uses this limiter before parsing and privileged forwarding, but setting trustProxy: true makes its bucket key the first value from the request-supplied X-Forwarded-For header. The option has no trusted proxy boundary, peer/proxy-chain verification, or adapter-provided client-IP resolver; the comment merely relies on deployment behavior. In a deployment where a proxy forwards or appends rather than overwrites the header, an unauthenticated caller can rotate the leading value to bypass the only relay-side request limit and drive arbitrary authenticated forwards using the shared server ingest key. The downstream server-key limiter is not equivalent because it is reached after this public route has accepted, parsed, and initiated each request.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: packages/runtime-node/src/relay.ts, packages/otlp-ingester/src/server.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Replace the boolean with a trusted client-IP resolver supplied by the deployment adapter, or validate the immediate peer/proxy chain before using X-Forwarded-For. Keep the socket address (Node) or a conservative shared bucket (fetch) when trusted client IP cannot be established, and test spoofed leading forwarded values cannot create new buckets.

Flagged by Autter security & observability checks.

? firstForwardedFor(request.headers.get("x-forwarded-for")) ||

Check failure on line 355 in packages/runtime-node/src/relay.ts

View check run for this annotation

Autter.dev / autter/review-gate

🔴 High · trustProxy enables requester-controlled rate-limit keys without proxy verification

Setting the new boolean to true makes the public fetch relay use the first `X-Forwarded-For` value directly. Neither handler establishes that the immediate peer is a configured proxy nor that the proxy overwrote this header, so a client can rotate the leading value and receive a new 120/min bucket for each request. This bypasses the relay's only pre-parse limit and permits repeated bounded parsing and authenticated forwards under the server ingest key. Documentation asking callers to enable it only behind a correct proxy is not an enforcement boundary; the same direct use is present in the Node handler. Suggested fix: Do not derive the limit key directly from a request header. Replace `trustProxy` with an adapter-supplied trusted client-IP resolver, or require trusted-proxy configuration that validates the socket peer/proxy chain before accepting a forwarded address. Add fetch and Node tests showing spoofed leading X-Forwarded-For values cannot produce distinct buckets when proxy verification is absent.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [ai] trustProxy enables requester-controlled rate-limit keys without proxy verification — Risk: 76/100

Setting the new boolean to true makes the public fetch relay use the first X-Forwarded-For value directly. Neither handler establishes that the immediate peer is a configured proxy nor that the proxy overwrote this header, so a client can rotate the leading value and receive a new 120/min bucket for each request. This bypasses the relay's only pre-parse limit and permits repeated bounded parsing and authenticated forwards under the server ingest key. Documentation asking callers to enable it only behind a correct proxy is not an enforcement boundary; the same direct use is present in the Node handler.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: packages/runtime-node/src/relay.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Do not derive the limit key directly from a request header. Replace `trustProxy` with an adapter-supplied trusted client-IP resolver, or require trusted-proxy configuration that validates the socket peer/proxy chain before accepting a forwarded address. Add fetch and Node tests showing spoofed leading X-Forwarded-For values cannot produce distinct buckets when proxy verification is absent.

Flagged by Autter security & observability checks.

"unknown"
: "shared";

Check failure on line 357 in packages/runtime-node/src/relay.ts

View check run for this annotation

Autter.dev / autter/review-gate

🔴 High · Default Next relay now globally rate-limits all browser clients

`createAutterRelayRoute` remains the public Next caller and passes its documented `{ apiKey }` options through unchanged. Since `trustProxy` is absent for that normal integration, the changed fetch handler selects the literal `"shared"` limiter key, so 120 aggregate POSTs in one minute cause every unrelated browser client using that route to receive 429. This removes the prior client-specific limiting behavior without updating the wrapper contract or giving its default caller a trusted client-IP source, creating a production availability regression. Suggested fix: Preserve a client-specific default rate-limit key for the Next adapter without trusting request-controlled X-Forwarded-For. Add an adapter-provided trusted client-IP resolver to RelayOptions and have createAutterRelayRoute supply it where available, or make the shared limiter an explicit opt-in rather than the default. Cover the documented createAutterRelayRoute({ apiKey }) path with two distinct clients and verify they do not consume one another's quota.

Check failure on line 357 in packages/runtime-node/src/relay.ts

View check run for this annotation

Autter.dev / autter/review-gate

🔴 High · Default fetch relay globally rate-limits all browser clients

The documented Next adapter passes RelayOptions through unchanged, so its standard integration leaves `trustProxy` unset. The changed fetch handler then uses the literal `shared` rate-limit key for every caller. Consequently, 120 aggregate requests in a minute from any mix of clients exhaust the sole bucket and cause unrelated legitimate users to receive 429 responses. This is a production availability regression on the deliberately public browser telemetry endpoint rather than the advertised per-IP protection; the adapter exposes no trusted client-IP source and the caller cannot retain a client-specific default key without opting into the spoofable header path. Suggested fix: Add an optional trusted client-IP resolver to RelayOptions and have supported fetch adapters provide a platform-trusted source where available. Otherwise make the shared limiter an explicit opt-in rather than the default, or document and require an external per-client rate-limit boundary. Add coverage through createAutterRelayRoute's default options that validates the chosen client-specific limiter behavior.

Check failure on line 357 in packages/runtime-node/src/relay.ts

View check run for this annotation

Autter.dev / autter/review-gate

🔴 High · Default Next relay collapses all browser clients into one rate-limit bucket

Without trustProxy, the fetch handler unconditionally keys its 120/min limiter as `shared`. The standard Next adapter passes RelayOptions through unchanged, and its documented default example supplies only apiKey, so every normal `createAutterRelayRoute` installation reaches this branch. After any aggregate 120 POSTs in a minute, unrelated clients receive 429 despite the RelayOptions contract documenting a per-IP default limit. This is a caller-visible availability regression introduced by the rate-limit hardening. Suggested fix: Provide a trusted client-IP resolver in RelayOptions and have the Next adapter use a platform-trusted source where available. Otherwise require explicit opt-in to a shared fetch limiter or preserve an appropriate per-client default without reading an untrusted header. Add a default createAutterRelayRoute test showing independent clients do not exhaust one another's quota.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [ai] Default fetch relay globally rate-limits all browser clients — Risk: 76/100

The documented Next adapter passes RelayOptions through unchanged, so its standard integration leaves trustProxy unset. The changed fetch handler then uses the literal shared rate-limit key for every caller. Consequently, 120 aggregate requests in a minute from any mix of clients exhaust the sole bucket and cause unrelated legitimate users to receive 429 responses. This is a production availability regression on the deliberately public browser telemetry endpoint rather than the advertised per-IP protection; the adapter exposes no trusted client-IP source and the caller cannot retain a client-specific default key without opting into the spoofable header path.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: packages/runtime-next/src/server.ts, packages/runtime-node/src/relay.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Add an optional trusted client-IP resolver to RelayOptions and have supported fetch adapters provide a platform-trusted source where available. Otherwise make the shared limiter an explicit opt-in rather than the default, or document and require an external per-client rate-limit boundary. Add coverage through createAutterRelayRoute's default options that validates the chosen client-specific limiter behavior.

Flagged by Autter security & observability checks.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [ai] Default Next relay collapses all browser clients into one rate-limit bucket — Risk: 76/100

Without trustProxy, the fetch handler unconditionally keys its 120/min limiter as shared. The standard Next adapter passes RelayOptions through unchanged, and its documented default example supplies only apiKey, so every normal createAutterRelayRoute installation reaches this branch. After any aggregate 120 POSTs in a minute, unrelated clients receive 429 despite the RelayOptions contract documenting a per-IP default limit. This is a caller-visible availability regression introduced by the rate-limit hardening.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: packages/runtime-node/src/relay.ts, packages/runtime-next/src/server.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Provide a trusted client-IP resolver in RelayOptions and have the Next adapter use a platform-trusted source where available. Otherwise require explicit opt-in to a shared fetch limiter or preserve an appropriate per-client default without reading an untrusted header. Add a default createAutterRelayRoute test showing independent clients do not exhaust one another's quota.

Flagged by Autter security & observability checks.

if (!limiter.allow(ip)) {
return new Response(JSON.stringify({ error: "rate limit exceeded" }), {
status: 429,
});
}
}
const text = await request.text();
if (text.length > maxBody) {
let bounded: { tooLarge: true } | { tooLarge: false; text: string };
try {
bounded = await readBodyBounded(request, maxBody);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [ai] Failed stream cancellation can turn an oversized body into a 400 response — Risk: 35/100

After total > maxBody, await reader.cancel() can reject. The outer catch then returns 400 { error: "invalid json" } even though the size limit was exceeded. Treat cancellation as best-effort (for example, catch its rejection locally) and return { tooLarge: true } so oversized requests consistently receive 413. Consider distinguishing stream-read failures from JSON parse failures as well.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Functions/symbols: sanitizeBrowserPayload, forward, firstForwardedFor, IpWindow.allow, respond, createBrowserRelayFetchHandler, createBrowserRelayHandler
  • Dependent files: node:http
  • Scopes: @autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Differentiate body-read failures from JSON parse failures. Catch `readBodyBounded` errors separately and return the appropriate non-JSON error response (or rethrow into the existing transport error handling), while keeping `JSON.parse` failures mapped to 400 invalid json. Add a test for a request body stream that rejects or errors mid-read. Blast radius — if this defect reaches production it can fail the downstream usage that depends on this file: functions `sanitizeBrowserPayload`, `forward`, `firstForwardedFor`, `IpWindow.allow`, `respond`, `createBrowserRelayFetchHandler`, `createBrowserRelayHandler`; scopes `@autter/runtime-node`; dependent files `node:http`.

Flagged by Autter security & observability checks.

} catch {
return new Response(JSON.stringify({ error: "invalid json" }), {
status: 400,
});
}
if (bounded.tooLarge) {
return new Response(JSON.stringify({ error: "payload too large" }), {
status: 413,
});
}
let raw: unknown;
try {
raw = JSON.parse(text);
raw = JSON.parse(bounded.text);
} catch {
return new Response(JSON.stringify({ error: "invalid json" }), {
status: 400,
Expand Down Expand Up @@ -236,8 +439,12 @@
return;
}
if (limiter) {
// Prefer the real socket peer; only trust X-Forwarded-For when the
// caller has explicitly opted into a trusted-proxy deployment.
const ip =
firstForwardedFor(req.headers["x-forwarded-for"]) ||
(opts.trustProxy === true

Check warning on line 445 in packages/runtime-node/src/relay.ts

View check run for this annotation

Autter.dev / autter/review-gate

🟠 Medium · `trustProxy` accepts an unvalidated forwarded IP for relay rate limiting

When `trustProxy === true`, the fetch and Node relay handlers key their local limiter from the first request-provided `X-Forwarded-For` value. The boolean option does not establish that the immediate peer is a trusted proxy or that the proxy overwrites this header. Deployments with a direct route or a proxy that preserves/appends client values allow callers to rotate the leading value and evade the per-IP limit. Use an adapter-supplied trusted client-IP resolver or validate the peer/proxy chain before honoring forwarded headers; otherwise retain the socket peer address (Node) or the shared fetch bucket. Suggested fix: Keep the default rate-limit key tied to a trusted client-IP source only. If proxy trust is needed, require an adapter-provided trusted-IP resolver or validated proxy chain instead of a boolean flag that reads `X-Forwarded-For` directly; otherwise fall back to the socket peer or a single shared bucket. Blast radius — if this auth regression ships it cascades to the downstream usage that depends on this guard: functions `sanitizeBrowserPayload`, `forward`, `IpWindow.allow`, `firstForwardedFor`, `respond`, `createBrowserRelayFetchHandler`, `createBrowserRelayHandler`, `RelayOptions`; scopes `@autter/runtime-node`; dependent files `node:http`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 trustProxy accepts an unvalidated forwarded IP for relay rate limiting — Risk: 68/100

When trustProxy === true, the fetch and Node relay handlers key their local limiter from the first request-provided X-Forwarded-For value. The boolean option does not establish that the immediate peer is a trusted proxy or that the proxy overwrites this header. Deployments with a direct route or a proxy that preserves/appends client values allow callers to rotate the leading value and evade the per-IP limit. Use an adapter-supplied trusted client-IP resolver or validate the peer/proxy chain before honoring forwarded headers; otherwise retain the socket peer address (Node) or the shared fetch bucket.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Functions/symbols: sanitizeBrowserPayload, forward, IpWindow.allow, firstForwardedFor, respond, createBrowserRelayFetchHandler, createBrowserRelayHandler, RelayOptions
  • Dependent files: node:http
  • Scopes: @autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Keep the default rate-limit key tied to a trusted client-IP source only. If proxy trust is needed, require an adapter-provided trusted-IP resolver or validated proxy chain instead of a boolean flag that reads `X-Forwarded-For` directly; otherwise fall back to the socket peer or a single shared bucket. Blast radius — if this auth regression ships it cascades to the downstream usage that depends on this guard: functions `sanitizeBrowserPayload`, `forward`, `IpWindow.allow`, `firstForwardedFor`, `respond`, `createBrowserRelayFetchHandler`, `createBrowserRelayHandler`, `RelayOptions`; scopes `@autter/runtime-node`; dependent files `node:http`.

Flagged by Autter security & observability checks.

? firstForwardedFor(req.headers["x-forwarded-for"])
: "") ||
req.socket?.remoteAddress ||
"unknown";
if (!limiter.allow(ip)) {
Expand Down
Loading
Loading