From 6268517a57e968cb4ea7a1adce55fdc1422f26c1 Mon Sep 17 00:00:00 2001 From: Jesse Merhi <79823012+jesse-merhi@users.noreply.github.com> Date: Mon, 27 Jul 2026 13:53:37 +1000 Subject: [PATCH] fix(runner): honor scanner env secrecy declarations --- docs/scanners.md | 18 +++-- internal/profiles/resolver.go | 45 ++++++----- internal/profiles/resolver_test.go | 28 +++++++ internal/runner/runner.go | 42 +++++++++-- internal/runner/runner_test.go | 99 +++++++++++++++++++++++++ internal/runner/user_defined_scanner.go | 33 +++++++-- 6 files changed, 225 insertions(+), 40 deletions(-) diff --git a/docs/scanners.md b/docs/scanners.md index e8da77e..684c505 100644 --- a/docs/scanners.md +++ b/docs/scanners.md @@ -32,6 +32,8 @@ profiles: - id: my-scanner command: my-scanner --json {{target}} env: + - MY_SCANNER_MODE + secretEnv: - MY_SCANNER_TOKEN targets: - skill @@ -47,7 +49,8 @@ that config-backed run and accept these fields: | --- | --- | --- | | `id` | yes | Scanner ID using letters, digits, `_`, and `-`, starting with a letter or digit. It must not match a built-in scanner ID. | | `command` | yes | Shell command to execute. Unquoted `{{target}}` is replaced with the safely passed resolved target; do not wrap the placeholder in shell quotes. | -| `env` | no | Required environment variable names. Values stay in the process environment and are never stored in the config or artifact. | +| `env` | no | Required non-secret environment variable names passed to the scanner. Their values are not automatically redacted from scanner error text. | +| `secretEnv` | no | Required secret environment variable names passed to the scanner. Their values are redacted from scanner error text regardless of how the names are spelled. | | `targets` | no | Supported target kinds: `skill`, `plugin`, and/or `url`. Defaults to `skill` and `url`. | | `gate` | no | Exit-code policy with optional `blockOnExitCode` and `warnOnExitCode` rules. | @@ -86,11 +89,14 @@ result. Required environment variables are checked before any scanner starts. Artifacts record each requirement as only `present` or `missing`. User-defined scanners use the same execution path as built-in command-backed -scanners. They run in the Docker sandbox by default, and declared `env` names -are added to its environment allowlist. Use `--sandbox off` only when you -intentionally want the command to run on the host. User-defined scanners are -local to the resolved config and do not appear in the built-in `clawscan -scanners` catalog. +scanners. They run in the Docker sandbox by default, and declared `env` and +`secretEnv` names are added to its environment allowlist. Put every sensitive +value in `secretEnv`; values declared only in `env` are treated as non-secret +and may appear in scanner error text. ClawScan preserves valid scanner stdout +as raw JSON evidence, so scanner authors must not print secrets into that JSON. +Use `--sandbox off` only when you intentionally want the command to run on the +host. User-defined scanners are local to the resolved config and do not appear +in the built-in `clawscan scanners` catalog. > **Trust boundary:** only load user-defined scanners from config files you > control. A scanner entry is executable code. The default sandbox limits its diff --git a/internal/profiles/resolver.go b/internal/profiles/resolver.go index cf497a4..16273c0 100644 --- a/internal/profiles/resolver.go +++ b/internal/profiles/resolver.go @@ -43,12 +43,13 @@ func (profile Profile) ScannerIDs() []string { } type ProfileScanner struct { - ID string - Command string - Env []string - Targets []string - Gate *ProfileScannerGate - custom bool + ID string + Command string + Env []string + SecretEnv []string + Targets []string + Gate *ProfileScannerGate + custom bool } type ProfileScannerGate struct { @@ -146,7 +147,7 @@ func (scanner *ProfileScanner) UnmarshalYAML(node *yaml.Node) error { case yaml.MappingNode: for index := 0; index < len(node.Content); index += 2 { switch node.Content[index].Value { - case "id", "command", "env", "targets", "gate": + case "id", "command", "env", "secretEnv", "targets", "gate": if node.Content[index].Value == "gate" { gateNode := resolvedYAMLNode(node.Content[index+1]) if gateNode.Kind != yaml.MappingNode { @@ -158,11 +159,12 @@ func (scanner *ProfileScanner) UnmarshalYAML(node *yaml.Node) error { } } var value struct { - ID string `yaml:"id"` - Command string `yaml:"command"` - Env []string `yaml:"env,omitempty"` - Targets []string `yaml:"targets,omitempty"` - Gate *ProfileScannerGate `yaml:"gate,omitempty"` + ID string `yaml:"id"` + Command string `yaml:"command"` + Env []string `yaml:"env,omitempty"` + SecretEnv []string `yaml:"secretEnv,omitempty"` + Targets []string `yaml:"targets,omitempty"` + Gate *ProfileScannerGate `yaml:"gate,omitempty"` } if err := node.Decode(&value); err != nil { return err @@ -170,6 +172,7 @@ func (scanner *ProfileScanner) UnmarshalYAML(node *yaml.Node) error { scanner.ID = value.ID scanner.Command = value.Command scanner.Env = value.Env + scanner.SecretEnv = value.SecretEnv scanner.Targets = value.Targets scanner.Gate = value.Gate scanner.custom = true @@ -191,12 +194,13 @@ func (scanner ProfileScanner) MarshalYAML() (interface{}, error) { return scanner.ID, nil } return struct { - ID string `yaml:"id"` - Command string `yaml:"command"` - Env []string `yaml:"env,omitempty"` - Targets []string `yaml:"targets,omitempty"` - Gate *ProfileScannerGate `yaml:"gate,omitempty"` - }{scanner.ID, scanner.Command, scanner.Env, scanner.Targets, scanner.Gate}, nil + ID string `yaml:"id"` + Command string `yaml:"command"` + Env []string `yaml:"env,omitempty"` + SecretEnv []string `yaml:"secretEnv,omitempty"` + Targets []string `yaml:"targets,omitempty"` + Gate *ProfileScannerGate `yaml:"gate,omitempty"` + }{scanner.ID, scanner.Command, scanner.Env, scanner.SecretEnv, scanner.Targets, scanner.Gate}, nil } func profileScannerIDs(scanners []ProfileScanner) []string { @@ -218,7 +222,7 @@ func profileScannerRegistry(scanners []ProfileScanner) (runner.ScannerRegistry, targets = []string{"skill", "url"} } adapter := runner.NewUserDefinedScanner(runner.UserDefinedScannerConfig{ - ID: scanner.ID, Command: scanner.Command, Env: scanner.Env, Targets: targets, + ID: scanner.ID, Command: scanner.Command, Env: scanner.Env, SecretEnv: scanner.SecretEnv, Targets: targets, }) var err error registry, err = registry.WithAdapters(adapter) @@ -1002,6 +1006,9 @@ func validateProfile(name string, profile Profile) error { if bad := invalidDeclaredEnvName(scanner.Env); bad != "" { return fmt.Errorf("User-defined scanner %s in profile %s has an invalid env entry %q; declare bare variable names and set values in the environment, not inline", scanner.ID, name, bad) } + if bad := invalidDeclaredEnvName(scanner.SecretEnv); bad != "" { + return fmt.Errorf("User-defined scanner %s in profile %s has an invalid secretEnv entry %q; declare bare variable names and set values in the environment, not inline", scanner.ID, name, bad) + } } if scanner.custom { allUnquoted, activeCount := scannerTargetPlaceholderState(scanner.Command) diff --git a/internal/profiles/resolver_test.go b/internal/profiles/resolver_test.go index fdb7f43..4ce4ab8 100644 --- a/internal/profiles/resolver_test.go +++ b/internal/profiles/resolver_test.go @@ -761,6 +761,8 @@ profiles: - id: my-scanner command: my-scanner --json {{target}} env: + - MY_SCANNER_MODE + secretEnv: - MY_SCANNER_TOKEN targets: - plugin @@ -783,6 +785,9 @@ profiles: if !adapter.SupportsTargetKind("plugin") { t.Fatal("plugin-only custom scanner does not support plugin targets") } + if got := adapter.Info().RequiredEnv; !reflect.DeepEqual(got, []string{"MY_SCANNER_MODE", "MY_SCANNER_TOKEN"}) { + t.Fatalf("required env = %#v", got) + } } func TestResolveArgsParsesUserDefinedScannerExitCodeGateRules(t *testing.T) { @@ -1141,6 +1146,29 @@ profiles: } } +func TestResolveArgsRejectsInlineUserDefinedScannerSecretEnvValue(t *testing.T) { + dir := t.TempDir() + config := filepath.Join(dir, ".clawscan.yml") + writeFile(t, config, `version: 1 +profiles: + review: + scanners: + - id: my-scanner + command: my-scanner {{target}} + secretEnv: + - SCANNER_AUTH=credential-sensitive-suffix +`) + + _, err := ResolveArgs([]string{"./skill", "--config", config, "--profile", "review"}, dir) + want := `User-defined scanner my-scanner in profile review has an invalid secretEnv entry "SCANNER_AUTH"; declare bare variable names and set values in the environment, not inline` + if err == nil || err.Error() != want { + t.Fatalf("err = %v, want %q", err, want) + } + if err != nil && strings.Contains(err.Error(), "credential-sensitive-suffix") { + t.Fatalf("error leaked the inline secretEnv value: %v", err) + } +} + func TestResolveArgsAllowsBareUserDefinedScannerEnvName(t *testing.T) { dir := t.TempDir() config := filepath.Join(dir, ".clawscan.yml") diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 390f167..04d266d 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -2284,22 +2284,50 @@ func envMapToEnviron(env map[string]string) []string { } func commandError(runErr error, stderr string, env map[string]string) string { - message := redactEnvValues(runErr.Error(), env) - if strings.TrimSpace(stderr) != "" { - message += ": " + redactEnvValues(strings.TrimSpace(stderr), env) + names := make([]string, 0) + for key := range env { + if isSecretEnvKey(key) { + names = append(names, key) + } + } + return commandErrorForEnvNames(runErr, stderr, env, names) +} + +func commandErrorForEnvNames(runErr error, stderr string, env map[string]string, names []string) string { + message := redactEnvValuesForNames(runErr.Error(), env, names) + redactedStderr := strings.TrimSpace(redactEnvValuesForNames(stderr, env, names)) + if redactedStderr != "" { + message += ": " + redactedStderr } return message } func redactEnvValues(value string, env map[string]string) string { - if value == "" || len(env) == 0 { + names := make([]string, 0) + for key := range env { + if isSecretEnvKey(key) { + names = append(names, key) + } + } + return redactEnvValuesForNames(value, env, names) +} + +func redactEnvValuesForNames(value string, env map[string]string, names []string) string { + if value == "" || len(env) == 0 || len(names) == 0 { return value } - secrets := make([]string, 0) - for key, secret := range env { - if strings.TrimSpace(secret) == "" || !isSecretEnvKey(key) { + secretSet := make(map[string]bool, len(names)) + for _, name := range names { + secret := env[name] + trimmed := strings.TrimSpace(secret) + if trimmed == "" { continue } + secretSet[secret] = true + secretSet[trimmed] = true + } + secrets := make([]string, 0, len(secretSet)) + for secret := range secretSet { secrets = append(secrets, secret) } sort.Slice(secrets, func(i int, j int) bool { diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index 8a5eb85..2dc37b7 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -990,6 +990,105 @@ func TestUserDefinedScannerMountsDockerTargetReadOnly(t *testing.T) { } } +func TestUserDefinedScannerRedactsOnlySecretEnvFromErrors(t *testing.T) { + adapter := NewUserDefinedScanner(UserDefinedScannerConfig{ + ID: "test", + Command: "test {{target}}", + Env: []string{"URL_SCANNER_TOKEN"}, + SecretEnv: []string{"SCANNER_AUTH"}, + Targets: []string{"skill"}, + }) + registry, err := NewScannerRegistry(adapter) + if err != nil { + t.Fatal(err) + } + commandRunner := &recordingCommandRunner{ + stderr: "mode visible-value auth credential-sensitive-suffix", + err: errCommandFailed, + } + result, err := (ExternalScannerRunner{ + Registry: registry, CommandRunner: commandRunner, + Env: map[string]string{ + "URL_SCANNER_TOKEN": "visible-value", + "SCANNER_AUTH": "credential-sensitive-suffix", + }, + SandboxMode: SandboxModeOff, + }).RunScanner("test", filepath.Join(t.TempDir(), "file.txt"), "2026-07-21T00:00:00Z") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(result.Error, "visible-value") { + t.Fatalf("plain env value was redacted: %q", result.Error) + } + if strings.Contains(result.Error, "credential-sensitive-suffix") || !strings.Contains(result.Error, "[redacted]") { + t.Fatalf("secretEnv value was not redacted: %q", result.Error) + } +} + +func TestUserDefinedScannerPreservesSecretEnvInRawEvidence(t *testing.T) { + adapter := NewUserDefinedScanner(UserDefinedScannerConfig{ + ID: "test", + Command: "test {{target}}", + SecretEnv: []string{"SCANNER_AUTH"}, + Targets: []string{"skill"}, + }) + registry, err := NewScannerRegistry(adapter) + if err != nil { + t.Fatal(err) + } + commandRunner := &recordingCommandRunner{ + stdout: `{"evidence":"credential-sensitive-suffix"}`, + stderr: "auth credential-sensitive-suffix", + err: errCommandFailed, + } + result, err := (ExternalScannerRunner{ + Registry: registry, CommandRunner: commandRunner, + Env: map[string]string{"SCANNER_AUTH": "credential-sensitive-suffix"}, + SandboxMode: SandboxModeOff, + }).RunScanner("test", filepath.Join(t.TempDir(), "file.txt"), "2026-07-21T00:00:00Z") + if err != nil { + t.Fatal(err) + } + if strings.Contains(result.Error, "credential-sensitive-suffix") { + t.Fatalf("secretEnv value leaked into error: %q", result.Error) + } + if !strings.Contains(string(result.Raw), "credential-sensitive-suffix") { + t.Fatalf("raw scanner evidence was modified: %s", result.Raw) + } +} + +func TestCommandErrorForEnvNamesRedactsOverlappingAndTrimmedValues(t *testing.T) { + env := map[string]string{ + "PLAIN_TOKEN": "credential", + "SCANNER_AUTH": " credential-sensitive-suffix ", + } + result := commandErrorForEnvNames( + errCommandFailed, + "credential-sensitive-suffix rejected; credential remains visible", + env, + []string{"SCANNER_AUTH"}, + ) + if result != "exit status 1: [redacted] rejected; credential remains visible" { + t.Fatalf("result = %q", result) + } +} + +func TestUserDefinedScannerRequirementsIncludeEnvAndSecretEnv(t *testing.T) { + adapter := NewUserDefinedScanner(UserDefinedScannerConfig{ + ID: "test", + Command: "test {{target}}", + Env: []string{"MODE"}, + SecretEnv: []string{"SCANNER_AUTH"}, + }) + requirements := adapter.Requirements(nil) + if len(requirements) != 2 || requirements[0].EnvVar != "MODE" || requirements[1].EnvVar != "SCANNER_AUTH" { + t.Fatalf("requirements = %#v", requirements) + } + if got := adapter.Info().RequiredEnv; !reflect.DeepEqual(got, []string{"MODE", "SCANNER_AUTH"}) { + t.Fatalf("required env = %#v", got) + } +} + func TestValidateRequirementsSkipsScannerResultCredentials(t *testing.T) { opts, err := ParseArgs([]string{ "./my-skill", diff --git a/internal/runner/user_defined_scanner.go b/internal/runner/user_defined_scanner.go index 60ac5c5..c0a8b6f 100644 --- a/internal/runner/user_defined_scanner.go +++ b/internal/runner/user_defined_scanner.go @@ -11,10 +11,11 @@ import ( ) type UserDefinedScannerConfig struct { - ID string - Command string - Env []string - Targets []string + ID string + Command string + Env []string + SecretEnv []string + Targets []string } func NewUserDefinedScanner(config UserDefinedScannerConfig) ScannerAdapter { @@ -33,15 +34,19 @@ type userDefinedScannerAdapter struct { func (adapter userDefinedScannerAdapter) ID() string { return adapter.config.ID } func (adapter userDefinedScannerAdapter) Requirements(_ map[string]string) []EnvRequirement { - requirements := make([]EnvRequirement, 0, len(adapter.config.Env)) - for _, name := range adapter.config.Env { + names := userDefinedScannerEnvNames(adapter.config) + requirements := make([]EnvRequirement, 0, len(names)) + for _, name := range names { requirements = append(requirements, EnvRequirement{EnvVar: name, Reason: adapter.config.ID + " scanner"}) } return requirements } func (adapter userDefinedScannerAdapter) Info() ScannerInfo { - return ScannerInfo{ID: adapter.config.ID, DisplayName: adapter.config.ID, RequiredEnv: append([]string(nil), adapter.config.Env...)} + return ScannerInfo{ + ID: adapter.config.ID, DisplayName: adapter.config.ID, + RequiredEnv: userDefinedScannerEnvNames(adapter.config), + } } func (adapter userDefinedScannerAdapter) InstallPlan() InstallPlan { @@ -108,7 +113,7 @@ func (adapter userDefinedScannerAdapter) Run(runner ExternalScannerRunner, targe completedAt := time.Now().UTC().Format(time.RFC3339Nano) raw := strings.TrimSpace(output.Stdout) if runErr != nil { - message := commandError(runErr, output.Stderr, runner.Env) + message := commandErrorForEnvNames(runErr, output.Stderr, runner.Env, adapter.config.SecretEnv) if json.Valid([]byte(raw)) { return ScannerResult{ Status: "completed", StartedAt: startedAt, CompletedAt: completedAt, Command: fullCommand, @@ -132,6 +137,18 @@ func (adapter userDefinedScannerAdapter) Run(runner ExternalScannerRunner, targe }, nil } +func userDefinedScannerEnvNames(config UserDefinedScannerConfig) []string { + names := make([]string, 0, len(config.Env)+len(config.SecretEnv)) + seen := make(map[string]bool, cap(names)) + for _, name := range append(append([]string(nil), config.Env...), config.SecretEnv...) { + if !seen[name] { + names = append(names, name) + seen[name] = true + } + } + return names +} + func gateEligibleExitCode(exitCode *int) *int { if exitCode == nil || *exitCode < 0 || *exitCode > MaxGateExitCode { return nil