From 1bccea55b279612ef7032861b9c1ef873dd87c9c Mon Sep 17 00:00:00 2001 From: stack72 Date: Tue, 5 May 2026 19:12:21 +0100 Subject: [PATCH] feat(security): redact sensitive method arg values from log, data, and summary outputs (swamp-club#243) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the existing SecretRedactor pattern to cover method input argument schemas, not just vault secrets. When a field in a method's argument schema is annotated with z.meta({ sensitive: true }), the framework now: 1. Registers all resolved string values from that field with SecretRedactor before method execution, so the per-run log file is scrubbed automatically. 2. Passes the redactor into createResourceWriter(), so result resource attributes written to disk are scrubbed before persistence. 3. Redacts the field to "***" in auto-generated method summary reports (both Markdown and JSON variants). Both string and string-array field types are supported. Array elements are registered individually so any occurrence of any element in output is scrubbed. Values shorter than 3 characters are skipped (SecretRedactor threshold) to prevent false-positive redaction. Follow-ups: swamp-club#258 (audit log), swamp-club#259 (exec env-var path investigation — confirmed not applicable at framework level), swamp-uat#190. Co-Authored-By: Claude Sonnet 4.6 --- design/vaults.md | 46 +++++++++ src/domain/drivers/raw_execution_driver.ts | 1 + src/domain/models/data_writer.ts | 6 +- src/domain/models/method_execution_service.ts | 4 + .../models/sensitive_field_extractor.ts | 43 ++++++++ .../models/sensitive_field_extractor_test.ts | 83 ++++++++++++++++ .../reports/report_execution_service_test.ts | 77 +++++++++++++++ src/domain/workflows/execution_service.ts | 29 ++++++ src/libswamp/models/run.ts | 26 +++++ src/libswamp/models/run_test.ts | 99 +++++++++++++++++++ 10 files changed, 413 insertions(+), 1 deletion(-) diff --git a/design/vaults.md b/design/vaults.md index 9c586fc9..1c67392d 100644 --- a/design/vaults.md +++ b/design/vaults.md @@ -194,6 +194,52 @@ keyData: ${{ vault.get('aws', 'machineKeyData') }} The expression evaluation system resolves these at runtime. +## Sensitive Method Arguments + +The same `z.meta({ sensitive: true })` annotation applies to **method input +argument schemas**, not just output resource schemas. When a method argument field +is marked sensitive, the framework: + +1. Registers all resolved values from that field with `SecretRedactor` before + executing the method — scrubbing them from the per-run log file automatically. +2. Applies the redactor when writing result resource attributes — scrubbing + sensitive values even if the extension model writes them into result attributes. +3. Redacts the field to `"***"` in the auto-generated method summary reports + (both Markdown and JSON variants). + +### Marking an Argument Field as Sensitive + +```typescript +methods: { + exec: { + description: "Run a command in the container", + arguments: z.object({ + // command may contain credentials — mark it sensitive + command: z.array(z.string()).meta({ sensitive: true }), + workdir: z.string().optional(), + }), + execute: async (args, context) => { ... }, + }, +}, +``` + +String and string-array values are both supported. For array fields, each element +is individually registered with the redactor so any occurrence of any element in +log output is scrubbed. + +### Behavior Comparison + +| Location | Output schema `sensitive: true` | Argument schema `sensitive: true` | +| -------- | ------------------------------- | --------------------------------- | +| Result resource attributes | Stored in vault, replaced with vault ref | Scrubbed by redactor at write time | +| Per-run log file | Vault secrets scrubbed | Argument values scrubbed | +| Method summary report | Rendered as vault ref | Rendered as `***` | +| Audit log | Not covered | Not covered (see follow-up #244) | + +Use output-schema sensitive marking when the value must be retrievable later via +`vault.get()`. Use argument-schema sensitive marking when the value is a +short-lived credential that should never be stored anywhere. + ## AWS Secrets Manager Provider The AWS Secrets Manager provider is the initial implementation supporting: diff --git a/src/domain/drivers/raw_execution_driver.ts b/src/domain/drivers/raw_execution_driver.ts index 3c438595..53b722e7 100644 --- a/src/domain/drivers/raw_execution_driver.ts +++ b/src/domain/drivers/raw_execution_driver.ts @@ -103,6 +103,7 @@ export class RawExecutionDriver implements ExecutionDriver { this.context.vaultService, this.methodName, this.context.onEvent, + this.context.redactor, ); const { diff --git a/src/domain/models/data_writer.ts b/src/domain/models/data_writer.ts index 7f1cf29e..5c26dcf9 100644 --- a/src/domain/models/data_writer.ts +++ b/src/domain/models/data_writer.ts @@ -433,6 +433,7 @@ export function createResourceWriter( vaultService?: VaultService, methodName?: string, onEvent?: (event: MethodExecutionEvent) => void, + redactor?: SecretRedactor, ): { writeResource: ( specName: string, @@ -597,7 +598,10 @@ export function createResourceWriter( resolvedOptions, ); - const handle = await writer.writeText(JSON.stringify(data)); + const serialized = redactor + ? redactor.redact(JSON.stringify(data)) + : JSON.stringify(data); + const handle = await writer.writeText(serialized); handles.push(handle); return handle; }; diff --git a/src/domain/models/method_execution_service.ts b/src/domain/models/method_execution_service.ts index 6d2e20eb..e88822e0 100644 --- a/src/domain/models/method_execution_service.ts +++ b/src/domain/models/method_execution_service.ts @@ -89,6 +89,7 @@ interface PersistContext extends | "tagOverrides" | "runtimeTags" | "vaultService" + | "redactor" > { resources: Record; files: Record; @@ -125,6 +126,8 @@ async function processDriverOutputs( persistContext.definitionName, persistContext.vaultService, persistContext.methodName, + undefined, // onEvent + persistContext.redactor, ); // Shape the raw content into resource data. // If content is valid JSON, use it directly. @@ -747,6 +750,7 @@ export class DefaultMethodExecutionService implements MethodExecutionService { definitionName: currentDefinition.name, vaultService: context.vaultService, methodName, + redactor: context.redactor, }); result = { dataHandles: currentHandles }; } diff --git a/src/domain/models/sensitive_field_extractor.ts b/src/domain/models/sensitive_field_extractor.ts index 6e23cdbd..db540527 100644 --- a/src/domain/models/sensitive_field_extractor.ts +++ b/src/domain/models/sensitive_field_extractor.ts @@ -179,6 +179,49 @@ export function extractSensitiveFields( return results; } +/** + * Extracts the runtime secret values from a data object based on its Zod schema. + * + * For each field marked `{ sensitive: true }` in the schema: + * - String values are collected directly. + * - Array values have each string element collected individually. + * - Undefined or null values are skipped. + * - Object values are skipped (nested object fields are found via recursion). + * + * Used to register sensitive argument values with SecretRedactor before + * method execution so they are scrubbed from log files and result resources. + * + * @param schema - A Zod schema (typically a method argument schema) + * @param data - The resolved data object to extract values from + * @returns Array of secret string values to register with SecretRedactor + */ +export function extractSensitiveFieldValues( + schema: z.ZodTypeAny, + data: Record, +): string[] { + const fields = extractSensitiveFields(schema); + const secrets: string[] = []; + + for (const field of fields) { + const value = getNestedValue(data, field.path); + if (value === undefined || value === null) { + continue; + } + if (typeof value === "string") { + secrets.push(value); + } else if (Array.isArray(value)) { + for (const element of value) { + if (typeof element === "string") { + secrets.push(element); + } + } + } + // Object values are skipped — nested fields are found by extractSensitiveFields recursion + } + + return secrets; +} + /** * Gets a nested value from an object by dot-separated path. */ diff --git a/src/domain/models/sensitive_field_extractor_test.ts b/src/domain/models/sensitive_field_extractor_test.ts index b425b948..ccb7d254 100644 --- a/src/domain/models/sensitive_field_extractor_test.ts +++ b/src/domain/models/sensitive_field_extractor_test.ts @@ -21,6 +21,7 @@ import { assertEquals } from "@std/assert"; import { z } from "zod"; import { extractSensitiveFields, + extractSensitiveFieldValues, getNestedValue, setNestedValue, } from "./sensitive_field_extractor.ts"; @@ -146,6 +147,88 @@ Deno.test("extractSensitiveFields: deeply nested", () => { assertEquals(fields[0].path, "level1.level2.secret"); }); +Deno.test("extractSensitiveFieldValues: string field", () => { + const schema = z.object({ + apiKey: z.string().meta({ sensitive: true }), + name: z.string(), + }); + const data = { apiKey: "s3cr3t", name: "test" }; + assertEquals(extractSensitiveFieldValues(schema, data), ["s3cr3t"]); +}); + +Deno.test("extractSensitiveFieldValues: array field collects each element", () => { + const schema = z.object({ + command: z.array(z.string()).meta({ sensitive: true }), + name: z.string(), + }); + const data = { command: ["sh", "-c", "echo TOKEN_HERE"], name: "test" }; + assertEquals(extractSensitiveFieldValues(schema, data), [ + "sh", + "-c", + "echo TOKEN_HERE", + ]); +}); + +Deno.test("extractSensitiveFieldValues: undefined field is skipped", () => { + const schema = z.object({ + apiKey: z.string().meta({ sensitive: true }).optional(), + }); + const data: Record = {}; + assertEquals(extractSensitiveFieldValues(schema, data), []); +}); + +Deno.test("extractSensitiveFieldValues: null field is skipped", () => { + const schema = z.object({ + apiKey: z.string().nullable().meta({ sensitive: true }), + }); + const data = { apiKey: null }; + assertEquals(extractSensitiveFieldValues(schema, data), []); +}); + +Deno.test("extractSensitiveFieldValues: non-sensitive fields ignored", () => { + const schema = z.object({ + apiKey: z.string().meta({ sensitive: true }), + region: z.string(), + }); + const data = { apiKey: "s3cr3t", region: "us-east-1" }; + assertEquals(extractSensitiveFieldValues(schema, data), ["s3cr3t"]); +}); + +Deno.test("extractSensitiveFieldValues: nested sensitive field", () => { + const schema = z.object({ + credentials: z.object({ + token: z.string().meta({ sensitive: true }), + }), + }); + const data = { credentials: { token: "tok_abc" } }; + assertEquals(extractSensitiveFieldValues(schema, data), ["tok_abc"]); +}); + +Deno.test("extractSensitiveFieldValues: multiple sensitive fields", () => { + const schema = z.object({ + apiKey: z.string().meta({ sensitive: true }), + secret: z.string().meta({ sensitive: true }), + name: z.string(), + }); + const data = { apiKey: "key123", secret: "sec456", name: "test" }; + const values = extractSensitiveFieldValues(schema, data); + assertEquals(values.sort(), ["key123", "sec456"]); +}); + +Deno.test("extractSensitiveFieldValues: array with non-string elements skips them", () => { + const schema = z.object({ + items: z.array(z.unknown()).meta({ sensitive: true }), + }); + const data = { items: ["str", 42, null, "other"] }; + assertEquals(extractSensitiveFieldValues(schema, data), ["str", "other"]); +}); + +Deno.test("extractSensitiveFieldValues: empty schema returns empty", () => { + const schema = z.object({}); + const data = {}; + assertEquals(extractSensitiveFieldValues(schema, data), []); +}); + Deno.test("getNestedValue: simple path", () => { const obj = { apiKey: "secret-value" }; assertEquals(getNestedValue(obj, "apiKey"), "secret-value"); diff --git a/src/domain/reports/report_execution_service_test.ts b/src/domain/reports/report_execution_service_test.ts index db4e96a4..e7cc72bb 100644 --- a/src/domain/reports/report_execution_service_test.ts +++ b/src/domain/reports/report_execution_service_test.ts @@ -1316,3 +1316,80 @@ Deno.test("executeReports: already-loaded reports are not re-promoted", async () assertEquals(summary.results.length, 1); assertEquals(summary.results[0].success, true); }); + +Deno.test("buildRedactSensitiveArgs: redacts array-typed sensitive method args to ***", async () => { + const typeName = "@test-redact/sensitive-array"; + const modelType = ModelType.create(typeName); + if (!modelRegistry.has(modelType)) { + modelRegistry.register({ + type: modelType, + version: "2026.01.01.1", + globalArguments: z.object({}), + methods: { + exec: { + description: "test exec", + arguments: z.object({ + command: z.array(z.string()).meta({ sensitive: true }), + name: z.string(), + }), + execute: () => Promise.resolve({ dataHandles: [] }), + }, + }, + }); + } + + const methodArgs = { + command: ["sh", "-c", "echo TOKEN_HERE | base64 -d"], + name: "test-step", + }; + const { report, getResults } = makeRedactionCapturingReport({}, methodArgs); + + const registry = new ReportRegistry(); + registry.register("redaction-test-array", report); + const { repo } = createInMemoryDataRepo(); + + const context: MethodReportContext = { + scope: "method", + repoDir: "/tmp/test", + logger: { + debug: () => {}, + info: () => {}, + warn: () => {}, + error: () => {}, + fatal: () => {}, + } as unknown as MethodReportContext["logger"], + // deno-lint-ignore no-explicit-any + dataRepository: repo as any, + // deno-lint-ignore no-explicit-any + definitionRepository: {} as any, + modelType, + modelId: "test-id", + definition: { id: "test-id", name: "test", version: 1, tags: {} }, + globalArgs: {}, + methodArgs, + methodName: "exec", + executionStatus: "succeeded", + dataHandles: [], + extensionFile: () => { + throw new Error("extensionFile not stubbed in this test"); + }, + }; + + await executeReports( + registry, + context, + modelType, + "test-id", + { require: ["redaction-test-array"] }, + {}, + undefined, + "exec", + ); + + const results = getResults(); + assertEquals(results !== null, true); + // The entire array is replaced with "***" + assertEquals(results!.redactedMethod.command, "***"); + // Non-sensitive fields are preserved + assertEquals(results!.redactedMethod.name, "test-step"); +}); diff --git a/src/domain/workflows/execution_service.ts b/src/domain/workflows/execution_service.ts index f2ea7b0f..c0fb74af 100644 --- a/src/domain/workflows/execution_service.ts +++ b/src/domain/workflows/execution_service.ts @@ -99,6 +99,7 @@ import { RepoMarkerRepository, } from "../../infrastructure/persistence/repo_marker_repository.ts"; import { createRepoMarkerLoader } from "../../infrastructure/persistence/repo_marker_loader.ts"; +import { extractSensitiveFieldValues } from "../models/sensitive_field_extractor.ts"; /** * Context for step execution. @@ -662,6 +663,34 @@ export class DefaultStepExecutor implements StepExecutor { driverConfig: evaluatedDefinition.driverConfig, }); + // Register sensitive argument values with the workflow redactor so they + // are scrubbed from the workflow log file. Must use post-vault-resolution + // values from evaluatedDefinition. + if (ctx.secretRedactor) { + const globalArgSchema = modelDef.globalArguments; + if (globalArgSchema) { + for ( + const secret of extractSensitiveFieldValues( + globalArgSchema, + evaluatedDefinition.globalArguments, + ) + ) { + ctx.secretRedactor.addSecret(secret); + } + } + const methodArgSchema = modelDef.methods[task.methodName]?.arguments; + if (methodArgSchema) { + for ( + const secret of extractSensitiveFieldValues( + methodArgSchema, + evaluatedDefinition.getMethodArguments(task.methodName), + ) + ) { + ctx.secretRedactor.addSecret(secret); + } + } + } + // Execute the method with the EVALUATED definition. The logger // handles both console and file persistence via RunFileSink. Data // is persisted by DataWriter during execution — no double-save. diff --git a/src/libswamp/models/run.ts b/src/libswamp/models/run.ts index 8823f321..c6e01033 100644 --- a/src/libswamp/models/run.ts +++ b/src/libswamp/models/run.ts @@ -58,6 +58,7 @@ import { InputValidationService, } from "../../domain/inputs/mod.ts"; import { extractInputReferences } from "../../domain/expressions/expression_parser.ts"; +import { extractSensitiveFieldValues } from "../../domain/models/sensitive_field_extractor.ts"; import { detectEnvVarUsageInDefinition } from "../../domain/models/env_var_detector.ts"; import { withEventBridge } from "../../infrastructure/stream/event_bridge.ts"; import type { MethodResult } from "../../domain/models/model.ts"; @@ -404,6 +405,31 @@ export async function* modelMethodRun( evaluatedDefinition = runtimeResult.definition; const secretBag = runtimeResult.secretBag; + // Register sensitive argument values with the redactor so they are + // scrubbed from per-run log files. Must use post-vault-resolution values. + const globalArgSchema = modelDef.globalArguments; + if (globalArgSchema) { + for ( + const secret of extractSensitiveFieldValues( + globalArgSchema, + evaluatedDefinition.globalArguments, + ) + ) { + redactor.addSecret(secret); + } + } + const methodArgSchema = modelDef.methods[input.methodName]?.arguments; + if (methodArgSchema) { + for ( + const secret of extractSensitiveFieldValues( + methodArgSchema, + evaluatedDefinition.getMethodArguments(input.methodName), + ) + ) { + redactor.addSecret(secret); + } + } + // --- Execute --- yield { kind: "executing", diff --git a/src/libswamp/models/run_test.ts b/src/libswamp/models/run_test.ts index 46ddcd94..4781c2e3 100644 --- a/src/libswamp/models/run_test.ts +++ b/src/libswamp/models/run_test.ts @@ -37,6 +37,7 @@ import { z } from "zod"; import { VaultSecretBag } from "../../domain/vaults/vault_secret_bag.ts"; import { initializeLogging } from "../../infrastructure/logging/logger.ts"; import type { DataQueryService } from "../../domain/data/data_query_service.ts"; +import { SecretRedactor } from "../../domain/secrets/secret_redactor.ts"; await initializeLogging({}); @@ -579,3 +580,101 @@ Deno.test("modelMethodRun: accepts known override input key", async () => { const completedEvent = events.find((e) => e.kind === "completed"); assertEquals(completedEvent?.kind, "completed"); }); + +// --- Sensitive argument registration tests --- + +Deno.test("modelMethodRun: registers sensitive string-array method arg values with the redactor", async () => { + const secretValue = "SUPER_SECRET_TOKEN_XYZ"; + const redactor = new SecretRedactor(); + + const modelDef: ModelDefinition = { + type: TEST_MODEL_TYPE, + version: "1.0.0", + methods: { + run: { + description: "Test method with sensitive array arg", + arguments: z.object({ + command: z.array(z.string()).meta({ sensitive: true }), + name: z.string().optional(), + }), + execute: (_args, _ctx) => Promise.resolve({ dataHandles: [] }), + }, + }, + }; + + const definition = Definition.create({ + name: "sensitive-arg-model", + methods: { + run: { + arguments: { command: ["sh", "-c", secretValue], name: "test-step" }, + }, + }, + }); + + const deps: ModelMethodRunDeps = { + ...createTestDeps(definition, modelDef), + createRunLog: () => + Promise.resolve({ + logFilePath: "/tmp/test.log", + redactor, + cleanup: () => {}, + }), + }; + + const ctx = createLibSwampContext(); + await collect( + modelMethodRun(ctx, deps, createTestInput("sensitive-arg-model", "run")), + ); + + // Long token must be scrubbed; short elements "sh" and "-c" fall below the 3-char threshold + assertEquals(redactor.redact(secretValue), "***"); + assertEquals(redactor.redact("not-a-secret"), "not-a-secret"); +}); + +Deno.test("modelMethodRun: registers sensitive string method arg values with the redactor", async () => { + const secretValue = "s3cr3t-api-key-abc123"; + const redactor = new SecretRedactor(); + + const modelDef: ModelDefinition = { + type: TEST_MODEL_TYPE, + version: "1.0.0", + methods: { + run: { + description: "Test method with sensitive string arg", + arguments: z.object({ + apiKey: z.string().meta({ sensitive: true }), + region: z.string().optional(), + }), + execute: (_args, _ctx) => Promise.resolve({ dataHandles: [] }), + }, + }, + }; + + const definition = Definition.create({ + name: "sensitive-string-model", + methods: { + run: { + arguments: { apiKey: secretValue, region: "us-east-1" }, + }, + }, + }); + + const deps: ModelMethodRunDeps = { + ...createTestDeps(definition, modelDef), + createRunLog: () => + Promise.resolve({ + logFilePath: "/tmp/test.log", + redactor, + cleanup: () => {}, + }), + }; + + const ctx = createLibSwampContext(); + await collect( + modelMethodRun(ctx, deps, createTestInput("sensitive-string-model", "run")), + ); + + assertEquals(redactor.redact(secretValue), "***"); + // Non-sensitive field value is not redacted + assertEquals(redactor.redact("us-east-1"), "us-east-1"); +});