fix(runtime): redact deeply nested attributes. - #19
Conversation
![autter-dev[bot]](https://avatars.githubusercontent.com/in/2920597?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔴 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.
| const drained = Promise.allSettled( | ||
| targets.map((target) => target.forceFlush()), | ||
| ).then(() => true); | ||
| ).then((results) => results.every((result) => result.status === "fulfilled")); |
There was a problem hiding this comment.
🟠 Silent exception swallowing — Risk: 78/100
This aggregate only observes whether each registered FlushTarget promise fulfills. The production target registered by initAutterServer can internally use Promise.allSettled for exporters and resolve normally after an exporter rejection, so this every(...) check reports success and installAutterAutoFlush can mark telemetry as flushed despite failed exporter delivery. Blast radius — if this failure path is hit it cascades to the downstream usage that depends on this file: functions installAutterAutoFlush, debugLog, CountingExporter.forceFlush, isDebugEnabled, TelemetryStats.markAllFlushed, activeFlushTargets, redactAttributes, makeRedactor; scopes @autter/runtime-node; dependent files @opentelemetry/core, @opentelemetry/sdk-trace-base.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Functions/symbols:
installAutterAutoFlush,debugLog,CountingExporter.forceFlush,isDebugEnabled,TelemetryStats.markAllFlushed,activeFlushTargets,redactAttributes,makeRedactor - Dependent files:
@opentelemetry/core,@opentelemetry/sdk-trace-base - Scopes:
@autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Make the active-server FlushTarget propagate failure from every underlying exporter and metric forceFlush, or inspect its allSettled results and reject/return failure when any operation fails. Add coverage for a target that converts an underlying rejection into a fulfilled promise, and verify installAutterAutoFlush reports the failed flush. Blast radius — if this failure path is hit it cascades to the downstream usage that depends on this file: functions `installAutterAutoFlush`, `debugLog`, `CountingExporter.forceFlush`, `isDebugEnabled`, `TelemetryStats.markAllFlushed`, `activeFlushTargets`, `redactAttributes`, `makeRedactor`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/core`, `@opentelemetry/sdk-trace-base`.
Flagged by Autter security & observability checks.
| const drained = Promise.allSettled( | ||
| targets.map((target) => target.forceFlush()), | ||
| ).then(() => true); | ||
| ).then((results) => results.every((result) => result.status === "fulfilled")); |
There was a problem hiding this comment.
🟠 [ai] Unhandled edge case (null / empty / zero / boundary) — Risk: 65/100
This only detects rejected FlushTarget promises, but the built-in target can resolve after Promise.allSettled has swallowed exporter failures. The flush is then reported as successful and telemetry may be marked flushed even though export failed. Blast radius — if this AI-generated slop ships it cascades to the downstream usage that depends on this file: functions installAutterAutoFlush, debugLog, CountingExporter.forceFlush, isDebugEnabled, TelemetryStats.markAllFlushed, activeFlushTargets, redactAttributes, makeRedactor; scopes @autter/runtime-node; dependent files @opentelemetry/core, @opentelemetry/sdk-trace-base.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Functions/symbols:
installAutterAutoFlush,debugLog,CountingExporter.forceFlush,isDebugEnabled,TelemetryStats.markAllFlushed,activeFlushTargets,redactAttributes,makeRedactor - Dependent files:
@opentelemetry/core,@opentelemetry/sdk-trace-base - Scopes:
@autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Make the active-server FlushTarget propagate failure when any underlying exporter or metric forceFlush rejects, or inspect its allSettled results and reject/return failure before this aggregate check evaluates the target. Blast radius — if this AI-generated slop ships it cascades to the downstream usage that depends on this file: functions `installAutterAutoFlush`, `debugLog`, `CountingExporter.forceFlush`, `isDebugEnabled`, `TelemetryStats.markAllFlushed`, `activeFlushTargets`, `redactAttributes`, `makeRedactor`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/core`, `@opentelemetry/sdk-trace-base`.
Flagged by Autter security & observability checks.
| ancestors.set(value, out); | ||
|
|
||
| for (const item of value) { | ||
| out.push(redactValue(item, r, ancestors)); |
There was a problem hiding this comment.
🟠 [ai] Unhandled edge case (null / empty / zero / boundary) — Risk: 55/100
Removing the traversal bound makes redaction recurse through arbitrarily deep caller-provided objects. A sufficiently deep attribute tree can exhaust the JavaScript call stack, causing telemetry code to throw instead of failing open. Blast radius — if this AI-generated slop ships it cascades to the downstream usage that depends on this file: functions redactAttributes, makeRedactor, redactString, redactValue, isSensitiveKey, redactWith, installAutterAutoFlush; scopes @autter/runtime-node; dependent files @opentelemetry/api.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Functions/symbols:
redactAttributes,makeRedactor,redactString,redactValue,isSensitiveKey,redactWith,installAutterAutoFlush - Dependent files:
@opentelemetry/api - Scopes:
@autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Retain a bounded traversal depth while tracking ancestors, or implement the deep traversal iteratively with an explicit work stack so hostile or pathological nesting cannot cause stack overflow. Blast radius — if this AI-generated slop ships it cascades to the downstream usage that depends on this file: functions `redactAttributes`, `makeRedactor`, `redactString`, `redactValue`, `isSensitiveKey`, `redactWith`, `installAutterAutoFlush`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/api`.
Flagged by Autter security & observability checks.
| const drained = Promise.allSettled( | ||
| targets.map((target) => target.forceFlush()), | ||
| ).then(() => true); | ||
| ).then((results) => results.every((result) => result.status === "fulfilled")); |
There was a problem hiding this comment.
🟠 [ai] Runtime error risk — Risk: 75/100
Promise.allSettled does not protect against a synchronous exception during targets.map: target.forceFlush is invoked before the promise list is created. A FlushTarget that throws synchronously therefore makes installAutterAutoFlush reject instead of returning the controlled failed-flush result; the timeout cleanup and callers' failure handling can also be bypassed. This touches exported/public surface code, so impact assessment should consider downstream callers. Blast radius — if this defect reaches production it can fail the downstream usage that depends on this file: functions installAutterAutoFlush, debugLog, CountingExporter.forceFlush, isDebugEnabled, TelemetryStats.markAllFlushed, activeFlushTargets, redactAttributes, makeRedactor; scopes @autter/runtime-node; dependent files @opentelemetry/core, @opentelemetry/sdk-trace-base.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Functions/symbols:
installAutterAutoFlush,debugLog,CountingExporter.forceFlush,isDebugEnabled,TelemetryStats.markAllFlushed,activeFlushTargets,redactAttributes,makeRedactor - Dependent files:
@opentelemetry/core,@opentelemetry/sdk-trace-base - Scopes:
@autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Wrap each target.forceFlush invocation in a promise boundary, such as Promise.resolve.then( => target.forceFlush), before passing the promises to Promise.allSettled. Preserve the aggregate fulfilled check and ensure the timeout is cleared in a finally block if doFlush exits unexpectedly. Blast radius — if this defect reaches production it can fail the downstream usage that depends on this file: functions `installAutterAutoFlush`, `debugLog`, `CountingExporter.forceFlush`, `isDebugEnabled`, `TelemetryStats.markAllFlushed`, `activeFlushTargets`, `redactAttributes`, `makeRedactor`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/core`, `@opentelemetry/sdk-trace-base`.
Flagged by Autter security & observability checks.
| ancestors.set(value, out); | ||
|
|
||
| for (const item of value) { | ||
| out.push(redactValue(item, r, ancestors)); |
There was a problem hiding this comment.
🟠 [ai] Unbounded recursive redaction can overflow the call stack — Risk: 65/100
redactValue now recursively traverses arbitrary acyclic nesting with no depth limit. A sufficiently deeply nested attribute supplied to redactAttributes or a makeRedactor result can trigger a RangeError and escape the redaction path. Add a bounded traversal strategy or depth/work limit so malformed attributes cannot throw from telemetry capture.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Functions/symbols:
redactAttributes,makeRedactor,redactString,redactValue,isSensitiveKey,redactWith,installAutterAutoFlush - Dependent files:
@opentelemetry/api - Scopes:
@autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Keep recursive redaction bounded so arbitrarily deep attribute values cannot cause a RangeError. Reintroduce a maximum depth or implement an iterative traversal with an explicit work limit, while retaining WeakMap-based cycle handling and ensuring deeper values are handled safely without throwing. Blast radius — if this defect reaches production it can fail the downstream usage that depends on this file: functions `redactAttributes`, `makeRedactor`, `redactString`, `redactValue`, `isSensitiveKey`, `redactWith`, `installAutterAutoFlush`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/api`.
Flagged by Autter security & observability checks.
| return out; | ||
| } | ||
| return value; | ||
| function redactValue( |
There was a problem hiding this comment.
🟠 [ai] High complexity in redactValue — Risk: 50/100
cyclomatic complexity 10 in redactValue, with nested array/object handling, cycle detection, loops, recursion, and conditional masking; this makes the redaction behavior harder to reason about and maintain. Blast radius — if this issue ships it degrades the downstream usage that depends on this file: functions redactAttributes, makeRedactor, redactString, redactValue, isSensitiveKey, redactWith, installAutterAutoFlush; scopes @autter/runtime-node; dependent files @opentelemetry/api.
Flagged by the Complexity Guard agent.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Functions/symbols:
redactAttributes,makeRedactor,redactString,redactValue,isSensitiveKey,redactWith,installAutterAutoFlush - Dependent files:
@opentelemetry/api - Scopes:
@autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Refactor redactValue into focused helpers for processing arrays, objects, and cycle tracking, keeping redactValue as a shallow dispatcher with early returns; preserve circular-reference handling and sensitive-key masking.
Flagged by Autter security & observability checks.
🚦 Pre-merge checks ·
|
| Check | Status | Explanation |
|---|---|---|
| Removed observability | 1 potential issue(s) detected (max risk 55/100): packages/runtime-node/src/redact.ts:329. | |
| Silent exception swallowing | 3 potential issue(s) detected (max risk 68/100): packages/runtime-node/src/redact.ts:158, packages/runtime-node/src/redact.ts:272, packages/runtime-node/src/redact.ts:385. | |
| Possible non-atomic read-modify-write | 1 potential issue(s) detected (max risk 55/100): packages/runtime-node/src/redact.ts:377. | |
| Batch size limit not detected | 2 potential issue(s) detected (max risk 58/100): packages/runtime-node/src/redact.ts:170, packages/runtime-node/src/redact.ts:136. | |
| Missing linked tracker issue | 1 potential issue(s) detected (max risk 60/100): packages/runtime-node/src/redact.ts:134. | |
| Missing CODEOWNERS reviewer approval | 1 potential issue(s) detected (max risk 80/100): packages/runtime-node/src/redact.ts:134. | |
| Missing security-team review on sensitive path | 1 potential issue(s) detected (max risk 84/100): packages/runtime-node/src/redact.ts:134. | |
| Source changes without matching tests | 1 potential issue(s) detected (max risk 75/100): packages/runtime-node/src/redact.ts:149. | |
| Multi-write without transaction wrapper | 1 potential issue(s) detected (max risk 90/100): packages/runtime-node/src/redact.ts:193. | |
| Generic placeholder identifier in production logic | 1 potential issue(s) detected (max risk 45/100): packages/runtime-node/src/redact.ts:200. | |
| Comment contradicts or fabricates code behaviour | 1 potential issue(s) detected (max risk 60/100): packages/runtime-node/src/redact.ts:305. | |
| Code style differs from rest of codebase | 1 potential issue(s) detected (max risk 45/100): packages/runtime-node/src/redact.ts:138. | |
| Public route touches private/PII data | 1 potential issue(s) detected (max risk 88/100): packages/runtime-node/src/redact.ts:348. | |
| Runtime error risk | 3 finding(s) on changed lines. | |
| Excessive complexity | 1 finding(s) on changed lines. | |
| Complexity Guard | 3 finding(s) on changed lines. | |
| Bundle Size Monitor | 2 finding(s) on changed lines. |
✅ Passed checks (152)
| Check | Status | Explanation |
|---|---|---|
| Too many files changed | ✅ Passed | Changed 2 file(s), within the limit of 50. |
| Too many lines changed | ✅ Passed | Changed 433 line(s), within the limit of 1000. |
| Too many unrelated chapters | ✅ Passed | 2 chapter(s) detected, within the limit of 6. |
| Generated files hiding real changes | ✅ Passed | Generated-file volume (0 lines) does not obscure the 433 hand-written line(s). |
| Missing PR context | ✅ Passed | PR context looks sufficient. |
| Mixed concerns (refactor + behavior change) | ✅ Passed | This PR is a focused behavior fix in runtime redaction plus regression tests; it does not mix a refactor-only cleanup with a separate behavior change. |
| Migration + app logic + UI combined in one PR | ✅ Passed | No database migrations, application logic, and UI changes are present in this PR. |
| 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. |
| 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. |
| 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. |
| Pagination not detected | ✅ Passed | No pagination not detected issues detected. |
| Publicly exposed storage | ✅ Passed | No publicly exposed storage issues detected. |
| Over-permissive IAM policy | ✅ Passed | No over-permissive iam policy issues detected. |
| Security group open to the internet | ✅ Passed | No security group open to the internet issues detected. |
| Unencrypted storage at rest | ✅ Passed | No unencrypted storage at rest issues detected. |
| Infrastructure missing access logging | ✅ Passed | No infrastructure missing access logging issues detected. |
| Hardcoded secret in IaC | ✅ Passed | No hardcoded secret in iac issues detected. |
| Infrastructure misconfiguration | ✅ Passed | No infrastructure misconfiguration issues detected. |
| Deprecated Kubernetes API version | ✅ Passed | No deprecated kubernetes api version issues detected. |
| Compound IaC attack chain | ✅ Passed | No compound iac attack chain 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. |
| 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. |
| 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. |
| 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. |
| 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. |
| Direct env-var access bypasses config module | ✅ Passed | No direct env-var access bypasses config module 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. |
| 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. |
| 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. |
| Doc-copy code with insecure defaults | ✅ Passed | No doc-copy code with insecure defaults 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. |
| 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. |
| API key in source | ✅ Passed | No api key in source detected. |
| 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. |
| 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. |
| Code correctness issue | ✅ Passed | No additional explanation was reported. |
| 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. |
| Simplifiable code | ✅ Passed | No additional explanation was reported. |
| Unnecessary type assertion | ✅ Passed | No additional explanation was reported. |
| Module smell | ✅ Passed | No additional explanation was reported. |
| Dead export (no callers) | ✅ Passed | No additional explanation was reported. |
| Code duplication / DRY violation | ✅ Passed | No additional explanation was reported. |
This comment is updated automatically whenever Autter reviews a new PR revision.
🔇 15 finding(s) suppressed as likely false positives by Autter's verification passThese were flagged by a detector but a second, full-file verification judged them not to be real issues. Listed here for transparency — review if you disagree.
🔕 1 finding(s) muted by prior team feedbackYour team previously dismissed or reacted 👎 to findings of the same type on these files, so Autter kept them out of inline comments. Listed here in case circumstances changed.
|
5fd9311 to
93f6c84
Compare
There was a problem hiding this comment.
🔴 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.
| const out: unknown[] = []; | ||
| ancestors.set(value, out); | ||
|
|
||
| for (const item of value) { |
There was a problem hiding this comment.
🟠 [ai] Runtime error risk — Risk: 75/100
The new traversal processes every array element recursively with no item, property, or total-work limit. A very large finite attribute structure can consume excessive CPU and memory during telemetry capture and block the host application; this affects redactAttributes and makeRedactor in @autter/runtime-node. This touches exported/public surface code, so impact assessment should consider downstream callers. Blast radius — if this defect reaches production it can fail the downstream usage that depends on this file: functions redactAttributes, makeRedactor, redactString, redactValue, isSensitiveKey, redactWith; scopes @autter/runtime-node; dependent files @opentelemetry/api.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Functions/symbols:
redactAttributes,makeRedactor,redactString,redactValue,isSensitiveKey,redactWith - Dependent files:
@opentelemetry/api - Scopes:
@autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Add a finite traversal budget, including limits for total visited values and collection entries, and stop safely when the budget is exhausted. Preserve masking of sensitive keys and cycle handling while ensuring pathological attribute trees cannot monopolize telemetry capture. Blast radius — if this defect reaches production it can fail the downstream usage that depends on this file: functions `redactAttributes`, `makeRedactor`, `redactString`, `redactValue`, `isSensitiveKey`, `redactWith`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/api`.
Flagged by Autter security & observability checks.
|
Autter's deep review traced 1 finding(s) to file(s) this PR does not change — they can't be shown as inline comments, but the change still affects them: 🟠 [ai] Active-server FlushTarget still fulfills when built-in exporters reject (risk 78/100)
The production call chain
|
|
Autter found 3 issue(s) it could not attach to the current diff (the anchor line is not part of a diff hunk, or the PR advanced during the review): 🟠 [ai] Active-server flush still reports exporter failures as success (risk 78/100)
The lifecycle aggregate now treats a target as failed only when its returned promise rejects, but the production target registered by
🟠 [ai] Synchronous FlushTarget throws bypass the failed-flush result (risk 72/100)
The changed aggregate check only observes promises after
🟠 [ai] Synchronous forceFlush throws escape Promise.allSettled (risk 72/100)
|
🧪 Autter test runAutter checked Execution summary: 6 checks executed · 6 passed. Project test commands
Autter targeted verification6 tests executed · 6 passed. Declared tests: 8 test file(s) found — 0 ran, 0 not observed in suite output, 8 did not run.
|
| Changed file | Related test | Result |
|---|---|---|
packages/runtime-node/src/redact.ts |
packages/runtime-node/test/redact.test.mjs |
✅ existing test passes |
🤖 Coverage-check evidence
packages/runtime-node/src/redact.ts
Ran: cd packages/runtime-node && npm run build && node --test test/redact.test.mjs; node .autter/scratch/redact-pr19-probe.mjs
Build succeeded; declared redact suite: 24/24 tests passed, including deep traversal bounds, hostile proxy/getter handling, collection truncation, usage-token masking, and circular-object masking. Targeted probe passed: self-referential arrays/objects JSON-serialize with cycle masks, 5,000-deep input does not throw, and a 1,001-entry array is truncated.
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 -w @autter/runtime-node. — verified by agent execution - ✅ Run
node --test packages/runtime-node/test/redact.test.mjs. — verified by agent execution - ✅ Call
redactAttributeswith nested objects and arrays containing passwords, emails, bearer tokens, and supported primitive values; verify secrets are masked and safe values are retained. — verified by agent execution - ✅ Exercise a deeply nested object and an oversized array/object graph; verify redaction returns without throwing. — verified by agent execution
- ✅ Exercise self-referential objects and arrays, plus throwing getters or revoked proxies; verify capture-safe redaction behavior and that the returned value can be passed through telemetry serialization safely. — verified by agent execution
🤖 Agent-executed checks
✅ Run npm run build -w @autter/runtime-node.
Ran: npm ci --include=dev && npm run build -w @autter/runtime-node
tsup v8.5.1 built ESM, CJS, and DTS outputs successfully.
✅ Run node --test packages/runtime-node/test/redact.test.mjs.
Ran: node --test packages/runtime-node/test/redact.test.mjs
24 tests passed; 0 failed.
✅ Call redactAttributes with nested objects and arrays containing passwords, emails, bearer tokens, and supported primitive values; verify secrets are masked and safe values are retained.
Ran: node .autter/scratch/redact-pr19-probe.mjs
probe: nested masking and safe primitives passed
✅ Exercise a deeply nested object and an oversized array/object graph; verify redaction returns without throwing.
Ran: node .autter/scratch/redact-pr19-probe.mjs
probe: 5000-deep and 1005-entry bounds passed
✅ Exercise self-referential objects and arrays, plus throwing getters or revoked proxies; verify capture-safe redaction behavior and that the returned value can be passed through telemetry serialization safely.
Ran: node .autter/scratch/redact-pr19-probe.mjs
probe: circular and hostile values serialize safely
⬜ items could not be verified automatically and still need a manual check.
|
Autter found 1 issue(s) it could not attach to the current diff (the anchor line is not part of a diff hunk, or the PR advanced during the review): 🟠 [verified] Change fails its execution check: packages/runtime-node/src/lifecycle.ts (risk 70/100)
No declared test covers this PR's change to Ran: |
| ancestors.set(value, out); | ||
|
|
||
| for (const item of value) { | ||
| out.push(redactValue(item, r, ancestors)); |
There was a problem hiding this comment.
🔴 [ai] Unbounded recursive traversal can throw during telemetry capture — Risk: 82/100
The new redaction walk recursively descends every nested array/object and has no depth, item, or work limit. A sufficiently deeply nested caller-supplied attribute can therefore hit a JavaScript RangeError, while a very large finite tree can monopolize CPU and memory. Because this function is called synchronously by the active server capture paths before the span is created, the exception escapes telemetry capture instead of being handled as a best-effort redaction failure; malformed attributes can make the application-facing capture call fail and prevent the error or message from being recorded. Cycle detection only handles repeated ancestors and does not bound acyclic depth or total work.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
packages/runtime-node/src/redact.ts,packages/runtime-node/src/server.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Bound redaction traversal with an explicit depth/work budget or use an iterative bounded walk. Preserve cycle handling, and define a safe behavior for values beyond the budget so redactAttributes and makeRedactor never throw on arbitrarily deep or oversized caller attributes.
Flagged by Autter security & observability checks.
There was a problem hiding this comment.
🔴 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.
| return true; | ||
| } | ||
|
|
||
| function redactValue( |
There was a problem hiding this comment.
🟠 [ai] Excessive complexity — Risk: 50/100
redactValue still combines type dispatch, cycle tracking, traversal budgeting, array/object iteration, truncation, and sensitive-key masking in one function, making the redaction path difficult to maintain. This affects the exported redactAttributes and makeRedactor functions in @autter/runtime-node and their telemetry capture callers. Blast radius — if this hygiene issue is left in it makes the downstream usage that depends on this file harder to change safely: functions redactAttributes, makeRedactor, redactString, redactValue, isSensitiveKey, redactWith; scopes @autter/runtime-node; dependent files @opentelemetry/api.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Functions/symbols:
redactAttributes,makeRedactor,redactString,redactValue,isSensitiveKey,redactWith - Dependent files:
@opentelemetry/api - Scopes:
@autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Refactor redactValue into a shallow dispatcher with focused array and object helpers. Keep traversal state, depth/work limits, ancestor cycle handling, collection truncation, and sensitive-key masking behavior unchanged, and update redactAttributes and makeRedactor to use the refactored helpers. Blast radius — if this hygiene issue is left in it makes the downstream usage that depends on this file harder to change safely: functions `redactAttributes`, `makeRedactor`, `redactString`, `redactValue`, `isSensitiveKey`, `redactWith`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/api`.
Flagged by Autter security & observability checks.
| : redactValue(v, r, state, depth + 1); | ||
| } | ||
|
|
||
| if (Object.keys(value).length > MAX_COLLECTION_ENTRIES) { |
There was a problem hiding this comment.
🟡 [ai] Simplifiable code — Risk: 32/100
The object is enumerated once with Object.entries and then enumerated again with Object.keys solely to detect truncation. Track whether the entry limit was reached during the existing loop instead of performing a second full enumeration. Blast radius — if this hygiene issue is left in it makes the downstream usage that depends on this file harder to change safely: functions redactAttributes, makeRedactor, redactString, redactValue, isSensitiveKey, redactWith; scopes @autter/runtime-node; dependent files @opentelemetry/api.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Functions/symbols:
redactAttributes,makeRedactor,redactString,redactValue,isSensitiveKey,redactWith - Dependent files:
@opentelemetry/api - Scopes:
@autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Replace the Object.keys(value).length check with a truncation flag maintained by the existing Object.entries loop. Preserve the current behavior of adding __redaction_truncated__ when more than MAX_COLLECTION_ENTRIES properties are present. Blast radius — if this hygiene issue is left in it makes the downstream usage that depends on this file harder to change safely: functions `redactAttributes`, `makeRedactor`, `redactString`, `redactValue`, `isSensitiveKey`, `redactWith`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/api`.
Flagged by Autter security & observability checks.
| state.ancestors.set(value, out); | ||
|
|
||
| let count = 0; | ||
| for (const [k, v] of Object.entries( |
There was a problem hiding this comment.
🟠 [ai] Runtime error risk — Risk: 65/100
Object.entries materializes every enumerable property before the MAX_COLLECTION_ENTRIES check at line 192, so a very large nested object can still allocate and enumerate its entire contents despite the collection limit. This can consume excessive CPU and memory during synchronous redaction on @autter/runtime-node capture paths. This touches exported/public surface code, so impact assessment should consider downstream callers. Blast radius — if this defect reaches production it can fail the downstream usage that depends on this file: functions redactAttributes, makeRedactor, redactString, redactValue, isSensitiveKey, redactWith; scopes @autter/runtime-node; dependent files @opentelemetry/api.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Functions/symbols:
redactAttributes,makeRedactor,redactString,redactValue,isSensitiveKey,redactWith - Dependent files:
@opentelemetry/api - Scopes:
@autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Implement the collection cap without first materializing the full entry list. Iterate lazily over enumerable properties, stop after MAX_COLLECTION_ENTRIES, and record truncation when another enumerable property exists. Ensure the implementation preserves sensitive-key masking and does not introduce uncaught accessor or proxy errors. Blast radius — if this defect reaches production it can fail the downstream usage that depends on this file: functions `redactAttributes`, `makeRedactor`, `redactString`, `redactValue`, `isSensitiveKey`, `redactWith`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/api`.
Flagged by Autter security & observability checks.
There was a problem hiding this comment.
Autter completed PR review for #19: 6 finding(s) remain below the merge-blocking bar, so this review stays neutral rather than approving. (Also detected: 5 finding(s) dismissed as likely false positives by verification and 1 finding(s) muted by earlier reviewer feedback.) See the findings below; the task checklist follows as the review's final comment.
|
|
||
| out[k] = isSensitiveKey(k, r) | ||
| ? r.mask | ||
| : redactValue(v, r, state, depth + 1); |
There was a problem hiding this comment.
🔴 [ai] Throwing accessors or proxy traps escape synchronous telemetry capture — Risk: 82/100
A caller can pass an object with a getter that throws while its value is read by the Object.entries result, or a Proxy whose ownKeys, descriptor, or getter trap throws. That exception propagates through redactValue and redactWith because there is no catch or safe fallback around traversal. The production captureException path calls activeRedactor(attributes) inline while constructing the span, and captureMessage does the same, so malformed attributes can make these void capture APIs throw before a span is created and prevent the original telemetry from being recorded. The new depth/work limits do not cover this failure mode.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
packages/runtime-node/src/redact.ts,packages/runtime-node/src/server.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Make redaction fail open for hostile object graphs: guard property enumeration and reads (including proxy/accessor traps), preserve already-redacted output where possible, and substitute the mask or omit the failing property so redactAttributes/makeRedactor do not throw into captureException or captureMessage.
Flagged by Autter security & observability checks.
| state.ancestors.set(value, out); | ||
|
|
||
| let count = 0; | ||
| for (const [k, v] of Object.entries( |
There was a problem hiding this comment.
🟠 [ai] Object redaction materializes the entire collection before applying its cap — Risk: 76/100
The object traversal uses Object.entries(value) at line 189, which eagerly creates an array containing every enumerable property before the loop can stop at MAX_COLLECTION_ENTRIES on line 192. It then calls Object.keys(value) at line 200, enumerating the complete key set a second time. Thus a caller can supply a very large nested object and still force full enumeration and allocation despite the stated 1,000-entry limit, consuming excessive synchronous CPU and memory in capture paths. The limit must be applied during lazy enumeration, with truncation detected without materializing the full property list.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
packages/runtime-node/src/redact.ts,packages/runtime-node/src/server.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Replace `Object.entries`/`Object.keys` with lazy enumerable-property iteration that processes at most `MAX_COLLECTION_ENTRIES` values and detects one additional enumerable property to mark truncation, while retaining sensitive-key masking.
Flagged by Autter security & observability checks.
| state.ancestors.set(value, out); | ||
|
|
||
| let count = 0; | ||
| for (const [k, v] of Object.entries( |
There was a problem hiding this comment.
🟠 [ai] Throwing nested accessors can escape telemetry capture — Risk: 76/100
The new arbitrary-object walk directly executes Object.entries(value) and subsequently reads each property value, so a nested object with an enumerable getter that throws causes redactValue to throw synchronously. Proxy traps can likewise throw during enumeration or key inspection. The active redactor is called directly by captureException and captureMessage before spans are created, with no surrounding redaction error boundary, so malformed caller attributes can make those production capture calls fail instead of failing open. The traversal needs guarded property access/enumeration or a safe fallback that cannot let accessor/proxy errors escape.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
packages/runtime-node/src/redact.ts,packages/runtime-node/src/server.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Guard nested enumeration and property reads against getter/proxy exceptions, and return a safe masked/truncated representation when an object cannot be inspected so capture callers remain best-effort and non-throwing.
Flagged by Autter security & observability checks.
|
|
||
| for (const [key, value] of Object.entries(attributes)) { | ||
| if (value === undefined) continue; | ||
| out[key] = isSensitiveKey(key, r) |
There was a problem hiding this comment.
🟠 [ai] Numeric GenAI usage attributes are incorrectly redacted — Risk: 72/100
The PR removed the special-case handling for canonical GenAI usage keys and now applies the generic /token/ pattern to every key. Consequently, valid numeric values such as gen_ai.usage.input_tokens and gen_ai.usage.output_tokens are replaced with the mask instead of remaining usable counts. This is reachable through the exported redactAttributes/makeRedactor APIs and through caller-supplied attributes passed to makeSafeCapture and LLM metadata; it regresses the prior contract and can make downstream LLM accounting read zero or lose the reported usage values.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
packages/runtime-node/src/redact.ts,packages/runtime-node/src/server.ts,packages/otlp-ingester/src/llm.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Restore the canonical GenAI usage-key exception: preserve finite, non-negative numeric counts for the known usage keys, while masking non-numeric/invalid values and continuing to mask token-like keys outside that allowlist.
Flagged by Autter security & observability checks.
| state.ancestors.set(value, out); | ||
|
|
||
| let count = 0; | ||
| for (const [k, v] of Object.entries( |
There was a problem hiding this comment.
🟠 [ai] Collection cap still materializes and re-enumerates the entire object — Risk: 65/100
This is not safe for hostile or merely oversized caller attributes: Object.entries at line 189 constructs the complete enumerable key/value array before the loop reaches the MAX_COLLECTION_ENTRIES break, and line 200 then calls Object.keys(value) to enumerate the object again. A nested object with millions of properties therefore still incurs full allocation and enumeration synchronously before redaction returns, defeating the intended collection bound and allowing capture-time CPU/memory spikes. The issue is reachable from captureException and captureMessage, which invoke the redactor synchronously and have no failure/resource guard around it.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
packages/runtime-node/src/redact.ts,packages/runtime-node/src/server.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Iterate lazily over enumerable keys/properties and stop after MAX_COLLECTION_ENTRIES without calling Object.entries or a second full Object.keys enumeration. Detect truncation by probing for one additional enumerable property, while preserving masking and safely handling accessor/proxy errors.
Flagged by Autter security & observability checks.
| return value; | ||
| } | ||
|
|
||
| function isSensitiveKey(key: string, r: CompiledRedactor): boolean { |
There was a problem hiding this comment.
🟠 [ai] Redaction no longer preserves valid GenAI usage counts — Risk: 58/100
The changed isSensitiveKey contract now only tests the configured key patterns and removed the previous special case that allowed finite non-negative values for gen_ai.usage.input_tokens, output_tokens, prompt_tokens, completion_tokens, total_tokens, and token_count. Consequently redactAttributes and the compiled function returned by makeRedactor now mask any usage key matching /token/ even when its value is a valid numeric count. This breaks callers that depend on redaction preserving telemetry usage data: server LLM attribute construction passes caller extras through activeRedactor at packages/runtime-node/src/server.ts:517->529, and makeSafeCapture invokes the exported redactor at packages/runtime-node/src/server.ts:1050->1052; the resulting spans can lose token accounting or safe caller-provided usage attributes. The removed return-shape guarantee is not handled by those callers.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
packages/runtime-node/src/redact.ts,packages/runtime-node/src/server.ts,packages/runtime-node/src/llm-instrument.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Restore the canonical GenAI usage-key exception in isSensitiveKey: preserve finite non-negative numeric token counts and mask non-numeric, negative, or otherwise sensitive values, while retaining the general sensitive-key patterns for all other keys.
Flagged by Autter security & observability checks.
|
I’ve addressed the latest review findings in commit |
|
I’ve confirmed the PR #17 merged behavior around GenAI usage token counts and am preserving it in this revision. I’m also tightening the traversal to avoid eager whole-object materialization and guarding hostile property access during redaction. |
There was a problem hiding this comment.
🔴 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.
|
|
||
| const limit = Math.min(value.length, MAX_COLLECTION_ENTRIES); | ||
| for (let i = 0; i < limit; i += 1) { | ||
| out.push(redactValue(value[i], r, state, depth + 1)); |
There was a problem hiding this comment.
🟠 [ai] Unhandled edge case (null / empty / zero / boundary) — Risk: 65/100
Array element access is unguarded, so an enumerable array getter or Proxy can throw during redaction and escape telemetry capture despite the object path handling accessor failures. Blast radius — if this AI-generated slop ships it cascades to the downstream usage that depends on this file: functions redactAttributes, makeRedactor, redactString, redactValue, isSensitiveKey, redactWith; scopes @autter/runtime-node; dependent files @opentelemetry/api.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Functions/symbols:
redactAttributes,makeRedactor,redactString,redactValue,isSensitiveKey,redactWith - Dependent files:
@opentelemetry/api - Scopes:
@autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Wrap array length and element reads in guarded access. On an accessor or Proxy failure, insert the redaction mask (or safely truncate the array) and continue without throwing, preserving the traversal and work limits. Blast radius — if this AI-generated slop ships it cascades to the downstream usage that depends on this file: functions `redactAttributes`, `makeRedactor`, `redactString`, `redactValue`, `isSensitiveKey`, `redactWith`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/api`.
Flagged by Autter security & observability checks.
|
|
||
| const limit = Math.min(value.length, MAX_COLLECTION_ENTRIES); | ||
| for (let i = 0; i < limit; i += 1) { | ||
| out.push(redactValue(value[i], r, state, depth + 1)); |
There was a problem hiding this comment.
🟠 [ai] Runtime error risk — Risk: 75/100
Array elements are read without a guard, so an attribute array containing a throwing getter (or a proxy with a failing get trap) can make redactValue throw synchronously. This affects exported redactAttributes and makeRedactor and can cause runtime-node capture paths to fail instead of failing open. This touches exported/public surface code, so impact assessment should consider downstream callers. Blast radius — if this defect reaches production it can fail the downstream usage that depends on this file: functions redactAttributes, makeRedactor, redactString, redactValue, isSensitiveKey, redactWith; scopes @autter/runtime-node; dependent files @opentelemetry/api.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Functions/symbols:
redactAttributes,makeRedactor,redactString,redactValue,isSensitiveKey,redactWith - Dependent files:
@opentelemetry/api - Scopes:
@autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Wrap array element access in try/catch, masking or safely truncating the element when access fails. Also guard array inspection for revoked or throwing proxies so redactAttributes and makeRedactor never propagate redaction errors into telemetry capture. Blast radius — if this defect reaches production it can fail the downstream usage that depends on this file: functions `redactAttributes`, `makeRedactor`, `redactString`, `redactValue`, `isSensitiveKey`, `redactWith`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/api`.
Flagged by Autter security & observability checks.
| ): unknown { | ||
| if (typeof value === "string") return redactString(value, r); | ||
|
|
||
| if (Array.isArray(value)) { |
There was a problem hiding this comment.
🔴 [ai] Array traversal leaves throwing proxy and accessor operations unhandled — Risk: 85/100
The array branch performs several hostile-object operations outside any try/catch: Array.isArray(value) can throw for a revoked Proxy, value.length can throw through a Proxy, each value[i] read can throw through an accessor or get trap, and the second value.length read can also throw. A caller-supplied array with one of these failures therefore escapes redactValue rather than inserting a mask or truncating safely. Since captureException and captureMessage invoke the active redactor synchronously while constructing the span, the failure can abort capture before telemetry is recorded.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
packages/runtime-node/src/redact.ts,packages/runtime-node/src/server.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Wrap array classification, length reads, element reads, and truncation checks in guarded operations. On a failed read, append the configured mask or a truncation marker and continue/return without propagating the redaction error, while retaining cycle and work limits.
Flagged by Autter security & observability checks.
| remainingWork: MAX_REDACTION_WORK, | ||
| }; | ||
|
|
||
| for (const [key, value] of Object.entries(attributes)) { |
There was a problem hiding this comment.
🔴 [ai] Top-level proxy enumeration can abort active telemetry capture — Risk: 84/100
The changed redaction entry point still calls Object.entries(attributes) without a guard. A caller-controlled Proxy can throw from ownKeys, getOwnPropertyDescriptor, or a getter while Object.entries is constructing the entries, so redactWith throws before returning any attributes. The active server invokes this redactor inline in captureException, captureMessage, and withProcessSpan, meaning those callers do not receive their prior best-effort/non-throwing behavior and an exception or message can be lost before its span is created (or the process callback is entered).
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
packages/runtime-node/src/redact.ts,packages/runtime-node/src/server.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Wrap top-level attribute enumeration and reads in the same fail-open boundary used for nested objects; on an enumeration failure return the attributes accumulated so far plus a masked truncation marker, without allowing redactAttributes or makeRedactor to throw.
Flagged by Autter security & observability checks.
| const out: unknown[] = []; | ||
| state.ancestors.set(value, out); | ||
|
|
||
| const limit = Math.min(value.length, MAX_COLLECTION_ENTRIES); |
There was a problem hiding this comment.
🔴 [ai] Array proxy length or element traps still escape redaction callers — Risk: 82/100
The array traversal reads value.length and then value[i] without guards. An array-like Proxy that reports a throwing length getter, a revoked Proxy passed to Array.isArray, or an array with a throwing indexed accessor can therefore throw from redactValue. That error propagates through redactWith and the active server's inline redactor calls, so captureException/captureMessage can fail before creating a span and withProcessSpan can fail before invoking the user's callback. The object branch's catch does not protect this array path.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
packages/runtime-node/src/redact.ts,packages/runtime-node/src/server.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Guard Array.isArray, length inspection, and each indexed element read. On any trap/accessor failure, append the configured mask or a truncation marker and continue/return a safe array without propagating the redaction error.
Flagged by Autter security & observability checks.
| const out: unknown[] = []; | ||
| state.ancestors.set(value, out); | ||
|
|
||
| const limit = Math.min(value.length, MAX_COLLECTION_ENTRIES); |
There was a problem hiding this comment.
🔴 [ai] Hostile array traps can bypass the redaction boundary and abort capture — Risk: 82/100
The nested-array branch reads value.length and then each value[i] without a protective catch. A caller can supply an array Proxy whose length/get trap throws, or an array with a throwing accessor, causing redactValue to propagate synchronously instead of returning a masked/truncated value. The redactor is invoked inline while constructing the error/message span, so captureException and captureMessage can throw before a span is created; this is not safe for the best-effort telemetry path and can prevent the original event from being recorded.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
packages/runtime-node/src/redact.ts,packages/runtime-node/src/server.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Guard Array.isArray/length inspection and every array element read. On a trap or accessor failure, append the redaction mask or stop with a masked truncation marker, and ensure the redactor never throws into captureException or captureMessage.
Flagged by Autter security & observability checks.
|
|
||
| const limit = Math.min(value.length, MAX_COLLECTION_ENTRIES); | ||
| for (let i = 0; i < limit; i += 1) { | ||
| out.push(redactValue(value[i], r, state, depth + 1)); |
There was a problem hiding this comment.
🔴 [ai] Guard array inspection and element reads during redaction — Risk: 82/100
The array branch reads value.length at line 166 and each value[i] at this line without a catch. An enumerable array getter, a Proxy get trap, or a revoked Proxy can throw synchronously, so redactAttributes/makeRedactor propagate the exception instead of returning a safe result. This is reachable from production capture paths such as captureException and captureMessage, which spread activeRedactor(attributes) inline before creating their spans; one hostile array attribute can therefore prevent the original telemetry from being recorded.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
packages/runtime-node/src/redact.ts,packages/runtime-node/src/server.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Wrap array length and indexed element access in guarded reads, and on failure insert the redaction mask or safely truncate the output while preserving the traversal and work limits. Also guard array classification/inspection for throwing or revoked proxies so the public redactors remain best-effort and non-throwing.
Flagged by Autter security & observability checks.
| remainingWork: MAX_REDACTION_WORK, | ||
| }; | ||
|
|
||
| for (const [key, value] of Object.entries(attributes)) { |
There was a problem hiding this comment.
🔴 [ai] Root attribute enumeration can still throw out of telemetry capture — Risk: 82/100
redactWith invokes Object.entries(attributes) without an error boundary. A caller can provide a Proxy whose ownKeys, property descriptor, or getter trap throws while the root attributes are materialized; that exception escapes redactAttributes/makeRedactor instead of returning a best-effort result. The production captureException and captureMessage paths call activeRedactor(attributes) inline before creating the span, so malformed attributes can prevent the original telemetry from being recorded.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
packages/runtime-node/src/redact.ts,packages/runtime-node/src/server.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Guard root attribute enumeration and reads in redactWith. On a root proxy/enumeration failure, preserve entries already copied and return a safe truncated or masked result so redaction remains non-throwing for captureException and captureMessage.
Flagged by Autter security & observability checks.
| remainingWork: MAX_REDACTION_WORK, | ||
| }; | ||
|
|
||
| for (const [key, value] of Object.entries(attributes)) { |
There was a problem hiding this comment.
🟠 [ai] Guard top-level attribute enumeration against proxy and accessor failures — Risk: 78/100
redactWith still calls Object.entries(attributes) without an exception boundary. A caller can supply an attributes Proxy whose ownKeys, descriptor, or getter trap throws; unlike nested object traversal, this failure escapes before any partial output is returned. redactWith is called directly by the active redactor in captureException and captureMessage while constructing the span, so malformed top-level attributes can make these capture APIs throw before a span is created, violating the best-effort redaction behavior implemented for nested objects.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
packages/runtime-node/src/redact.ts,packages/runtime-node/src/server.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Enumerate top-level attributes through a guarded path. Preserve already-redacted entries where possible and omit or mask the property that fails, or return a safely truncated partial result, without allowing proxy/accessor exceptions to escape the public redactors or telemetry capture callers.
Flagged by Autter security & observability checks.
|
I’ve addressed the current array redaction safety finding locally by guarding array length and indexed element access, with a regression test; local runtime-node build and 20/20 redaction tests pass. I’m also reviewing the remaining top-level proxy/enumeration and complexity findings before the next push. |
There was a problem hiding this comment.
🔴 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.
| }; | ||
|
|
||
| try { | ||
| for (const key in attributes as Record<string, unknown>) { |
There was a problem hiding this comment.
🟠 [ai] Top-level attributes bypass redaction collection limits — Risk: 50/100
redactWith has no top-level entry limit or work-budget check, so an unusually large caller-supplied attributes object can cause unbounded synchronous enumeration and output allocation. Apply the same collection/work bounds to top-level attributes.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Functions/symbols:
redactAttributes,makeRedactor,redactString,redactValue,isSensitiveKey,redactWith - Dependent files:
@opentelemetry/api - Scopes:
@autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Bound top-level attribute enumeration with MAX_COLLECTION_ENTRIES and the remaining-work budget, and mark or safely truncate the output when the limit is reached. Ensure this remains fail-open without throwing. Blast radius — if this AI-generated slop ships it cascades to the downstream usage that depends on this file: functions `redactAttributes`, `makeRedactor`, `redactString`, `redactValue`, `isSensitiveKey`, `redactWith`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/api`.
Flagged by Autter security & observability checks.
| depth: number, | ||
| ): unknown { | ||
| const existing = state.ancestors.get(value); | ||
| if (existing) return existing; |
There was a problem hiding this comment.
🟠 [ai] Runtime error risk — Risk: 75/100
Returning the previously allocated output for an ancestor preserves cycles in the redacted result. For example, an attribute containing an array that references itself produces a cyclic array, so downstream serialization such as JSON.stringify can throw a TypeError and prevent telemetry capture through the exported redactAttributes and makeRedactor APIs in @autter/runtime-node. This touches exported/public surface code, so impact assessment should consider downstream callers. Blast radius — if this defect reaches production it can fail the downstream usage that depends on this file: functions redactAttributes, makeRedactor, redactString, redactValue, isSensitiveKey, redactWith; scopes @autter/runtime-node; dependent files @opentelemetry/api.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Functions/symbols:
redactAttributes,makeRedactor,redactString,redactValue,isSensitiveKey,redactWith - Dependent files:
@opentelemetry/api - Scopes:
@autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
When a value is already present in state.ancestors, return the redaction mask (or another non-cyclic safe representation) instead of returning the existing output container. Add coverage for self-referential arrays and objects and verify that redactAttributes and makeRedactor results can be serialized without throwing. Blast radius — if this defect reaches production it can fail the downstream usage that depends on this file: functions `redactAttributes`, `makeRedactor`, `redactString`, `redactValue`, `isSensitiveKey`, `redactWith`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/api`.
Flagged by Autter security & observability checks.
| return out; | ||
| } | ||
|
|
||
| function redactObject( |
There was a problem hiding this comment.
🟡 [ai] Moderate control-flow complexity in redactObject — Risk: 30/100
redactObject combines enumeration, enumerable-property filtering, collection-limit handling, guarded property reads, and truncation/exception handling in one helper. The control flow is more complex than a shallow dispatcher; extracting enumeration/truncation or property-reading logic would improve maintainability.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Functions/symbols:
redactAttributes,makeRedactor,redactString,redactValue,isSensitiveKey,redactWith - Dependent files:
@opentelemetry/api - Scopes:
@autter/runtime-node
🛠 AI fix prompt (copy & paste into your coding agent)
Refactor redactObject into focused helpers for safe property enumeration/access and collection-limit handling. Keep the existing redaction budget, masking, getter/proxy protection, cycle tracking, and truncation behavior unchanged.
Flagged by Autter security & observability checks.
| depth: number, | ||
| ): unknown { | ||
| const existing = state.ancestors.get(value); | ||
| if (existing) return existing; |
There was a problem hiding this comment.
🔴 [ai] Cycle handling returns cyclic output containers — Risk: 82/100
When a nested value refers to an ancestor, the redaction walk returns the previously allocated output container instead of a safe leaf value. Thus redactAttributes({ context: selfReferentialObject }) produces an output whose context.self points back to context; the same occurs for arrays. The result is not safely serializable, so an OpenTelemetry exporter or downstream span serialization that calls JSON.stringify can throw after captureException or captureMessage has synchronously passed these attributes through activeRedactor, preventing the telemetry from being exported. This is not merely a test concern: the changed test at lines 256-268 asserts the cyclic result rather than proving serializability.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
packages/runtime-node/src/redact.ts,packages/runtime-node/src/server.ts,packages/runtime-node/test/redact.test.mjs
🛠 AI fix prompt (copy & paste into your coding agent)
When state.ancestors already contains the current object or array, return the redaction mask (or another acyclic safe representation) instead of the existing output container. Add coverage asserting that results from redactAttributes and makeRedactor can be JSON serialized for self-referential objects and arrays.
Flagged by Autter security & observability checks.
|
Autter found 1 issue(s) it could not attach to the current diff (the anchor line is not part of a diff hunk, or the PR advanced during the review): 🟠 [verified] Change fails its execution check: packages/runtime-node/src/redact.ts (risk 70/100)
No declared test covers this PR's change to Ran: |
There was a problem hiding this comment.
🔴 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.
There was a problem hiding this comment.
Autter approved #19: no unresolved findings remain after review — 23 finding(s) dismissed as likely false positives by verification and 2 finding(s) muted by earlier reviewer feedback; they stay listed with their verdicts in the Autter review dashboard. Generated the PR summary; the task checklist follows as the review's final comment.
| const attributes = {}; | ||
|
|
||
| for (let i = 0; i < 1005; i += 1) { | ||
| attributes["key_" + i] = "value"; |
There was a problem hiding this comment.
🟠 [deterministic] Biome: lint/style/useTemplate — Risk: 55/100
Template literals are preferred over string concatenation.
🛠 AI fix prompt (copy & paste into your coding agent)
Fix the Biome `lint/style/useTemplate` issue at packages/runtime-node/test/redact.test.mjs:260: Template literals are preferred over string concatenation.
Flagged by Autter security & observability checks.
Autter task list
Generated from PR diff, blast radius, and context. Issues found
Also detected but not listed above: 25 finding(s) dismissed as likely false positives by verification — see the Autter review dashboard for their verdicts. 🛠 Fix optionsCheck one option and Autter will start a fix run for the unresolved issues above.
Checking a box triggers the fix run immediately — Autter comments back with the issues being fixed and the branch created for each. |
2 participants
Summary
This PR fixes a redaction gap in the Node runtime where sensitive attributes nested beyond the previous fixed traversal depth could remain unredacted.
The existing redaction implementation used a fixed maximum traversal depth of
4. While this was sufficient for common attribute structures, callers can provide arbitrary nested objects. As a result, sensitive keys located deeper than the configured traversal depth were not inspected and could potentially retain their original values.This change removes the fixed traversal-depth limitation and makes nested attribute redaction recursive while also adding protection for circular object references.
Problem
The previous implementation passed a depth value of
4into the recursive redaction function.For example, a structure such as:
context
└── level1
└── level2
└── level3
└── level4
└── password: "SECRET"
could reach the traversal limit before the sensitive
passwordkey was inspected.This created a redaction gap for deeply nested caller-provided objects.
What Changed
1. Removed the fixed traversal depth
The redaction path no longer relies on
maxDepth: 4.Instead, nested objects and arrays continue to be traversed recursively until their reachable values have been processed.
This ensures sensitive keys are not left unredacted simply because they occur deeper than an arbitrary traversal limit.
2. Added circular-reference protection
Recursive traversal introduces another important edge case: circular object graphs.
For example:
const context = {};
context.self = context;
A naive recursive implementation would continue following
selfindefinitely and eventually fail with a stack overflow.To prevent this, the implementation uses a
WeakMapto keep track of objects currently being traversed and their corresponding redacted copies.When an already-visited object is encountered, the previously created redacted copy is reused instead of recursively traversing the same object again.
This allows circular structures to be handled safely while preserving the circular relationship in the resulting redacted object.
3. Preserved existing redaction behavior
The existing sensitive-key matching and string-value redaction logic remain in place.
Sensitive keys such as passwords, tokens, credentials, authorization values, etc. continue to be masked using the configured redaction marker.
String values continue to go through the existing value-level scrubbing logic.
Arrays are also traversed element-by-element.
4. Added regression coverage for deep nesting
A regression test was added with a sensitive
passwordattribute nested beyond the previous traversal depth.The test verifies that the sensitive value is still replaced with the redaction mask.
This ensures the original depth-related gap does not regress in the future.
5. Added circular-reference test coverage
A dedicated test was added for circular references.
The test verifies that:
Validation
The following checks were performed successfully:
npm run build -w @autter/runtime-node.d.ts) build passednode --test packages\runtime-node\test\redact.test.mjsgit diff --checkThe redaction test suite now covers the existing redaction behavior together with the new deep-nesting and circular-reference cases.
Result
The Node runtime redaction logic is now able to safely process arbitrarily deep nested objects and arrays without relying on the previous fixed traversal depth, while also protecting against circular references.
This closes the identified deep-nesting redaction gap and adds regression coverage for both the original issue and the circular-reference edge case.
Summary
Summary generated by Autter.
Extends
@autter/runtime-nodeattribute redaction to traverse nested caller-provided objects and arrays, so sensitive keys and token/PII-like string values are masked before telemetry is exported. Traversal is bounded by depth, work, and collection-entry limits to preserve the runtime SDK’s fail-open behavior for malformed or oversized attribute graphs.Changes
redactAttributesand redactors created bymakeRedactorto recursively process nested attribute values rather than only top-level values.Acceptance Criteria
Test Plan
npm run build -w @autter/runtime-node.node --test packages/runtime-node/test/redact.test.mjs.redactAttributeswith nested objects and arrays containing passwords, emails, bearer tokens, and supported primitive values; verify secrets are masked and safe values are retained.Rollback Plan
packages/runtime-node/src/redact.tsandpackages/runtime-node/test/redact.test.mjs.@autter/runtime-nodepackage version if the new traversal behavior causes capture-path regressions.captureException,captureMessage, and process-span calls until the prior runtime package is restored.Related Issues
No linked issue was identified.
Written for commit c69f8bc. Summary will update on new commits.