-
Notifications
You must be signed in to change notification settings - Fork 1
fix(runtime): redact deeply nested attributes. #19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
93f6c84
33f33d6
c83a39a
b29d899
c69f8bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -131,27 +131,189 @@ function redactString(value: string, r: CompiledRedactor): string { | |
| return out; | ||
| } | ||
|
|
||
| function redactValue(value: unknown, r: CompiledRedactor, depth: number): unknown { | ||
| if (typeof value === "string") return redactString(value, r); | ||
| if (Array.isArray(value)) { | ||
| if (depth <= 0) return value; | ||
| return value.map((item) => redactValue(item, r, depth - 1)); | ||
| } | ||
| // Defensive: OTLP attributes are flat, but callers hand us arbitrary | ||
| // objects — walk one more level so nothing sensitive hides inside. | ||
| if (typeof value === "object" && value !== null && depth > 0) { | ||
| const out: Record<string, unknown> = {}; | ||
| for (const [k, v] of Object.entries(value as Record<string, unknown>)) { | ||
| out[k] = isSensitiveKey(k, r) ? r.mask : redactValue(v, r, depth - 1); | ||
| } | ||
| return out; | ||
| } | ||
| return value; | ||
| const MAX_REDACTION_DEPTH = 64; | ||
| const MAX_REDACTION_WORK = 10_000; | ||
| const MAX_COLLECTION_ENTRIES = 1_000; | ||
|
|
||
| interface RedactionState { | ||
| ancestors: WeakMap<object, object>; | ||
| remainingWork: number; | ||
| } | ||
|
|
||
| function canTraverse(depth: number, state: RedactionState): boolean { | ||
| if (depth > MAX_REDACTION_DEPTH || state.remainingWork <= 0) return false; | ||
| state.remainingWork -= 1; | ||
| return true; | ||
| } | ||
|
|
||
| function redactValue( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 [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
🛠 AI fix prompt (copy & paste into your coding agent)Flagged by Autter security & observability checks. |
||
| value: unknown, | ||
| r: CompiledRedactor, | ||
| state: RedactionState, | ||
| depth: number, | ||
| ): unknown { | ||
| if (typeof value === "string") return redactString(value, r); | ||
|
|
||
| let isArray = false; | ||
| try { | ||
| isArray = Array.isArray(value); | ||
| } catch { | ||
| return r.mask; | ||
| } | ||
|
|
||
| if (isArray) { | ||
| return redactArray(value as unknown[], r, state, depth); | ||
| } | ||
|
|
||
| if (typeof value === "object" && value !== null) { | ||
| return redactObject(value, r, state, depth); | ||
| } | ||
|
|
||
| return value; | ||
| } | ||
|
|
||
| function redactArray( | ||
| value: unknown[], | ||
| r: CompiledRedactor, | ||
| state: RedactionState, | ||
| depth: number, | ||
| ): unknown { | ||
| const existing = state.ancestors.get(value); | ||
| if (existing) return r.mask; | ||
|
|
||
| if (!canTraverse(depth, state)) return r.mask; | ||
|
|
||
| const out: unknown[] = []; | ||
| state.ancestors.set(value, out); | ||
|
|
||
| let length: number; | ||
| try { | ||
| length = value.length; | ||
| } catch { | ||
| state.ancestors.delete(value); | ||
| return r.mask; | ||
| } | ||
|
|
||
| const limit = Math.min(length, MAX_COLLECTION_ENTRIES); | ||
|
|
||
| for (let i = 0; i < limit; i += 1) { | ||
| let item: unknown; | ||
|
|
||
| try { | ||
| item = value[i]; | ||
| } catch { | ||
| out.push(r.mask); | ||
| continue; | ||
| } | ||
|
|
||
| out.push(redactValue(item, r, state, depth + 1)); | ||
| } | ||
|
|
||
| if (length > limit) { | ||
| out.push(r.mask); | ||
| } | ||
|
|
||
| state.ancestors.delete(value); | ||
| return out; | ||
| } | ||
|
|
||
| function redactObject( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 [ai] Moderate control-flow complexity in redactObject — Risk: 30/100
🛠 AI fix prompt (copy & paste into your coding agent)Flagged by Autter security & observability checks. |
||
| value: object, | ||
| r: CompiledRedactor, | ||
| state: RedactionState, | ||
| depth: number, | ||
| ): unknown { | ||
| const existing = state.ancestors.get(value); | ||
| if (existing) return r.mask; | ||
|
|
||
| if (!canTraverse(depth, state)) return r.mask; | ||
|
|
||
| const out: Record<string, unknown> = {}; | ||
| state.ancestors.set(value, out); | ||
|
|
||
| let count = 0; | ||
| let truncated = false; | ||
|
|
||
| try { | ||
| for (const key in value as Record<string, unknown>) { | ||
| if ( | ||
| !Object.prototype.propertyIsEnumerable.call( | ||
| value, | ||
| key, | ||
| ) | ||
| ) { | ||
| continue; | ||
| } | ||
|
|
||
| if (count >= MAX_COLLECTION_ENTRIES) { | ||
| truncated = true; | ||
| break; | ||
| } | ||
|
|
||
| let nestedValue: unknown; | ||
| try { | ||
| nestedValue = (value as Record<string, unknown>)[key]; | ||
| } catch { | ||
| out[key] = r.mask; | ||
| count += 1; | ||
| continue; | ||
| } | ||
|
|
||
| count += 1; | ||
| out[key] = isSensitiveKey(key, nestedValue, r) | ||
| ? r.mask | ||
| : redactValue( | ||
| nestedValue, | ||
| r, | ||
| state, | ||
| depth + 1, | ||
| ); | ||
| } | ||
| } catch { | ||
| truncated = true; | ||
| } | ||
|
|
||
| if (truncated) { | ||
| out.__redaction_truncated__ = r.mask; | ||
| } | ||
|
|
||
| state.ancestors.delete(value); | ||
| return out; | ||
| } | ||
| const USAGE_TOKEN_KEYS = new Set([ | ||
| "input_tokens", | ||
| "output_tokens", | ||
| "prompt_tokens", | ||
| "completion_tokens", | ||
| "total_tokens", | ||
| "token_count", | ||
| "gen_ai.usage.input_tokens", | ||
| "gen_ai.usage.output_tokens", | ||
| "gen_ai.usage.prompt_tokens", | ||
| "gen_ai.usage.completion_tokens", | ||
| "gen_ai.usage.total_tokens", | ||
| "gen_ai.usage.token_count", | ||
| ]); | ||
|
|
||
| function isSensitiveKey( | ||
| key: string, | ||
| value: unknown, | ||
| r: CompiledRedactor, | ||
| ): boolean { | ||
| const lowered = key.toLowerCase(); | ||
|
|
||
| // Canonical GenAI usage attributes are safe when they contain | ||
| // valid non-negative numeric counts. | ||
| if (USAGE_TOKEN_KEYS.has(lowered)) { | ||
| return !( | ||
| typeof value === "number" && | ||
| Number.isFinite(value) && | ||
| value >= 0 | ||
| ); | ||
| } | ||
|
|
||
| function isSensitiveKey(key: string, r: CompiledRedactor): boolean { | ||
| const lowered = key.toLowerCase(); | ||
| return r.keyPatterns.some((re) => re.test(lowered)); | ||
| // All other sensitive keys, including token-like keys, are redacted. | ||
| return r.keyPatterns.some((re) => re.test(lowered)); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -164,23 +326,71 @@ export function redactAttributes( | |
| options?: RedactOptions, | ||
| ): Attributes { | ||
| const r = compile(options); | ||
| return redactWith(attributes, r, 4); | ||
| return redactWith(attributes, r); | ||
| } | ||
|
|
||
| function redactWith( | ||
| attributes: Attributes | null | undefined, | ||
| r: CompiledRedactor, | ||
| maxDepth: number, | ||
| attributes: Attributes | null | undefined, | ||
| r: CompiledRedactor, | ||
| ): Attributes { | ||
| const out: Attributes = {}; | ||
| if (!attributes) return out; | ||
| for (const [key, value] of Object.entries(attributes)) { | ||
| if (value === undefined) continue; | ||
| out[key] = isSensitiveKey(key, r) | ||
| ? r.mask | ||
| : (redactValue(value, r, maxDepth) as Attributes[string]); | ||
| } | ||
| return out; | ||
| const out: Attributes = {}; | ||
| if (!attributes) return out; | ||
|
|
||
| const state: RedactionState = { | ||
| ancestors: new WeakMap<object, object>(), | ||
| remainingWork: MAX_REDACTION_WORK, | ||
| }; | ||
|
|
||
| let count = 0; | ||
| let truncated = false; | ||
|
|
||
| try { | ||
| for (const key in attributes as Record<string, unknown>) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 [ai] Top-level attributes bypass redaction collection limits — Risk: 50/100
🛠 AI fix prompt (copy & paste into your coding agent)Flagged by Autter security & observability checks. |
||
| if ( | ||
| !Object.prototype.propertyIsEnumerable.call( | ||
| attributes, | ||
| key, | ||
| ) | ||
| ) { | ||
| continue; | ||
| } | ||
|
|
||
| if ( | ||
| count >= MAX_COLLECTION_ENTRIES || | ||
| state.remainingWork <= 0 | ||
| ) { | ||
| truncated = true; | ||
| break; | ||
| } | ||
|
|
||
| let value: unknown; | ||
| try { | ||
| value = (attributes as Record<string, unknown>)[key]; | ||
| } catch { | ||
| out[key] = r.mask; | ||
| count += 1; | ||
| state.remainingWork -= 1; | ||
| continue; | ||
| } | ||
|
|
||
| count += 1; | ||
| state.remainingWork -= 1; | ||
|
|
||
| if (value === undefined) continue; | ||
|
|
||
| out[key] = isSensitiveKey(key, value, r) | ||
| ? r.mask | ||
| : (redactValue(value, r, state, 0) as Attributes[string]); | ||
| } | ||
| } catch { | ||
| truncated = true; | ||
| } | ||
|
|
||
| if (truncated) { | ||
| out.__redaction_truncated__ = r.mask; | ||
| } | ||
|
|
||
| return out; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -194,5 +404,5 @@ export function makeRedactor( | |
| return (attributes) => ({ ...(attributes ?? {}) }); | ||
| } | ||
| const r = compile(options === true ? undefined : options); | ||
| return (attributes) => redactWith(attributes, r, 4); | ||
| return (attributes) => redactWith(attributes, r); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 [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.
redactAttributes,makeRedactor,redactString,redactValue,isSensitiveKey,redactWith,installAutterAutoFlush@opentelemetry/api@autter/runtime-node🛠 AI fix prompt (copy & paste into your coding agent)
Flagged by Autter security & observability checks.