diff --git a/docs/scanners.md b/docs/scanners.md index cbb2f1c..7fa6d8d 100644 --- a/docs/scanners.md +++ b/docs/scanners.md @@ -33,6 +33,8 @@ profiles: command: my-scanner --json {{target}} env: - MY_SCANNER_TOKEN + secretEnv: + - MY_SECRET_KEY targets: - skill - plugin @@ -47,7 +49,8 @@ that config-backed run and accept these fields: | --- | --- | --- | | `id` | yes | Scanner ID using lowercase letters, digits, `_`, and `-`, starting with a letter or digit, at most 64 characters. It must not match a built-in scanner ID or a reserved Windows device name such as `con`, `nul`, or `com1`. | | `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 | Environment variable names passed through to the scanner. Values are shown in output unless the name itself looks like a credential (e.g., ends with `_TOKEN` or `_PASSWORD`), in which case the heuristic backstop redacts them. | +| `secretEnv` | no | Environment variable names passed through to the scanner and always redacted from persisted output, regardless of name. Use this for credentials and sensitive values. | | `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. | @@ -90,10 +93,10 @@ value if they must be redacted. Required environment variables are checked before any scanner starts. Artifacts record each requirement as only `present` or `missing`. -Scanner credentials should be supplied through environment variables declared under `env:` (or the sandbox env allowlist). ClawScan can only redact values it was told about via declared env vars; a secret written directly into the command — whether as an inline `NAME=value` assignment or as a flag value such as `--token sk-live` — is outside every redaction scope and can leak into saved evidence if the scanner echoes its arguments. ClawScan does not block either form; keeping all credentials in declared environment variables is the operator's responsibility. +Scanner credentials should be supplied through environment variables. Use `secretEnv:` to declare environment variables that must always be redacted, even if their names don't look like credentials. Use `env:` for non-secret configuration that should appear in output; credential-named variables (e.g., `MY_TOKEN`, `API_KEY`) in `env:` are still redacted as a safety backstop. ClawScan can only redact values it was told about via declared env vars; a secret written directly into the command — whether as an inline `NAME=value` assignment or as a flag value such as `--token sk-live` — is outside every redaction scope and can leak into saved evidence if the scanner echoes its arguments. ClawScan does not block either form; keeping all credentials in declared environment variables is the operator's responsibility. 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 +scanners. They run in the Docker sandbox by default, and declared `env` and `secretEnv` 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 diff --git a/internal/profiles/resolver.go b/internal/profiles/resolver.go index 2744ba6..f7518f2 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 { @@ -153,34 +154,35 @@ 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": default: return fmt.Errorf("field %s not found in type profiles.ProfileScanner", node.Content[index].Value) } } for index := 0; index < len(node.Content); index += 2 { - if node.Content[index].Value != "env" { + if node.Content[index].Value != "env" && node.Content[index].Value != "secretEnv" { continue } envNode := node.Content[index+1] nullScalar := envNode.Kind == yaml.ScalarNode && (envNode.Tag == "!!null" || envNode.Value == "") if envNode.Kind != yaml.SequenceNode && !nullScalar { - return errors.New("scanner env must be a list of variable names") + return fmt.Errorf("scanner %s must be a list of variable names", node.Content[index].Value) } if envNode.Kind == yaml.SequenceNode { for entryIndex, entry := range envNode.Content { if entry.Kind != yaml.ScalarNode { - return fmt.Errorf("scanner env entry #%d must be a variable name", entryIndex+1) + return fmt.Errorf("scanner %s entry #%d must be a variable name", node.Content[index].Value, entryIndex+1) } } } } 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 @@ -188,6 +190,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 @@ -202,12 +205,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 { @@ -227,12 +231,15 @@ func profileScannerRegistry(scanners []ProfileScanner) (runner.ScannerRegistry, if bad := runner.InvalidUserDefinedEnvName(scanner.Env); bad != "" { return runner.ScannerRegistry{}, fmt.Errorf("scanner %s env entry %s is not a variable name; declare bare names and set values in the environment", scanner.ID, bad) } + if bad := runner.InvalidUserDefinedEnvName(scanner.SecretEnv); bad != "" { + return runner.ScannerRegistry{}, fmt.Errorf("scanner %s secretEnv entry %s is not a variable name; declare bare names and set values in the environment", scanner.ID, bad) + } targets := append([]string(nil), scanner.Targets...) if len(targets) == 0 { 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) @@ -244,7 +251,7 @@ func profileScannerRegistry(scanners []ProfileScanner) (runner.ScannerRegistry, } // declaredEnvNames unions the env var names every profile in the registry -// declares (scanner env plus sandbox passthrough). A single-profile run +// declares (scanner secretEnv plus sandbox passthrough). A single-profile run // with --sandbox off inherits the full host environment, so a blandly // named credential declared only by a sibling profile in the same config // must still be redacted from persisted output. @@ -260,9 +267,9 @@ func (registry ProfileRegistry) declaredEnvNames() []string { } for _, resolved := range registry.profiles { for _, scanner := range resolved.profile.Scanners { - // scanner env: entries are credentials by declaration whatever - // their spelling. - for _, name := range scanner.Env { + // scanner secretEnv: entries are credentials by declaration + // whatever their spelling. + for _, name := range scanner.SecretEnv { add(name) } } diff --git a/internal/profiles/resolver_test.go b/internal/profiles/resolver_test.go index ce416dc..0ba8860 100644 --- a/internal/profiles/resolver_test.go +++ b/internal/profiles/resolver_test.go @@ -186,7 +186,7 @@ profiles: scanners: - id: alpha command: alpha {{target}} - env: [SHARED_ACCESS] + secretEnv: [SHARED_ACCESS] profile-b: scanners: - id: beta diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index bec9293..eb0780f 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -1381,7 +1381,7 @@ func TestRunHostRedactionCoversSkippedScannersEnv(t *testing.T) { ID: "alpha", Command: "alpha {{target}}", Targets: []string{"skill"}, }) pluginOnly := NewUserDefinedScanner(UserDefinedScannerConfig{ - ID: "plugin-only", Command: "plugin-only {{target}}", Env: []string{"BETA_LICENSE"}, Targets: []string{"plugin"}, + ID: "plugin-only", Command: "plugin-only {{target}}", SecretEnv: []string{"BETA_LICENSE"}, Targets: []string{"plugin"}, }) registry, err := NewScannerRegistry(alpha, pluginOnly) if err != nil { @@ -1421,7 +1421,7 @@ func TestRunHostRedactionCoversUnselectedRegistryScannersEnv(t *testing.T) { t.Fatal(err) } alpha := NewUserDefinedScanner(UserDefinedScannerConfig{ - ID: "alpha", Command: "alpha {{target}}", Env: []string{"ALPHA_ACCESS"}, Targets: []string{"skill"}, + ID: "alpha", Command: "alpha {{target}}", SecretEnv: []string{"ALPHA_ACCESS"}, Targets: []string{"skill"}, }) beta := NewUserDefinedScanner(UserDefinedScannerConfig{ ID: "beta", Command: "beta {{target}}", Targets: []string{"skill"}, @@ -1467,7 +1467,7 @@ func TestRunRedactsDeclaredCredentialsFromFixtureResults(t *testing.T) { t.Fatal(err) } alpha := NewUserDefinedScanner(UserDefinedScannerConfig{ - ID: "alpha", Command: "alpha {{target}}", Env: []string{"SCANNER_ACCESS"}, Targets: []string{"skill"}, + ID: "alpha", Command: "alpha {{target}}", SecretEnv: []string{"SCANNER_ACCESS"}, Targets: []string{"skill"}, }) registry, err := NewScannerRegistry(alpha) if err != nil { @@ -1500,7 +1500,7 @@ func TestRunScannerRedactsDeclaredCredentialsFromBuiltinAdapters(t *testing.T) { // built-in scanner exposes the credential to both, and the built-in's // stdout/stderr must be scrubbed too. alpha := NewUserDefinedScanner(UserDefinedScannerConfig{ - ID: "alpha", Command: "alpha {{target}}", Env: []string{"SCANNER_ACCESS"}, Targets: []string{"skill"}, + ID: "alpha", Command: "alpha {{target}}", SecretEnv: []string{"SCANNER_ACCESS"}, Targets: []string{"skill"}, }) registry, err := DefaultScannerRegistry().WithAdapters(alpha) if err != nil { @@ -1559,7 +1559,7 @@ func TestRedactionEnvNamesDockerExcludesUnexposedSiblingCredentials(t *testing.T // enters the container; scrubbing its value (ALPHA_ACCESS=clean) would // rewrite legitimate "clean" verdicts without preventing a leak. alpha := NewUserDefinedScanner(UserDefinedScannerConfig{ - ID: "alpha", Command: "alpha {{target}}", Env: []string{"ALPHA_ACCESS"}, Targets: []string{"skill"}, + ID: "alpha", Command: "alpha {{target}}", SecretEnv: []string{"ALPHA_ACCESS"}, Targets: []string{"skill"}, }) beta := NewUserDefinedScanner(UserDefinedScannerConfig{ ID: "beta", Command: "beta {{target}}", Targets: []string{"skill"}, @@ -1605,7 +1605,7 @@ func TestRunDockerRedactionSkipsNonRunnableScannerCredentials(t *testing.T) { ID: "alpha", Command: "alpha {{target}}", Targets: []string{"skill"}, }) pluginOnly := NewUserDefinedScanner(UserDefinedScannerConfig{ - ID: "plugin-only", Command: "plugin-only {{target}}", Env: []string{"BETA_LICENSE"}, Targets: []string{"plugin"}, + ID: "plugin-only", Command: "plugin-only {{target}}", SecretEnv: []string{"BETA_LICENSE"}, Targets: []string{"plugin"}, }) registry, err := NewScannerRegistry(alpha, pluginOnly) if err != nil { @@ -1773,7 +1773,7 @@ func TestRunProfileBatchRedactsSiblingProfileCredentials(t *testing.T) { t.Fatal(err) } alpha := NewUserDefinedScanner(UserDefinedScannerConfig{ - ID: "alpha", Command: "alpha {{target}}", Env: []string{"ALPHA_ACCESS"}, Targets: []string{"skill"}, + ID: "alpha", Command: "alpha {{target}}", SecretEnv: []string{"ALPHA_ACCESS"}, Targets: []string{"skill"}, }) alphaRegistry, err := NewScannerRegistry(alpha) if err != nil { @@ -4777,7 +4777,7 @@ func TestRunJudgeRedactsDeclaredScannerEnvFromResult(t *testing.T) { t.Fatal(err) } custom := NewUserDefinedScanner(UserDefinedScannerConfig{ - ID: "custom", Command: "custom {{target}}", Env: []string{"SCANNER_ACCESS"}, Targets: []string{"skill"}, + ID: "custom", Command: "custom {{target}}", SecretEnv: []string{"SCANNER_ACCESS"}, Targets: []string{"skill"}, }) registry, err := DefaultScannerRegistry().WithAdapters(custom) if err != nil { @@ -4824,7 +4824,7 @@ func TestRunJudgeRedactsNumericDeclaredCredentialScalar(t *testing.T) { t.Fatal(err) } custom := NewUserDefinedScanner(UserDefinedScannerConfig{ - ID: "custom", Command: "custom {{target}}", Env: []string{"SCANNER_ACCESS"}, Targets: []string{"skill"}, + ID: "custom", Command: "custom {{target}}", SecretEnv: []string{"SCANNER_ACCESS"}, Targets: []string{"skill"}, }) registry, err := DefaultScannerRegistry().WithAdapters(custom) if err != nil { @@ -4875,7 +4875,7 @@ func TestRunFixtureScannerEnvStillRedactedOnHost(t *testing.T) { t.Fatal(err) } fixtureScanner := NewUserDefinedScanner(UserDefinedScannerConfig{ - ID: "fixture-scanner", Command: "fixture {{target}}", Env: []string{"SCANNER_ACCESS"}, Targets: []string{"skill"}, + ID: "fixture-scanner", Command: "fixture {{target}}", SecretEnv: []string{"SCANNER_ACCESS"}, Targets: []string{"skill"}, }) live := NewUserDefinedScanner(UserDefinedScannerConfig{ ID: "live-scanner", Command: "live {{target}}", Targets: []string{"skill"}, diff --git a/internal/runner/sandbox.go b/internal/runner/sandbox.go index 0ca6cc9..212378a 100644 --- a/internal/runner/sandbox.go +++ b/internal/runner/sandbox.go @@ -432,9 +432,19 @@ func redactionEnvNames(opts Options, env map[string]string, sandboxMode string) // credentials — except explicit user-defined env: declarations, which // are credentials whatever their name. declared := declaredCredentialEnvNames(opts) + // Plain env: declarations are operator-chosen configuration, shown in + // evidence. They reach the redaction sweep via the reachability set + // (Requirements/RequiredEnv union env+secretEnv), so exempt them from the + // fail-closed CredentialEnvName default below. A plain name that is also a + // credential by declaration (declared) or by spelling (isSecretEnvKey) is + // not exempt: it stays redacted as a backstop. + plain := declaredNonCredentialEnvNames(opts) names := collected[:0] seen := map[string]bool{} for _, name := range collected { + if plain[name] && !declared[name] && !isSecretEnvKey(name) { + continue + } if declared[name] || CredentialEnvName(name) { names = append(names, name) seen[name] = true @@ -480,6 +490,32 @@ func declaredCredentialEnvNames(opts Options) map[string]bool { return declared } +// declaredNonCredentialEnvNames collects plain env: declarations across the +// resolved registry. These names are exempt from redaction's fail-closed +// default so their values stay visible in evidence; see redactionEnvNames. +func declaredNonCredentialEnvNames(opts Options) map[string]bool { + type nonCredentialDeclarer interface { + DeclaredNonCredentialEnv() []string + } + plain := map[string]bool{} + registry := registryForOptions(opts) + for _, id := range registry.IDs() { + adapter, ok := registry.Adapter(id) + if !ok { + continue + } + if declarer, ok := adapter.(nonCredentialDeclarer); ok { + for _, name := range declarer.DeclaredNonCredentialEnv() { + name = strings.TrimSpace(name) + if name != "" { + plain[name] = true + } + } + } + } + return plain +} + // collectEnvNames gathers env var names in scope for a run. wholeRegistry // widens the sweep to every adapter's declared credentials for host-mode // redaction; false yields the executing-scanner passthrough set used both diff --git a/internal/runner/scanner_registry_test.go b/internal/runner/scanner_registry_test.go index 07aaa75..a92e57a 100644 --- a/internal/runner/scanner_registry_test.go +++ b/internal/runner/scanner_registry_test.go @@ -286,7 +286,7 @@ func TestUserDefinedScannerSkipsExistenceCheckForURLTargets(t *testing.T) { func TestUserDefinedScannerRedactsDeclaredEnvOnFailure(t *testing.T) { adapter := NewUserDefinedScanner(UserDefinedScannerConfig{ - ID: "demo", Command: "demo {{target}}", Env: []string{"SCANNER_ACCESS"}, Targets: []string{"skill"}, + ID: "demo", Command: "demo {{target}}", SecretEnv: []string{"SCANNER_ACCESS"}, Targets: []string{"skill"}, }) registry, err := NewScannerRegistry(adapter) if err != nil { @@ -311,7 +311,7 @@ func TestUserDefinedScannerRedactsDeclaredEnvOnFailure(t *testing.T) { func TestUserDefinedScannerRedactsDeclaredEnvInRawJSON(t *testing.T) { adapter := NewUserDefinedScanner(UserDefinedScannerConfig{ - ID: "demo", Command: "demo {{target}}", Env: []string{"SCANNER_ACCESS"}, Targets: []string{"skill"}, + ID: "demo", Command: "demo {{target}}", SecretEnv: []string{"SCANNER_ACCESS"}, Targets: []string{"skill"}, }) registry, err := NewScannerRegistry(adapter) if err != nil { @@ -951,7 +951,7 @@ func TestEnvValueForNameFindsNonEmptyExactMatch(t *testing.T) { func TestUserDefinedScannerInfoSanitizesMalformedEnvEntries(t *testing.T) { adapter := NewUserDefinedScanner(UserDefinedScannerConfig{ ID: "alpha", Command: "scanner {{target}}", - Env: []string{"API_TOKEN=sk-live-info-leak", "=sk-live-eqzero", "GOOD_NAME"}, Targets: []string{"skill"}, + SecretEnv: []string{"API_TOKEN=sk-live-info-leak", "=sk-live-eqzero", "GOOD_NAME"}, Targets: []string{"skill"}, }) want := []string{"API_TOKEN", "GOOD_NAME"} if got := adapter.Info().RequiredEnv; !reflect.DeepEqual(got, want) { @@ -1226,7 +1226,7 @@ func TestUserDefinedScannerRedactsEscapedUndeclaredSecretsFromErrors(t *testing. func TestUserDefinedScannerRedactsAlternateEncodedSecretsFromErrors(t *testing.T) { adapter := NewUserDefinedScanner(UserDefinedScannerConfig{ - ID: "alpha", Command: "alpha {{target}}", Env: []string{"SCANNER_ACCESS"}, Targets: []string{"skill"}, + ID: "alpha", Command: "alpha {{target}}", SecretEnv: []string{"SCANNER_ACCESS"}, Targets: []string{"skill"}, }) registry, err := NewScannerRegistry(adapter) if err != nil { @@ -1368,7 +1368,7 @@ func TestRedactScannerStdoutMarkerSubstringSecrets(t *testing.T) { func TestUserDefinedScannerMarkerSubstringSecretInErrors(t *testing.T) { adapter := NewUserDefinedScanner(UserDefinedScannerConfig{ - ID: "alpha", Command: "alpha {{target}}", Env: []string{"SCANNER_ACCESS"}, Targets: []string{"skill"}, + ID: "alpha", Command: "alpha {{target}}", SecretEnv: []string{"SCANNER_ACCESS"}, Targets: []string{"skill"}, }) registry, err := NewScannerRegistry(adapter) if err != nil { @@ -1526,7 +1526,7 @@ func TestJSONSecretLeavesIncludesNumericCredential(t *testing.T) { func TestUserDefinedScannerRedactsNumericJSONCredentialLeaf(t *testing.T) { adapter := NewUserDefinedScanner(UserDefinedScannerConfig{ - ID: "demo", Command: "demo {{target}}", Env: []string{"SCANNER_ACCESS"}, Targets: []string{"skill"}, + ID: "demo", Command: "demo {{target}}", SecretEnv: []string{"SCANNER_ACCESS"}, Targets: []string{"skill"}, }) registry, err := NewScannerRegistry(adapter) if err != nil { diff --git a/internal/runner/user_defined_scanner.go b/internal/runner/user_defined_scanner.go index 727cad0..8956a60 100644 --- a/internal/runner/user_defined_scanner.go +++ b/internal/runner/user_defined_scanner.go @@ -19,10 +19,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 { @@ -41,15 +42,15 @@ 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 sanitizedDeclaredEnvNames(adapter.config.Env) { + requirements := make([]EnvRequirement, 0, len(adapter.config.Env)+len(adapter.config.SecretEnv)) + for _, name := range sanitizedDeclaredEnvNames(append(append([]string(nil), adapter.config.Env...), adapter.config.SecretEnv...)) { 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: sanitizedDeclaredEnvNames(adapter.config.Env)} + return ScannerInfo{ID: adapter.config.ID, DisplayName: adapter.config.ID, RequiredEnv: sanitizedDeclaredEnvNames(append(append([]string(nil), adapter.config.Env...), adapter.config.SecretEnv...))} } func (adapter userDefinedScannerAdapter) InstallPlan() InstallPlan { @@ -63,10 +64,20 @@ func (adapter userDefinedScannerAdapter) SupportsTargetKind(kind string) bool { func (adapter userDefinedScannerAdapter) CommandBacked() bool { return true } // DeclaredCredentialEnv lists env vars that are credentials by declaration: -// whatever their spelling, a user-defined scanner's env: entries exist to -// hand the command secrets, so their values must always be redacted from -// persisted output. +// only secretEnv: entries are credentials-by-declaration and always +// redacted; plain env: entries are passed through and shown unless the +// name-heuristic backstop (isSecretEnvKey) classifies them as a credential. func (adapter userDefinedScannerAdapter) DeclaredCredentialEnv() []string { + return sanitizedDeclaredEnvNames(adapter.config.SecretEnv) +} + +// DeclaredNonCredentialEnv lists plain env: entries. These are passed through +// to the scanner and shown in evidence, so redaction must exempt them from its +// fail-closed default (CredentialEnvName treats every unknown name as a +// credential). A plain env: name whose spelling looks secret (isSecretEnvKey) +// is still redacted at the call site as a backstop, and secretEnv: never +// appears here, so a name declared as both credential and plain stays redacted. +func (adapter userDefinedScannerAdapter) DeclaredNonCredentialEnv() []string { return sanitizedDeclaredEnvNames(adapter.config.Env) } @@ -83,7 +94,7 @@ func (adapter userDefinedScannerAdapter) Run(runner ExternalScannerRunner, targe }, nil } } - // env: entries must be bare variable names: `env: [API_TOKEN=sk-live]` + // env: and secretEnv: entries must be bare variable names: `env: [API_TOKEN=sk-live]` // would otherwise flow into the missing-variable diagnostic verbatim, // leaking the inline value into terminal and CI logs. if bad := invalidDeclaredEnvName(adapter.config.Env); bad != "" { @@ -92,6 +103,12 @@ func (adapter userDefinedScannerAdapter) Run(runner ExternalScannerRunner, targe Error: fmt.Sprintf("User-defined scanner %s env entry %s is not a variable name; declare bare names and set values in the environment", adapter.config.ID, bad), }, nil } + if bad := invalidDeclaredEnvName(adapter.config.SecretEnv); bad != "" { + return ScannerResult{ + Status: "failed", StartedAt: startedAt, CompletedAt: time.Now().UTC().Format(time.RFC3339Nano), + Error: fmt.Sprintf("User-defined scanner %s secretEnv entry %s is not a variable name; declare bare names and set values in the environment", adapter.config.ID, bad), + }, nil + } if commandReparsesTarget(adapter.config.Command) { return ScannerResult{ Status: "failed", StartedAt: startedAt, CompletedAt: time.Now().UTC().Format(time.RFC3339Nano), @@ -152,15 +169,16 @@ func (adapter userDefinedScannerAdapter) Run(runner ExternalScannerRunner, targe // so structural redaction of decoded strings suffices — and byte-level // replacement must not run first, or a short secret like "1" would corrupt // non-string JSON tokens and flip a healthy scan to failed. - // Scrub this adapter's declared env plus everything else exposed to - // scanners this run (sandbox allowlist, other adapters' credentials): - // under Docker every scanner sees the whole passthrough set, and a name - // like BETA_LICENSE evades the isSecretEnvKey heuristic. - scrubNames := append(append([]string(nil), adapter.config.Env...), runner.ExposedEnvNames...) - // Under Docker only allowlisted names reach the container; scrubbing an - // unrelated host secret's value (CI_TOKEN=clean) would corrupt evidence - // without preventing any leak. --sandbox off keeps the full host env. - visibleEnv := commandVisibleEnv(runner.Env, scrubNames, runner.SandboxMode) + // reachableNames narrows redaction's env view to what the scanner can + // actually see (Docker allowlist), so the name-heuristic backstop still + // catches a secret-named plain env: value. Both buckets are reachable. + reachableNames := append(append(append([]string(nil), adapter.config.Env...), adapter.config.SecretEnv...), runner.ExposedEnvNames...) + // scrubNames are the force-redacted credentials: secretEnv plus run-wide + // declared credentials. Plain env: is intentionally excluded so non-secret + // config stays in evidence; secret-named env: is still caught via the + // heuristic sweep inside scannerSecretValues over visibleEnv. + scrubNames := append(append([]string(nil), adapter.config.SecretEnv...), runner.ExposedEnvNames...) + visibleEnv := commandVisibleEnv(runner.Env, reachableNames, runner.SandboxMode) stdout := output.Stdout // Evidence is rejected outright — not repaired — when structural // redaction cannot see everything the raw bytes hold: invalid UTF-8 @@ -180,7 +198,9 @@ func (adapter userDefinedScannerAdapter) Run(runner ExternalScannerRunner, targe } if runErr != nil { // Failure text needs the same coverage as stdout: declared env plus - // everything exposed to scanners this run, whatever the spelling. + // everything exposed to scanners this run. secretEnv: and env: values + // both reach the scanner, but only secretEnv: values are force-redacted; + // env: values are shown unless the heuristic backstop catches them. // commandError's own secret-named sweep must also use the visible // env: pre-scrubbing with the whole host env would corrupt Docker // diagnostics matching an unexposed host value by coincidence. diff --git a/internal/runner/user_defined_scanner_env_test.go b/internal/runner/user_defined_scanner_env_test.go new file mode 100644 index 0000000..9e48482 --- /dev/null +++ b/internal/runner/user_defined_scanner_env_test.go @@ -0,0 +1,268 @@ +package runner + +import ( + "strings" + "testing" +) + +// declaredCredentialEnv reaches DeclaredCredentialEnv, which is exposed via a +// type assertion (the ScannerAdapter interface does not declare it). +func declaredCredentialEnv(t *testing.T, adapter ScannerAdapter) []string { + t.Helper() + declarer, ok := adapter.(interface{ DeclaredCredentialEnv() []string }) + if !ok { + t.Fatalf("adapter %T does not expose DeclaredCredentialEnv", adapter) + } + return declarer.DeclaredCredentialEnv() +} + +// TestSecretEnvScrubbedPlainEnvShownEndToEnd is the core behavior contract: +// a secretEnv value is scrubbed from persisted stdout while a non-secret-named +// plain env value stays in evidence, both through a real adapter.Run. +func TestSecretEnvScrubbedPlainEnvShownEndToEnd(t *testing.T) { + scanner := NewUserDefinedScanner(UserDefinedScannerConfig{ + ID: "alpha", + Command: "alpha {{target}}", + Env: []string{"MODE"}, + SecretEnv: []string{"BETA_LICENSE"}, + Targets: []string{"skill"}, + }) + registry, err := NewScannerRegistry(scanner) + if err != nil { + t.Fatal(err) + } + env := map[string]string{"MODE": "fastmode-shown", "BETA_LICENSE": "beta-secret-xyz"} + run := ExternalScannerRunner{ + Registry: registry, + CommandRunner: &recordingCommandRunner{stdout: `{"mode":"fastmode-shown","license":"beta-secret-xyz"}`}, + Env: env, SandboxMode: SandboxModeOff, + } + result, err := run.RunScanner("alpha", t.TempDir(), "2026-07-21T00:00:00Z") + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(result.Raw), "beta-secret-xyz") { + t.Fatalf("secretEnv value leaked into raw evidence: %s", result.Raw) + } + if !strings.Contains(string(result.Raw), "fastmode-shown") { + t.Fatalf("non-secret plain env value was scrubbed from evidence: %s", result.Raw) + } +} + +// TestSecretEnvSplitHonoredThroughComputedExposedEnvNames closes the gap the +// other end-to-end tests leave open: they never set ExposedEnvNames, so they +// miss that a real run populates it from redactionEnvNames — whose fail-closed +// CredentialEnvName default would otherwise redact the plain env value the +// reachability union (env+secretEnv) feeds into the sweep. This test computes +// ExposedEnvNames exactly as the runner does and asserts the plain value still +// shows while the secretEnv value is scrubbed, in both sandbox modes. +func TestSecretEnvSplitHonoredThroughComputedExposedEnvNames(t *testing.T) { + scanner := NewUserDefinedScanner(UserDefinedScannerConfig{ + ID: "alpha", + Command: "alpha {{target}}", + Env: []string{"MODE"}, + SecretEnv: []string{"BETA_LICENSE"}, + Targets: []string{"skill"}, + }) + registry, err := NewScannerRegistry(scanner) + if err != nil { + t.Fatal(err) + } + env := map[string]string{"MODE": "fastmode-shown", "BETA_LICENSE": "beta-secret-xyz"} + opts := Options{Scanners: []string{"alpha"}, ScannerRegistry: registry} + for _, mode := range []string{SandboxModeOff, SandboxModeDocker} { + exposed := redactionEnvNames(opts, env, mode) + run := ExternalScannerRunner{ + Registry: registry, + CommandRunner: &recordingCommandRunner{stdout: `{"mode":"fastmode-shown","license":"beta-secret-xyz"}`}, + Env: env, + SandboxMode: mode, + ExposedEnvNames: exposed, + } + result, err := run.RunScanner("alpha", t.TempDir(), "2026-07-21T00:00:00Z") + if err != nil { + t.Fatalf("mode %s: %v", mode, err) + } + if strings.Contains(string(result.Raw), "beta-secret-xyz") { + t.Fatalf("mode %s: secretEnv value leaked into evidence: %s", mode, result.Raw) + } + if !strings.Contains(string(result.Raw), "fastmode-shown") { + t.Fatalf("mode %s: plain env value scrubbed from evidence via ExposedEnvNames %v: %s", mode, exposed, result.Raw) + } + } +} + +// TestRedactionEnvNamesExemptsPlainNonSecretEnv pins the redaction-set filter: +// a plain env: name that is not credential-spelled is excluded (shown), while a +// secret-named plain env and a secretEnv name are included (redacted). +func TestRedactionEnvNamesExemptsPlainNonSecretEnv(t *testing.T) { + scanner := NewUserDefinedScanner(UserDefinedScannerConfig{ + ID: "alpha", + Command: "alpha {{target}}", + Env: []string{"TEAMNAME", "MY_TOKEN"}, + SecretEnv: []string{"APIKEY"}, + Targets: []string{"skill"}, + }) + registry, err := NewScannerRegistry(scanner) + if err != nil { + t.Fatal(err) + } + opts := Options{Scanners: []string{"alpha"}, ScannerRegistry: registry} + env := map[string]string{"TEAMNAME": "acme", "MY_TOKEN": "sk-tok", "APIKEY": "sk-live"} + for _, mode := range []string{SandboxModeOff, SandboxModeDocker} { + got := map[string]bool{} + for _, name := range redactionEnvNames(opts, env, mode) { + got[name] = true + } + if got["TEAMNAME"] { + t.Fatalf("mode %s: plain non-secret env TEAMNAME must be exempt from redaction, got %v", mode, got) + } + if !got["MY_TOKEN"] { + t.Fatalf("mode %s: secret-named plain env MY_TOKEN must stay redacted, got %v", mode, got) + } + if !got["APIKEY"] { + t.Fatalf("mode %s: secretEnv APIKEY must stay redacted, got %v", mode, got) + } + } +} + +// TestDeclaredNonCredentialEnvOnlyIncludesPlainEnv: the plain-env declaration +// surface (which drives the redaction exemption) lists env entries only, never +// secretEnv entries. +func TestDeclaredNonCredentialEnvOnlyIncludesPlainEnv(t *testing.T) { + adapter := NewUserDefinedScanner(UserDefinedScannerConfig{ + ID: "test-scanner", + Command: "test {{target}}", + Env: []string{"CONFIG_VAR", "TEAMNAME"}, + SecretEnv: []string{"API_KEY"}, + }) + declarer, ok := adapter.(interface{ DeclaredNonCredentialEnv() []string }) + if !ok { + t.Fatalf("adapter %T does not expose DeclaredNonCredentialEnv", adapter) + } + got := map[string]bool{} + for _, name := range declarer.DeclaredNonCredentialEnv() { + got[name] = true + } + if !got["CONFIG_VAR"] || !got["TEAMNAME"] { + t.Fatalf("DeclaredNonCredentialEnv() = %v, want CONFIG_VAR and TEAMNAME", got) + } + if got["API_KEY"] { + t.Fatalf("DeclaredNonCredentialEnv() must not include secretEnv entries: %v", got) + } +} + +// TestSecretNamedPlainEnvStillScrubbedByBackstop verifies the safety net: a +// secret-NAMED value placed in plain env (not secretEnv) is still redacted by +// the isSecretEnvKey heuristic, so a migration cannot silently un-redact it. +func TestSecretNamedPlainEnvStillScrubbedByBackstop(t *testing.T) { + scanner := NewUserDefinedScanner(UserDefinedScannerConfig{ + ID: "alpha", + Command: "alpha {{target}}", + Env: []string{"MY_TOKEN"}, + Targets: []string{"skill"}, + }) + registry, err := NewScannerRegistry(scanner) + if err != nil { + t.Fatal(err) + } + env := map[string]string{"MY_TOKEN": "tokenvalue-abc"} + run := ExternalScannerRunner{ + Registry: registry, + CommandRunner: &recordingCommandRunner{stdout: `{"seen":"tokenvalue-abc"}`}, + Env: env, SandboxMode: SandboxModeOff, + } + result, err := run.RunScanner("alpha", t.TempDir(), "2026-07-21T00:00:00Z") + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(result.Raw), "tokenvalue-abc") { + t.Fatalf("secret-named plain env value escaped the heuristic backstop: %s", result.Raw) + } +} + +// TestSecretEnvScrubbedFromErrorText verifies secretEnv values are scrubbed +// from failure text (OS error + stderr), not just stdout. +func TestSecretEnvScrubbedFromErrorText(t *testing.T) { + scanner := NewUserDefinedScanner(UserDefinedScannerConfig{ + ID: "alpha", + Command: "alpha {{target}}", + SecretEnv: []string{"BETA_LICENSE"}, + Targets: []string{"skill"}, + }) + registry, err := NewScannerRegistry(scanner) + if err != nil { + t.Fatal(err) + } + env := map[string]string{"BETA_LICENSE": "beta-secret-xyz"} + run := ExternalScannerRunner{ + Registry: registry, + CommandRunner: &recordingCommandRunner{stderr: "auth beta-secret-xyz rejected", err: errCommandFailed}, + Env: env, SandboxMode: SandboxModeOff, + } + result, err := run.RunScanner("alpha", t.TempDir(), "2026-07-21T00:00:00Z") + if err != nil { + t.Fatal(err) + } + if strings.Contains(result.Error, "beta-secret-xyz") { + t.Fatalf("secretEnv value leaked into error text: %q", result.Error) + } +} + +// TestDeclaredCredentialEnvOnlyIncludesSecretEnv: only secretEnv entries are +// credentials-by-declaration; plain env entries are not. +func TestDeclaredCredentialEnvOnlyIncludesSecretEnv(t *testing.T) { + adapter := NewUserDefinedScanner(UserDefinedScannerConfig{ + ID: "test-scanner", + Command: "test {{target}}", + Env: []string{"CONFIG_VAR", "SECRET_TOKEN"}, + SecretEnv: []string{"API_KEY", "PASSWORD"}, + }) + declared := declaredCredentialEnv(t, adapter) + got := map[string]bool{} + for _, name := range declared { + got[name] = true + } + if len(declared) != 2 || !got["API_KEY"] || !got["PASSWORD"] { + t.Fatalf("DeclaredCredentialEnv() = %v, want exactly [API_KEY PASSWORD]", declared) + } + if got["CONFIG_VAR"] || got["SECRET_TOKEN"] { + t.Fatalf("DeclaredCredentialEnv() must not include plain env entries: %v", declared) + } +} + +// TestRequirementsAndInfoIncludeBothBuckets: both env and secretEnv must reach +// the scanner, so both surface as requirements/required env. +func TestRequirementsAndInfoIncludeBothBuckets(t *testing.T) { + adapter := NewUserDefinedScanner(UserDefinedScannerConfig{ + ID: "test-scanner", + Command: "test {{target}}", + Env: []string{"CONFIG_VAR"}, + SecretEnv: []string{"API_KEY"}, + }) + reqNames := map[string]bool{} + for _, req := range adapter.Requirements(nil) { + reqNames[req.EnvVar] = true + } + if !reqNames["CONFIG_VAR"] || !reqNames["API_KEY"] { + t.Fatalf("Requirements() = %v, want both CONFIG_VAR and API_KEY", reqNames) + } + infoNames := map[string]bool{} + for _, name := range adapter.Info().RequiredEnv { + infoNames[name] = true + } + if !infoNames["CONFIG_VAR"] || !infoNames["API_KEY"] { + t.Fatalf("Info().RequiredEnv = %v, want both CONFIG_VAR and API_KEY", infoNames) + } +} + +// TestSecretEnvEntriesValidatedAsBareNames: secretEnv rejects NAME=value +// entries exactly like env does. +func TestSecretEnvEntriesValidatedAsBareNames(t *testing.T) { + if InvalidUserDefinedEnvName([]string{"API_KEY=sk-live"}) == "" { + t.Fatal("secretEnv validation must reject NAME=value entries") + } + if bad := InvalidUserDefinedEnvName([]string{"API_KEY", "BETA_LICENSE"}); bad != "" { + t.Fatalf("bare names must be accepted, rejected %q", bad) + } +}