Skip to content

fix(runtime-node): bound browser relay context and count body size in bytes - #20

Merged
sagnik11 merged 6 commits into
Autter-dev:mainfrom
rajeshaipython-stack:fix/relay-context-bounds
Oct 8, 2026
Merged

sagnik11 merged 6 commits into
Autter-dev:mainfrom
rajeshaipython-stack:fix/relay-context-bounds

Conversation

@rajeshaipython-stack

@rajeshaipython-stack rajeshaipython-stack commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The browser relay's payload sanitiser (sanitizeBrowserPayload) length-caps every event field it forwards — message, stack, name, filename, route — but context was forwarded verbatim, with no bound. That quietly contradicts the sanitiser's own contract:

Whitelist sanitiser — anything not listed here is dropped, so a compromised or buggy client can't smuggle cookies/DOM/bodies through the relay.

context is exactly the field a client can use to push an unbounded, arbitrarily-nested object through the relay. This PR closes that gap, fixes a related size-limit inaccuracy, and adds the first test coverage for relay.ts.

Changes

1. Bound context (sanitizeBrowserPayload)
context now passes through boundContext, a bounded, cycle-safe deep copy that mirrors the caps already applied to the other fields: a depth limit, a total-node budget, a per-string length cap, and array-length / key-count caps. Cycles are detected (the repeated reference is dropped) so a self-referential object can't loop, and reads that throw (hostile getters) or non-serialisable values fail open — that value is dropped and sanitising continues, so it never throws on the capture path.

- ? { context: e.context }
+ ? { context: boundContext(e.context) }

2. Count body size in bytes (createBrowserRelayFetchHandler)
The fetch-handler size guard compared text.length (UTF-16 code units) against maxBodyBytes, so a multi-byte payload (non-Latin text, emoji) could carry several times the configured byte budget. It now measures UTF-8 bytes, matching the option's name and the Node handler (which already counts real bytes). TextEncoder keeps it portable to the edge runtimes this handler targets (no Buffer).

- if (text.length > maxBody) {
+ if (byteLength(text) > maxBody) {

3. First tests for relay.ts
relay.ts had no test file. Added test/relay.test.mjs covering context depth / string / array bounding, cyclic context, non-serialisable values, and the byte-vs-code-unit size limit.

Testing

npm run build succeeds; the new relay.test.mjs and the existing redact / lifecycle suites pass.

Notes

  • The traversal goes deep but stays bounded — enough for realistic nested context, capped so a pathologically deep or wide payload can't exhaust CPU/stack.
  • Limits are conservative constants; happy to make them configurable or tune them if you'd prefer.

View code changes stack in Autter

Summary

Summary generated by Autter.

Why this change

The browser relay accepted arbitrary client-supplied context objects, so a hostile or oversized payload could smuggle unbounded data (or crash the sanitizer via cycles or hostile Proxies) through the public route. The body size limit also counted UTF-16 code units rather than bytes, so multibyte payloads could slip past maxBodyBytes.

What changes for users

Browser events forwarded by the relay now carry a bounded, cycle-safe copy of context, with limits on depth, node count, string length, array length, and key count. Secret-looking keys and secret-shaped values in that context are redacted. Request body limits in the fetch handler are enforced in UTF-8 bytes, and oversized bodies return 413.

Implementation and review notes

  • New boundContext in relay.ts does a bounded deep copy: max depth 6, 256 nodes, strings capped at 4000 chars, arrays and key counts capped at 100, with a WeakSet for cycle detection. Anything over a limit, cyclic, or non-serialisable is dropped.
  • It is meant to fail open: Array.isArray, length reads, key enumeration and element reads are wrapped in try/catch, so revoked or hostile Proxies drop that value instead of throwing into sanitizeBrowserPayload or the handlers. Reviewers should confirm every Proxy-observable operation is covered.
  • The tests cover key-based redaction (top-level and nested password, token, authorization, cookie) and value-based scrubbing of Bearer and JWT-shaped strings. Numeric usage counts under token/session keys are preserved. Prior review feedback flags that value redaction does not yet cover other credential formats (e.g. sk-, AKIA, PEM blocks, URL credentials) under benign keys, so check that gap.
  • New exported byteLength uses TextEncoder. readBodyBounded handles the fetch handler's body reading, and a test checks that a 413 is still returned when the stream's cancel() never settles.
  • sanitizeBrowserPayload and createBrowserRelayFetchHandler were changed; relay.test.mjs adds about 214 lines of tests, including multibyte (Tamil) body sizing and oversized-body rejection.

Changes

  • packages/runtime-node/src/relay.ts (+163/-7)
    • Adds the limit constants CONTEXT_MAX_DEPTH (6), CONTEXT_MAX_NODES (256), CONTEXT_MAX_STRING (4000), CONTEXT_MAX_ARRAY (100) and CONTEXT_MAX_KEYS (100).
    • Adds exported byteLength(text), which returns the UTF-8 byte length via TextEncoder and works on edge runtimes.
    • Adds exported boundContext(value):
      • Walks untrusted context with a WeakSet for cycle detection and a shared node budget.
      • Drops functions, symbols, and other non-serialisable values.
      • Guards Array.isArray, length, key enumeration, and element reads, so revoked or throwing Proxies drop only that value.
      • Redacts secret-named keys at every nesting level and scrubs Bearer/Basic- and JWT-shaped string values.
      • Preserves numeric usage counts under token/session-style keys.
    • sanitizeBrowserPayload now routes e.context through boundContext instead of passing it through raw (small net change in the existing hunk, +2/-4).
    • Adds readBodyBounded and uses it in createBrowserRelayFetchHandler so maxBodyBytes is enforced in UTF-8 bytes while the stream is read. An oversized body returns 413 even if the stream's cancel() never settles.
  • packages/runtime-node/test/relay.test.mjs (+214/-1)
    • Covers deep nesting, string/array caps, cycles, non-serialisable values, a revoked Proxy, a hostile length trap, and a hostile element getter.
    • Covers fetch-handler 413 behaviour for multibyte (Tamil) bodies, oversized bodies, and a never-settling cancel().
    • Covers key- and value-based redaction and preservation of numeric usage counts.
  • No changes to the ingester, ClickHouse schema, migrations, fingerprinting, or the wire contract. The /v1/browser version 1 payload shape is unchanged; only context content is bounded and redacted before forwarding.

Acceptance Criteria

  • sanitizeBrowserPayload never throws for any context value, including cyclic graphs, revoked Proxies, and Proxies with throwing length, get, or enumeration traps. The offending value is dropped and the rest of the event is kept.
  • Forwarded context is bounded: depth ≤ 6, ≤ 256 nodes, strings ≤ 4000 chars, arrays ≤ 100 elements, objects ≤ 100 keys.
  • Secret-named keys (password, token, authorization, cookie, etc.) are [redacted] at top level and when nested. Bearer/Basic- and JWT-shaped values under benign keys are [redacted]. Plain strings and numeric usage counts (input_tokens, sessions, …) are unchanged.
  • createBrowserRelayFetchHandler returns 413 when the body exceeds maxBodyBytes measured in UTF-8 bytes. A 6-character Tamil string (18 bytes) is rejected at maxBodyBytes: 10.
  • Oversized bodies return 413 without hanging, even when the request stream's cancel() never settles.
  • Valid payloads still return 202, with unchanged behaviour for rate limiting, event-type validation, and the private key staying server-side.
  • npm run build and npm test -w @autter/runtime-node pass.
  • Known follow-up: value-based redaction does not yet cover other credential formats (sk-…, AKIA…, GitHub/Slack tokens, PEM blocks, URL credentials, email) under benign keys. This PR does not close that gap.

Test Plan

  • Run npm run build at the repo root and confirm it passes.
  • Run npm test -w @autter/runtime-node and confirm all relay.test.mjs cases pass, including the Proxy, cycle, multibyte, and never-settling cancel() cases.
  • Start an app that mounts createBrowserRelayFetchHandler with maxBodyBytes: 10. POST a body of six Tamil characters ("அ".repeat(6), 6 code units, 18 bytes) and confirm the response is 413.
  • POST a ~500-byte body with maxBodyBytes: 100 and confirm 413. POST a normal browser payload under the limit and confirm 202.
  • POST an event whose context is nested 30+ levels deep, one with a 9000-char string, and one with a 500-element array. Confirm the forwarded payload (inspect via onError/a local ingester log) is truncated to the documented limits.
  • Call sanitizeBrowserPayload in a Node REPL with a cyclic context, a revoked Proxy.revocable object, and an array Proxy whose length trap throws. Confirm none throw and the event is still returned.
  • Send context: { request: { headers: { cookie: "sid=1", authorization: "Bearer x" } }, note: "Bearer supersecrettoken12345", keep: "ok" } through the relay to a local ingester (docker compose on port 4318). Read the row back from ClickHouse and confirm the secret values are [redacted] and keep is intact.
  • Send context: { input_tokens: 500, sessions: 3 } and confirm the numeric values are preserved in the stored row.
  • Optional gap check: send context: { note: "sk-…" } or { debug: "AKIA…" } and note whether the value survives (expected today, per the known follow-up).

Rollback Plan

  • Revert commit 763e04e (merge or squash revert) on main. The change is confined to packages/runtime-node/src/relay.ts and test/relay.test.mjs, so the revert is clean and needs no data or schema action.
  • No ClickHouse migration, schemaStatements() change, or wire-format change is involved, so ingester deployments (AWS ECS, single-server) need no rollback.
  • If a release was already published, ship a patch release of @autter/runtime-node (independent semver) from the revert. Dependents such as @autter/runtime-next, which re-exports createBrowserRelayFetchHandler, can pin the previous @autter/runtime-node version in the meantime.
  • If the new handling misbehaves in production, a mitigation that needs no code change is to raise maxBodyBytes (the default stays 64 KB) or temporarily disable the relay route.
  • After a rollback the relay returns to passing context through unbounded and counting body size in UTF-16 code units. Keep perIpRateLimit enabled and consider a WAF body-size limit in front of the route until a fix lands.

Related Issues

No linked issue was identified in the branch name, commit metadata, or diff context.

Written for commit 763e04e. Summary will update on new commits.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 Autter review in progress — running security, correctness & dependency checks on this PR. Follow live step-by-step progress on the autter/review-gate check in the merge box. Merge is blocked until the gate completes; Autter approves automatically when the review comes back clean, and releases this hold with a neutral review when it finds non-blocking issues.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter posted 1 finding(s) as review threads below (🟠 1). Each carries a copy-paste AI fix prompt.

Comment thread packages/runtime-node/src/relay.ts Outdated
}
const text = await request.text();
if (text.length > maxBody) {
if (byteLength(text) > maxBody) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 [ai] Fetch relay enforces the body limit only after fully buffering the request — Risk: 75/100

await request.text() buffers and decodes the entire request before byteLength(text) > maxBody is checked. The byte calculation is correct, but it cannot protect the fetch-style relay from memory/CPU spent on oversized requests. Reject a known oversized content-length before reading when available, and read request.body incrementally with a byte budget (cancelling once exceeded) so the configured limit is enforced during consumption.

⚠ 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)
Enforce the body limit while streaming the request instead of after `request.text`. Read the request body incrementally, accumulate bytes as chunks arrive, and abort with 413 as soon as the byte budget is exceeded.

Flagged by Autter security & observability checks.

@autter-dev

autter-dev Bot commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

🚦 Pre-merge checks · ⚠️ 7 warning, ✅ 161 passed

Needs attention

Check Status Explanation
Missing linked tracker issue ⚠️ Warning 1 potential issue(s) detected (max risk 45/100): packages/runtime-node/src/relay.ts:95.
Missing security-team review on sensitive path ⚠️ Warning 1 potential issue(s) detected (max risk 75/100): packages/runtime-node/src/relay.ts:297.
Comment contradicts or fabricates code behaviour ⚠️ Warning 1 potential issue(s) detected (max risk 35/100): packages/runtime-node/src/relay.ts:146.
Code correctness issue ⚠️ Warning 1 finding(s) on changed lines.
Dead export (no callers) ⚠️ Warning 1 finding(s) on changed lines.
Complexity Guard ⚠️ Warning 1 finding(s) on changed lines.
Release Notes Curator ⚠️ Warning 1 finding(s) on changed lines.
✅ Passed checks (161)
Check Status Explanation
Too many files changed ✅ Passed Changed 2 file(s), within the limit of 50.
Too many lines changed ✅ Passed Changed 385 line(s), within the limit of 1000.
Too many unrelated chapters ✅ Passed 3 chapter(s) detected, within the limit of 6.
Generated files hiding real changes ✅ Passed Generated-file volume (0 lines) does not obscure the 385 hand-written line(s).
Missing PR context ✅ Passed PR context looks sufficient.
Mixed concerns (refactor + behavior change) ✅ Passed The PR is a hardening fix with tests. Bounding context and counting body size in bytes are both behavior changes in the same area. No renames, moves, or pure refactors are mixed in; new helpers such as boundContext, byteLength and readBodyBounded exist only to implement the fixes.
Migration + app logic + UI combined in one PR ✅ Passed No database migrations or UI changes. Only relay.ts in the runtime package and its test file are touched.
Sensitive data in logs ✅ Passed No sensitive data in logs issues detected.
Log injection ✅ Passed No log injection issues detected.
Missing audit logging ✅ Passed No missing audit logging issues detected.
Removed observability ✅ Passed No removed observability issues detected.
Silent exception swallowing ✅ Passed No silent exception swallowing issues remaining after verification.
Unhandled promise rejection ✅ Passed No unhandled promise rejection issues detected.
Circuit breaker not detected ✅ Passed No circuit breaker not detected issues detected.
Stack trace leakage ✅ Passed No stack trace leakage issues detected.
Multi-write without detected transaction ✅ Passed No multi-write without detected transaction issues detected.
Possible TOCTOU in critical path ✅ Passed No possible toctou in critical path issues detected.
Idempotency key not detected ✅ Passed No idempotency key not detected issues detected.
Possible non-atomic read-modify-write ✅ Passed No possible non-atomic read-modify-write issues detected.
Optimistic locking not detected ✅ Passed No optimistic locking not detected issues detected.
Rate limiting not detected ✅ Passed No rate limiting not detected issues detected.
Rate limiting removed ✅ Passed No rate limiting removed issues detected.
Batch size limit not detected ✅ Passed No batch size limit not detected issues remaining after verification.
Pagination not detected ✅ Passed No pagination not detected issues detected.
Prompt injection risk ✅ Passed No LLM/AI-integration code touched by this diff.
LLM output used in a dangerous sink ✅ Passed No LLM/AI-integration code touched by this diff.
Sensitive data in prompt or system-prompt leakage ✅ Passed No LLM/AI-integration code touched by this diff.
Over-privileged LLM tool / excessive agency ✅ Passed No LLM/AI-integration code touched by this diff.
Missing validation on an LLM-driven decision ✅ Passed No LLM/AI-integration code touched by this diff.
Unbounded LLM usage (denial-of-wallet) ✅ Passed No LLM/AI-integration code touched by this diff.
Table exposed without row-level security ✅ Passed No row-level-security-related code touched by this diff.
Over-broad row-level security policy ✅ Passed No row-level-security-related code touched by this diff.
Code path that bypasses row-level security ✅ Passed No row-level-security-related code touched by this diff.
Privileged database credential reachable from the client ✅ Passed No row-level-security-related code touched by this diff.
Privileged query without row-level scoping ✅ Passed No row-level-security-related code touched by this diff.
Row-level security enabled with no policy ✅ Passed No row-level-security-related code touched by this diff.
Template-default gradient styling ✅ Passed No added frontend pages or design-slop markers in this diff.
Interchangeable AI marketing copy ✅ Passed No added frontend pages or design-slop markers in this diff.
Placeholder content shipped to users ✅ Passed No added frontend pages or design-slop markers in this diff.
Emoji standing in for an icon system ✅ Passed No added frontend pages or design-slop markers in this diff.
Call-to-action that goes nowhere ✅ Passed No added frontend pages or design-slop markers in this diff.
Templated page composition ✅ Passed No added frontend pages or design-slop markers in this diff.
Color outside the brand palette ✅ Passed No added frontend markup or brand-relevant style changes in this diff.
Hardcoded style bypassing design tokens ✅ Passed No added frontend markup or brand-relevant style changes in this diff.
Typography outside the brand type system ✅ Passed No added frontend markup or brand-relevant style changes in this diff.
One-off UI instead of the shared component ✅ Passed No added frontend markup or brand-relevant style changes in this diff.
Copy that does not match brand voice ✅ Passed No added frontend markup or brand-relevant style changes in this diff.
Screen does not match the rest of the product ✅ Passed No added frontend markup or brand-relevant style changes in this diff.
Merge-blocking marker left in the change ✅ Passed No pending-work markers added by this diff.
Known-defect marker shipped in code ✅ Passed No pending-work markers added by this diff.
Untracked TODO without an issue reference ✅ Passed No pending-work markers added by this diff.
Test disabled or left pending ✅ Passed No pending-work markers added by this diff.
PII in logs ✅ Passed No pii in logs issues detected.
PII or internals leaked in error response ✅ Passed No pii or internals leaked in error response issues detected.
PII stored without application-level encryption ✅ Passed No pii stored without application-level encryption issues detected.
User data stored without retention controls ✅ Passed No user data stored without retention controls issues detected.
PII sent to external / cross-border destination ✅ Passed No pii sent to external / cross-border destination issues detected.
Lockfile resolution / integrity tampered ✅ Passed No lockfile resolution / integrity tampered issues detected.
Dependency runs install-time lifecycle script ✅ Passed No dependency runs install-time lifecycle script issues detected.
Possible dependency-confusion attack ✅ Passed No possible dependency-confusion attack issues detected.
Lockfile resolves a dependency the manifest does not declare ✅ Passed No lockfile resolves a dependency the manifest does not declare issues detected.
Checked-in build artefact modified without source change ✅ Passed No checked-in build artefact modified without source change issues detected.
Dockerfile build-step is insecure ✅ Passed No dockerfile build-step is insecure issues detected.
External artefact pulled in without integrity pinning ✅ Passed No external artefact pulled in without integrity pinning issues detected.
Changed export, importer not updated ✅ Passed No changed export with an un-updated importer detected.
Missing CODEOWNERS reviewer approval ✅ Passed No missing codeowners reviewer approval issues detected.
Source changes without matching tests ✅ Passed No source changes without matching tests issues detected.
Migration missing rollback / down step ✅ Passed No migration missing rollback / down step issues detected.
Frontend importing database client directly ✅ Passed No frontend importing database client directly issues detected.
Route handler bypassing service layer ✅ Passed No route handler bypassing service layer issues detected.
Backend service importing UI module ✅ Passed No backend service importing ui module issues detected.
Cross-context internals import ✅ Passed No cross-context internals import issues detected.
Workspace package rule violation ✅ Passed No workspace package rule violation issues detected.
Inconsistent logging pattern ✅ Passed No inconsistent logging pattern issues detected.
Inconsistent error handling ✅ Passed No inconsistent error handling issues detected.
Endpoint missing input validation ✅ Passed No endpoint missing input validation issues detected.
Multi-write without transaction wrapper ✅ Passed No multi-write without transaction wrapper issues detected.
New feature shipped without feature flag ✅ Passed No new feature shipped without feature flag issues detected.
Module placed in the wrong workspace package ✅ Passed No module placed in the wrong workspace package issues detected.
Hallucinated import (package not installed) ✅ Passed No hallucinated import (package not installed) issues detected.
Nonexistent package (not found in registry) ✅ Passed No nonexistent package (not found in registry) issues detected.
Call to function that does not exist ✅ Passed No call to function that does not exist issues detected.
Generic placeholder identifier in production logic ✅ Passed No generic placeholder identifier in production logic issues remaining after verification.
Repetitive boilerplate (duplicated block) ✅ Passed No repetitive boilerplate (duplicated block) issues detected.
Overbroad try/catch swallowing all exceptions ✅ Passed No overbroad try/catch swallowing all exceptions issues detected.
TODO / FIXME on critical path ✅ Passed No todo / fixme on critical path issues detected.
Abstraction defined but never used ✅ Passed No abstraction defined but never used issues detected.
Code style differs from rest of codebase ✅ Passed No code style differs from rest of codebase issues detected.
Established pattern ignored ✅ Passed No established pattern ignored issues detected.
Unhandled edge case (null / empty / zero / boundary) ✅ Passed No unhandled edge case (null / empty / zero / boundary) issues detected.
Insecure training-data / doc-example pattern ✅ Passed No insecure training-data / doc-example pattern issues detected.
Dead code (defined but never referenced) ✅ Passed No dead code (defined but never referenced) issues detected.
Deprecated API call ✅ Passed No deprecated api call issues detected.
API pattern from wrong library version ✅ Passed No api pattern from wrong library version issues detected.
API endpoint removed ✅ Passed No api endpoint removed issues detected.
HTTP method changed (GET ↔ POST etc.) ✅ Passed No http method changed (get ↔ post etc.) issues detected.
New required field added to request ✅ Passed No new required field added to request issues detected.
Field removed from response schema ✅ Passed No field removed from response schema issues detected.
Response field type changed ✅ Passed No response field type changed issues detected.
HTTP status code changed ✅ Passed No http status code changed issues detected.
Auth requirement added / removed / changed ✅ Passed No auth requirement added / removed / changed issues detected.
Error response shape changed ✅ Passed No error response shape changed issues detected.
Pagination behaviour changed ✅ Passed No pagination behaviour changed issues detected.
Outbound webhook payload schema changed ✅ Passed No outbound webhook payload schema changed issues detected.
GraphQL field removed without deprecation ✅ Passed No graphql field removed without deprecation issues detected.
GraphQL enum value removed ✅ Passed No graphql enum value removed issues detected.
Vendor API consumer ✅ Passed This PR does not touch call sites of an open vendor API change.
SQL injection ✅ Passed No sql injection issues detected.
Cross-site scripting (XSS) ✅ Passed No cross-site scripting (xss) issues detected.
Path traversal ✅ Passed No path traversal issues detected.
Command injection ✅ Passed No command injection issues detected.
Insecure deserialization ✅ Passed No insecure deserialization issues detected.
Weak cryptography ✅ Passed No weak cryptography issues detected.
Hardcoded secret ✅ Passed No hardcoded secret issues detected.
Insecure randomness for security material ✅ Passed No insecure randomness for security material issues detected.
Unsafe file upload ✅ Passed No unsafe file upload issues detected.
Missing input validation ✅ Passed No missing input validation issues detected.
Unsafe CORS configuration ✅ Passed No unsafe cors configuration issues detected.
Unsafe / open redirect ✅ Passed No unsafe / open redirect issues detected.
Missing CSRF protection ✅ Passed No missing csrf protection issues detected.
Unsafe cookie / session settings ✅ Passed No unsafe cookie / session settings issues detected.
Sensitive data exposure ✅ Passed No sensitive data exposure issues detected.
Invisible Unicode / Trojan Source characters ✅ Passed No invisible unicode / trojan source characters issues detected.
API key in source ✅ Passed No api key in source issues remaining after verification.
Access token in source ✅ Passed No access token in source detected.
Private key in source ✅ Passed No private key in source detected.
Database connection URL with embedded credentials ✅ Passed No database connection url with embedded credentials detected.
Cloud credential in source ✅ Passed No cloud credential in source detected.
Webhook signing secret in source ✅ Passed No webhook signing secret in source detected.
OAuth client secret in source ✅ Passed No oauth client secret in source detected.
JWT signing secret in source ✅ Passed No jwt signing secret in source detected.
Hardcoded password ✅ Passed No hardcoded password detected.
Auth middleware removed from route ✅ Passed No auth middleware removed from route issues detected.
Route protection changed (protected → public) ✅ Passed No route protection changed (protected → public) issues detected.
Permission / RBAC check removed ✅ Passed No permission / rbac check removed issues detected.
Required role weakened ✅ Passed No required role weakened issues detected.
Admin-only route exposed to lower privilege ✅ Passed No admin-only route exposed to lower privilege issues detected.
Token validation skipped in middleware chain ✅ Passed No token validation skipped in middleware chain issues detected.
JWT verification weakened or changed ✅ Passed No jwt verification weakened or changed issues detected.
Session expiration / TTL changed ✅ Passed No session expiration / ttl changed issues detected.
Password reset flow changed ✅ Passed No password reset flow changed issues detected.
OAuth callback / redirect handling changed ✅ Passed No oauth callback / redirect handling changed issues detected.
Webhook endpoint missing signature verification ✅ Passed No webhook endpoint missing signature verification issues detected.
Public route touches private/PII data ✅ Passed No public route touches private/pii data issues detected.
Frontend performance issue ✅ Passed No additional explanation was reported.
Frontend security issue ✅ Passed No additional explanation was reported.
Frontend correctness issue ✅ Passed No additional explanation was reported.
Accessibility issue ✅ Passed No additional explanation was reported.
Frontend maintainability issue ✅ Passed No additional explanation was reported.
Runtime error risk ✅ Passed No runtime error risk issues remaining after verification.
Resource leak risk ✅ Passed No additional explanation was reported.
Data integrity risk ✅ Passed No additional explanation was reported.
Maintainability issue ✅ Passed No additional explanation was reported.
Co-change coupling ✅ Passed No additional explanation was reported.
Redundant alias / duplicate import ✅ Passed No additional explanation was reported.
Redundant type construct ✅ Passed No additional explanation was reported.
Unnecessary type assertion ✅ Passed No additional explanation was reported.
Module smell ✅ Passed No additional explanation was reported.
Code duplication / DRY violation ✅ Passed No additional explanation was reported.
Intent vs. implementation ✅ Passed Yes — the diff bounds context with a cycle-safe, fail-open copier and meters body size in real UTF-8 bytes, matching the PR's stated goals; test coverage was omitted from review. Requirements: 5 implemented. (Intent from product context, PR description.)
Bundle Size Monitor ✅ Passed No additional explanation was reported.

This comment is updated automatically whenever Autter reviews a new PR revision.

@autter-dev

autter-dev Bot commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor
💡 2 suggestion(s) — conventions, hardening and hygiene, not defects

Inline comments are reserved for concrete defects. These are things worth knowing that the diff does not prove wrong — a missing hardening layer, a convention the repo usually follows, a file that historically changes alongside one you touched. Skim, adopt what fits, ignore the rest.

  • 🟡 Context walker has high cyclomatic complexity (risk 15/100) — packages/runtime-node/src/relay.ts:121
  • 🟡 Throwing length trap yields [] instead of dropping the value as documented (risk 15/100) — packages/runtime-node/src/relay.ts:146
⚠️ 1 unconfirmed finding(s) — flagged by a detector but not proven by Autter's verification pass

Inline comments are reserved for findings that survived verification. These are plausible but could not be confirmed from the available context, so they are listed here as FYIs instead — review the ones that look real to you.

  • 🟡 Dead export (no callers) (risk 15/100) — packages/runtime-node/src/relay.ts:108 — byteLength and boundContext are exported, and the provided windows show only internal callers (readBodyBounded and relayContext). However, the package index and the test file relay.test.mjs (which has changed symbols in this PR) are not shown. The exports may be intentional for tests or re-exported publicly, so a dead export cannot be confirmed. Impact is minor hygiene at most.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter completed PR review for #20: 2 finding(s) remain below the merge-blocking bar, so this review stays neutral rather than approving. (Also detected: 30 finding(s) dismissed as likely false positives by verification.) See the findings below; the task checklist follows as the review's final comment.

@autter-dev

autter-dev Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

🤖 Release Notes Curator

Impact: patch — fixes relay input handling and request-size accounting without changing the public API.

Changelog entry: Fixed browser relay validation to safely bound context data and enforce request body limits using UTF-8 byte size.

Custom agent · runs after review · configured in Autter

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter posted 3 finding(s) as review threads below (🔴 1 · 🟠 2). Each carries a copy-paste AI fix prompt.

Comment thread packages/runtime-node/src/relay.ts Outdated
const obj = v as object;
if (seen.has(obj)) return undefined;
seen.add(obj);
if (Array.isArray(v)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 [ai] Guard proxy classification and array length before relay sanitization — Risk: 80/100

boundContext promises never to throw, but both Array.isArray(v) and the subsequent v.length access execute Proxy traps outside the existing catches. A revoked Proxy (or an array Proxy with a throwing get trap for length) supplied as event.context therefore throws from the newly added sanitizer. createBrowserRelayFetchHandler invokes sanitizeBrowserPayload without catching that error, so its public Next.js/App Router handler rejects instead of returning a response; the parsed-body Node path likewise calls it outside a catch. This changes the callers’ valid-payload contract from accepting a context object to allowing a hostile context to abort the request, so it is not safe to ship.

⚠ 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)
Make the entire `boundContext` walk non-throwing: guard `Array.isArray`, array length inspection, `WeakSet` operations, and all other proxy-observable operations. On an inspection failure, drop that context value. Add cases for a revoked Proxy and an array Proxy whose `length` getter/trap throws through both fetch and parsed-body relay handlers.

Flagged by Autter security & observability checks.

Comment thread packages/runtime-node/src/relay.ts Outdated
const obj = v as object;
if (seen.has(obj)) return undefined;
seen.add(obj);
if (Array.isArray(v)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 [ai] Guard array classification and length reads in context sanitization — Risk: 78/100

boundContext promises never to throw, but Array.isArray(v) and the loop condition's v.length are both outside a catch. A revoked array Proxy makes Array.isArray throw, and an array Proxy with a throwing get trap for length throws when the loop condition is evaluated. sanitizeBrowserPayload calls this walker without an error boundary while building each event, so an already-parsed hostile body passed to createBrowserRelayHandler (or direct callers of the exported sanitizer) causes the handler to throw instead of returning its controlled 400/202 response. That bypasses the relay's failure handling and can leave the request unresponded. Treat array classification and length inspection as untrusted operations and drop that context value on failure.

⚠ 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)
Wrap `Array.isArray(v)` and obtaining a bounded array length in a try/catch before entering the loop. If either operation throws, return `undefined` for this context value. Keep each indexed read guarded as it is now, and add coverage for both a revoked array Proxy and an array Proxy whose `length` getter trap throws through `sanitizeBrowserPayload` and the Node handler path.

Flagged by Autter security & observability checks.

Comment thread packages/runtime-node/src/relay.ts Outdated
const obj = v as object;
if (seen.has(obj)) return undefined;
seen.add(obj);
if (Array.isArray(v)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 [ai] Guard array classification and length access in bounded context — Risk: 76/100

boundContext promises to never throw for hostile context graphs, but Array.isArray(v) is outside a guard and the loop condition then reads v.length outside the existing element-read guard. Array.isArray throws for a revoked Proxy, and an array Proxy can throw from its get trap for length; either exception propagates through sanitizeBrowserPayload rather than dropping the context. This is reachable through the package-exported sanitizeBrowserPayload and through createBrowserRelayHandler when a body parser supplies an object body, so the relay handler can fail its request instead of returning its controlled response. The stated fail-open behavior is therefore not safe to ship.

⚠ 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/index.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Wrap array classification and length inspection in the same fail-open boundary as element reads. If either operation throws, return `undefined` from `walk` so the offending context value is omitted; add coverage using a revoked Proxy and an array Proxy whose `length` read throws.

Flagged by Autter security & observability checks.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter posted 1 finding(s) as review threads below (🟠 1). Each carries a copy-paste AI fix prompt.

Comment thread packages/runtime-node/src/relay.ts Outdated
for (let i = 0; i < keys.length && i < CONTEXT_MAX_KEYS; i++) {
if (nodes >= CONTEXT_MAX_NODES) break;
nodes++;
const key = keys[i]!;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 [deterministic] Biome: lint/style/noNonNullAssertion — Risk: 55/100

Forbidden non-null assertion.

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the Biome `lint/style/noNonNullAssertion` issue at packages/runtime-node/src/relay.ts:127: Forbidden non-null assertion.

Flagged by Autter security & observability checks.

@autter-dev

autter-dev Bot commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

🧪 Autter test run

Autter checked 763e04e5.

This PR changes 2 source files. No project test command ran for the touched workspace. It also executed 7 checks from the PR's test plan.

Targeted agent checks: Autter ran targeted checks against the files this PR changed — it executed uncovered test-plan items and, where a changed file had no matching test, wrote a temporary test to verify the new behavior.

Execution summary: 11 checks executed · 10 passed · 1 failed.

Project test commands

No project test commands were detected in this repository.

Autter targeted verification

8 tests executed · 8 passed.

Code-level simulations

Revision 763e04e50766 · 324.4s · up to 2 scenarios in parallel

  • Bound and safely copy hostile browser context — execution incomplete (code_diff, code_diff, code_diff)
    Source: packages/runtime-node/src/relay.ts — const CONTEXT_MAX_DEPTH = 6; const CONTEXT_MAX_NODES = 256; const CONTEXT_MAX_STRING = 4000;
    Source: packages/runtime-node/src/relay.ts — if (seen.has(obj)) return undefined;
    Source: packages/runtime-node/src/relay.ts — ? relayContext(e.context, redactor)
    Ran: node '.autter/scratch/relay-hostile-context.mjs'
    Could not complete: Error: Simulation emitted an invalid assertion report. "strings":13,"maxDepth":0,"largestArray":0,"largestObject":100},"responseStatus":202},{"handler":"node","name":"depth","passed":true,"stats":{"values":6,"strings":14,"maxDepth":5,"largestArray":0…
  • Check secret-bearing context across privileged relay forwarding — failed (code_diff, code_diff, spec)
    Source: packages/runtime-node/src/relay.ts — return Array.isArray(bounded) ? {} : { context: redactor(bounded as Attributes) };
    Source: packages/runtime-node/src/relay.ts — authorization: Bearer ${opts.apiKey},
    Source: PR description — Whitelist sanitiser — anything not listed here is dropped, so a compromised or buggy client can't smuggle cookies/DOM/bodies through the relay.
    Ran: node '.autter/scratch/relay-context-harness.mjs'
    ✅ The relay returns 202 for the valid event and sends the configured server key only on the server-to-ingester request; the browser-facing response does not contain that key.: expected Both responses are 202; the configured key appears on both ingester requests and in neither browser response.; observed Fetch status=202; Node status=202; captured forwards=2; server-key authorization on both forwards=true; browser responses contain server key=false. (integration)
    ❌ No planted secret value is present raw in the serialized forwarded context, including values under benign nested keys or array entries; if an opaque password/session marker survives, record that as a privacy-boundary failure.: expected Zero planted secret values appear raw in either serialized forwarded context.; observed Raw planted secret occurrences in forwarded contexts=2; per-handler occurrence counts=1,1. (integration)
    ✅ Numeric otp and sessionId credentials are not forwarded raw, while the safe input_tokens count remains available.: expected otp/sessionId are absent or redacted in both contexts; input_tokens remains 42 in both.; observed Raw otp forwarded per handler=false,false; raw sessionId forwarded per handler=false,false; input_tokens values=42,42. (integration)
    Diagnosis could not complete: no valid structured result (exit=0, hadFile=true);
    Regression: Identical harness and assertions failed on revision d20942e.
  • Enforce request limits using UTF-8 bytes in both relay handlers — passed (code_diff, code_diff)
    Source: packages/runtime-node/src/relay.ts — total += value.byteLength;
    Source: packages/runtime-node/src/relay.ts — if (total > maxBody) {
    Ran: node '.autter/scratch/relay-byte-limits.mjs'
    ✅ Each multi-byte body exceeding the byte budget is rejected with 413 even if its JavaScript string length is within the limit.: expected {"fetchHandler":413,"nodeHandler":413}; observed {"fetchHandler":413,"nodeHandler":413} (integration)
    ✅ Each valid ASCII body exactly at the byte limit is accepted with 202 and forwarded; the limit is inclusive at the boundary.: expected {"fetchHandlerStatus":202,"nodeHandlerStatus":202,"forwardedRequests":2,"forwardedPaths":["/v1/browser","/v1/browser"],"forwardedMessagesMatch":true}; observed {"fetchHandlerStatus":202,"nodeHandlerStatus":202,"forwardedRequests":2,"forwardedPaths":["/v1/browser","/v1/browser"],"forwardedMessagesMatch":true} (integration)
  • Return promptly when cancelling an oversized request stream — passed (code_diff, code_diff, code_diff)
    Source: packages/runtime-node/src/relay.ts — void Promise.resolve()
    Source: packages/runtime-node/src/relay.ts — .catch(() => {});
    Source: packages/runtime-node/src/relay.ts — return { tooLarge: true };
    Ran: node '.autter/scratch/relay-cancel-harness.mjs'
    ✅ Both oversized requests return 413 promptly even when cancellation hangs or rejects.: expected Both requests return HTTP 413 before the 1000 ms timeout.; observed HTTP 413 in 2 ms; HTTP 413 in 1 ms (code)
    ✅ A stream read failure returns a controlled 400 response rather than leaving the handler promise rejected or hanging.: expected HTTP 400 before the 1000 ms timeout, with no rejected handler promise.; observed HTTP 400 in 0 ms (code)
    Source unavailable: Production route aggregates are unavailable: Runtime queries are not configured. No retained production error patterns were available for this repository.
    Coverage limitation: No production runtime IDs or routes were supplied, so the simulations use local fake ingesters only.
    Coverage limitation: A fake ingester can verify the privileged forwarded request but cannot establish downstream database persistence or sink behavior; no disposable database is required for these checks.
    Coverage limitation: Opaque credentials in free-text values cannot be identified reliably by format alone; the planted opaque markers specifically test whether the public context boundary needs a stricter allowlist.

Declared tests: 42 test file(s) found — 1 ran, 0 not observed in suite output, 41 did not run.

The project's command reported 0 of them. Autter ran the other 1 file one by one so every declared test has a verdict (1 passed, 0 failed).

"Did not run" means no test command that Autter executed covers the file, so nothing in it was checked.

⚠️ Test cases that did not run
  • ⛔ adapters/python/test_compat.py — no test suite covers this file
  • ⛔ adapters/python/test_redact.py — no test suite covers this file
  • ⛔ packages/otlp-ingester/src/coded-errors.test.ts (16 cases) — no test suite covers this file
  • ⛔ packages/otlp-ingester/src/compat.test.ts (9 cases) — no test suite covers this file
  • ⛔ packages/otlp-ingester/src/continuous-detection.test.ts (6 cases) — no test suite covers this file
  • ⛔ packages/otlp-ingester/src/fingerprint.test.ts (10 cases) — no test suite covers this file
  • ⛔ packages/otlp-ingester/src/latency.test.ts (5 cases) — no test suite covers this file
  • ⛔ packages/otlp-ingester/src/logs.test.ts (4 cases) — no test suite covers this file
  • ⛔ packages/otlp-ingester/src/normalize-browser.test.ts (5 cases) — no test suite covers this file
  • ⛔ packages/otlp-ingester/src/normalize.test.ts (5 cases) — no test suite covers this file
  • ⛔ packages/otlp-ingester/src/redact.test.ts (5 cases) — no test suite covers this file
  • ⛔ packages/otlp-ingester/src/server.compat.test.ts (4 cases) — no test suite covers this file
  • ⛔ packages/otlp-ingester/src/server.logs.test.ts (1 case) — no test suite covers this file
  • ⛔ packages/otlp-ingester/src/server.sink.test.ts (2 cases) — no test suite covers this file
  • ⛔ packages/otlp-ingester/src/sink.test.ts (9 cases) — no test suite covers this file
  • ⛔ packages/runtime-browser/test/before-send.test.mjs (1 case) — no test suite covers this file
  • ⛔ packages/runtime-browser/test/client-context.test.mjs (1 case) — no test suite covers this file
  • ⛔ packages/runtime-browser/test/coded-errors.test.mjs (5 cases) — no test suite covers this file
  • ⛔ packages/runtime-browser/test/csp-actions.test.mjs (1 case) — no test suite covers this file
  • ⛔ packages/runtime-browser/test/network.test.mjs (1 case) — no test suite covers this file
  • ⛔ packages/runtime-browser/test/redact-e2e.test.mjs (1 case) — no test suite covers this file
  • ⛔ packages/runtime-browser/test/redact-hardening.test.mjs (2 cases) — no test suite covers this file
  • ⛔ packages/runtime-browser/test/redact-vectors.test.mjs (3 cases) — no test suite covers this file
  • ⛔ packages/runtime-browser/test/redact.test.mjs (5 cases) — no test suite covers this file
  • ⛔ packages/runtime-core/test/core.test.mjs (15 cases) — no test suite covers this file
  • ⛔ packages/runtime-edge/test/edge.test.mjs (7 cases) — no test suite covers this file
  • ⛔ packages/runtime-next/test/next.test.mjs (3 cases) — no test suite covers this file
  • ⛔ packages/runtime-node/test/autoflush.test.mjs (7 cases) — no test suite covers this file
  • ⛔ packages/runtime-node/test/compat.test.mjs (7 cases) — no test suite covers this file
  • ⛔ packages/runtime-node/test/doctor.test.mjs (5 cases) — no test suite covers this file
  • ⛔ packages/runtime-node/test/golden.test.mjs (1 case) — no test suite covers this file
  • ⛔ packages/runtime-node/test/hook-mode.test.mjs (1 case) — no test suite covers this file
  • ⛔ packages/runtime-node/test/lifecycle.test.mjs (4 cases) — no test suite covers this file
  • ⛔ packages/runtime-node/test/logger-ingester.test.mjs (1 case) — no test suite covers this file
  • ⛔ packages/runtime-node/test/logger.test.mjs (2 cases) — no test suite covers this file
  • ⛔ packages/runtime-node/test/logging-only.test.mjs (1 case) — no test suite covers this file
  • ⛔ packages/runtime-node/test/operations.test.mjs (5 cases) — no test suite covers this file
  • ⛔ packages/runtime-node/test/redact.test.mjs (33 cases) — no test suite covers this file
  • ⛔ packages/runtime-node/test/relay-compat.test.mjs (2 cases) — no test suite covers this file
  • ⛔ packages/runtime-node/test/requests.test.mjs (5 cases) — no test suite covers this file
  • …and 1 more

Tests for this change

Autter ran the existing test for 1 changed file.

Change coverage (is each changed file's behavior verified by a test?)

Changed file Related test Result
packages/runtime-node/src/relay.ts packages/runtime-node/test/relay.test.mjs ✅ existing test passes
🤖 Coverage-check evidence

packages/runtime-node/src/relay.ts

Temporary tests are written under .autter/scratch/ for verification only — they are never committed to the repository.

Test plan (from the PR description)

  • ✅ Run npm run build at the repo root and confirm it passes. — verified by agent execution
  • ✅ Run npm test -w @autter/runtime-node and confirm all relay.test.mjs cases pass, including the Proxy, cycle, multibyte, and never-settling cancel() cases. — verified by agent execution
  • ✅ Start an app that mounts createBrowserRelayFetchHandler with maxBodyBytes: 10. POST a body of six Tamil characters ("அ".repeat(6), 6 code units, 18 bytes) and confirm the response is 413. — verified by agent execution
  • ✅ POST a ~500-byte body with maxBodyBytes: 100 and confirm 413. POST a normal browser payload under the limit and confirm 202. — verified by agent execution
  • ✅ POST an event whose context is nested 30+ levels deep, one with a 9000-char string, and one with a 500-element array. Confirm the forwarded payload (inspect via onError/a local ingester log) is truncated to the documented limits. — verified by agent execution
  • ✅ Call sanitizeBrowserPayload in a Node REPL with a cyclic context, a revoked Proxy.revocable object, and an array Proxy whose length trap throws. Confirm none throw and the event is still returned. — verified by agent execution
  • ⬜ Send context: { request: { headers: { cookie: "sid=1", authorization: "Bearer x" } }, note: "Bearer supersecrettoken12345", keep: "ok" } through the relay to a local ingester (docker compose on port 4318). Read the row back from ClickHouse and confirm the secret values are [redacted] and keep is intact. — needs manual verification
  • ⬜ Send context: { input_tokens: 500, sessions: 3 } and confirm the numeric values are preserved in the stored row. — needs manual verification
  • ✅ Optional gap check: send context: { note: "sk-…" } or { debug: "AKIA…" } and note whether the value survives (expected today, per the known follow-up). — verified by agent execution
🤖 Agent-executed checks

✅ Run npm run build at the repo root and confirm it passes.
Root build completed successfully through runtime-core, runtime-node, runtime-edge, runtime-browser, otlp-ingester, and runtime-next; exit 0.

✅ Run npm test -w @autter/runtime-node and confirm all relay.test.mjs cases pass, including the Proxy, cycle, multibyte, and never-settling cancel() cases.
All 124 tests passed; 0 failed, 1 skipped. relay.test.mjs passed Proxy, cycle, UTF-8 multibyte, and never-settling/rejecting cancel() cases.

✅ Start an app that mounts createBrowserRelayFetchHandler with maxBodyBytes: 10. POST a body of six Tamil characters ("அ".repeat(6), 6 code units, 18 bytes) and confirm the response is 413.
Six Tamil characters measured 6 code units / 18 bytes; POST returned 413 with maxBodyBytes=10.

✅ POST a ~500-byte body with maxBodyBytes: 100 and confirm 413. POST a normal browser payload under the limit and confirm 202.
500-byte body returned 413 with maxBodyBytes=100; valid 90-byte browser payload returned 202.

✅ POST an event whose context is nested 30+ levels deep, one with a 9000-char string, and one with a 500-element array. Confirm the forwarded payload (inspect via onError/a local ingester log) is truncated to the documented limits.
Forwarded context was bounded to depth 5, string length 4000, and array length 100.

✅ Call sanitizeBrowserPayload in a Node REPL with a cyclic context, a revoked Proxy.revocable object, and an array Proxy whose length trap throws. Confirm none throw and the event is still returned.
Cyclic context, revoked Proxy, and array Proxy with throwing length trap did not throw; each sanitizeBrowserPayload result contained the event.

✅ Optional gap check: send context: { note: "sk-…" } or { debug: "AKIA…" } and note whether the value survives (expected today, per the known follow-up).
The abbreviated sample values 'sk-…' and 'AKIA…' survived unchanged. Complete representative OpenAI and AWS-shaped values were [redacted] by the sanitizer.

⬜ items could not be verified automatically and still need a manual check.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter blocked this PR after its agentic checks (build/test/deep scans) completed: 2 confirmed correctness/runtime finding(s). See the findings below and the full PR review for details.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 Autter review in progress — running security, correctness & dependency checks on this PR. Follow live step-by-step progress on the autter/review-gate check in the merge box. Merge is blocked until the gate completes; Autter approves automatically when the review comes back clean, and releases this hold with a neutral review when it finds non-blocking issues.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter posted 1 finding(s) as review threads below (🟠 1). Each carries a copy-paste AI fix prompt.

Comment thread packages/runtime-node/src/relay.ts Outdated
: {}),
...(typeof e.context === "object" && e.context !== null
? { context: e.context }
? { context: boundContext(e.context) }

Copy link
Copy Markdown
Contributor

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.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter posted 1 finding(s) as review threads below (🟡 1). Each carries a copy-paste AI fix prompt.

if (text.length > maxBody) {
let bounded: { tooLarge: true } | { tooLarge: false; text: string };
try {
bounded = await readBodyBounded(request, maxBody);

Copy link
Copy Markdown
Contributor

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.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter completed PR review for #20: 3 finding(s) remain below the merge-blocking bar, so this review stays neutral rather than approving. (Also detected: 34 finding(s) dismissed as likely false positives by verification.) See the findings below; the task checklist follows as the review's final comment.

@autter-dev

autter-dev Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

🤖 Release Notes Curator

Impact: patch — fixes relay input handling and request-size enforcement without changing the public API.

Changelog: Fixed browser relay validation to safely bound context data and enforce request body limits using UTF-8 byte size.

Custom agent · runs after review · configured in Autter

…y-size check

The browser relay sanitiser length-caps every event field except context, which was forwarded as-is. Bound context with a depth, node, string, and array/key limit (cycle-safe, and proxy-safe: revoked or hostile Proxy traps on classification, length, key enumeration, or element reads are dropped rather than thrown out of the sanitizer). Enforce maxBodyBytes in the fetch handler while consuming the request stream, counting real UTF-8 bytes and cancelling once the limit is crossed, instead of buffering the whole body via request.text() first. Add the first tests for relay.ts. No public API or behaviour change.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 Autter review in progress — running security, correctness & dependency checks on this PR. Follow live step-by-step progress on the autter/review-gate check in the merge box. Merge is blocked until the gate completes; Autter approves automatically when the review comes back clean, and releases this hold with a neutral review when it finds non-blocking issues.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter posted 1 finding(s) as review threads below (🟠 1). Each carries a copy-paste AI fix prompt.

Comment thread packages/runtime-node/src/relay.ts Outdated
for (let i = 0; i < keys.length && i < CONTEXT_MAX_KEYS; i++) {
if (nodes >= CONTEXT_MAX_NODES) break;
nodes++;
const key = keys[i]!;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 [deterministic] Biome: lint/style/noNonNullAssertion — Risk: 55/100

Forbidden non-null assertion.

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the Biome `lint/style/noNonNullAssertion` issue at packages/runtime-node/src/relay.ts:141: Forbidden non-null assertion.

Flagged by Autter security & observability checks.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter posted 2 finding(s) as review threads below (🔴 1 · 🟡 1). Each carries a copy-paste AI fix prompt.

@@ -0,0 +1,115 @@
import { test } from "node:test";
import assert from "node:assert/strict";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 [heuristic] Hallucinated import (package not installed) — Risk: 80/100

Import target node:assert/strict is not declared in the project's manifests (no matching entry in indexed external_dependencies for ecosystem npm).

References:

  • https://pypi.org/project/pkgguard-cli/ — # pkgguard-cli v0.1.0 Vet the packages and repos your AI assistant recommended — before you install. Catches hallucinated/slopsquatted names, malware signals, license traps, dead r
  • https://pypi.org/project/slopgate/ — # slopgate v0.1.0 A CI gate against slopsquatting and hallucinated dependencies (Python / npm / Go). - Author: Nullra - License: MIT - Python: >=3.11 - Package URL: https://pypi
🛠 AI fix prompt (copy & paste into your coding agent)
In `packages/runtime-node/test/relay.test.mjs` around line 2, this import target is not in the project's package manifests or lockfile and was not found in the indexed external_dependencies. Either install it via the package manager, fix the typo, or — if a local module was intended — use the correct relative path. AI agents commonly hallucinate plausible-looking but nonexistent packages.

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
Contributor

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.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter completed PR review for #20: 9 finding(s) remain below the merge-blocking bar, so this review stays neutral rather than approving. See the findings below; the task checklist follows as the review's final comment.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 Autter review in progress — running security, correctness & dependency checks on this PR. Follow live step-by-step progress on the autter/review-gate check in the merge box. Merge is blocked until the gate completes; Autter approves automatically when the review comes back clean, and releases this hold with a neutral review when it finds non-blocking issues.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter posted 1 finding(s) as review threads below (🔴 1). Each carries a copy-paste AI fix prompt.

Comment thread packages/runtime-node/src/relay.ts Outdated
: {}),
...(typeof e.context === "object" && e.context !== null
? { context: e.context }
? { context: boundContext(e.context) }

Copy link
Copy Markdown
Contributor

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.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter posted 1 finding(s) as review threads below (🟡 1). Each carries a copy-paste AI fix prompt.

Comment thread packages/runtime-node/src/relay.ts Outdated
: {}),
...(typeof e.context === "object" && e.context !== null
? { context: e.context }
? { context: boundContext(e.context) }

Copy link
Copy Markdown
Contributor

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.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter completed PR review for #20: 3 finding(s) remain below the merge-blocking bar, so this review stays neutral rather than approving. (Also detected: 28 finding(s) dismissed as likely false positives by verification.) See the findings below; the task checklist follows as the review's final comment.

@autter-dev

autter-dev Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

🤖 Release Notes Curator

Impact: patch — fixes relay payload handling and request-size enforcement without changing the public API.

Changelog: Fixed browser relay handling to safely bound event context and enforce request body limits by UTF-8 byte size.

Custom agent · runs after review · configured in Autter

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter posted 1 finding(s) as review threads below (🔴 1). Each carries a copy-paste AI fix prompt.

Comment thread packages/runtime-node/src/relay.ts Outdated
: {}),
...(typeof e.context === "object" && e.context !== null
? { context: e.context }
? { context: boundContext(e.context) }

Copy link
Copy Markdown
Contributor

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.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter posted 1 finding(s) as review threads below (🟠 1). Each carries a copy-paste AI fix prompt.

Comment thread packages/runtime-node/src/relay.ts Outdated
// must still surface as "too large" (413), never fall through to
// the caller's JSON-error branch and become a 400.
try {
await reader.cancel();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 [ai] Do not await an unbounded stream cancellation before responding 413 — Risk: 78/100

When an oversized request body is detected, this path awaits reader.cancel() before returning { tooLarge: true }. A ReadableStream implementation may return a cancellation promise that never settles (for example while waiting on an unresponsive upstream). In that failure mode the catch is never entered, readBodyBounded never resolves, and createBrowserRelayFetchHandler never reaches its 413 response at lines 321-324. This is reachable through the public Next.js App Router wrapper, so a malicious or failed upload can leave a route request hanging indefinitely after the size limit has already been crossed.

⚠ 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)
Once the byte budget is exceeded, make cancellation strictly best-effort without awaiting an unbounded promise. Call `reader.cancel()` inside a synchronous try/catch and attach a rejection handler (for example `void reader.cancel().catch(() => {})`), then return `{ tooLarge: true }` immediately. Add a test whose `cancel()` returns a never-settling promise and assert the fetch handler resolves 413.

Flagged by Autter security & observability checks.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 Autter review in progress — running security, correctness & dependency checks on this PR. Follow live step-by-step progress on the autter/review-gate check in the merge box. Merge is blocked until the gate completes; Autter approves automatically when the review comes back clean, and releases this hold with a neutral review when it finds non-blocking issues.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter posted 1 finding(s) as review threads below (🟠 1). Each carries a copy-paste AI fix prompt.

Comment thread packages/runtime-node/src/relay.ts Outdated
* `false` to disable (e.g. when a WAF already rate-limits).
*/
perIpRateLimit?: number | false;
/**

Copy link
Copy Markdown
Contributor

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.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter posted 1 finding(s) as review threads below (🔴 1). Each carries a copy-paste AI fix prompt.

Comment thread packages/runtime-node/src/relay.ts Outdated
: {}),
...(typeof e.context === "object" && e.context !== null
? { context: e.context }
? { context: boundContext(e.context) }

Copy link
Copy Markdown
Contributor

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.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter posted 1 finding(s) as review threads below (🟠 1). Each carries a copy-paste AI fix prompt.

Comment thread packages/runtime-node/src/relay.ts Outdated
// they can never be a credential, and this preserves usage counts such as
// `input_tokens`.
const REDACTED = "[redacted]";
const SECRET_KEY_RE =

Copy link
Copy Markdown
Contributor

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.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter posted 1 finding(s) as review threads below (🟠 1). Each carries a copy-paste AI fix prompt.

Comment thread packages/runtime-node/src/relay.ts Outdated
// caller has explicitly opted into a trusted-proxy deployment.
const ip =
firstForwardedFor(req.headers["x-forwarded-for"]) ||
(opts.trustProxy === true

Copy link
Copy Markdown
Contributor

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.

@autter-dev

autter-dev Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

🤖 Release Notes Curator

Impact: patch — fixes browser relay payload sanitization and byte-accurate request-size enforcement without changing the public API.

Changelog: Hardened the browser telemetry relay to safely bound event context data, redact credentials, and enforce request size limits accurately for all character sets.

Custom agent · runs after review · configured in Autter

@rajeshaipython-stack

Copy link
Copy Markdown
Contributor Author

Hi Sagnik, thanks a lot for taking the time to review this — I really appreciate it.

I've pushed fixes for the actionable findings on fix/relay-context-bounds:

Context redaction — boundContext now recursively redacts secret-bearing keys (authorization, cookie, token, *_secret, *_key, session, jwt, and similar) at every nesting level, and scrubs Bearer/Basic/JWT-shaped string values even under benign keys. Numeric values such as usage counts (input_tokens) are preserved.
Rate-limit — the forwarded header is now honored only behind a strict trustProxy === true; otherwise the fetch handler keys a shared bucket and the Node handler keys the socket peer.
Oversized body — the stream cancel is now detached, so a 413 returns immediately.

All covered by tests (19 passing), and the earlier JWT-in-test flag is resolved.

There are two remaining items where I'd really value your direction, since both feel like calls that should be yours rather than mine to assume:

First — the "secret value under a benign key" case (e.g. an API-key-shaped string under a key like note). A blacklist can't fully close this; the complete fix is an allowlist of approved context keys, which would change context from free-form metadata into a fixed schema. Would you like me to move to an allowlist, or keep the current recursive redaction?

Second — the rate limiter. Fully resolving the forwarded-IP concern points toward a trusted-proxy client-IP resolver, which is a larger change and touches code that predates this PR. Would you prefer I take that on here, keep the current safe-by-default behaviour, or handle it in a separate PR?

I'm also happy to split this into focused commits/PRs if that's easier for you to review. Whichever direction you feel is right for Autter, I'm glad to follow.

Thanks again — really enjoying working in this codebase.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter posted 12 finding(s) as review threads below (🔴 4 · 🟠 8). Each carries a copy-paste AI fix prompt.

Comment thread packages/runtime-node/src/relay.ts Outdated
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));

Copy link
Copy Markdown
Contributor

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.

Comment thread packages/runtime-node/src/relay.ts Outdated
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));

Copy link
Copy Markdown
Contributor

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.

Comment thread packages/runtime-node/src/relay.ts Outdated
// back to one shared bucket (a conservative global limit).
const ip =
firstForwardedFor(request.headers.get("x-forwarded-for")) || "unknown";
opts.trustProxy === true

Copy link
Copy Markdown
Contributor

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.

Comment thread packages/runtime-node/src/relay.ts Outdated
// back to one shared bucket (a conservative global limit).
const ip =
firstForwardedFor(request.headers.get("x-forwarded-for")) || "unknown";
opts.trustProxy === true

Copy link
Copy Markdown
Contributor

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.

Comment thread packages/runtime-node/src/relay.ts Outdated
continue;
}
const rt = typeof raw;
out[key] = rt === "number" || rt === "boolean" ? raw : REDACTED;

Copy link
Copy Markdown
Contributor

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.

Comment thread packages/runtime-node/src/relay.ts Outdated
opts.trustProxy === true
? firstForwardedFor(request.headers.get("x-forwarded-for")) ||
"unknown"
: "shared";

Copy link
Copy Markdown
Contributor

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.

Comment thread packages/runtime-node/src/relay.ts Outdated
const ip =
firstForwardedFor(request.headers.get("x-forwarded-for")) || "unknown";
opts.trustProxy === true
? firstForwardedFor(request.headers.get("x-forwarded-for")) ||

Copy link
Copy Markdown
Contributor

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.

Comment thread packages/runtime-node/src/relay.ts Outdated
opts.trustProxy === true
? firstForwardedFor(request.headers.get("x-forwarded-for")) ||
"unknown"
: "shared";

Copy link
Copy Markdown
Contributor

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.

Comment thread packages/runtime-node/src/relay.ts Outdated
continue;
}
const rt = typeof raw;
out[key] = rt === "number" || rt === "boolean" ? raw : REDACTED;

Copy link
Copy Markdown
Contributor

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.

Comment thread packages/runtime-node/src/relay.ts Outdated
// back to one shared bucket (a conservative global limit).
const ip =
firstForwardedFor(request.headers.get("x-forwarded-for")) || "unknown";
opts.trustProxy === true

Copy link
Copy Markdown
Contributor

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.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter blocked this PR: 5 confirmed correctness/runtime finding(s). See the findings below and the full PR review for details.

@autter-dev

autter-dev Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

🔧 Autter is processing fixes for 18 unresolved issue(s) — requested by @sagnik11.

Packaging: one PR with all unresolved fixes — every fix lands on branch autter/fix-20, and a single fix PR will target fix/relay-context-bounds.

Issues being fixed:

  1. Browser context still forwards credential formats under benign keys — packages/runtime-node/src/relay.ts:124
  2. Public relay still forwards credential-shaped values under benign context keys — packages/runtime-node/src/relay.ts:124
  3. trustProxy directly re-enables a requester-spoofable rate-limit key — packages/runtime-node/src/relay.ts:354
  4. trustProxy makes the public relay limiter key attacker-controlled — packages/runtime-node/src/relay.ts:354
  5. Browser relay preserves opaque secret values under benign context keys — packages/runtime-node/src/relay.ts:295
  6. Relay leaves recognized credential formats under benign context keys — packages/runtime-node/src/relay.ts:124
  7. Sensitive numeric context values are forwarded intact — packages/runtime-node/src/relay.ts:185
  8. Public relay preserves numeric credentials under sensitive context keys — packages/runtime-node/src/relay.ts:185
  9. Default Next relay collapses all browser clients into one rate-limit bucket — packages/runtime-node/src/relay.ts:357
  10. Default Next relay now globally rate-limits all browser clients — packages/runtime-node/src/relay.ts:357
  11. Default fetch relay globally rate-limits all browser clients — packages/runtime-node/src/relay.ts:357
  12. trustProxy enables requester-controlled rate-limit keys without proxy verification — packages/runtime-node/src/relay.ts:355
  13. Sensitive numeric context values are explicitly forwarded — packages/runtime-node/src/relay.ts:185
  14. trustProxy enables request-controlled IP rotation on the public relay — packages/runtime-node/src/relay.ts:354
  15. trustProxy accepts an unvalidated forwarded IP for relay rate limiting — packages/runtime-node/src/relay.ts:445
  16. Rate limit fallback is still spoofable in fetch runtime — packages/runtime-node/src/relay.ts:354
  17. Browser relay retains API-key-shaped values under benign context keys — packages/runtime-node/src/relay.ts:97
  18. Rate limiting not detected — packages/runtime-node/src/relay.ts:24

1 unresolved finding(s) need a human instead of a patch — they are left out of this run and listed in a separate checklist comment below.

Autter will comment here as fixes are pushed and fix PRs open. Issues that can't be safely automated are reported instead of guessed at.

Run ID: 0357068b-88b3-4cca-9054-61b1340171b0

@autter-dev

autter-dev Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

👤 1 unresolved issue(s) need a human — Autter won't auto-fix these — requested by @sagnik11.

These can't be resolved by a code patch. Autter is handling the automatable findings separately (see the fix run above) — this checklist is yours:

  • PR mixes refactor and behavior change (packages/runtime-node/src/relay.ts:21) — 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 po…

Tick items off as you resolve them — Autter re-checks these findings on the next pushed revision.

@autter-dev

autter-dev Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

🛑 Autter finished this fix run without pushing a fix branch — requested by @sagnik11.

The generated patch was withheld after adversarial self-review judged it unsafe to propose: The new BEARER_RE is narrower than the previous redaction: it only redacts Bearer [REDACTED] with at least 10 characters from a restricted character set. Valid short or opaque Bearer [REDACTED] (for example containing characters outside that set) previously matched ^\s*(bearer|basic)\s+\S+ but will now be forwarded under benign context keys. This is a direct secret-redaction regression on the public-to-private relay boundary. Additionally, the OpenAI pattern misses common sk-proj-... / hyph…

Findings the agent reported it could not safely automate:

  • Default Next relay collapses all browser clients into one rate-limit bucket (packages/runtime-node/src/relay.ts:357) — The generic Next Fetch API has no verified peer/client-IP signal. The safe shared fallback remains; per-client default limiting requires a platform-specific trusted resolver contract.
  • Default Next relay now globally rate-limits all browser clients (packages/runtime-node/src/relay.ts:357) — A safe client-specific Next default cannot be implemented from a standard Request without trusting attacker-controlled headers; a deployment adapter must provide verified identity.
  • Default fetch relay globally rate-limits all browser clients (packages/runtime-node/src/relay.ts:357) — The secure shared fallback is retained. Per-client rate limits for generic fetch handlers require a platform-specific trusted identity source.

No fix branch or PR was created. To retry, uncheck and re-check the fix option on the task comment.

Run ID: 0357068b-88b3-4cca-9054-61b1340171b0

Resolve the add/add conflict in packages/runtime-node/test/relay.test.mjs by
keeping both the relay bounding tests from this branch and the CSP-violation
tests added on main.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter posted 2 finding(s) as review threads below (🟠 2). Each carries a copy-paste AI fix prompt.

Comment thread packages/runtime-node/src/relay.ts Outdated
Comment thread packages/runtime-node/src/relay.ts Outdated
@autter-dev

autter-dev Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

🤖 Release Notes Curator

Impact: minor. The PR adds a new opt-in trustProxy relay option. It also changes defaults: context is now bounded and redacted, and the body-size limit counts bytes. Deployments that relied on X-Forwarded-For for per-IP rate limiting will see different behavior unless they enable the option.

The PR now carries both release: patch and release: minor. I couldn't remove the existing patch label, so a maintainer should drop release: patch.

Changelog entry

Browser relay: safer defaults and a new trustProxy option. The relay now limits the size and depth of browser-supplied context and redacts values that look like credentials before forwarding. Request size limits now count bytes rather than characters, so non-Latin text can no longer slip past the cap. Per-IP rate limiting no longer trusts the client-supplied X-Forwarded-For header by default, because it can be spoofed. If you run behind a proxy or CDN you control, set trustProxy: true to restore per-client rate limiting.

Custom agent · runs after review · configured in Autter

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter posted 3 finding(s) as review threads below (🔴 2 · 🟡 1). Each carries a copy-paste AI fix prompt.

Comment thread packages/runtime-node/src/relay.ts Outdated

/** 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;

Copy link
Copy Markdown
Contributor

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 secrets under benign context keys — Risk: 80/100

The relay is intentionally unauthenticated, but it attaches its configured server ingest key when forwarding browser events. Its value scrubber only recognizes Bearer/Basic prefixes and JWT-shaped strings, so values such as OpenAI/AWS/GitHub/Slack credentials, PEM blocks, URL credentials, or opaque secrets under benign nested keys (for example context.debug or context.request.note) survive this boundary. The secret-key branch also preserves numeric values such as context.otp or context.sessionId. These values are forwarded with the server credential and can be persisted: /v1/browser authenticates the forwarded request, while its context scrubber only checks top-level keys and email strings and does not recursively inspect nested values (nor does its key expression cover otp). This leaves a browser-controlled credential disclosure path into privileged 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/normalize-browser.ts, packages/otlp-ingester/src/server.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Use a strict allowlist for browser context, or recursively scrub values at the relay boundary with coverage equivalent to the runtime-node redactor (including private-key blocks, API tokens, URL credentials, and email). Drop all values under sensitive keys regardless of type, except for an exact allowlist of validated usage-count keys. Add relay-to-ingester coverage proving representative nested secrets and numeric OTP/session values are absent from persisted/sink payloads.

Flagged by Autter security & observability checks.

Comment thread packages/runtime-node/src/relay.ts Outdated
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));

Copy link
Copy Markdown
Contributor

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 credentials in browser context — Risk: 80/100

This public relay does not establish the claimed secret-redaction boundary: strings under benign context keys are scrubbed only when they start with Bearer/Basic or match the JWT shape, so values such as OpenAI/AWS/GitHub/Slack keys, PEM private keys, URL credentials, and nested email/credential strings remain raw. In addition, the sensitive-key branch explicitly preserves all numeric/boolean values, so inputs such as {context:{otp:123456}} or {context:{sessionId:987654}} also survive. sanitizeBrowserPayload attaches this result to the event and forward serializes it in a request authenticated with the server ingest key. The ingester accepts arbitrary nested context and scrubContext only checks each top-level key plus email strings, leaving these nested credential values (and numeric otp/sessionId) available for storage and sink delivery. Use a strict context allowlist or recursively apply equivalent server-side key/value redaction before privileged forwarding, without exempting numeric values under secret-bearing 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/otlp-ingester/src/normalize-browser.ts, packages/otlp-ingester/src/server.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Harden boundContext at the public relay boundary: use a strict allowlist or recursively redact/drop credential-shaped strings using the same formats as packages/runtime-node/src/redact.ts, and redact all value types under sensitive keys (except an exact allowlist of validated usage counters). Add relay-to-forwarding regression coverage for nested OpenAI/AWS/GitHub/Slack keys, private-key blocks, URL credentials, email, and numeric otp/sessionId values.

Flagged by Autter security & observability checks.

Comment thread packages/runtime-node/src/relay.ts Outdated
opts.trustProxy === true
? firstForwardedFor(request.headers.get("x-forwarded-for")) ||
"unknown"
: "shared";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 [ai] Fetch relay rate limit shares one bucket across all clients — Risk: 40/100

The public fetch-style relay keys every request to the literal shared by default because a Fetch Request has no peer-IP field. Consequently, any unauthenticated caller can consume the default 120-request window and cause legitimate clients of that relay instance to receive 429 responses until the window rolls over; this is a trivial cross-client availability attack on the browser telemetry endpoint. The Node handler can key on the socket peer, but this fetch path is used by Next.js and other supported Fetch runtimes. Key the default limiter by a trustworthy platform-provided client IP, or require a trusted proxy configuration that supplies a non-spoofable client address.

⚠ 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)
Change the fetch relay's default rate-limit key from a shared constant to a trustworthy per-client IP supplied by the hosting adapter/platform, or require an explicitly trusted proxy configuration for Fetch runtimes; never use a client-controlled header as the identity source.

Flagged by Autter security & observability checks.

@autter-dev

autter-dev Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

packages/runtime-node/src/relay.ts:1 · test_failure

🟠 [verified] Change fails its execution check: packages/runtime-node/src/relay.ts — Risk: 70/100

The execution check failed while exercising the changed behavior in packages/runtime-node/src/relay.ts.

Ran: NODE_ENV=production npm run build -w @autter/runtime-node && NODE_ENV=test node --test test/relay.test.mjs (cwd: packages/runtime-node); NODE_ENV=test node --test .autter/scratch/relay-secret-regression.test.mjs

Declared relay suite passed: 21 tests, 0 failures, but it does not test these raw credential forms. Temporary mocked-fetch relay regression failed: forwarded nested context retained OpenAI/GitHub/AWS/Slack token examples, PEM material, URL credentials, email, an opaque session-like value, and numeric otp/sessionId/passcode/recoveryCode. Failure boundary: changed_code. Techniques: executed declared tests and a focused handler-to-forwarding regression; property/fuzz/mutation testing not used for this focused regression. Cases: nested benign-key secrets in arrays/objects and numeric credentials. Limitations: downstream network/sink was mocked; temporary test removed after execution.
🛠 AI fix prompt (copy & paste into your coding agent)
The change in packages/runtime-node/src/relay.ts produced wrong observed behavior when exercised: Declared relay suite passed: 21 tests, 0 failures, but it does not test these raw credential forms. Temporary mocked-fetch relay regression failed: forwarded nested context retained OpenAI/GitHub/AWS/Slack token examples, PEM material, URL credentials, email, an opaque session-like value, and numeric otp/sessionId/passcode/recoveryCode. Failure boundary: changed_code. Techniques: executed declared. Fix the changed behavior and add a real test covering it.

Flagged by Autter security & observability checks.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter blocked this PR: test; test_failure; 1 confirmed correctness/runtime finding(s). See the findings below and the full PR review for details.

@autter-dev

autter-dev Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Autter review completed — merge blocked

Autter completed its review and found at least one confirmed merge-blocking finding.

Autter blocked this PR: test; test_failure; 1 confirmed correctness/runtime finding(s). See the findings below and the full PR review for details.

Other findings explain the additional warnings, but they did not independently block this PR.

The merge gate remains blocked until the confirmed finding is resolved.

Retry

Retrying reruns the review on the same commit. If the commit is unchanged, the same finding and block may be reproduced.

  • Click Retry review on the autter/review-gate check below, or
  • Comment @autter review on this PR

Autter will post a new comment when the retry starts.

…ctor

- Keep today's per-client rate-limit keying. Defaulting trustProxy off put
  every visitor of a default Next.js install (and any Node app behind a
  proxy) into one shared 120/min bucket.
- Bound context first, then scrub it with the relay's shared redactor from
  main instead of a second, narrower set of secret patterns.
- Guard the array check behind boundContext so a revoked Proxy context is
  dropped instead of throwing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sagnik11
sagnik11 merged commit 95516db into Autter-dev:main Oct 8, 2026
1 check passed

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter approved #20: no unresolved blocking findings remain after review.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter approved #20: no unresolved blocking findings remain after review.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter posted 1 finding(s) as review threads below (🟡 1). Each carries a copy-paste AI fix prompt.

} catch {
child = undefined;
}
if (child !== undefined) out[key] = child;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 [ai] Code correctness issue — Risk: 30/100

boundContext copies context property names verbatim (out[key] = child) without applying CONTEXT_MAX_STRING or any key-length limit, so a long key bypasses the per-string cap. Impact is bounded by the overall request body limit (maxBodyBytes), but consider truncating or dropping keys longer than a reasonable limit.

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

  • Functions/symbols: byteLength, boundContext, sanitizeBrowserPayload, createBrowserRelayFetchHandler, readBodyBounded, relayContext
  • Dependent files: ../../otlp-ingester/src/compat.js, ./compat.js, ./redact.js, @opentelemetry/api, node:http
  • Scopes: @autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Cap context property-name length before adding each property to `out` (or drop keys exceeding the limit). Add a test showing that an overlong context key is truncated or omitted by `sanitizeBrowserPayload`. Blast radius — if this defect reaches production it can fail the downstream usage that depends on this file: functions `byteLength`, `boundContext`, `sanitizeBrowserPayload`, `createBrowserRelayFetchHandler`, `readBodyBounded`, `relayContext`; scopes `@autter/runtime-node`; dependent files `../../otlp-ingester/src/compat.js`, `./compat.js`, `./redact.js`, `@opentelemetry/api`, `node:http`.

Flagged by Autter security & observability checks.

@autter-dev

autter-dev Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

🤖 Release Notes Curator

Impact: patch. This is a bug fix that hardens the browser relay. It caps the size of browser-supplied context and counts the request body limit in bytes. No API or option was removed or changed.

The PR also carries a release: minor label that I didn't add, and I can't remove it with my tools. Please drop it if patch is the intended level.

Changelog entry

Fixed: The Node browser relay now limits how much context data a browser can send. Deeply nested, oversized or circular context is trimmed or dropped instead of being forwarded as-is. The request body size limit is now measured in bytes, so bodies with multi-byte characters can no longer slip past it.

Custom agent · runs after review · configured in Autter

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Autter approved #20 with notes: the merge gate is clear, and 7 non-blocking finding(s) remain as review detail. (Also detected: 10 finding(s) dismissed as likely false positives by verification.)

@autter-dev

autter-dev Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Autter task list

  • Extend value-based redaction to other credential formats (packages/runtime-node/src/relay.ts, packages/runtime-node/test/relay.test.mjs) - Add value-shape scrubbing in boundContext for sk- keys, AKIA access keys, GitHub/Slack tokens, PEM blocks and URL-embedded credentials under benign keys, and add tests for each format.
  • Verify boundContext survives every Proxy trap (packages/runtime-node/src/relay.ts, packages/runtime-node/test/relay.test.mjs) - Audit each Proxy-observable operation in boundContext (Array.isArray, length, key enumeration, property get, descriptor reads) to confirm it sits inside try/catch, and add tests for any trap not yet covered, such as ownKeys and getOwnPropertyDescriptor.

Generated from PR diff, blast radius, and context.

Issues found

No unresolved code findings remain for this revision — 10 finding(s) dismissed as likely false positives by verification. Dismissed findings stay listed with their verdicts in the Autter review dashboard.

9 additional non-blocking finding(s) are available in the Autter review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants