diff --git a/docs/project-changelog.md b/docs/project-changelog.md index 0f0c1d65..326e7cd2 100644 --- a/docs/project-changelog.md +++ b/docs/project-changelog.md @@ -6,6 +6,22 @@ Significant changes, features, and fixes in reverse chronological order. ## 2026-05-29 +### RapidAPI cron SecureCLI diagnostics (issue #74) + +**Fixes** + +- Added a built-in `rapidapi` SecureCLI preset with required `RAPIDAPI_KEY`, 60s timeout, and verbose/debug flag denials. +- Credentialed exec now validates required preset env keys before binary resolution, so missing RapidAPI credentials return a GoClaw diagnostic instead of downstream `RAPIDAPI_KEY required`. +- Added safe SecureCLI env diagnostics that log env key names/count context only, never credential values. +- Added `RAPIDAPI_KEY` to fall-through exec env scrubbing. + +**Tests** + +- Added RapidAPI preset contract and deny-pattern coverage. +- Added credentialed exec regressions for missing RapidAPI env and successful direct exec with injected `RAPIDAPI_KEY`. + +--- + ### Sandbox tenant workspace isolation (issue #68) **Security** diff --git a/internal/cron/service.go b/internal/cron/service.go index b12bc046..0e8d7a03 100644 --- a/internal/cron/service.go +++ b/internal/cron/service.go @@ -214,7 +214,8 @@ func (cs *Service) GetJob(jobID string) (*Job, bool) { for i, job := range cs.store.Jobs { if job.ID == jobID { - return &cs.store.Jobs[i], true + jobCopy := cs.store.Jobs[i] + return &jobCopy, true } } return nil, false diff --git a/internal/tools/credential_presets.go b/internal/tools/credential_presets.go index 221ee113..ef70cf76 100644 --- a/internal/tools/credential_presets.go +++ b/internal/tools/credential_presets.go @@ -139,6 +139,17 @@ var CLIPresets = map[string]CLIPreset{ Tips: "Use -A -t for plain output suitable for piping", AdapterName: "psql", }, + "rapidapi": { + BinaryName: "rapidapi", + Description: "RapidAPI CLI", + EnvVars: []EnvVarDef{ + {Name: "RAPIDAPI_KEY", Desc: "RapidAPI application key"}, + }, + DenyArgs: nil, + DenyVerbose: []string{`--verbose`, `--debug`, `-v`}, + Timeout: 60, + Tips: "Use read-only/search commands for scheduled jobs. Configure RAPIDAPI_KEY as a per-user credential and grant the target agent.", + }, } // GetPreset returns a preset by name, or nil if not found. @@ -159,3 +170,23 @@ func ListPresetNames() []string { sort.Strings(names) return names } + +func requiredCredentialEnvVars(binary string) []string { + name := normalizeBinaryName(binary) + if name != "rapidapi" { + return nil + } + preset := GetPreset(name) + if preset == nil { + return nil + } + names := make([]string, 0, len(preset.EnvVars)) + for _, envVar := range preset.EnvVars { + if envVar.Optional { + continue + } + names = append(names, envVar.Name) + } + sort.Strings(names) + return names +} diff --git a/internal/tools/credential_presets_rapidapi_test.go b/internal/tools/credential_presets_rapidapi_test.go new file mode 100644 index 00000000..381aaa5a --- /dev/null +++ b/internal/tools/credential_presets_rapidapi_test.go @@ -0,0 +1,55 @@ +package tools + +import ( + "encoding/json" + "slices" + "testing" +) + +func TestRapidAPIPresetContract(t *testing.T) { + preset := GetPreset("rapidapi") + if preset == nil { + t.Fatal("rapidapi preset is missing") + } + if preset.BinaryName != "rapidapi" { + t.Fatalf("BinaryName = %q, want rapidapi", preset.BinaryName) + } + + envNames := make([]string, 0, len(preset.EnvVars)) + for _, envVar := range preset.EnvVars { + envNames = append(envNames, envVar.Name) + } + if !slices.Contains(envNames, "RAPIDAPI_KEY") { + t.Fatalf("rapidapi preset env vars = %v, missing RAPIDAPI_KEY", envNames) + } +} + +func TestRapidAPIPresetBlocksVerboseSecretLeakFlags(t *testing.T) { + preset := GetPreset("rapidapi") + if preset == nil { + t.Fatal("rapidapi preset is missing") + } + denyVerbose, err := json.Marshal(preset.DenyVerbose) + if err != nil { + t.Fatal(err) + } + + for _, args := range [][]string{ + {"--verbose"}, + {"--debug"}, + {"-v"}, + } { + if got := matchesBinaryVerbose(args, denyVerbose); got == "" { + t.Fatalf("matchesBinaryVerbose(%v) did not block verbose/debug flag", args) + } + } +} + +func TestRequiredCredentialEnvVarsScopedToRapidAPI(t *testing.T) { + if got := requiredCredentialEnvVars("rapidapi"); !slices.Contains(got, "RAPIDAPI_KEY") { + t.Fatalf("requiredCredentialEnvVars(rapidapi) = %v, missing RAPIDAPI_KEY", got) + } + if got := requiredCredentialEnvVars("gh"); len(got) != 0 { + t.Fatalf("requiredCredentialEnvVars(gh) = %v, want no behavior change for existing presets", got) + } +} diff --git a/internal/tools/credentialed_exec.go b/internal/tools/credentialed_exec.go index 6d630b2b..200732e3 100644 --- a/internal/tools/credentialed_exec.go +++ b/internal/tools/credentialed_exec.go @@ -374,17 +374,7 @@ func (t *ExecTool) executeCredentialed(ctx context.Context, cred *store.SecureCL return credentialedShellOperatorError(rawCommand, ops) } - // Step 2: Resolve binary to absolute path and verify against config - absPath, err := resolveAndMatchBinary(binary, cred.BinaryPath) - if err != nil { - r := credentialedPathError(binary, err) - if t.sandboxMgr != nil && sandboxKey != "" { - r.ForLLM += hintBinaryNotFound - } - return r - } - - // Step 3: Per-binary deny check (deny_args) + // Step 2: Per-binary deny check (deny_args) denyArgs, allowAudits := applyCommandKeywordAllowlist(binary, args, t.commandKeywordAllowlistSnapshot()) if p := matchesBinaryDeny(denyArgs, cred.DenyArgs); p != "" { return credentialedDenyError(binary, args, p) @@ -409,18 +399,46 @@ func (t *ExecTool) executeCredentialed(ctx context.Context, cred *store.SecureCL ) } - // Step 4: Decrypt env vars from store (already decrypted by store layer). + // Step 3: Decrypt env vars from store (already decrypted by store layer). // Per-user env overrides take priority over binary/grant env. envMap, err := mergeCredentialedEnv(cred) if err != nil { return ErrorResult(fmt.Sprintf("credentialed exec: invalid env JSON for %q: %v", binary, err)) } + slog.Debug("secure_cli.env_merged", + "binary", binary, + "agent_id", store.AgentIDFromContext(ctx), + "tenant_id", store.TenantIDFromContext(ctx), + "credential_user_id_present", store.CredentialUserIDFromContext(ctx) != "", + "env_keys", sortedKeys(envMap), + ) + if missing := missingRequiredCredentialEnv(binary, envMap); len(missing) > 0 { + slog.Warn("secure_cli.missing_required_env", + "binary", binary, + "agent_id", store.AgentIDFromContext(ctx), + "tenant_id", store.TenantIDFromContext(ctx), + "credential_user_id_present", store.CredentialUserIDFromContext(ctx) != "", + "missing_env_keys", missing, + "env_keys", sortedKeys(envMap), + ) + return credentialedMissingEnvError(binary, missing) + } - // Step 5: Register credential values for output scrubbing + // Step 4: Register credential values for output scrubbing for _, v := range envMap { AddCredentialScrubValues(v) } + // Step 5: Resolve binary to absolute path and verify against config + absPath, err := resolveAndMatchBinary(binary, cred.BinaryPath) + if err != nil { + r := credentialedPathError(binary, err) + if t.sandboxMgr != nil && sandboxKey != "" { + r.ForLLM += hintBinaryNotFound + } + return r + } + // Step 6: Determine timeout timeout := time.Duration(cred.TimeoutSeconds) * time.Second if timeout <= 0 { @@ -531,6 +549,20 @@ func mergeCredentialedEnv(cred *store.SecureCLIBinary) (map[string]string, error return envMap, nil } +func missingRequiredCredentialEnv(binary string, envMap map[string]string) []string { + required := requiredCredentialEnvVars(binary) + if len(required) == 0 { + return nil + } + missing := make([]string, 0, len(required)) + for _, key := range required { + if strings.TrimSpace(envMap[key]) == "" { + missing = append(missing, key) + } + } + return missing +} + // validateExecCwd checks that cmd.Dir exists and is a directory before // cmd.Start(). Empty cwd is allowed (Go interprets as "inherit parent's dir"). // @@ -825,6 +857,17 @@ func credentialedDenyError(binary string, args []string, pattern string) *Result } } +func credentialedMissingEnvError(binary string, keys []string) *Result { + return &Result{ + ForLLM: fmt.Sprintf("[CREDENTIALED EXEC] Credential config missing required credential env.\n"+ + "Binary: %s\nMissing env keys: %s\n"+ + "Configure the missing key as a SecureCLI user credential for the same user context, then grant this binary to the target agent.", + binary, strings.Join(keys, ", ")), + ForUser: fmt.Sprintf("CLI credential for %q is missing required env: %s.", binary, strings.Join(keys, ", ")), + IsError: true, + } +} + func credentialedExecFailError(binary string, args []string, exitCode int, output string) *Result { return &Result{ ForLLM: fmt.Sprintf("[CREDENTIALED EXEC] Command failed (exit code %d).\n"+ diff --git a/internal/tools/credentialed_exec_env_test.go b/internal/tools/credentialed_exec_env_test.go index 91e48cb5..7186d15e 100644 --- a/internal/tools/credentialed_exec_env_test.go +++ b/internal/tools/credentialed_exec_env_test.go @@ -1,8 +1,16 @@ package tools import ( + "context" + "encoding/json" + "os" + "path/filepath" + "runtime" + "strings" "testing" + "github.com/google/uuid" + "github.com/nextlevelbuilder/goclaw/internal/store" ) @@ -64,3 +72,76 @@ func TestMergeCredentialedEnvFlattensSensitiveValueEntries(t *testing.T) { t.Fatalf("PUBLIC_BASE_URL = %q", env["PUBLIC_BASE_URL"]) } } + +func TestExec_RapidAPIMissingRequiredEnvFailsBeforeBinaryResolution(t *testing.T) { + stub := newStubSecureCLIStore() + stub.byName["rapidapi"] = &store.SecureCLIBinary{ + BinaryName: "rapidapi", + EncryptedEnv: []byte("{}"), + TimeoutSeconds: 10, + DenyArgs: json.RawMessage("[]"), + DenyVerbose: json.RawMessage("[]"), + } + + tool := NewExecTool(t.TempDir(), false) + tool.SetSecureCLIStore(stub) + + ctx := store.WithTenantID(store.WithAgentID(context.Background(), uuid.New()), uuid.New()) + ctx = store.WithCredentialUserID(ctx, "tenant-user-rapidapi") + result := tool.Execute(ctx, map[string]any{"command": "rapidapi search weather"}) + + if !result.IsError { + t.Fatalf("expected missing RAPIDAPI_KEY to fail") + } + if !strings.Contains(result.ForLLM, "missing required credential env") { + t.Fatalf("expected actionable missing-env error, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "RAPIDAPI_KEY") { + t.Fatalf("expected missing key name in error, got: %s", result.ForLLM) + } + if strings.Contains(result.ForLLM, "not found in PATH") || strings.Contains(result.ForLLM, "Binary resolution failed") { + t.Fatalf("missing env should be reported before binary resolution, got: %s", result.ForLLM) + } + if strings.Contains(result.ForLLM, "tenant-user-rapidapi") { + t.Fatalf("credential user id leaked into user-facing error: %s", result.ForLLM) + } +} + +func TestExec_RapidAPIWithRequiredEnvReachesDirectExec(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("shell script fixture is POSIX-only") + } + + binDir := t.TempDir() + binPath := filepath.Join(binDir, "rapidapi") + if err := os.WriteFile(binPath, []byte("#!/bin/sh\n[ -n \"$RAPIDAPI_KEY\" ] || exit 43\nprintf 'rapidapi args:%s\\n' \"$*\"\n"), 0o755); err != nil { + t.Fatal(err) + } + + stub := newStubSecureCLIStore() + stub.byName["rapidapi"] = &store.SecureCLIBinary{ + BinaryName: "rapidapi", + BinaryPath: &binPath, + EncryptedEnv: []byte("{}"), + UserEnv: []byte(`{"RAPIDAPI_KEY":"test-key-not-real"}`), + TimeoutSeconds: 10, + DenyArgs: json.RawMessage("[]"), + DenyVerbose: json.RawMessage("[]"), + } + + tool := NewExecTool(t.TempDir(), false) + tool.SetSecureCLIStore(stub) + + ctx := store.WithTenantID(store.WithAgentID(context.Background(), uuid.New()), uuid.New()) + result := tool.Execute(ctx, map[string]any{"command": "rapidapi search weather"}) + + if result.IsError { + t.Fatalf("expected rapidapi direct exec to run, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "rapidapi args:search weather") { + t.Fatalf("expected script output, got: %s", result.ForLLM) + } + if strings.Contains(result.ForLLM, "test-key-not-real") { + t.Fatalf("credential value leaked into output: %s", result.ForLLM) + } +} diff --git a/internal/tools/env_scrub.go b/internal/tools/env_scrub.go index bd26f969..f2e8ece6 100644 --- a/internal/tools/env_scrub.go +++ b/internal/tools/env_scrub.go @@ -24,6 +24,7 @@ var staticCredentialEnvKeys = []string{ "AWS_SECRET_ACCESS_KEY", "AWS_SESSION_TOKEN", "GOOGLE_APPLICATION_CREDENTIALS", + "RAPIDAPI_KEY", "HUGGINGFACE_TOKEN", "HF_TOKEN", "GCP_SA_KEY", diff --git a/internal/tools/env_scrub_test.go b/internal/tools/env_scrub_test.go index c431239d..033cabec 100644 --- a/internal/tools/env_scrub_test.go +++ b/internal/tools/env_scrub_test.go @@ -21,6 +21,7 @@ func TestScrubCredentialEnv_StripsStatic(t *testing.T) { "GH_TOKEN=secret-abc", "PATH=/usr/bin", "AWS_SECRET_ACCESS_KEY=topsecret", + "RAPIDAPI_KEY=rapid-secret", } out := scrubCredentialEnv(in, nil) @@ -30,6 +31,9 @@ func TestScrubCredentialEnv_StripsStatic(t *testing.T) { if envContains(out, "AWS_SECRET_ACCESS_KEY") { t.Fatalf("AWS_SECRET_ACCESS_KEY must be scrubbed, got: %v", out) } + if envContains(out, "RAPIDAPI_KEY") { + t.Fatalf("RAPIDAPI_KEY must be scrubbed, got: %v", out) + } if !envContains(out, "HOME") || !envContains(out, "PATH") { t.Fatalf("essential vars must be preserved, got: %v", out) } diff --git a/plans/260528-1801-rapidapi-cron-securecli-credentials/phase-01-diagnose-current-rapidapi-credential-path.md b/plans/260528-1801-rapidapi-cron-securecli-credentials/phase-01-diagnose-current-rapidapi-credential-path.md new file mode 100644 index 00000000..d6c1af00 --- /dev/null +++ b/plans/260528-1801-rapidapi-cron-securecli-credentials/phase-01-diagnose-current-rapidapi-credential-path.md @@ -0,0 +1,124 @@ +--- +phase: 1 +title: "Diagnose Current RapidAPI Credential Path" +status: complete +priority: P1 +effort: "1.5h" +dependencies: [] +--- + +# Phase 1: Diagnose Current RapidAPI Credential Path + +## Context Links + +- Issue: `digitopvn/goclaw#74` +- Related closed issue: `digitopvn/goclaw#54` +- Cron payload model: `internal/store/cron_store.go` +- Cron run injection: `cmd/gateway_cron.go` +- SecureCLI lookup: `internal/tools/credentialed_exec.go` + +## Overview + +Prove the concrete failure mode before code. Current code already preserves cron credential user identity; #74 may be RapidAPI config, grant, env key, or legacy job payload. + +## Key Insights + +- `CronPayload.CredentialUserID` exists and is redacted from API responses. +- `PGCronStore.AddJob` and `SQLiteCronStore.AddJob` store explicit credential user identity. +- `makeCronJobHandler` injects that identity into cron run context. +- `lookupCredentialedBinary` resolves per-user env through `CredentialUserIDFromContext`. +- No built-in RapidAPI preset exists today. +- 2026-05-29 verification: + - #54 context path is present: cron handler injects `payload.credentialUserId` before scheduling. + - Redaction path is present: response copies clear `payload.credentialUserId`. + - `rapidapi` is absent from `CLIPresets`, so operators get no guided `RAPIDAPI_KEY` setup. + - Credentialed exec merges base/user env but does not validate required preset env keys before process execution. + +## Requirements + +- Functional: identify whether `rapidapi` is configured, granted, and populated with `RAPIDAPI_KEY`. +- Functional: identify if failing cron payload is legacy and lacks `credentialUserId`. +- Non-functional: no secret values in terminal, docs, issue comment, tests, or logs. +- Non-functional: read-only live checks unless operator explicitly approves mutation. + +## Architecture + +Cron creation captures credential identity in payload. Cron execution injects that identity into context. Exec tool uses context to join SecureCLI per-user env. Diagnosis must trace all three links. + +```text +cron.create ctx -> payload.credentialUserId -> makeCronJobHandler ctx -> exec LookupByBinary(userID) -> RAPIDAPI_KEY env +``` + +## Related Code Files + +- Modify: none in this phase. +- Read: `internal/store/pg/cron_crud.go` +- Read: `internal/store/sqlitestore/cron_crud.go` +- Read: `cmd/gateway_cron.go` +- Read: `internal/tools/credentialed_exec.go` +- Read: `internal/store/pg/secure_cli.go` +- Read: `internal/store/sqlitestore/secure-cli.go` + +## Implementation Steps + +1. Write failing characterization notes before code: + - `rapidapi` absent from SecureCLI registry. + - `rapidapi` present but grant missing/disabled. + - grant present but `RAPIDAPI_KEY` missing from merged env. + - cron payload lacks `credentialUserId`. +2. Inspect local DB or live DB only with read-only commands if credentials available: + - SecureCLI binary rows for `rapidapi`. + - grants for the cron agent. + - sanitized env keys only; never reveal value. + - cron payload JSON for target job. +3. Add or prepare unit-level characterization tests before changing product code: + - cron handler passes `credentialUserId` through. + - SecureCLI env merge exposes `RAPIDAPI_KEY` when configured. + - missing user env produces actionable branch in diagnostics plan. +4. Decide root cause: + - config-only: document operator fix and still consider preset/logging. + - code gap: proceed Phase 2. + +## Todo List + +- [x] Confirm current branch contains #54 fix. +- [x] Confirm `rapidapi` preset absence. +- [x] Inspect sanitized SecureCLI config path. +- [ ] Inspect failing cron payload shape if available. +- [x] Record exact failure class. + +## Success Criteria + +- [x] Root cause class documented with code/file evidence. +- [x] No secret material displayed or persisted. +- [x] Tests-before list finalized for Phase 2. + +## Diagnosis Result + +Root cause class: product support gap plus weak diagnostics, not missing cron identity plumbing. + +Code evidence: + +- `cmd/gateway_cron.go` injects `store.WithCredentialUserID` when `job.Payload.CredentialUserID` is present. +- `internal/store/cron_store.go` redacts `CredentialUserID` from response-safe cron jobs. +- `internal/tools/credentialed_exec.go` uses `store.CredentialUserIDFromContext(ctx)` for `LookupByBinary`. +- `internal/tools/credential_presets.go` has no `rapidapi` preset. +- `internal/tools/credentialed_exec.go` returns downstream CLI output when env is empty, so a RapidAPI failure surfaces as raw `RAPIDAPI_KEY required` instead of a GoClaw credential diagnostic. + +Live DB inspection was not run in this phase because no operator-approved credentialed environment or target cron job was provided. Phase 2 proceeds with code-level regression coverage for the confirmed local gap. + +## Risk Assessment + +- Risk: live DB access reveals secrets. Mitigation: query key names/sanitized responses only. +- Risk: legacy cron payload mistaken for current bug. Mitigation: compare job creation time against fix version. +- Risk: RapidAPI CLI command shape unknown. Mitigation: treat smoke command as operator-provided until verified. + +## Security Considerations + +- Never use `printenv`, `env`, or CLI debug flags to prove secret presence. +- Validate by CLI success/failure and sanitized env key metadata. + +## Next Steps + +- If root cause is missing preset or diagnostics, Phase 2. +- If root cause is stale cron payload only, Phase 3 documents remediation and issue reply. diff --git a/plans/260528-1801-rapidapi-cron-securecli-credentials/phase-02-add-rapidapi-preset-and-credential-diagnostics.md b/plans/260528-1801-rapidapi-cron-securecli-credentials/phase-02-add-rapidapi-preset-and-credential-diagnostics.md new file mode 100644 index 00000000..43b2815c --- /dev/null +++ b/plans/260528-1801-rapidapi-cron-securecli-credentials/phase-02-add-rapidapi-preset-and-credential-diagnostics.md @@ -0,0 +1,133 @@ +--- +phase: 2 +title: "Add RapidAPI Preset and Credential Diagnostics" +status: complete +priority: P1 +effort: "2.5h" +dependencies: [1] +--- + +# Phase 2: Add RapidAPI Preset and Credential Diagnostics + +## Context Links + +- Preset registry: `internal/tools/credential_presets.go` +- SecureCLI execution: `internal/tools/credentialed_exec.go` +- Env parsing/flattening: `internal/store/secure_cli_env.go` +- UI consumes presets through existing SecureCLI preset endpoints. + +## Overview + +Add the smallest product support needed for RapidAPI cron use: a SecureCLI preset and precise diagnostics. Do not add RapidAPI-specific runtime branches. + +## Key Insights + +- SecureCLI presets are static templates only; adding `rapidapi` is low blast radius. +- Credentialed exec logs currently show lookup found/no found, but not enough operator-safe reason detail. +- Host fall-through exec scrubs credential env keys; that is correct and should not be bypassed. +- 2026-05-29 implementation: + - Added built-in `rapidapi` preset with required `RAPIDAPI_KEY`. + - Added required preset env validation before binary resolution/execution. + - Added safe env-key-only diagnostics for merged env and missing required keys. + - Added `RAPIDAPI_KEY` to fall-through exec env scrub list. + +## Requirements + +- Functional: `rapidapi` appears in preset list with required `RAPIDAPI_KEY`. +- Functional: credentialed exec logs safe metadata for lookup and injection attempt. +- Functional: missing/empty merged env yields actionable error, not only downstream `RAPIDAPI_KEY required`. +- Non-functional: no secret values in logs; key names allowed. +- Non-functional: no cron API contract change. + +## Architecture + +Use existing SecureCLI: + +```text +Preset -> admin creates binary -> optional agent grant -> optional user env -> exec direct argv mode +``` + +Diagnostics live at two layers: + +- Lookup layer: found config? agent grant? credential user id present? +- Injection layer: merged env key names count, required key missing, binary path resolution. + +## Related Code Files + +- Modify: `internal/tools/credential_presets.go` +- Modify: `internal/tools/credentialed_exec.go` +- Modify: tests near `internal/tools/credential*_test.go` +- Modify if needed: `docs/project-changelog.md` +- Avoid unless required: DB migrations, cron schema, HTTP API contracts. + +## Implementation Steps + +### Tests Before + +1. Add preset test: + - `GetPreset("rapidapi")` exists. + - env vars include required `RAPIDAPI_KEY`. + - deny patterns avoid interactive/export/debug secret leakage if RapidAPI CLI has such commands. +2. Add credentialed env validation test: + - `rapidapi` config with no env returns an actionable missing-env error before executing binary. + - `rapidapi` config with `RAPIDAPI_KEY` proceeds to binary resolution/execution path. +3. Add logging behavior test only where stable without asserting log formatting too tightly. + +### Refactor / Implementation + +4. Add `rapidapi` preset: + - `BinaryName: "rapidapi"` + - `EnvVars: RAPIDAPI_KEY` + - conservative timeout, e.g. 60s. + - tips: use read-only/list/query commands; avoid debug output. +5. Add optional required-env guard for preset-required keys if current SecureCLI model supports it without schema change. + - If schema change would be needed, skip guard and rely on diagnostics/logging; YAGNI. +6. Improve operator-safe diagnostics: + - log binary, agent id, tenant id, credential user id presence, env key names only. + - distinguish no credential row vs found row with empty env. + - never log env values. +7. Ensure non-credentialed fallback remains scrubbed and blocked for registered non-global binaries. + +### Tests After + +8. Run focused tests: + - `go test ./internal/tools` + - `go test ./internal/store` +9. Run compile gate: + - `go build ./...` + +## Todo List + +- [x] Preset regression test first. +- [x] Missing-env diagnostic test first. +- [x] Add minimal preset. +- [x] Add safe diagnostics. +- [x] Update changelog if code changes. + +## Success Criteria + +- [x] `rapidapi` preset available through existing preset API. +- [x] Missing `RAPIDAPI_KEY` is distinguishable from policy block and binary-not-found. +- [x] Logs expose only safe metadata. +- [x] Existing SecureCLI security tests still pass. + +## Validation + +- `go test ./internal/tools -run 'RapidAPI|MissingRequiredEnv|MergeCredentialedEnv'` +- `go test ./internal/tools` + +## Risk Assessment + +- Risk: required-env guard changes all SecureCLI behavior. Mitigation: only add if scoped and test-protected; otherwise skip. +- Risk: logs leak identity detail. Mitigation: no values, no command args beyond existing truncation, key names only. +- Risk: RapidAPI CLI binary name differs. Mitigation: verify command package before final implementation; if command is not `rapidapi`, update preset name to real binary. + +## Security Considerations + +- Keep direct argv execution for credentialed mode. +- Do not support shell wrappers for RapidAPI credentials. +- Deny verbose/debug options if RapidAPI CLI can print HTTP headers. + +## Next Steps + +- Phase 3 validates cron path end-to-end and prepares GitHub issue reply. diff --git a/plans/260528-1801-rapidapi-cron-securecli-credentials/phase-03-validate-cron-smoke-and-issue-handoff.md b/plans/260528-1801-rapidapi-cron-securecli-credentials/phase-03-validate-cron-smoke-and-issue-handoff.md new file mode 100644 index 00000000..5f73b550 --- /dev/null +++ b/plans/260528-1801-rapidapi-cron-securecli-credentials/phase-03-validate-cron-smoke-and-issue-handoff.md @@ -0,0 +1,128 @@ +--- +phase: 3 +title: "Validate Cron Smoke and Issue Handoff" +status: complete +priority: P1 +effort: "2h" +dependencies: [2] +--- + +# Phase 3: Validate Cron Smoke and Issue Handoff + +## Context Links + +- Cron methods: `internal/gateway/methods/cron.go` +- Cron handler: `cmd/gateway_cron.go` +- Cron docs: `docs/08-scheduling-cron.md` +- SecureCLI docs: `docs/03-tools-system.md` + +## Overview + +Validate cron-triggered RapidAPI usage through tests and one operator-approved smoke path. Then update docs/changelog and reply to issue #74 with exact result and plan path. + +## Key Insights + +- Existing tests already cover cron credential context injection. +- #74 acceptance asks for smoke test and clear logs. +- Live smoke may need real RapidAPI key; no fake credential should be used to claim success. +- 2026-05-29 validation: + - Cron credential context injection and redaction tests are already present and covered by targeted test runs. + - RapidAPI direct exec is covered with a local script fixture and injected `RAPIDAPI_KEY`. + - Real RapidAPI cron smoke was not run because no operator-approved key, target agent grant, or harmless endpoint was provided in this session. + +## Requirements + +- Functional: cron-created-after-fix can access configured RapidAPI credential through SecureCLI. +- Functional: missing credentials return actionable error. +- Functional: GitHub issue gets concise summary and plan path. +- Non-functional: no secret printing or durable secret artifacts. +- Non-functional: skip load/stress tests per repo rule. + +## Architecture + +Validation combines: + +- unit tests for context and env wiring, +- optional integration/manual smoke for real RapidAPI CLI, +- issue handoff comment with branch/plan path. + +## Related Code Files + +- Modify if implementation occurred: `docs/project-changelog.md` +- Modify if docs need command examples: `docs/03-tools-system.md` +- Test: `cmd/gateway_cron_test.go` +- Test: `internal/tools/credentialed_exec*_test.go` +- Test: `internal/store/*secure_cli*_test.go` + +## Implementation Steps + +### Tests Before + +1. Add/confirm regression proving cron context carries `credentialUserId` into scheduler run. +2. Add test proving redacted cron API response never exposes `credentialUserId`. +3. Add test proving RapidAPI env key can be merged for credentialed exec without exposing value. + +### Validation + +4. Run targeted tests: + - `go test ./cmd ./internal/tools ./internal/store` +5. Run build gates if code changed: + - `go build ./...` + - `go build -tags sqliteonly ./...` +6. If real RapidAPI credentials available and user approves: + - create/update SecureCLI `rapidapi` config with sanitized workflow. + - grant target agent. + - create fresh cron job after #54 fix. + - run harmless RapidAPI read command. + - verify run log success and no secret output. +7. If live credentials unavailable: + - document skipped live smoke explicitly. + - keep issue open or mark plan blocked by credential availability. + +### Handoff + +8. Update changelog/docs only if implementation changes behavior. +9. Reply GitHub issue #74: + - plan path, + - root-cause hypothesis, + - chosen approach, + - validation gates, + - open operator questions. + +## Todo List + +- [x] Regression tests pass. +- [x] Build gates pass if code changed. +- [x] Smoke result recorded or skipped with reason. +- [ ] Issue #74 replied with plan summary and filepath after PR is opened. + +## Success Criteria + +- [x] Tests prove cron and SecureCLI credential path for RapidAPI. +- [x] Manual smoke either passes or is clearly blocked by missing real credentials. +- [x] No secrets in logs, tests, docs, or issue comment. +- [ ] GitHub issue comment links this plan and summarizes next implementation path after PR is opened. + +## Validation + +- `go test ./cmd ./internal/tools ./internal/store` +- `go build ./...` +- `go build -tags sqliteonly ./...` +- `go vet ./...` +- `TEST_DATABASE_URL="postgres://postgres:test@localhost:5433/goclaw_test?sslmode=disable" go test -race -tags integration ./tests/integration/` + +## Risk Assessment + +- Risk: smoke uses fake data and gives false confidence. Mitigation: fake keys only test error wording; real smoke must use real read-only endpoint. +- Risk: issue comment overstates completion. Mitigation: label plan as pending until implementation/smoke complete. +- Risk: docs drift from actual CLI syntax. Mitigation: verify RapidAPI CLI command syntax before adding command examples. + +## Security Considerations + +- Do not paste RapidAPI key in commands. +- Do not use command forms that echo env. +- Keep issue comment plan-only, no live config values. + +## Next Steps + +- After plan approval, run `/ck:cook /Volumes/GOON/www/digitop/goclaw/plans/260528-1801-rapidapi-cron-securecli-credentials/plan.md --tdd`. diff --git a/plans/260528-1801-rapidapi-cron-securecli-credentials/plan.md b/plans/260528-1801-rapidapi-cron-securecli-credentials/plan.md new file mode 100644 index 00000000..0e30e86a --- /dev/null +++ b/plans/260528-1801-rapidapi-cron-securecli-credentials/plan.md @@ -0,0 +1,69 @@ +--- +title: "RapidAPI cron SecureCLI credential fix" +description: "Diagnose and fix RapidAPI CLI credential injection from cron-triggered agent turns without changing cron credential architecture." +status: complete +priority: P1 +effort: 6h +issue: 74 +branch: "codex/issue-74-rapidapi-cron-plan" +tags: [bugfix, backend, security, cron, secure-cli, tdd] +blockedBy: [] +blocks: [] +created: "2026-05-28T11:01:28.893Z" +createdBy: "ck:plan" +source: skill +--- + +# RapidAPI cron SecureCLI credential fix + +## Overview + +Issue #74 reports `rapidapi` CLI failing from cron with `RAPIDAPI_KEY required`. +Broader cron credential context is already fixed by storing `payload.credentialUserId` on create and injecting it into cron runs. This plan keeps that architecture and focuses on RapidAPI-specific SecureCLI setup, diagnostics, and regression smoke. + +## Phases + +| Phase | Name | Status | +|-------|------|--------| +| 1 | [Diagnose Current RapidAPI Credential Path](./phase-01-diagnose-current-rapidapi-credential-path.md) | Complete | +| 2 | [Add RapidAPI Preset and Credential Diagnostics](./phase-02-add-rapidapi-preset-and-credential-diagnostics.md) | Complete | +| 3 | [Validate Cron Smoke and Issue Handoff](./phase-03-validate-cron-smoke-and-issue-handoff.md) | Complete | + +## Dependencies + +- GitHub issue: `digitopvn/goclaw#74` +- Related closed issue: `digitopvn/goclaw#54` +- Cron context path: `cmd/gateway_cron.go`, `internal/store/cron_store.go`, `internal/gateway/methods/cron.go` +- SecureCLI path: `internal/tools/credentialed_exec.go`, `internal/tools/credential_presets.go`, `internal/store/secure_cli_store.go` +- Docs to update if behavior changes: `docs/project-changelog.md`, `docs/03-tools-system.md` + +## Recommended Scope + +- First prove whether live config is missing `rapidapi`, missing agent grant, missing user env, or legacy cron payload. +- Add built-in RapidAPI preset only if config gap confirmed or reproducible from local tests. +- Add logs/tests that distinguish: no SecureCLI config, no grant, no `RAPIDAPI_KEY`, invalid env JSON, binary not found. + +## Out of Scope + +- New credential store. +- New cron auth model. +- Exposing credential values in API/logs. +- Load/stress tests. +- Direct live write smoke unless operator provides explicit credentialed environment. + +## Validation Gates + +- `go test ./internal/tools ./internal/store ./cmd` +- Targeted cron/SecureCLI regression tests before implementation changes. +- Manual or documented smoke: cron-created-after-fix invokes harmless `rapidapi` command and does not print secret. + +## Completion Notes + +- 2026-05-29: all code/test/build gates passed, including PG and SQLite builds, `go vet`, and integration race suite with local PG test DB. +- Real RapidAPI cron smoke remains operator-gated because this session did not include an approved RapidAPI key, target SecureCLI grant, or harmless endpoint. + +## Unresolved Questions + +- Is `rapidapi` registered in the live SecureCLI table now? +- Was the failing cron created before `v3.12.0-beta.25` / commit `f9440bac`? +- Which harmless RapidAPI endpoint should be the operator-approved smoke target?