diff --git a/src/handlers/harness/create/index.tsx b/src/handlers/harness/create/index.tsx index 6c5546105..f9b6e4d0a 100644 --- a/src/handlers/harness/create/index.tsx +++ b/src/handlers/harness/create/index.tsx @@ -20,88 +20,73 @@ export const createCreateHarnessHandler = (core: Core) => name: "create", description: "create a harness", flags: [ - flag("name", "the name of the harness", z.string().optional(), parameterHelp.name), + flag("name", "the name of the harness", z.string().optional(), { help: parameterHelp.name }), flag( "execution-role-arn", "IAM role the harness assumes; a default role is created when omitted", z.string().optional(), - parameterHelp.executionRoleArn, - ), - flag( - "system-prompt", - "the agent's system prompt", - z.string().optional(), - parameterHelp.systemPrompt, - ), - flag( - "model", - "model configuration (JSON HarnessModelConfiguration)", - z.string().optional(), - parameterHelp.model, - ), - flag( - "tools", - "tools available to the agent (JSON HarnessTool[])", - z.string().optional(), - parameterHelp.tools, - ), - flag( - "skills", - "skills available to the agent (JSON HarnessSkill[])", - z.string().optional(), - parameterHelp.skills, + { help: parameterHelp.executionRoleArn }, ), + flag("system-prompt", "the agent's system prompt", z.string().optional(), { + help: parameterHelp.systemPrompt, + }), + flag("model", "model configuration (JSON HarnessModelConfiguration)", z.string().optional(), { + help: parameterHelp.model, + }), + flag("tools", "tools available to the agent (JSON HarnessTool[])", z.string().optional(), { + help: parameterHelp.tools, + }), + flag("skills", "skills available to the agent (JSON HarnessSkill[])", z.string().optional(), { + help: parameterHelp.skills, + }), flag( "allowed-tools", "tool allowlist patterns (e.g. * or @serverName/toolName)", z.array(z.string()).optional(), - parameterHelp.allowedTools, + { help: parameterHelp.allowedTools }, ), flag( "memory", "memory configuration (JSON HarnessMemoryConfiguration)", z.string().optional(), - parameterHelp.memory, + { help: parameterHelp.memory }, ), flag( "truncation", "context truncation configuration (JSON HarnessTruncationConfiguration)", z.string().optional(), - parameterHelp.truncation, + { help: parameterHelp.truncation }, ), flag( "environment", "compute environment configuration (JSON HarnessEnvironmentProviderRequest)", z.string().optional(), - parameterHelp.environment, + { help: parameterHelp.environment }, ), flag( "environment-artifact", "environment artifact, e.g. a container image (JSON HarnessEnvironmentArtifact)", z.string().optional(), - parameterHelp.environmentArtifact, + { help: parameterHelp.environmentArtifact }, ), flag( "environment-variables", "environment variables (JSON object of key/value strings)", z.string().optional(), - parameterHelp.environmentVariables, + { help: parameterHelp.environmentVariables }, ), flag( "authorizer-configuration", "inbound authorizer configuration (JSON AuthorizerConfiguration)", z.string().optional(), - parameterHelp.authorizerConfiguration, + { help: parameterHelp.authorizerConfiguration }, ), flag("max-iterations", "max agent loop iterations per invocation", z.number().optional()), flag("max-tokens", "max total output tokens per invocation", z.number().optional()), flag("timeout-seconds", "max duration in seconds per invocation", z.number().optional()), - flag( - "tags", - "tags to apply (JSON object of key/value strings)", - z.string().optional(), - parameterHelp.tags, - ), + flag("tags", "tags to apply (JSON object of key/value strings)", z.string().optional(), { + help: parameterHelp.tags, + }), flag("client-token", "idempotency token", z.string().optional()), ], handle: async (ctx, flags) => { diff --git a/src/handlers/harness/update/index.tsx b/src/handlers/harness/update/index.tsx index 3d8c908f9..87e74c51a 100644 --- a/src/handlers/harness/update/index.tsx +++ b/src/handlers/harness/update/index.tsx @@ -29,47 +29,32 @@ export const createUpdateHarnessHandler = (core: Core) => description: "update a harness (creates a new version)", flags: [ flag("id", "the ID of the harness to update", z.string().max(48).optional()), - flag( - "execution-role-arn", - "IAM role the harness assumes", - z.string().optional(), - parameterHelp.executionRoleArn, - ), - flag( - "system-prompt", - "the agent's system prompt", - z.string().optional(), - parameterHelp.systemPrompt, - ), - flag( - "model", - "model configuration (JSON HarnessModelConfiguration)", - z.string().optional(), - parameterHelp.model, - ), - flag( - "tools", - "tools available to the agent (JSON HarnessTool[])", - z.string().optional(), - parameterHelp.tools, - ), - flag( - "skills", - "skills available to the agent (JSON HarnessSkill[])", - z.string().optional(), - parameterHelp.skills, - ), + flag("execution-role-arn", "IAM role the harness assumes", z.string().optional(), { + help: parameterHelp.executionRoleArn, + }), + flag("system-prompt", "the agent's system prompt", z.string().optional(), { + help: parameterHelp.systemPrompt, + }), + flag("model", "model configuration (JSON HarnessModelConfiguration)", z.string().optional(), { + help: parameterHelp.model, + }), + flag("tools", "tools available to the agent (JSON HarnessTool[])", z.string().optional(), { + help: parameterHelp.tools, + }), + flag("skills", "skills available to the agent (JSON HarnessSkill[])", z.string().optional(), { + help: parameterHelp.skills, + }), flag( "allowed-tools", "tool allowlist patterns (e.g. * or @serverName/toolName)", z.array(z.string()).optional(), - parameterHelp.allowedTools, + { help: parameterHelp.allowedTools }, ), flag( "memory", "memory configuration (JSON HarnessMemoryConfiguration)", z.string().optional(), - parameterHelp.memory, + { help: parameterHelp.memory }, ), flag( "clear-memory", @@ -80,19 +65,19 @@ export const createUpdateHarnessHandler = (core: Core) => "truncation", "context truncation configuration (JSON HarnessTruncationConfiguration)", z.string().optional(), - parameterHelp.truncation, + { help: parameterHelp.truncation }, ), flag( "environment", "compute environment configuration (JSON HarnessEnvironmentProviderRequest)", z.string().optional(), - parameterHelp.environment, + { help: parameterHelp.environment }, ), flag( "environment-artifact", "environment artifact, e.g. a container image (JSON HarnessEnvironmentArtifact)", z.string().optional(), - parameterHelp.environmentArtifact, + { help: parameterHelp.environmentArtifact }, ), flag( "clear-environment-artifact", @@ -103,13 +88,13 @@ export const createUpdateHarnessHandler = (core: Core) => "environment-variables", "environment variables (JSON object; replaces all existing)", z.string().optional(), - parameterHelp.environmentVariables, + { help: parameterHelp.environmentVariables }, ), flag( "authorizer-configuration", "inbound authorizer configuration (JSON AuthorizerConfiguration)", z.string().optional(), - parameterHelp.authorizerConfiguration, + { help: parameterHelp.authorizerConfiguration }, ), flag( "clear-authorizer-configuration", diff --git a/src/middleware/withLogging.test.ts b/src/middleware/withLogging.test.ts index e8fd67b4a..68e56af0d 100644 --- a/src/middleware/withLogging.test.ts +++ b/src/middleware/withLogging.test.ts @@ -1,5 +1,6 @@ import { test, describe, beforeEach, afterEach } from "bun:test"; -import { Router, createHandler } from "../router"; +import z from "zod"; +import { Router, createHandler, flag } from "../router"; import { withLogging } from "./withLogging"; import { createFileLogger } from "../logging/fileLogger"; import { LOG_LEVEL, type AsyncLogger } from "../logging/types"; @@ -25,6 +26,42 @@ describe("withLogging", () => { await rm(tempDir, { recursive: true, force: true }); }); + test("redacts sensitive flags and preserves all non-sensitive flags", async () => { + const app = new Router("myapp", "test app"); + app.use(withLogging({ logger })); + app.handler( + createHandler({ + name: "login", + description: "login with credentials", + flags: [ + flag("name", "the provider name", z.string().optional()), + flag("api-key", "the secret key", z.string().optional(), { sensitive: true }), + ], + handle: async () => {}, + }), + ); + + await app.route([ + "node", + "myapp", + "login", + "--name", + "my-provider", + "--api-key", + "super-secret", + ]); + + await assertLogsMatch(tempDir, [ + { + filter: (log: any) => + log.msg === "executing command" && + log.flags?.name === "my-provider" && + log.flags?.["api-key"] === "[REDACTED]", + expectedCount: 1, + }, + ]); + }); + test("logs success and error with correct command path bindings", async () => { const app = new Router("myapp", "test app"); app.use(withLogging({ logger })); diff --git a/src/middleware/withLogging.tsx b/src/middleware/withLogging.tsx index ef9b9df59..2b89084de 100644 --- a/src/middleware/withLogging.tsx +++ b/src/middleware/withLogging.tsx @@ -1,11 +1,28 @@ import type { Logger } from "../logging"; -import type { Middleware } from "../router"; +import type { Flag, Middleware } from "../router"; import { LoggerKey, PathKey } from "../router"; interface WithLoggingConfig { logger: Logger; } +const REDACTED = "[REDACTED]"; + +// replace the values of sensitive flags with a placeholder +function redactSensitiveFlags( + flags: Record, + flagDefs: Flag[], +): Record { + const sensitiveNames = new Set(flagDefs.filter((f) => f.sensitive).map((f) => f.name)); + if (sensitiveNames.size === 0) return flags; + + const redacted: Record = {}; + for (const [key, value] of Object.entries(flags)) { + redacted[key] = sensitiveNames.has(key) && value !== undefined ? REDACTED : value; + } + return redacted; +} + /** * Middleware that creates a child logger bound to the current command path * and logs execution start, success, and failure. @@ -23,8 +40,8 @@ export function withLogging(config: WithLoggingConfig): Middleware { const commandPath = ctx.require(PathKey); const logger = config.logger.child({ commandPath }); try { - // TODO: ensure sensitive fields are redacted from flags/args. - logger.child({ flags, args }).debug("executing command"); + const safeFlags = redactSensitiveFlags(flags, h.flags()); + logger.child({ flags: safeFlags, args }).debug("executing command"); await h.handle(ctx.withValue(LoggerKey, logger), flags, args); logger.debug("command executed successfully"); } catch (err) { diff --git a/src/router/handler.tsx b/src/router/handler.tsx index cc3f2ecd2..a760a2318 100644 --- a/src/router/handler.tsx +++ b/src/router/handler.tsx @@ -14,6 +14,8 @@ export interface Flag { // Its first line is the type annotation shown next to the flag name; the // remaining lines are the body — prose, JSON syntax, examples. help?: string; + // sensitive flags are redacted from debug logs by the withLogging middleware. + sensitive?: boolean; } // GlobalFlag is a group-level flag that is *also* a typed ContextKey: declared on @@ -32,9 +34,9 @@ export function flag( name: N, description: string, schema: z.ZodType, - help?: string, + options?: { help?: string; sensitive?: boolean }, ): Flag { - return { name, description, schema, help }; + return { name, description, schema, help: options?.help, sensitive: options?.sensitive }; } // globalFlag constructs a GlobalFlag. The returned value doubles as the typed diff --git a/src/router/router.test.ts b/src/router/router.test.ts index d52650256..11d798fac 100644 --- a/src/router/router.test.ts +++ b/src/router/router.test.ts @@ -632,12 +632,9 @@ test("flags with long-form help render a Parameter details section", async () => description: "", flags: [ flag("name", "the name", z.string().optional()), - flag( - "model", - "model config (JSON)", - z.string().optional(), - `(JSON: tagged union object)\nThe model configuration.\n\nExample:\n --model '{"a":1}'`, - ), + flag("model", "model config (JSON)", z.string().optional(), { + help: `(JSON: tagged union object)\nThe model configuration.\n\nExample:\n --model '{"a":1}'`, + }), ], handle: async () => {}, });