From 4bbce49c9faa68f7110d3b65ef1026e9dd39aa6a Mon Sep 17 00:00:00 2001 From: Heewon Oh Date: Fri, 23 Oct 2026 21:47:00 +0900 Subject: [PATCH] fix(tool-boundaries): close guard and trace retention gaps --- src/adapters/shellCommandGuard.test.ts | 8 ++++ src/adapters/shellCommandGuard.ts | 14 ++++++- src/core/traceCollector.test.ts | 26 ++++++++++++- src/core/traceCollector.ts | 54 +++++++++++++++++++++++--- 4 files changed, 94 insertions(+), 8 deletions(-) diff --git a/src/adapters/shellCommandGuard.test.ts b/src/adapters/shellCommandGuard.test.ts index 7e04b11a..548a7226 100644 --- a/src/adapters/shellCommandGuard.test.ts +++ b/src/adapters/shellCommandGuard.test.ts @@ -41,6 +41,14 @@ const rewriteBypasses = [ ]; describe('destructive-command guard sees what the shell will run (AGT-3436)', () => { + it.each([ + 'cat <(rm -rf /foo)', + 'diff <(git reset --hard) out.txt', + 'tee >(rm -rf /foo)', + ])('blocks destructive process substitution: %s', (command) => { + expect(isCommandBlocked(command)).toBe(true); + }); + it.each(rewriteBypasses)('blocks the shell-rewritten form: %s', (command) => { expect(isCommandBlocked(command)).toBe(true); }); diff --git a/src/adapters/shellCommandGuard.ts b/src/adapters/shellCommandGuard.ts index 9c148619..dc277e62 100644 --- a/src/adapters/shellCommandGuard.ts +++ b/src/adapters/shellCommandGuard.ts @@ -186,8 +186,9 @@ interface SubstitutionSpan { */ function substitutionSpanAt(command: string, i: number): SubstitutionSpan | 'none' | 'unclosed' { const char = command[i]; + const processSubstitution = (char === '<' || char === '>') && command[i + 1] === '('; const dollar = char === '$' && (command[i + 1] === '(' || command[i + 1] === '{'); - if (char !== '`' && !dollar) return 'none'; + if (char !== '`' && !dollar && !processSubstitution) return 'none'; const end = endOfSubstitution(command, i); if (end < 0) return 'unclosed'; const runsCommand = char === '`' || command[i + 1] === '('; @@ -299,6 +300,17 @@ function resolveCommands(command: string): ResolvedCommand[] | null { continue; } if (char === '>' || char === '<') { + const span = substitutionSpanAt(command, i); + if (span === 'unclosed') return null; + if (span !== 'none') { + if (isWordChar(word[word.length - 1]) || isWordChar(command[span.end])) return null; + word += span.text; + wordStarted = true; + atWordStart = false; + if (span.runsCommand) nested.push(span.body); + i = span.end - 1; + continue; + } endWord(); atWordStart = true; redirectTarget = false; diff --git a/src/core/traceCollector.test.ts b/src/core/traceCollector.test.ts index 6ef1131f..53c569a8 100644 --- a/src/core/traceCollector.test.ts +++ b/src/core/traceCollector.test.ts @@ -8,6 +8,27 @@ describe('TraceCollector', () => { collector = new TraceCollector(); }); + it('bounds retained names, metadata, and error payloads by UTF-8 bytes', () => { + const bounded = new TraceCollector(); + const traceId = bounded.startTrace('😀'.repeat(100_000), { kept: 'yes', huge: 'x'.repeat(100_000) }); + const spanId = bounded.startSpan(traceId, 'é'.repeat(100_000), undefined, { kept: 'yes', huge: 'y'.repeat(100_000) })!; + bounded.recordError(traceId, spanId, { + message: 'm'.repeat(100_000), stack: 's'.repeat(100_000), code: 'c'.repeat(100_000), + }); + + const trace = bounded.getTrace(traceId)!; + const span = trace.spans[0]; + expect(Buffer.byteLength(trace.name, 'utf8')).toBeLessThanOrEqual(1024); + expect(Buffer.byteLength(span.name, 'utf8')).toBeLessThanOrEqual(1024); + expect(Buffer.byteLength(JSON.stringify(trace.metadata), 'utf8')).toBeLessThanOrEqual(4096); + expect(Buffer.byteLength(JSON.stringify(span.metadata), 'utf8')).toBeLessThanOrEqual(4096); + expect(Buffer.byteLength(JSON.stringify(span.errorInfo), 'utf8')).toBeLessThanOrEqual(4096); + expect(trace.metadata.kept).toBe('yes'); + expect(span.metadata.kept).toBe('yes'); + expect(trace.name.endsWith('…')).toBe(true); + expect(span.name.endsWith('…')).toBe(true); + }); + // ============================================ // Trace 생성/종료 // ============================================ @@ -321,7 +342,7 @@ describe('TraceCollector', () => { expect(span.metadata.array).toHaveLength(3); }); - it('should handle very large metadata values', () => { + it('should bound very large metadata values', () => { const traceId = collector.startTrace('session'); const largeString = 'A'.repeat(100000); collector.startSpan(traceId, 'worker', undefined, { @@ -330,7 +351,8 @@ describe('TraceCollector', () => { const trace = collector.getTrace(traceId); const span = trace!.spans[0]; - expect((span.metadata.largeData as string).length).toBe(100000); + expect(Buffer.byteLength(span.metadata.largeData as string, 'utf8')).toBeLessThanOrEqual(4096); + expect((span.metadata.largeData as string).endsWith('…')).toBe(true); }); it('should handle special characters in span names and traces', () => { diff --git a/src/core/traceCollector.ts b/src/core/traceCollector.ts index 2ae68d43..d3661b7c 100644 --- a/src/core/traceCollector.ts +++ b/src/core/traceCollector.ts @@ -5,6 +5,50 @@ import { randomUUID } from 'node:crypto'; +const MAX_TRACE_NAME_BYTES = 1024; +const MAX_METADATA_BYTES = 4096; +const MAX_ERROR_BYTES = 4096; + +function boundText(value: string, maxBytes: number): string { + if (Buffer.byteLength(value, 'utf8') <= maxBytes) return value; + let result = ''; + for (const char of value) { + if (Buffer.byteLength(result + char + '…', 'utf8') > maxBytes) break; + result += char; + } + return `${result}…`; +} + +function boundMetadata(metadata: Record): Record { + const bounded: Record = {}; + for (const [key, value] of Object.entries(metadata)) { + try { + const encoded = JSON.stringify({ ...bounded, [key]: value }); + if (encoded !== undefined && Buffer.byteLength(encoded, 'utf8') <= MAX_METADATA_BYTES) bounded[key] = value; + else if (typeof value === 'string') { + const room = MAX_METADATA_BYTES - Buffer.byteLength(JSON.stringify(bounded), 'utf8') - Buffer.byteLength(JSON.stringify(key), 'utf8') - 8; + const shortened = boundText(value, Math.max(0, room)); + const candidate = JSON.stringify({ ...bounded, [key]: shortened }); + if (candidate && Buffer.byteLength(candidate, 'utf8') <= MAX_METADATA_BYTES) bounded[key] = shortened; + } + } catch { + // Ignore unserializable values rather than retaining them unchecked. + } + } + return bounded; +} + +function boundError(error: { message: string; stack?: string; code?: string }): { message: string; stack?: string; code?: string } { + const result = { + message: boundText(error.message, 1024), + ...(error.stack === undefined ? {} : { stack: boundText(error.stack, 2048) }), + ...(error.code === undefined ? {} : { code: boundText(error.code, 256) }), + }; + return Buffer.byteLength(JSON.stringify(result), 'utf8') <= MAX_ERROR_BYTES + ? result + : { message: boundText(result.message, MAX_ERROR_BYTES - 16) }; +} + // Types export type SpanStatus = 'running' | 'completed' | 'failed'; @@ -95,11 +139,11 @@ export class TraceCollector { const traceId = randomUUID(); const trace: Trace = { traceId, - name, + name: boundText(name, MAX_TRACE_NAME_BYTES), startTime: Date.now(), status: 'running', spans: [], - metadata, + metadata: boundMetadata(metadata), }; this.traces.set(traceId, trace); return traceId; @@ -150,10 +194,10 @@ export class TraceCollector { spanId, traceId, parentSpanId, - name, + name: boundText(name, MAX_TRACE_NAME_BYTES), status: 'running', startTime: Date.now(), - metadata, + metadata: boundMetadata(metadata), }; trace.spans.push(span); return spanId; @@ -190,7 +234,7 @@ export class TraceCollector { const span = trace.spans.find((s) => s.spanId === spanId); if (!span) return false; - span.errorInfo = error; + span.errorInfo = boundError(error); span.status = 'failed'; span.endTime = span.endTime ?? Date.now(); return true;