feat(security): redact sensitive method arg values from log, data, and summary outputs (swamp-club#243)

[stack72]

Summary

Extends the existing SecretRedactor pattern to cover method input argument schemas. When a field in a method's argument schema is annotated with z.meta({ sensitive: true }), the framework now:

• Per-run log file: registers all resolved string values from that field with SecretRedactor before execution, so they are scrubbed from every log line automatically via RunFileSink
• Result resource attributes: passes the redactor into createResourceWriter() so data written to disk is scrubbed before persistence — covers both the raw execution driver (TypeScript extensions) and the Docker driver path via method_execution_service.ts
• Method summary reports: redacts the field to "***" in both Markdown and JSON summary variants

Both string and string[] field types are supported. Array elements are registered individually. Values under 3 characters are skipped (existing SecretRedactor threshold) to prevent false-positive redaction.

No breaking changes — all new parameters are optional and appended to existing function signatures.

Test Plan

• extractSensitiveFieldValues() — 9 new unit tests in sensitive_field_extractor_test.ts covering string fields, array fields, null/undefined skipping, nested paths, and non-string element filtering
• buildRedactSensitiveArgs — new test in report_execution_service_test.ts verifying array-typed sensitive args render as *** in summary reports with non-sensitive siblings preserved
• modelMethodRun redactor registration — 2 new integration tests in run_test.ts verifying a real SecretRedactor receives sensitive string-array and string values after execution

Full suite: 5472 passed, 0 failed.

Follow-ups

• swamp-club#258 — audit log redaction (bash hook captures CLI commands; needs schema awareness at hook time, separate work)
• swamp-club#259 — exec env-var injection path (investigated: not applicable at framework level — method args flow as JSON, not env vars)
• swamp-uat#190 — UAT adversarial scenarios

🤖 Generated with Claude Code

[github-actions]

CLI UX Review

Blocking

None.

Suggestions

None.

Verdict

PASS — This PR does not meaningfully change the CLI UX. All changes are internal security infrastructure: extending SecretRedactor to cover method input argument schemas. No new flags, commands, help text, error messages, or output formats are introduced. The only user-visible effect — sensitive fields rendering as *** in method summary reports — is the intended behavior and consistent with the existing redaction pattern.

[github-actions]

Code Review

Clean, well-scoped security improvement. The approach of extending the existing SecretRedactor pattern to cover method argument schemas is sound — it threads redaction through three distinct output paths (log files, persisted data, summary reports) without breaking existing callers since all new parameters are optional.

Blocking Issues

None.

Suggestions

1. createResourceWriter positional parameter growth (data_writer.ts:433): The function now takes 13 positional parameters with several undefined placeholders at call sites (e.g., method_execution_service.ts:129). A future cleanup pass to consolidate into an options object would improve readability and reduce the risk of positional mistakes. Not worth doing in this PR, but worth tracking.

2. Duplicate registration pattern (run.ts:386-408 vs execution_service.ts:666-692): The ~20-line block that iterates global + method arg schemas through extractSensitiveFieldValues and registers each secret is nearly identical in both call sites. If a third consumer appears, consider extracting a registerSensitiveArgValues(redactor, modelDef, evaluatedDefinition, methodName) helper into the domain layer.

3. Verbose JSDoc on extractSensitiveFieldValues (sensitive_field_extractor.ts:182-196): Per CLAUDE.md comment policy, the multi-paragraph docstring is heavier than typical. It follows the existing style in the file though, so this is very minor — the function signature and name already communicate the intent well.

All three are optional improvements. The PR is ready to merge as-is.

[github-actions]

Adversarial Review

Traced every code path introduced by this PR — the new extractSensitiveFieldValues function, redactor threading through data_writer.ts, method_execution_service.ts, execution_service.ts, run.ts, and raw_execution_driver.ts.

Critical / High

None.

Medium

None.

Low

1. Redundant redactor registration for vault-sourced sensitive args (src/libswamp/models/run.ts:385-408, src/domain/workflows/execution_service.ts:669-692): After resolveRuntimeExpressionsInDefinition, vault-sourced fields in evaluatedDefinition contain sentinel tokens (not raw values). The raw secret values are already registered with the redactor by the resolution process. The new code registers the sentinel tokens — harmless (Set deduplication, and sentinels are short-lived internal values that won't match real log content), but worth knowing for future maintenance.

2. 3-character threshold gap (src/domain/models/sensitive_field_extractor.ts:198-222): extractSensitiveFieldValues returns all string values including short ones, but SecretRedactor.addSecret silently drops values < 3 chars. A 1-2 character sensitive value (e.g., a single-letter flag) would pass through the extractor but never be registered for redaction. Unlikely in practice for real secrets; the threshold exists to prevent false-positive redaction of common short strings.

Verdict

PASS — Clean, well-structured security enhancement. The ordering is correct (post-vault-resolution, pre-execution), the redactor is applied at both the log-file and data-persistence layers, SecretRedactor handles JSON-escaped forms of secrets, split/join avoids regex pitfalls, the createResourceWriter signature extension is backwards-compatible (optional trailing parameter), and all callers pass arguments in the correct positional order. Tests cover string fields, array fields, null/undefined skipping, nested paths, and the short-value threshold. No security bypasses found.

[github-actions]

CLI UX Review

Blocking

None.

Suggestions

None.

Verdict

PASS — This PR makes no changes to CLI commands, flags, help text, or output structure. The only user-visible effect is that method argument fields annotated with z.meta({ sensitive: true }) now render as "***" in method summary reports (both Markdown and JSON variants) instead of their raw values. Both output modes are consistently updated through the shared redactedMethod object, so log/JSON parity is preserved. The design doc update in design/vaults.md gives extension authors the documentation they need to use the feature.

[github-actions]

Code Review

Clean, well-tested security feature. The implementation correctly extends the existing SecretRedactor pattern to cover method argument schemas, placing the domain logic appropriately and threading the redactor through as a dependency.

DDD assessment: Good separation — extractSensitiveFieldValues is a pure stateless domain function in the models layer; secret registration is orchestrated in the application layer (run.ts) and workflow executor (execution_service.ts); SecretRedactor is injected as a dependency. No domain model leaks.

Import boundary: src/libswamp/models/run.ts importing from ../../domain/models/sensitive_field_extractor.ts follows the existing pattern in that file (other domain imports already present). The CLAUDE.md rule applies to CLI commands/renderers importing libswamp internals, not to libswamp importing from domain.

Test coverage: Thorough — 9 unit tests for extractSensitiveFieldValues, integration tests for both modelMethodRun (string + string-array redactor registration) and buildRedactSensitiveArgs (report-level redaction). Edge cases (null, undefined, non-string array elements, empty schema) are covered.

Suggestions

1. The sensitive-field registration block in run.ts:408-431 and execution_service.ts:669-692 is near-identical. A shared helper like registerSensitiveArgs(redactor, modelDef, evaluatedDefinition, methodName) in sensitive_field_extractor.ts would reduce duplication. Not blocking since the two call sites differ slightly (redactor vs ctx.secretRedactor, input.methodName vs task.methodName).

2. createResourceWriter now accepts 13 positional parameters — pre-existing debt, but the undefined, // onEvent placeholder at the method_execution_service.ts call site (line 129) signals it's getting unwieldy. A future cleanup to an options object would help readability. Not introduced by this PR so not blocking.

3. The JSDoc on extractSensitiveFieldValues (13 lines) is more verbose than CLAUDE.md's "one short line max" guideline, but it matches the existing style of extractSensitiveFields directly above it — consistent within the file.

[github-actions]

Adversarial Review

Critical / High

None found.

Medium

1. No integration test for data_writer.ts redaction path — The PR tests that secrets are registered with the redactor (run_test.ts) and that buildRedactSensitiveArgs works for reports (report_execution_service_test.ts), but there is no test verifying that createResourceWriter actually applies the redactor when serializing resource data to disk (the redactor.redact(JSON.stringify(data)) path at src/domain/models/data_writer.ts:601). If someone refactors that block and drops the redactor call, no test would catch it. Consider adding a unit test that passes a pre-populated SecretRedactor to createResourceWriter and asserts the serialized output has the secret scrubbed.

Low

1. Redundant secret registration for vault-sourced sensitive arguments — In both run.ts:408-431 and execution_service.ts:666-692, extractSensitiveFieldValues runs on the post-vault-resolution evaluatedDefinition. For vault-sourced fields, the extracted values are sentinel tokens (not actual secrets), because resolveRuntimeExpressionsInDefinition already replaced them and registered the raw values with the redactor. Registering sentinel tokens is harmless but is wasted work. Not blocking.

2. null values in report redaction — buildRedactSensitiveArgs (report_execution_service.ts:312) checks getNestedValue !== undefined but does not check for null. A sensitive field explicitly set to null would be redacted to *** in the report summary, which could be mildly confusing since no actual secret was present. Cosmetic only.

Verdict

PASS — The implementation is well-structured, follows existing patterns, and correctly handles all three redaction surfaces (log file, resource data, summary report). Positional parameters in createResourceWriter calls are correctly ordered. The new redactor parameter is optional and appended, preserving backwards compatibility. Edge cases are handled and tested. The SecretRedactor's existing JSON-escaping in addSecret and the < 3 char threshold are leveraged correctly.