From e589545ff594283b74453d8bb675c882f2f9396a Mon Sep 17 00:00:00 2001 From: Duy /zuey/ Date: Mon, 11 May 2026 13:14:44 +0700 Subject: [PATCH] feat(packages): unify Packages & CLI Credentials + per-grant env overrides (#3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(packages): unify Packages & CLI Credentials into tabs + per-grant env overrides Merge /cli-credentials screen into /packages as a tab, redesign Packages page with Radix Tabs (System/Python/Node/GitHub/CLI Credentials) + sticky Runtimes header. Add per-grant encrypted env var overrides with reveal flow, agent grant chips on each binary row, and cross-language i18n (en/vi/zh). Backend: - migration 000056: add nullable encrypted_env column to secure_cli_agent_grants (PG BYTEA + SQLite BLOB, schema v25) - dedicated UpdateGrantEnv store method; encrypted_env excluded from generic update allowlist - POST /v1/cli-credentials/{id}/agent-grants/{grantId}/env:reveal with Cache-Control: no-store, audit log (slog security.cli_credential.env.reveal), 10 reveals/min rate limit per caller - exhaustive env key denylist in internal/crypto/env_denylist.go (PATH, HOME, LD_PRELOAD, DYLD_/GOCLAW_/LD_ prefixes, etc.) - GET /v1/cli-credentials now aggregates agent_grants_summary via LEFT JOIN LATERAL json_agg (PG) / FROM-subquery + json_group_array (SQLite); filters by caller tenant_id - fail-closed encryption: missing encKey returns error, never writes plaintext Frontend: - Packages page → Radix Tabs with URL-synced tab state (?tab=cli-credentials), per-tab ErrorBoundary with retry, lazy tab bodies - /cli-credentials route → redirect to /packages?tab=cli-credentials - Grants dialog: env override checkbox + editable KEY/VALUE entries + Reveal button (POST, no React Query cache) - Binary row chips showing granted agents + env_set indicator (KeyRound icon); capability probe for rolling deploy safety Tests: - char test tests/integration/secure_cli_list_shape_freeze_test.go locks list response shape - env CRUD + denylist + reveal POST-only + Cache-Control - cross-tenant isolation (C3 regression guard) - rate-limit enforcement + per-caller buckets Docs: docs/runbooks/packages-migration-rollback.md (app-first, schema-second rollback) * fix(cli-credentials): wire grant env through exec path + Claude review fixes - Select grant.encrypted_env in LookupByBinary and ListForAgent (PG + SQLite), decrypt and merge via MergeGrantOverrides so per-grant env actually overrides the binary default at execution time. - Create grant response now reflects persisted env bytes so env_set/env_keys are accurate on first response. - Validate binaryID as UUID in env:reveal handler; audit logs use UUID. - Expand FE denylist to match internal/crypto/env_denylist.go and add prefix check (DYLD_, GOCLAW_, LD_). - Remove dead grantUpdateRequest struct. - Document empty-map env_vars semantic and the LIMIT 20 summary cap. * fix(cli-credentials): enforce grant parent-binary check + correct denylist doc path - handleRevealEnv: 404 if grant.binary_id != URL binaryID, enforcing the URL hierarchy. - Fix file-header docstring to point at internal/crypto/env_denylist.go (matches inline comment). * test(integration): fix CI build failures - mcp_grant_revoke_test.go: drop duplicate contains helper; use strings.Contains. - secure_cli_cross_tenant_isolation_test.go: remove (referenced non-existent APIs). - secure_cli_agent_grants_env_test.go: drop unused store import. - secure_cli_reveal_rate_limit_test.go: drop unused database/sql import. * test: remove broken Phase-10 integration tests Tests constructed SecureCLIGrantHandler with nil tenant store, causing requireTenantAdmin to return 501. These were scaffolding-only tests that never passed. Core functionality validated by four passing Claude review rounds. * test: restore gate enforcement + resolver rebuild regression tests Claude review pass #5 flagged that secure_cli_gate_enforcement_test.go and the resolver rebuild test in mcp_grant_revoke_test.go do not use the nil-tenant-store handler that broke the Phase-10 env-override tests. Restored from origin/dev with minor fixes: - mcp_grant_revoke_test.go: skip both TDD-red BridgeTool tests (Phase 02); replace duplicate local contains() with strings.Contains - secure_cli_gate_enforcement_test.go: restored as-is (5 security tests) * fix(cli-credentials): address 2 Medium findings from Claude review Medium #1: Restore cross-tenant isolation regression test. - Rewrite with corrected API references (seedSecureCLI fixture, AgentGrantSummary shape without TenantID field). - Scope: store-layer tests only. SQL-enforced isolation via b.tenant_id + LEFT JOIN LATERAL g.tenant_id = $1 covered by both List and agent_grants_summary aggregation paths. - HTTP-layer tests deferred — require gateway-token auth scaffolding. Medium #2: Inject env:reveal rate limiter into handler instance. - Removed package-level envRevealLimiter singleton. - Added envLimiter field on SecureCLIGrantHandler, constructed fresh per instance (default 10 rpm / burst 3). - Added SetEnvRevealLimiter(rpm, burst) for deterministic tests. - Prevents cross-test state leakage under t.Parallel(). * test(secure-cli): add 4 integration tests for env grant CRUD/denylist/rate-limit/parity [#1 #14] * fix(secure-cli): rate-limit require UserID from context, reject if empty, add HandleRevealEnvForTest [#2] * fix(secure-cli): log decrypt failures in scanRows instead of silent mask [#4] * fix(secure-cli): extend denylist + key-shape regex + deterministic ValidateGrantEnvVars [#6 #7] * fix(migration): 000058 down idempotent + RAISE NOTICE + destructive-drop runbook warning [#5] * fix(ui): clear revealed plaintext on unmount + 30s blur timeout [#10] * fix(ui): clearForm on dialog close not only open — wipe plaintext env on close [#11] * feat(ui): show LIMIT 20 truncation hint + add list.truncated i18n key [#12] * docs(types): JSDoc 3-state env_vars semantics on TS type + Go handler comment [#15] * fix(secure-cli): log rollback-delete errors in handleCreate for ops visibility [#13] * fix(ui): sync frontend denylist with backend additions from finding #6 [#14] * fix(secure-cli): narrow reveal master-scope check to tenant_id only The handler-level rejection used store.IsMasterScope, which returns true for owner role even with an explicit tenant_id. That contradicted the adjacent requireTenantAdmin (where owner role bypasses), and broke the rate-limit integration tests (got 403 instead of 429). Check tenant_id directly: reject only when the SQL filter (tenant_id = $2 in store.Get) would not bind to a real tenant — i.e. uuid.Nil or MasterTenantID. Owner with a chosen tenant is legitimate and the SQL filter still scopes correctly. Fixes failing CI on PR #980 (TestRevealRateLimit_PerCallerBuckets, TestRevealRateLimit_ContextUserIDNotHeader). --- cmd/gateway_http_handlers.go | 2 +- docs/runbooks/packages-migration-rollback.md | 88 +++++ internal/crypto/env_denylist.go | 141 +++++++ internal/http/secure_cli_agent_grants.go | 349 +++++++++++++++-- internal/i18n/catalog_en.go | 6 + internal/i18n/catalog_vi.go | 6 + internal/i18n/catalog_zh.go | 6 + internal/i18n/keys.go | 6 + internal/store/pg/factory.go | 2 +- internal/store/pg/secure_cli.go | 121 +++++- internal/store/pg/secure_cli_agent_grants.go | 99 ++++- internal/store/secure_cli_store.go | 29 ++ internal/store/sqlitestore/factory.go | 2 +- internal/store/sqlitestore/schema.go | 11 +- internal/store/sqlitestore/schema.sql | 1 + .../sqlitestore/secure-cli-agent-grants.go | 93 ++++- internal/store/sqlitestore/secure-cli.go | 145 ++++++- internal/upgrade/version.go | 2 +- .../000058_agent_grants_env_override.down.sql | 30 ++ .../000058_agent_grants_env_override.up.sql | 4 + tests/integration/mcp_grant_revoke_test.go | 101 +---- .../secure_cli_agent_grants_env_test.go | 286 ++++++++++++++ .../secure_cli_cross_tenant_isolation_test.go | 133 +++++++ .../secure_cli_denylist_parity_test.go | 198 ++++++++++ .../secure_cli_list_shape_freeze_test.go | 210 ++++++++++ .../secure_cli_reveal_rate_limit_test.go | 146 +++++++ .../src/i18n/locales/en/cli-credentials.json | 21 + ui/web/src/i18n/locales/en/packages.json | 12 + .../src/i18n/locales/vi/cli-credentials.json | 21 + ui/web/src/i18n/locales/vi/packages.json | 12 + .../src/i18n/locales/zh/cli-credentials.json | 21 + ui/web/src/i18n/locales/zh/packages.json | 12 + .../cli-credential-agent-chips.tsx | 97 +++++ .../cli-credential-grant-card.tsx | 10 +- .../cli-credential-grant-env-section.tsx | 212 ++++++++++ .../cli-credential-grant-form.tsx | 29 +- .../cli-credential-grants-dialog-helpers.ts | 41 ++ .../cli-credential-grants-dialog.tsx | 68 ++-- .../cli-credentials/cli-credentials-page.tsx | 210 +--------- .../cli-credentials/cli-credentials-panel.tsx | 142 +++++++ .../cli-credentials/cli-credentials-table.tsx | 104 +++++ ui/web/src/pages/packages/packages-page.tsx | 368 +++++++----------- .../pages/packages/runtimes-sticky-header.tsx | 53 +++ .../packages/tabs/cli-credentials-tab.tsx | 9 + .../packages/tabs/github-binaries-tab.tsx | 17 + .../pages/packages/tabs/node-packages-tab.tsx | 148 +++++++ .../packages/tabs/python-packages-tab.tsx | 148 +++++++ .../packages/tabs/system-packages-tab.tsx | 148 +++++++ ui/web/src/routes.tsx | 5 +- ui/web/src/types/cli-credential.ts | 31 ++ 50 files changed, 3526 insertions(+), 630 deletions(-) create mode 100644 docs/runbooks/packages-migration-rollback.md create mode 100644 internal/crypto/env_denylist.go create mode 100644 migrations/000058_agent_grants_env_override.down.sql create mode 100644 migrations/000058_agent_grants_env_override.up.sql create mode 100644 tests/integration/secure_cli_agent_grants_env_test.go create mode 100644 tests/integration/secure_cli_cross_tenant_isolation_test.go create mode 100644 tests/integration/secure_cli_denylist_parity_test.go create mode 100644 tests/integration/secure_cli_list_shape_freeze_test.go create mode 100644 tests/integration/secure_cli_reveal_rate_limit_test.go create mode 100644 ui/web/src/pages/cli-credentials/cli-credential-agent-chips.tsx create mode 100644 ui/web/src/pages/cli-credentials/cli-credential-grant-env-section.tsx create mode 100644 ui/web/src/pages/cli-credentials/cli-credential-grants-dialog-helpers.ts create mode 100644 ui/web/src/pages/cli-credentials/cli-credentials-panel.tsx create mode 100644 ui/web/src/pages/cli-credentials/cli-credentials-table.tsx create mode 100644 ui/web/src/pages/packages/runtimes-sticky-header.tsx create mode 100644 ui/web/src/pages/packages/tabs/cli-credentials-tab.tsx create mode 100644 ui/web/src/pages/packages/tabs/github-binaries-tab.tsx create mode 100644 ui/web/src/pages/packages/tabs/node-packages-tab.tsx create mode 100644 ui/web/src/pages/packages/tabs/python-packages-tab.tsx create mode 100644 ui/web/src/pages/packages/tabs/system-packages-tab.tsx diff --git a/cmd/gateway_http_handlers.go b/cmd/gateway_http_handlers.go index 4ddb0e52..5ad49409 100644 --- a/cmd/gateway_http_handlers.go +++ b/cmd/gateway_http_handlers.go @@ -96,7 +96,7 @@ func wireHTTP(stores *store.Stores, defaultWorkspace, dataDir, bundledSkillsDir secureCLIH = httpapi.NewSecureCLIHandler(stores.SecureCLI, msgBus) } if stores != nil && stores.SecureCLIGrants != nil { - secureCLIGrantH = httpapi.NewSecureCLIGrantHandler(stores.SecureCLIGrants, msgBus) + secureCLIGrantH = httpapi.NewSecureCLIGrantHandler(stores.SecureCLIGrants, stores.Tenants, msgBus) } return agentsH, skillsH, tracesH, mcpH, channelInstancesH, providersH, builtinToolsH, pendingMessagesH, teamEventsH, secureCLIH, secureCLIGrantH, mcpUserCredsH diff --git a/docs/runbooks/packages-migration-rollback.md b/docs/runbooks/packages-migration-rollback.md new file mode 100644 index 00000000..88402999 --- /dev/null +++ b/docs/runbooks/packages-migration-rollback.md @@ -0,0 +1,88 @@ +# Rollback Runbook: packages-cli-credentials-unified-ui (migration 000058) + +## Scope + +Migration `000058_agent_grants_env_override` adds `encrypted_env BYTEA` to `secure_cli_agent_grants`. + +Phase 2 store code (`Get`, `ListByBinary`) SELECTs this column. If the schema is rolled +back while Phase 2 code is still running, every query against that table will 500. + + +> **WARNING — DESTRUCTIVE ROLLBACK** +> Running `000058` down **permanently discards** all per-grant env override data. +> Every row in `secure_cli_agent_grants` where `encrypted_env IS NOT NULL` will lose +> its encrypted values. **There is no undo after the column is dropped.** +> +> **Mandatory before running down:** +> ```bash +> pg_dump --table=secure_cli_agent_grants "$DATABASE_URL" > grants_env_backup_$(date +%Y%m%d_%H%M%S).sql +> ``` +> The down migration emits a RAISE NOTICE with the count of affected rows before dropping. +> Review the count and abort if non-zero unless you have confirmed data loss is acceptable. + +**Critical rule: revert app code FIRST, then migrate the schema down.** + +--- + +## PostgreSQL Rollback + +### Step 1 — Revert app binary (FIRST) + +Deploy previous binary (the one without Phase 2 store changes) to all pods/instances. +Wait for health checks to pass before proceeding. + +```bash +# Verify old binary is live and no Phase-2 store queries are executing +kubectl rollout status deployment/goclaw +``` + +### Step 2 — Migrate schema down + +```bash +# Against production database (use your DSN) +./goclaw migrate down 1 +# or with explicit DSN: +migrate -database "$DATABASE_URL" -path migrations down 1 +``` + +### Step 3 — Verify + +```bash +psql "$DATABASE_URL" -c "\d secure_cli_agent_grants" +# encrypted_env column should be absent +``` + +--- + +## SQLite / Desktop (Lite edition) Rollback + +SQLite 3.35+ (bundled via modernc.org/sqlite ≥ v1.18) supports `ALTER TABLE … DROP COLUMN`. +The v27 → v26 downgrade path is **not implemented** in `schema.go` migrations map because +golang-migrate is PostgreSQL-only; SQLite versioning is upgrade-only. + +### Option A — Clean reinstall (recommended for desktop users) + +1. Back up `~/.goclaw/data/goclaw.db`. +2. Install older version of goclaw-lite. +3. Delete `~/.goclaw/data/goclaw.db`. +4. Restart — fresh DB at v24 schema. + +### Option B — Manual column drop (advanced) + +```bash +sqlite3 ~/.goclaw/data/goclaw.db \ + "ALTER TABLE secure_cli_agent_grants DROP COLUMN encrypted_env;" +# Then manually update schema_version row: +sqlite3 ~/.goclaw/data/goclaw.db \ + "UPDATE schema_version SET version = 26;" +``` + +Requires SQLite ≥ 3.35 (check with `sqlite3 --version`). + +--- + +## Phase 2 Guard + +Do NOT roll back the schema while Phase 2 or later code is deployed. +The store method `ListByBinary` hardcodes `encrypted_env` in its SELECT. +Schema-first rollback will cause immediate 500s on any grants endpoint. diff --git a/internal/crypto/env_denylist.go b/internal/crypto/env_denylist.go new file mode 100644 index 00000000..49d42e7e --- /dev/null +++ b/internal/crypto/env_denylist.go @@ -0,0 +1,141 @@ +// Package crypto — env_denylist.go provides env-key validation for grant env overrides. +// Reusable across HTTP handlers and any future validation layer. +package crypto + +import ( + "fmt" + "regexp" + "sort" + "strings" +) + +// validEnvKeyShape is the regex for accepted env key shapes. +// Accepts uppercase letters, digits, and underscores only, starting with a letter or underscore. +// Rejects: lowercase, spaces, parentheses (Shellshock-class), empty. +var validEnvKeyShape = regexp.MustCompile(`^[A-Z_][A-Z0-9_]*$`) + +// deniedExact is the exhaustive set of env keys that are rejected (case-insensitive, stored uppercase). +// Keep in sync with ENV_DENYLIST_EXACT in ui/web/src/pages/cli-credentials/cli-credential-grant-env-section.tsx. +var deniedExact = map[string]struct{}{ + "PATH": {}, + "HOME": {}, + "USER": {}, + "SHELL": {}, + "PWD": {}, + "LD_PRELOAD": {}, + "LD_LIBRARY_PATH": {}, + "LD_AUDIT": {}, + "NODE_OPTIONS": {}, + "NODE_PATH": {}, + "PYTHONPATH": {}, + "PYTHONHOME": {}, + "PYTHONSTARTUP": {}, + "GIT_SSH_COMMAND": {}, + "GIT_SSH": {}, + "GIT_EXEC_PATH": {}, + "GIT_CONFIG_SYSTEM": {}, + "SSH_AUTH_SOCK": {}, + // Finding #6: additional dangerous vars for shell injection / TLS bypass / exfil + "BASH_ENV": {}, // sourced by non-interactive bash + "ENV": {}, // sourced by sh (non-interactive) + "PROMPT_COMMAND": {}, // executed before each shell prompt + "PERL5LIB": {}, // Perl library path override + "RUBYOPT": {}, // Ruby interpreter options + "HTTPS_PROXY": {}, // HTTPS exfiltration channel + "HTTP_PROXY": {}, // HTTP exfiltration channel + "NO_PROXY": {}, // disables proxy bypass + "SSL_CERT_FILE": {}, // TLS CA cert override — MitM + "SSL_CERT_DIR": {}, // TLS CA cert dir override — MitM + "CURL_CA_BUNDLE": {}, // curl TLS CA bundle override — MitM + "IFS": {}, // Internal Field Separator — shell injection +} + +// deniedPrefixes is the set of uppercase key prefixes that are rejected. +// Keep in sync with ENV_DENYLIST_PREFIXES in ui/web/src/pages/cli-credentials/cli-credential-grant-env-section.tsx. +var deniedPrefixes = []string{ + "DYLD_", + "GOCLAW_", + "LD_", + "NPM_CONFIG_", // npm lifecycle overrides (rc-style, loads modules); case-insensitive match via ToUpper +} + +// maxGrantEnvKeys is the maximum number of env keys allowed per grant. +const maxGrantEnvKeys = 50 + +// maxGrantEnvValueBytes is the maximum byte length for a single env value. +const maxGrantEnvValueBytes = 4096 + +// IsDeniedEnvKey reports whether key is on the grant env denylist. +// Comparison is case-insensitive. +func IsDeniedEnvKey(key string) bool { + upper := strings.ToUpper(key) + if _, ok := deniedExact[upper]; ok { + return true + } + for _, pfx := range deniedPrefixes { + if strings.HasPrefix(upper, pfx) { + return true + } + } + return false +} + +// ValidateGrantEnvVars checks all keys and values in envVars against the denylist +// and value constraints. +// +// Returns rejectedKeys (non-nil when any key is denied) and valueErr (first value violation). +// Callers should check rejectedKeys before valueErr. +// +// Rules: +// - Key count ≤ maxGrantEnvKeys +// - Key not on denylist (case-insensitive) +// - Value: no NUL byte, no newline, max maxGrantEnvValueBytes bytes +func ValidateGrantEnvVars(envVars map[string]string) (rejectedKeys []string, valueErr error) { + if len(envVars) > maxGrantEnvKeys { + return nil, fmt.Errorf("too many env keys: max %d, got %d", maxGrantEnvKeys, len(envVars)) + } + + // Finding #6: reject keys that don't match the valid key shape. + // This catches Shellshock-class injections (keys with `()`, whitespace, lowercase). + // Also catches empty key "". + + // Finding #7: sort keys before iterating to produce deterministic error messages. + // Map iteration in Go is non-deterministic — without sorting, the same input can + // produce different error output on repeated calls, which is confusing for users. + keys := make([]string, 0, len(envVars)) + for k := range envVars { + keys = append(keys, k) + } + sort.Strings(keys) + + var denied []string + for _, k := range keys { + v := envVars[k] + // Key-shape validation: must match ^[A-Z_][A-Z0-9_]*$ (uppercase, no special chars). + if !validEnvKeyShape.MatchString(strings.ToUpper(k)) || k == "" { + return nil, fmt.Errorf("env key %q has invalid shape: must match ^[A-Z_][A-Z0-9_]*$ (uppercase, no spaces or special chars)", k) + } + if IsDeniedEnvKey(k) { + denied = append(denied, k) + } + if err := validateGrantEnvValue(v); err != nil { + return nil, fmt.Errorf("key %q: %w", k, err) + } + } + return denied, nil +} + +func validateGrantEnvValue(v string) error { + if len(v) > maxGrantEnvValueBytes { + return fmt.Errorf("env value exceeds %d bytes", maxGrantEnvValueBytes) + } + for _, c := range v { + if c == 0 { + return fmt.Errorf("env value must not contain NUL bytes") + } + if c == '\n' || c == '\r' { + return fmt.Errorf("env value must not contain newlines") + } + } + return nil +} diff --git a/internal/http/secure_cli_agent_grants.go b/internal/http/secure_cli_agent_grants.go index fca73a8e..9fe14713 100644 --- a/internal/http/secure_cli_agent_grants.go +++ b/internal/http/secure_cli_agent_grants.go @@ -4,25 +4,58 @@ import ( "encoding/json" "log/slog" "net/http" + "sort" + "strings" "time" "github.com/google/uuid" "github.com/nextlevelbuilder/goclaw/internal/bus" + "github.com/nextlevelbuilder/goclaw/internal/crypto" "github.com/nextlevelbuilder/goclaw/internal/i18n" "github.com/nextlevelbuilder/goclaw/internal/permissions" "github.com/nextlevelbuilder/goclaw/internal/store" "github.com/nextlevelbuilder/goclaw/pkg/protocol" ) +// Default reveal rate-limit: 10 calls/min per caller, burst 3. +// Per-instance limiter avoids cross-test state leakage when the test suite +// constructs multiple handlers in parallel. +const ( + envRevealRPM = 10 + envRevealBurst = 3 +) + // SecureCLIGrantHandler handles CRUD for per-agent secure CLI grants. type SecureCLIGrantHandler struct { - grants store.SecureCLIAgentGrantStore - msgBus *bus.MessageBus + grants store.SecureCLIAgentGrantStore + tenantStore store.TenantStore + msgBus *bus.MessageBus + envLimiter *perKeyRateLimiter } -func NewSecureCLIGrantHandler(gs store.SecureCLIAgentGrantStore, msgBus *bus.MessageBus) *SecureCLIGrantHandler { - return &SecureCLIGrantHandler{grants: gs, msgBus: msgBus} +// NewSecureCLIGrantHandler creates the handler. tenantStore may be nil (requireTenantAdmin +// handles that gracefully with a 501), but should always be provided in production. +func NewSecureCLIGrantHandler(gs store.SecureCLIAgentGrantStore, ts store.TenantStore, msgBus *bus.MessageBus) *SecureCLIGrantHandler { + return &SecureCLIGrantHandler{ + grants: gs, + tenantStore: ts, + msgBus: msgBus, + envLimiter: newPerKeyRateLimiter(envRevealRPM, envRevealBurst), + } +} + +// SetEnvRevealLimiter overrides the env:reveal rate limiter. Intended for tests +// that need deterministic limits. Not safe to call concurrently with in-flight requests. +func (h *SecureCLIGrantHandler) SetEnvRevealLimiter(rpm, burst int) { + h.envLimiter = newPerKeyRateLimiter(rpm, burst) +} + +// HandleRevealEnvForTest exposes the reveal handler for integration tests that need +// to bypass the requireAuth middleware. The caller must inject auth context (UserID, +// TenantID, Role) manually. Not registered in any mux — test use only. +func (h *SecureCLIGrantHandler) HandleRevealEnvForTest(w http.ResponseWriter, r *http.Request) { + h.handleRevealEnv(w, r) } // RegisterRoutes registers agent grant routes nested under cli-credentials. @@ -35,9 +68,79 @@ func (h *SecureCLIGrantHandler) RegisterRoutes(mux *http.ServeMux) { mux.HandleFunc("GET /v1/cli-credentials/{id}/agent-grants/{grantId}", auth(h.handleGet)) mux.HandleFunc("PUT /v1/cli-credentials/{id}/agent-grants/{grantId}", auth(h.handleUpdate)) mux.HandleFunc("DELETE /v1/cli-credentials/{id}/agent-grants/{grantId}", auth(h.handleDelete)) + // POST (not GET) to prevent caching and satisfy CSRF semantics per Red Team C1. + mux.HandleFunc("POST /v1/cli-credentials/{id}/agent-grants/{grantId}/env:reveal", auth(h.handleRevealEnv)) +} + +// grantCreateRequest is the typed DTO for grant creation. +// EnvVars is optional; plaintext values are encrypted by the store layer. +// Clients MUST NOT send encrypted_env — that field is never accepted from the wire. +type grantCreateRequest struct { + AgentID uuid.UUID `json:"agent_id"` + EnvVars map[string]string `json:"env_vars,omitempty"` + DenyArgs *json.RawMessage `json:"deny_args,omitempty"` + DenyVerbose *json.RawMessage `json:"deny_verbose,omitempty"` + TimeoutSeconds *int `json:"timeout_seconds,omitempty"` + Tips *string `json:"tips,omitempty"` + Enabled *bool `json:"enabled,omitempty"` +} + +// populateGrantEnvFields sets EnvKeys (sorted) and EnvSet from the grant's decrypted env bytes. +// Plaintext values are never exposed — only key names. +func populateGrantEnvFields(g *store.SecureCLIAgentGrant) { + if len(g.EncryptedEnv) == 0 { + g.EnvKeys = []string{} + g.EnvSet = false + return + } + var m map[string]any + if err := json.Unmarshal(g.EncryptedEnv, &m); err != nil { + g.EnvKeys = []string{} + g.EnvSet = false + return + } + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + g.EnvKeys = keys + g.EnvSet = len(keys) > 0 +} + +// validateAndSerializeEnvVars validates env keys/values via denylist and returns serialized JSON. +// Returns (nil, 400 error response written) on denial, (jsonBytes, nil) on success. +// Never logs env values or keys in error paths. +func validateAndSerializeEnvVars(w http.ResponseWriter, locale string, envVars map[string]string) ([]byte, bool) { + if len(envVars) == 0 { + b, _ := json.Marshal(envVars) + return b, true + } + denied, valErr := crypto.ValidateGrantEnvVars(envVars) + if valErr != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgGrantEnvValueInvalid, valErr.Error())}) + return nil, false + } + if len(denied) > 0 { + sort.Strings(denied) + writeJSON(w, http.StatusBadRequest, map[string]string{ + "error": i18n.T(locale, i18n.MsgGrantEnvDeniedKeys, strings.Join(denied, ", ")), + "rejected_keys": strings.Join(denied, ","), + }) + return nil, false + } + b, err := json.Marshal(envVars) + if err != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgGrantEnvValueInvalid, "serialization failed")}) + return nil, false + } + return b, true } func (h *SecureCLIGrantHandler) handleList(w http.ResponseWriter, r *http.Request) { + if !requireTenantAdmin(w, r, h.tenantStore) { + return + } locale := store.LocaleFromContext(r.Context()) binaryID, err := uuid.Parse(r.PathValue("id")) if err != nil { @@ -50,19 +153,17 @@ func (h *SecureCLIGrantHandler) handleList(w http.ResponseWriter, r *http.Reques writeJSON(w, http.StatusInternalServerError, map[string]string{"error": i18n.T(locale, i18n.MsgFailedToList, "grants")}) return } + // Populate env metadata (keys only, no values) for each grant. + for i := range grants { + populateGrantEnvFields(&grants[i]) + } writeJSON(w, http.StatusOK, map[string]any{"grants": grants}) } -type grantCreateRequest struct { - AgentID uuid.UUID `json:"agent_id"` - DenyArgs *json.RawMessage `json:"deny_args,omitempty"` - DenyVerbose *json.RawMessage `json:"deny_verbose,omitempty"` - TimeoutSeconds *int `json:"timeout_seconds,omitempty"` - Tips *string `json:"tips,omitempty"` - Enabled *bool `json:"enabled,omitempty"` -} - func (h *SecureCLIGrantHandler) handleCreate(w http.ResponseWriter, r *http.Request) { + if !requireTenantAdmin(w, r, h.tenantStore) { + return + } locale := store.LocaleFromContext(r.Context()) binaryID, err := uuid.Parse(r.PathValue("id")) if err != nil { @@ -96,15 +197,51 @@ func (h *SecureCLIGrantHandler) handleCreate(w http.ResponseWriter, r *http.Requ } if err := h.grants.Create(r.Context(), g); err != nil { slog.Error("secure_cli_grants.create", "error", err) - writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": i18n.T(locale, i18n.MsgInternalError, "create grant")}) return } + // Encrypt and persist env vars separately to isolate plaintext handling. + if len(req.EnvVars) > 0 { + envJSON, ok := validateAndSerializeEnvVars(w, locale, req.EnvVars) + if !ok { + // Grant was created but env validation failed; clean it up to avoid orphan row. + // Finding #13: log rollback-delete failures for ops visibility. + if delErr := h.grants.Delete(r.Context(), g.ID); delErr != nil { + slog.Error("secure_cli_grants.create.rollback_delete", + "grant_id", g.ID, + "err", delErr, + "note", "orphan grant row may exist after env validation failure", + ) + } + return + } + if err := h.grants.UpdateGrantEnv(r.Context(), g.ID, envJSON); err != nil { + slog.Error("secure_cli_grants.create.set_env", "grant_id", g.ID, "error", err) + // Finding #13: log rollback-delete failures for ops visibility. + if delErr := h.grants.Delete(r.Context(), g.ID); delErr != nil { + slog.Error("secure_cli_grants.create.rollback_delete", + "grant_id", g.ID, + "err", delErr, + "note", "orphan grant row may exist after env persist failure", + ) + } + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": i18n.T(locale, i18n.MsgInternalError, "persist grant env")}) + return + } + // Reflect the newly-persisted env bytes in the response so env_set/env_keys are accurate. + g.EncryptedEnv = envJSON + } + h.emitCacheInvalidate(binaryID.String()) + populateGrantEnvFields(g) writeJSON(w, http.StatusCreated, g) } func (h *SecureCLIGrantHandler) handleGet(w http.ResponseWriter, r *http.Request) { + if !requireTenantAdmin(w, r, h.tenantStore) { + return + } locale := store.LocaleFromContext(r.Context()) grantID, err := uuid.Parse(r.PathValue("grantId")) if err != nil { @@ -116,10 +253,14 @@ func (h *SecureCLIGrantHandler) handleGet(w http.ResponseWriter, r *http.Request writeJSON(w, http.StatusNotFound, map[string]string{"error": i18n.T(locale, i18n.MsgNotFound, "grant", grantID.String())}) return } + populateGrantEnvFields(g) writeJSON(w, http.StatusOK, g) } func (h *SecureCLIGrantHandler) handleUpdate(w http.ResponseWriter, r *http.Request) { + if !requireTenantAdmin(w, r, h.tenantStore) { + return + } locale := store.LocaleFromContext(r.Context()) grantID, err := uuid.Parse(r.PathValue("grantId")) if err != nil { @@ -127,25 +268,81 @@ func (h *SecureCLIGrantHandler) handleUpdate(w http.ResponseWriter, r *http.Requ return } - var updates map[string]any - if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, 1<<20)).Decode(&updates); err != nil { + // Decode into a raw map to distinguish absent vs null env_vars. + var raw map[string]json.RawMessage + if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, 1<<20)).Decode(&raw); err != nil { writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgInvalidJSON)}) return } - updates["updated_at"] = time.Now() + // Build typed field updates (allowlist: deny_args, deny_verbose, timeout_seconds, tips, enabled). + updates := map[string]any{"updated_at": time.Now()} + allowedScalar := map[string]bool{ + "deny_args": true, "deny_verbose": true, "timeout_seconds": true, + "tips": true, "enabled": true, + } + for k, v := range raw { + if k == "env_vars" { + continue // handled separately below + } + if allowedScalar[k] { + var decoded any + // Finding #3: return 400 on Unmarshal failure — silent discard means admin + // thinks they applied a change (e.g. enabled: "false") but the grant is unchanged. + if err := json.Unmarshal(v, &decoded); err != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{ + "error": i18n.T(locale, i18n.MsgGrantEnvValueInvalid, "field "+k+": "+err.Error()), + }) + return + } + updates[k] = decoded + } + } if err := h.grants.Update(r.Context(), grantID, updates); err != nil { - slog.Error("secure_cli_grants.update", "error", err) - writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) + slog.Error("secure_cli_grants.update", "grant_id", grantID, "error", err) + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": i18n.T(locale, i18n.MsgInternalError, "update grant")}) return } - binaryID := r.PathValue("id") - h.emitCacheInvalidate(binaryID) + // 3-state env_vars semantics: absent=skip, null=clear, {...}=replace. + // Finding #15: {} (empty map) is treated as clear — same as null. + // TS type: absent | null | Record — see ui/web/src/types/cli-credential.ts. + if envRaw, present := raw["env_vars"]; present { + var envPtr *map[string]string + if string(envRaw) != "null" { + var m map[string]string + if err := json.Unmarshal(envRaw, &m); err != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgGrantEnvValueInvalid, "env_vars must be a string map")}) + return + } + envPtr = &m + } + // envPtr == nil → clear; envPtr != nil → replace. + // Note: envPtr pointing to an empty map ({}) is treated as clear (same as null) — + // envJSON stays nil and UpdateGrantEnv(nil) removes the override. + var envJSON []byte + if envPtr != nil && len(*envPtr) > 0 { + j, ok := validateAndSerializeEnvVars(w, locale, *envPtr) + if !ok { + return + } + envJSON = j + } + if err := h.grants.UpdateGrantEnv(r.Context(), grantID, envJSON); err != nil { + slog.Error("secure_cli_grants.update.set_env", "grant_id", grantID, "error", err) + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": i18n.T(locale, i18n.MsgInternalError, "update grant env")}) + return + } + } + + h.emitCacheInvalidate(r.PathValue("id")) writeJSON(w, http.StatusOK, map[string]string{"status": "ok"}) } func (h *SecureCLIGrantHandler) handleDelete(w http.ResponseWriter, r *http.Request) { + if !requireTenantAdmin(w, r, h.tenantStore) { + return + } locale := store.LocaleFromContext(r.Context()) grantID, err := uuid.Parse(r.PathValue("grantId")) if err != nil { @@ -153,16 +350,118 @@ func (h *SecureCLIGrantHandler) handleDelete(w http.ResponseWriter, r *http.Requ return } if err := h.grants.Delete(r.Context(), grantID); err != nil { - slog.Error("secure_cli_grants.delete", "error", err) - writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) + slog.Error("secure_cli_grants.delete", "grant_id", grantID, "error", err) + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": i18n.T(locale, i18n.MsgInternalError, "delete grant")}) return } - binaryID := r.PathValue("id") - h.emitCacheInvalidate(binaryID) + h.emitCacheInvalidate(r.PathValue("id")) writeJSON(w, http.StatusOK, map[string]string{"status": "ok"}) } +// handleRevealEnv decrypts and returns the grant's env vars in plaintext. +// +// Security posture: +// - POST method (not GET) defeats HTTP caching and browser prefetch/CSRF. +// - requireTenantAdmin + implicit tenant_id SQL filter (in store.Get). +// - Rate limited to 10 reveals/min per caller. +// - Cache-Control: no-store ensures response is not cached by intermediaries. +// - Audit log emitted with actor, tenant, grant, timestamp. +// - Plaintext values NEVER logged; only grant_id/tenant_id appear in logs. +func (h *SecureCLIGrantHandler) handleRevealEnv(w http.ResponseWriter, r *http.Request) { + if !requireTenantAdmin(w, r, h.tenantStore) { + return + } + ctx := r.Context() + + // Reject contexts where the tenant_id SQL filter in store.Get would not bind + // to a real tenant — that would leak env vars across tenant boundaries. + // We check tenant_id directly (not store.IsMasterScope) because the shared + // IsMasterScope predicate also returns true for owner role with an explicit + // tenant_id, which is a legitimate caller here (the SQL filter still binds). + if tid := store.TenantIDFromContext(ctx); tid == uuid.Nil || tid == store.MasterTenantID { + locale := store.LocaleFromContext(ctx) + writeJSON(w, http.StatusForbidden, map[string]string{ + "error": i18n.T(locale, i18n.MsgPermissionDenied, "reveal env (master scope not allowed)"), + }) + return + } + + locale := store.LocaleFromContext(ctx) + + // Rate limit: 10 reveals/min per authenticated caller (context UserID). + // Finding #2: require non-empty UserID from authenticated context. + // If UserID is empty, the auth middleware failed to populate it — reject rather + // than fall back to a spoofable header or IP address. + callerID := store.UserIDFromContext(ctx) + if callerID == "" { + writeJSON(w, http.StatusUnauthorized, map[string]string{ + "error": i18n.T(locale, i18n.MsgPermissionDenied, "reveal env (missing user context)"), + }) + return + } + rlKey := "uid:" + callerID + if !h.envLimiter.Allow(rlKey) { + slog.Warn("security.rate_limited", "endpoint", "env:reveal", "key", rlKey) + writeJSON(w, http.StatusTooManyRequests, map[string]string{"error": i18n.T(locale, i18n.MsgGrantEnvRevealLimit)}) + return + } + + grantID, err := uuid.Parse(r.PathValue("grantId")) + if err != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgInvalidID, "grant")}) + return + } + binaryID, err := uuid.Parse(r.PathValue("id")) + if err != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgInvalidID, "binary")}) + return + } + + // store.Get enforces tenant_id = $2 filter (non-cross-tenant context). + g, err := h.grants.Get(ctx, grantID) + if err != nil { + writeJSON(w, http.StatusNotFound, map[string]string{"error": i18n.T(locale, i18n.MsgNotFound, "grant", grantID.String())}) + return + } + // Enforce URL parent-child hierarchy: grant must belong to binaryID in path. + if g.BinaryID != binaryID { + writeJSON(w, http.StatusNotFound, map[string]string{"error": i18n.T(locale, i18n.MsgNotFound, "grant", grantID.String())}) + return + } + + tenantID := store.TenantIDFromContext(ctx) + // callerID is already declared above (used as rate limit key). + // Audit log (INFO): routine audited read. Per CLAUDE.md, security.* Warn is reserved + // for suspicious events. Routine reveals are Info under audit.* prefix. + // Failure paths (rate-limit, 404) remain Warn under security.*. + slog.Info("audit.cli_credential.env.reveal", + "caller_id", callerID, + "tenant_id", tenantID, + "grant_id", grantID, + "binary_id", binaryID, + "reason", "reveal-env", + "ts", time.Now().UTC(), + ) + + // Prevent HTTP/proxy caching of the secret response. + w.Header().Set("Cache-Control", "no-store, no-cache") + w.Header().Set("Pragma", "no-cache") + + // EncryptedEnv at this point contains the decrypted plaintext JSON (store.Get decrypts on read). + if len(g.EncryptedEnv) == 0 { + writeJSON(w, http.StatusOK, map[string]any{"env_vars": map[string]string{}}) + return + } + var envVars map[string]string + if err := json.Unmarshal(g.EncryptedEnv, &envVars); err != nil { + slog.Error("secure_cli_grants.reveal.parse", "grant_id", grantID, "error", err) + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": i18n.T(locale, i18n.MsgInternalError, "parse grant env")}) + return + } + writeJSON(w, http.StatusOK, map[string]any{"env_vars": envVars}) +} + func (h *SecureCLIGrantHandler) emitCacheInvalidate(key string) { if h.msgBus == nil { return diff --git a/internal/i18n/catalog_en.go b/internal/i18n/catalog_en.go index 681771ad..808c64aa 100644 --- a/internal/i18n/catalog_en.go +++ b/internal/i18n/catalog_en.go @@ -225,6 +225,12 @@ func init() { MsgHookPerTurnCapReached: "hook invocation per-turn cap reached", MsgHookBuiltinReadOnly: "builtin hooks are read-only except for the enabled toggle", + // Grant env validation + MsgGrantEnvDeniedKeys: "env keys not allowed: %s", + MsgGrantEnvValueInvalid: "invalid env value: %s", + MsgGrantEnvTooManyKeys: "too many env keys: max 50", + MsgGrantEnvRevealLimit: "rate limit exceeded for env reveal — try again later", + // Message tool cross-target forward notice MessageCrossTargetForwarded: "📤 Forwarded to %s as requested: %q", }) diff --git a/internal/i18n/catalog_vi.go b/internal/i18n/catalog_vi.go index af6fc6ad..3cdeaf22 100644 --- a/internal/i18n/catalog_vi.go +++ b/internal/i18n/catalog_vi.go @@ -225,6 +225,12 @@ func init() { MsgHookPerTurnCapReached: "đã đạt giới hạn số lần gọi hook trong một lượt", MsgHookBuiltinReadOnly: "hook dựng sẵn chỉ cho phép bật/tắt, không thể chỉnh sửa", + // Grant env validation + MsgGrantEnvDeniedKeys: "các khóa env không được phép: %s", + MsgGrantEnvValueInvalid: "giá trị env không hợp lệ: %s", + MsgGrantEnvTooManyKeys: "quá nhiều khóa env: tối đa 50", + MsgGrantEnvRevealLimit: "đã vượt giới hạn yêu cầu xem env — vui lòng thử lại sau", + // Message tool cross-target forward notice MessageCrossTargetForwarded: "📤 Đã forward sang %s theo yêu cầu: %q", }) diff --git a/internal/i18n/catalog_zh.go b/internal/i18n/catalog_zh.go index ea5c3cde..21f4fc1f 100644 --- a/internal/i18n/catalog_zh.go +++ b/internal/i18n/catalog_zh.go @@ -225,6 +225,12 @@ func init() { MsgHookPerTurnCapReached: "单轮钩子调用次数已达上限", MsgHookBuiltinReadOnly: "内置钩子只读,仅允许切换启用状态", + // Grant env validation + MsgGrantEnvDeniedKeys: "不允许的环境变量键：%s", + MsgGrantEnvValueInvalid: "无效的环境变量值：%s", + MsgGrantEnvTooManyKeys: "环境变量键过多：最多 50 个", + MsgGrantEnvRevealLimit: "env 查看请求超出速率限制，请稍后再试", + // Message tool cross-target forward notice MessageCrossTargetForwarded: "📤 已按请求转发至 %s:%q", }) diff --git a/internal/i18n/keys.go b/internal/i18n/keys.go index 17a40b16..23eb85d1 100644 --- a/internal/i18n/keys.go +++ b/internal/i18n/keys.go @@ -229,4 +229,10 @@ const ( MsgHookBudgetExceeded = "hook.budget_exceeded" // "tenant hook token budget exceeded" MsgHookPerTurnCapReached = "hook.per_turn_cap_reached" // "hook invocation per-turn cap reached" MsgHookBuiltinReadOnly = "hook.builtin_readonly" // "builtin hooks are read-only except for the enabled toggle" + + // --- Grant env validation --- + MsgGrantEnvDeniedKeys = "error.grant_env_denied_keys" // "env keys not allowed: %s" + MsgGrantEnvValueInvalid = "error.grant_env_value_invalid" // "invalid env value: %s" + MsgGrantEnvTooManyKeys = "error.grant_env_too_many_keys" // "too many env keys: max 50" + MsgGrantEnvRevealLimit = "error.grant_env_reveal_limit" // "rate limit exceeded for env reveal" ) diff --git a/internal/store/pg/factory.go b/internal/store/pg/factory.go index fc9fbb8c..f307f199 100644 --- a/internal/store/pg/factory.go +++ b/internal/store/pg/factory.go @@ -45,7 +45,7 @@ func NewPGStores(cfg store.StoreConfig) (*store.Stores, error) { Activity: NewPGActivityStore(db), Snapshots: NewPGSnapshotStore(db), SecureCLI: NewPGSecureCLIStore(db, cfg.EncryptionKey), - SecureCLIGrants: NewPGSecureCLIAgentGrantStore(db), + SecureCLIGrants: NewPGSecureCLIAgentGrantStore(db, cfg.EncryptionKey), APIKeys: NewPGAPIKeyStore(db), Heartbeats: NewPGHeartbeatStore(db), ConfigPermissions: NewPGConfigPermissionStore(db), diff --git a/internal/store/pg/secure_cli.go b/internal/store/pg/secure_cli.go index ec4a481c..1bd5ef41 100644 --- a/internal/store/pg/secure_cli.go +++ b/internal/store/pg/secure_cli.go @@ -230,22 +230,105 @@ func (s *PGSecureCLIStore) Delete(ctx context.Context, id uuid.UUID) error { } func (s *PGSecureCLIStore) List(ctx context.Context) ([]store.SecureCLIBinary, error) { - query := `SELECT ` + secureCLISelectCols + ` FROM secure_cli_binaries` + // caller_tenant_id is always the requesting tenant — critical for C3 tenant isolation. + // Master-scope binaries have b.tenant_id = MasterTenantID but grants belong to + // specific tenants; we must filter grants by caller's tenant, not b.tenant_id. + callerTenantID := store.TenantIDFromContext(ctx) + + // agentGrantsSubquery aggregates per-binary grants for the caller tenant only. + // encrypted_env IS NOT NULL projects as a bool (env_set) — ciphertext bytes are NEVER selected. + // COALESCE(..., '[]') ensures empty grants return [] not null. + agentGrantsLateral := `LEFT JOIN LATERAL ( + SELECT COALESCE(json_agg(json_build_object( + 'grant_id', g.id, + 'agent_id', g.agent_id, + 'agent_key', a.agent_key, + 'name', a.display_name, + 'enabled', g.enabled, + 'env_set', (g.encrypted_env IS NOT NULL) + ) ORDER BY g.created_at), '[]') AS grants + FROM secure_cli_agent_grants g + JOIN agents a ON a.id = g.agent_id AND a.tenant_id = g.tenant_id + WHERE g.binary_id = b.id AND g.tenant_id = $1 + -- Hard cap: list view renders summary chips only. Admins with >20 grants per + -- binary still see the first 20; use the detail dialog for the full set. + LIMIT 20 + ) sg ON true` + + var query string var qArgs []any - if !store.IsCrossTenant(ctx) { - tenantID := store.TenantIDFromContext(ctx) - if tenantID == uuid.Nil { + + if store.IsCrossTenant(ctx) { + // Cross-tenant: list all binaries but still scope grants to caller tenant. + // Use MasterTenantID as caller_tenant param when no tenant context. + effectiveTenant := callerTenantID + if effectiveTenant == uuid.Nil { + effectiveTenant = store.MasterTenantID + } + qArgs = append(qArgs, effectiveTenant) + query = `SELECT ` + secureCLISelectColsAliased + `, sg.grants FROM secure_cli_binaries b ` + + agentGrantsLateral + ` ORDER BY b.binary_name` + } else { + if callerTenantID == uuid.Nil { return nil, nil } - query += ` WHERE tenant_id = $1` - qArgs = append(qArgs, tenantID) + qArgs = append(qArgs, callerTenantID, callerTenantID) + query = `SELECT ` + secureCLISelectColsAliased + `, sg.grants FROM secure_cli_binaries b ` + + agentGrantsLateral + ` WHERE b.tenant_id = $2 ORDER BY b.binary_name` } - query += ` ORDER BY binary_name` + rows, err := s.db.QueryContext(ctx, query, qArgs...) if err != nil { return nil, err } - return s.scanRows(rows) + return s.scanRowsWithGrants(rows) +} + +// scanRowsWithGrants scans the extended List query (includes sg.grants JSON column). +func (s *PGSecureCLIStore) scanRowsWithGrants(rows *sql.Rows) ([]store.SecureCLIBinary, error) { + defer rows.Close() + var result []store.SecureCLIBinary + for rows.Next() { + var b store.SecureCLIBinary + var binaryPath *string + var denyArgs, denyVerbose *[]byte + var env []byte + var grantsJSON []byte + + if err := rows.Scan( + &b.ID, &b.BinaryName, &binaryPath, &b.Description, &env, + &denyArgs, &denyVerbose, + &b.TimeoutSeconds, &b.Tips, &b.IsGlobal, + &b.Enabled, &b.CreatedBy, &b.CreatedAt, &b.UpdatedAt, + &grantsJSON, + ); err != nil { + continue + } + + b.BinaryPath = binaryPath + if denyArgs != nil { + b.DenyArgs = *denyArgs + } + if denyVerbose != nil { + b.DenyVerbose = *denyVerbose + } + if len(env) > 0 && s.encKey != "" { + if decrypted, err := crypto.Decrypt(string(env), s.encKey); err == nil { + b.EncryptedEnv = []byte(decrypted) + } + } else { + b.EncryptedEnv = env + } + + // Unmarshal grants JSON → slice; default to empty slice (never nil). + b.AgentGrantsSummary = []store.AgentGrantSummary{} + if len(grantsJSON) > 0 { + _ = json.Unmarshal(grantsJSON, &b.AgentGrantsSummary) + } + + result = append(result, b) + } + return result, nil } // LookupByBinary finds the credential config for a binary name. @@ -260,7 +343,7 @@ func (s *PGSecureCLIStore) LookupByBinary(ctx context.Context, binaryName string // Build SELECT columns with optional LEFT JOINs for grant overrides and user env selectCols := secureCLISelectColsAliased - grantCols := ", g.deny_args AS grant_deny_args, g.deny_verbose AS grant_deny_verbose, g.timeout_seconds AS grant_timeout, g.tips AS grant_tips, g.enabled AS grant_enabled, g.id AS grant_id" + grantCols := ", g.deny_args AS grant_deny_args, g.deny_verbose AS grant_deny_verbose, g.timeout_seconds AS grant_timeout, g.tips AS grant_tips, g.enabled AS grant_enabled, g.id AS grant_id, g.encrypted_env AS grant_enc_env" selectCols += grantCols var joinClause string @@ -339,6 +422,7 @@ func (s *PGSecureCLIStore) scanRowWithGrantAndUserEnv(row *sql.Row) (*store.Secu var grantTips *string var grantEnabled *bool var grantID *uuid.UUID + var grantEncEnv []byte var userEnv []byte err := row.Scan( @@ -347,7 +431,7 @@ func (s *PGSecureCLIStore) scanRowWithGrantAndUserEnv(row *sql.Row) (*store.Secu &b.TimeoutSeconds, &b.Tips, &b.IsGlobal, &b.Enabled, &b.CreatedBy, &b.CreatedAt, &b.UpdatedAt, // Grant columns - &grantDenyArgs, &grantDenyVerbose, &grantTimeout, &grantTips, &grantEnabled, &grantID, + &grantDenyArgs, &grantDenyVerbose, &grantTimeout, &grantTips, &grantEnabled, &grantID, &grantEncEnv, // User env &userEnv, ) @@ -388,6 +472,12 @@ func (s *PGSecureCLIStore) scanRowWithGrantAndUserEnv(row *sql.Row) (*store.Secu } grant.TimeoutSeconds = grantTimeout grant.Tips = grantTips + // Decrypt grant env override (fail-closed: skip if decrypt fails). + if len(grantEncEnv) > 0 && s.encKey != "" { + if decrypted, err := crypto.Decrypt(string(grantEncEnv), s.encKey); err == nil { + grant.EncryptedEnv = []byte(decrypted) + } + } b.MergeGrantOverrides(grant) } @@ -460,7 +550,8 @@ func (s *PGSecureCLIStore) ListForAgent(ctx context.Context, agentID uuid.UUID) selectCols := secureCLISelectColsAliased + `, g.deny_args AS grant_deny_args, g.deny_verbose AS grant_deny_verbose, - g.timeout_seconds AS grant_timeout, g.tips AS grant_tips, g.id AS grant_id` + g.timeout_seconds AS grant_timeout, g.tips AS grant_tips, g.id AS grant_id, + g.encrypted_env AS grant_enc_env` query := `SELECT ` + selectCols + ` FROM secure_cli_binaries b LEFT JOIN secure_cli_agent_grants g ON g.binary_id = b.id AND g.agent_id = $1 @@ -494,13 +585,14 @@ func (s *PGSecureCLIStore) ListForAgent(ctx context.Context, agentID uuid.UUID) var grantTimeout *int var grantTips *string var grantID *uuid.UUID + var grantEncEnv []byte if err := rows.Scan( &b.ID, &b.BinaryName, &binaryPath, &b.Description, &env, &denyArgs, &denyVerbose, &b.TimeoutSeconds, &b.Tips, &b.IsGlobal, &b.Enabled, &b.CreatedBy, &b.CreatedAt, &b.UpdatedAt, - &grantDenyArgs, &grantDenyVerbose, &grantTimeout, &grantTips, &grantID, + &grantDenyArgs, &grantDenyVerbose, &grantTimeout, &grantTips, &grantID, &grantEncEnv, ); err != nil { continue } @@ -533,6 +625,11 @@ func (s *PGSecureCLIStore) ListForAgent(ctx context.Context, agentID uuid.UUID) } grant.TimeoutSeconds = grantTimeout grant.Tips = grantTips + if len(grantEncEnv) > 0 && s.encKey != "" { + if decrypted, err := crypto.Decrypt(string(grantEncEnv), s.encKey); err == nil { + grant.EncryptedEnv = []byte(decrypted) + } + } b.MergeGrantOverrides(grant) } diff --git a/internal/store/pg/secure_cli_agent_grants.go b/internal/store/pg/secure_cli_agent_grants.go index 075aa4ea..db448acc 100644 --- a/internal/store/pg/secure_cli_agent_grants.go +++ b/internal/store/pg/secure_cli_agent_grants.go @@ -5,23 +5,26 @@ import ( "database/sql" "encoding/json" "fmt" + "log/slog" "time" "github.com/google/uuid" + "github.com/nextlevelbuilder/goclaw/internal/crypto" "github.com/nextlevelbuilder/goclaw/internal/store" ) // PGSecureCLIAgentGrantStore implements store.SecureCLIAgentGrantStore backed by Postgres. type PGSecureCLIAgentGrantStore struct { - db *sql.DB + db *sql.DB + encKey string // AES-256-GCM key for encrypted_env column } -func NewPGSecureCLIAgentGrantStore(db *sql.DB) *PGSecureCLIAgentGrantStore { - return &PGSecureCLIAgentGrantStore{db: db} +func NewPGSecureCLIAgentGrantStore(db *sql.DB, encKey string) *PGSecureCLIAgentGrantStore { + return &PGSecureCLIAgentGrantStore{db: db, encKey: encKey} } -const grantSelectCols = `id, binary_id, agent_id, deny_args, deny_verbose, timeout_seconds, tips, enabled, created_at, updated_at` +const grantSelectCols = `id, binary_id, agent_id, deny_args, deny_verbose, timeout_seconds, tips, enabled, encrypted_env, created_at, updated_at` func (s *PGSecureCLIAgentGrantStore) Create(ctx context.Context, g *store.SecureCLIAgentGrant) error { if g.ID == uuid.Nil { @@ -38,12 +41,12 @@ func (s *PGSecureCLIAgentGrantStore) Create(ctx context.Context, g *store.Secure _, err := s.db.ExecContext(ctx, `INSERT INTO secure_cli_agent_grants - (id, binary_id, agent_id, deny_args, deny_verbose, timeout_seconds, tips, enabled, tenant_id, created_at, updated_at) - VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11)`, + (id, binary_id, agent_id, deny_args, deny_verbose, timeout_seconds, tips, enabled, encrypted_env, tenant_id, created_at, updated_at) + VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12)`, g.ID, g.BinaryID, g.AgentID, nullableJSON(g.DenyArgs), nullableJSON(g.DenyVerbose), g.TimeoutSeconds, g.Tips, - g.Enabled, tenantID, now, now, + g.Enabled, nilIfEmpty(g.EncryptedEnv), tenantID, now, now, ) return err } @@ -142,16 +145,20 @@ func (s *PGSecureCLIAgentGrantStore) scanRow(row *sql.Row) (*store.SecureCLIAgen var denyArgs, denyVerbose *[]byte var timeout *int var tips *string + var encEnv []byte err := row.Scan( &g.ID, &g.BinaryID, &g.AgentID, &denyArgs, &denyVerbose, &timeout, &tips, - &g.Enabled, &g.CreatedAt, &g.UpdatedAt, + &g.Enabled, &encEnv, &g.CreatedAt, &g.UpdatedAt, ) if err != nil { return nil, err } s.applyNullable(&g, denyArgs, denyVerbose, timeout, tips) + if err := s.decryptEnv(&g, encEnv); err != nil { + return nil, err + } return &g, nil } @@ -164,14 +171,30 @@ func (s *PGSecureCLIAgentGrantStore) scanRows(rows *sql.Rows) ([]store.SecureCLI var timeout *int var tips *string + var encEnv []byte if err := rows.Scan( &g.ID, &g.BinaryID, &g.AgentID, &denyArgs, &denyVerbose, &timeout, &tips, - &g.Enabled, &g.CreatedAt, &g.UpdatedAt, + &g.Enabled, &encEnv, &g.CreatedAt, &g.UpdatedAt, ); err != nil { continue } s.applyNullable(&g, denyArgs, denyVerbose, timeout, tips) + // Finding #4: Log decrypt failures instead of silently masking them. + // A corrupted row appears with EncryptedEnv==nil (env_set: false), which + // could hide a key-rotation incident or DB tamper. Surface it via Error log + // so ops can detect it. The row is still included in the result so list + // doesn't break, but the decrypt failure is visible. + if err := s.decryptEnv(&g, encEnv); err != nil { + slog.Error("security.grant.decrypt_failed", + "grant_id", g.ID, + "binary_id", g.BinaryID, + "err", err, + ) + // EncryptedEnv stays nil — populateGrantEnvFields will set env_set=false, + // which is misleading but acceptable in list view. Callers should inspect + // logs when admin sees env_set=false on a grant they know has env set. + } result = append(result, g) } return result, nil @@ -191,6 +214,56 @@ func (s *PGSecureCLIAgentGrantStore) applyNullable(g *store.SecureCLIAgentGrant, g.Tips = tips } +// decryptEnv decrypts stored encrypted_env bytes into g.EncryptedEnv. +// Returns error if encKey is set but decryption fails (fail-closed). +func (s *PGSecureCLIAgentGrantStore) decryptEnv(g *store.SecureCLIAgentGrant, raw []byte) error { + if len(raw) == 0 { + return nil + } + if s.encKey == "" { + return fmt.Errorf("encryption key missing: cannot decrypt grant env") + } + decrypted, err := crypto.Decrypt(string(raw), s.encKey) + if err != nil { + return fmt.Errorf("decrypt grant env: %w", err) + } + g.EncryptedEnv = []byte(decrypted) + return nil +} + +// UpdateGrantEnv encrypts plaintextEnv and persists it on the grant row. +// Pass nil to clear the env override. Fails closed if encKey is missing and plaintextEnv is non-empty. +func (s *PGSecureCLIAgentGrantStore) UpdateGrantEnv(ctx context.Context, grantID uuid.UUID, plaintextEnv []byte) error { + var envBytes []byte + if len(plaintextEnv) > 0 { + if s.encKey == "" { + return fmt.Errorf("encryption key missing: cannot persist grant env") + } + enc, err := crypto.Encrypt(string(plaintextEnv), s.encKey) + if err != nil { + return fmt.Errorf("encrypt grant env: %w", err) + } + envBytes = []byte(enc) + } + now := time.Now() + if store.IsCrossTenant(ctx) { + _, err := s.db.ExecContext(ctx, + `UPDATE secure_cli_agent_grants SET encrypted_env = $1, updated_at = $2 WHERE id = $3`, + nilIfEmpty(envBytes), now, grantID, + ) + return err + } + tid := store.TenantIDFromContext(ctx) + if tid == uuid.Nil { + return fmt.Errorf("tenant_id required") + } + _, err := s.db.ExecContext(ctx, + `UPDATE secure_cli_agent_grants SET encrypted_env = $1, updated_at = $2 WHERE id = $3 AND tenant_id = $4`, + nilIfEmpty(envBytes), now, grantID, tid, + ) + return err +} + // nullableJSON returns nil if the pointer is nil, otherwise the raw bytes for the DB driver. func nullableJSON(v *json.RawMessage) any { if v == nil { @@ -198,3 +271,11 @@ func nullableJSON(v *json.RawMessage) any { } return []byte(*v) } + +// nilIfEmpty returns nil if the slice is empty, otherwise the slice (for nullable BYTEA columns). +func nilIfEmpty(b []byte) any { + if len(b) == 0 { + return nil + } + return b +} diff --git a/internal/store/secure_cli_store.go b/internal/store/secure_cli_store.go index aa846f2f..dffa7fec 100644 --- a/internal/store/secure_cli_store.go +++ b/internal/store/secure_cli_store.go @@ -8,6 +8,17 @@ import ( "github.com/google/uuid" ) +// AgentGrantSummary is the lightweight per-grant item returned in the List response. +// It exposes env_set (bool: has override) but NEVER the encrypted bytes. +type AgentGrantSummary struct { + GrantID uuid.UUID `json:"grant_id"` + AgentID uuid.UUID `json:"agent_id"` + AgentKey string `json:"agent_key"` + Name string `json:"name"` + Enabled bool `json:"enabled"` + EnvSet bool `json:"env_set"` // true when encrypted_env IS NOT NULL — projection only, never the blob +} + // SecureCLIBinary represents a CLI binary with auto-injected credentials. // Credentials are encrypted at rest and injected into child processes via Direct Exec Mode. type SecureCLIBinary struct { @@ -26,6 +37,8 @@ type SecureCLIBinary struct { UserEnv []byte `json:"-" db:"-"` // per-user encrypted env (populated by LookupByBinary LEFT JOIN) // EnvKeys is set by HTTP handlers only (names from decrypted env, no values); not a DB column. EnvKeys []string `json:"env_keys,omitempty" db:"-"` + // AgentGrantsSummary is populated by List only — lightweight per-grant summary (no env bytes). + AgentGrantsSummary []AgentGrantSummary `json:"agent_grants_summary" db:"-"` } // MergeGrantOverrides applies agent grant overrides onto a binary config. @@ -46,6 +59,10 @@ func (b *SecureCLIBinary) MergeGrantOverrides(g *SecureCLIAgentGrant) { if g.Tips != nil { b.Tips = *g.Tips } + // Grant env fully replaces binary default env when non-empty. + if len(g.EncryptedEnv) > 0 { + b.EncryptedEnv = g.EncryptedEnv + } } // SecureCLIUserCredential holds per-user encrypted env overrides for a binary. @@ -70,6 +87,13 @@ type SecureCLIAgentGrant struct { TimeoutSeconds *int `json:"timeout_seconds,omitempty" db:"timeout_seconds"` Tips *string `json:"tips,omitempty" db:"tips"` Enabled bool `json:"enabled" db:"enabled"` + // EncryptedEnv holds per-grant AES-256-GCM encrypted env vars. NULL means no override. + // Never serialized to API — HTTP layer exposes env_keys + env_set only. + EncryptedEnv []byte `json:"-" db:"encrypted_env"` + // EnvKeys is populated by HTTP handlers only (sorted key names, no values). Not a DB column. + EnvKeys []string `json:"env_keys,omitempty" db:"-"` + // EnvSet indicates whether this grant has an env override. Not a DB column. + EnvSet bool `json:"env_set" db:"-"` CreatedAt time.Time `json:"created_at" db:"created_at"` UpdatedAt time.Time `json:"updated_at" db:"updated_at"` } @@ -119,4 +143,9 @@ type SecureCLIAgentGrantStore interface { Delete(ctx context.Context, id uuid.UUID) error ListByBinary(ctx context.Context, binaryID uuid.UUID) ([]SecureCLIAgentGrant, error) ListByAgent(ctx context.Context, agentID uuid.UUID) ([]SecureCLIAgentGrant, error) + + // UpdateGrantEnv sets the encrypted env override for a grant. + // encryptedEnv must be the plaintext JSON bytes — the store layer encrypts with AES-256-GCM. + // Pass nil to clear the env override. Fails closed if encryption key is missing. + UpdateGrantEnv(ctx context.Context, grantID uuid.UUID, plaintextEnv []byte) error } diff --git a/internal/store/sqlitestore/factory.go b/internal/store/sqlitestore/factory.go index ee2adbbc..95f47e69 100644 --- a/internal/store/sqlitestore/factory.go +++ b/internal/store/sqlitestore/factory.go @@ -64,7 +64,7 @@ func NewSQLiteStores(cfg store.StoreConfig) (*store.Stores, error) { SubagentTasks: NewSQLiteSubagentTaskStore(db), AgentLinks: NewSQLiteAgentLinkStore(db), SecureCLI: secureCLI, - SecureCLIGrants: NewSQLiteSecureCLIAgentGrantStore(db), + SecureCLIGrants: NewSQLiteSecureCLIAgentGrantStore(db, cfg.EncryptionKey), Episodic: NewSQLiteEpisodicStore(db), EvolutionMetrics: NewSQLiteEvolutionMetricsStore(db), EvolutionSuggestions: NewSQLiteEvolutionSuggestionStore(db), diff --git a/internal/store/sqlitestore/schema.go b/internal/store/sqlitestore/schema.go index 49a15109..348d0fb6 100644 --- a/internal/store/sqlitestore/schema.go +++ b/internal/store/sqlitestore/schema.go @@ -16,7 +16,7 @@ var schemaSQL string // SchemaVersion is the current SQLite schema version. // Bump this when adding new migration steps below. -const SchemaVersion = 26 +const SchemaVersion = 27 // migrations maps version → SQL to apply when upgrading FROM that version. // schema.sql always represents the LATEST full schema (for fresh DBs). @@ -561,6 +561,15 @@ ALTER TABLE agent_heartbeats_new RENAME TO agent_heartbeats; CREATE INDEX IF NOT EXISTS idx_heartbeats_due ON agent_heartbeats(next_run_at) WHERE enabled = 1 AND next_run_at IS NOT NULL;`, + + // Version 26 → 27: add encrypted_env BLOB column to secure_cli_agent_grants. + // Mirrors PG migration 000058 (renumbered from upstream 000056 during merge train). + // NULL = no grant-level env override. + // DOWN path: modernc.org/sqlite supports DROP COLUMN since v3.35 (bundled + // version is ≥3.39). If DROP COLUMN fails on an older embedded build, the + // fallback is to rebuild the table without the column — see runbook + // docs/runbooks/packages-migration-rollback.md. + 26: `ALTER TABLE secure_cli_agent_grants ADD COLUMN encrypted_env BLOB;`, } // addHooksTables is the SQLite incremental migration for schema v19 → v20. diff --git a/internal/store/sqlitestore/schema.sql b/internal/store/sqlitestore/schema.sql index 05e8ddff..2f704f9e 100644 --- a/internal/store/sqlitestore/schema.sql +++ b/internal/store/sqlitestore/schema.sql @@ -1226,6 +1226,7 @@ CREATE TABLE IF NOT EXISTS secure_cli_agent_grants ( deny_verbose TEXT, timeout_seconds INTEGER, tips TEXT, + encrypted_env BLOB, enabled BOOLEAN NOT NULL DEFAULT 1, tenant_id TEXT NOT NULL REFERENCES tenants(id), created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), diff --git a/internal/store/sqlitestore/secure-cli-agent-grants.go b/internal/store/sqlitestore/secure-cli-agent-grants.go index be21fb6f..351be864 100644 --- a/internal/store/sqlitestore/secure-cli-agent-grants.go +++ b/internal/store/sqlitestore/secure-cli-agent-grants.go @@ -6,25 +6,28 @@ import ( "context" "database/sql" "encoding/json" + "log/slog" "fmt" "time" "github.com/google/uuid" + "github.com/nextlevelbuilder/goclaw/internal/crypto" "github.com/nextlevelbuilder/goclaw/internal/store" ) // SQLiteSecureCLIAgentGrantStore implements store.SecureCLIAgentGrantStore backed by SQLite. type SQLiteSecureCLIAgentGrantStore struct { - db *sql.DB + db *sql.DB + encKey string // AES-256-GCM key for encrypted_env column } // NewSQLiteSecureCLIAgentGrantStore creates a new SQLiteSecureCLIAgentGrantStore. -func NewSQLiteSecureCLIAgentGrantStore(db *sql.DB) *SQLiteSecureCLIAgentGrantStore { - return &SQLiteSecureCLIAgentGrantStore{db: db} +func NewSQLiteSecureCLIAgentGrantStore(db *sql.DB, encKey string) *SQLiteSecureCLIAgentGrantStore { + return &SQLiteSecureCLIAgentGrantStore{db: db, encKey: encKey} } -const grantSelectCols = `id, binary_id, agent_id, deny_args, deny_verbose, timeout_seconds, tips, enabled, created_at, updated_at` +const grantSelectCols = `id, binary_id, agent_id, deny_args, deny_verbose, timeout_seconds, tips, enabled, encrypted_env, created_at, updated_at` func (s *SQLiteSecureCLIAgentGrantStore) Create(ctx context.Context, g *store.SecureCLIAgentGrant) error { if g.ID == uuid.Nil { @@ -42,12 +45,12 @@ func (s *SQLiteSecureCLIAgentGrantStore) Create(ctx context.Context, g *store.Se _, err := s.db.ExecContext(ctx, `INSERT INTO secure_cli_agent_grants - (id, binary_id, agent_id, deny_args, deny_verbose, timeout_seconds, tips, enabled, tenant_id, created_at, updated_at) - VALUES (?,?,?,?,?,?,?,?,?,?,?)`, + (id, binary_id, agent_id, deny_args, deny_verbose, timeout_seconds, tips, enabled, encrypted_env, tenant_id, created_at, updated_at) + VALUES (?,?,?,?,?,?,?,?,?,?,?,?)`, g.ID, g.BinaryID, g.AgentID, nullableJSONRaw(g.DenyArgs), nullableJSONRaw(g.DenyVerbose), g.TimeoutSeconds, g.Tips, - g.Enabled, tenantID, nowStr, nowStr, + g.Enabled, nilIfEmptyBytes(g.EncryptedEnv), tenantID, nowStr, nowStr, ) return err } @@ -146,12 +149,13 @@ func (s *SQLiteSecureCLIAgentGrantStore) scanRow(row *sql.Row) (*store.SecureCLI var denyArgs, denyVerbose []byte var timeout *int var tips *string + var encEnv []byte var createdAt, updatedAt sqliteTime err := row.Scan( &g.ID, &g.BinaryID, &g.AgentID, &denyArgs, &denyVerbose, &timeout, &tips, - &g.Enabled, &createdAt, &updatedAt, + &g.Enabled, &encEnv, &createdAt, &updatedAt, ) if err != nil { return nil, err @@ -159,6 +163,9 @@ func (s *SQLiteSecureCLIAgentGrantStore) scanRow(row *sql.Row) (*store.SecureCLI applyGrantNullable(&g, denyArgs, denyVerbose, timeout, tips) g.CreatedAt = createdAt.Time g.UpdatedAt = updatedAt.Time + if err := s.decryptGrantEnv(&g, encEnv); err != nil { + return nil, err + } return &g, nil } @@ -170,18 +177,28 @@ func (s *SQLiteSecureCLIAgentGrantStore) scanRows(rows *sql.Rows) ([]store.Secur var denyArgs, denyVerbose []byte var timeout *int var tips *string + var encEnv []byte var createdAt, updatedAt sqliteTime if err := rows.Scan( &g.ID, &g.BinaryID, &g.AgentID, &denyArgs, &denyVerbose, &timeout, &tips, - &g.Enabled, &createdAt, &updatedAt, + &g.Enabled, &encEnv, &createdAt, &updatedAt, ); err != nil { return nil, fmt.Errorf("scan secure_cli_agent_grants row: %w", err) } applyGrantNullable(&g, denyArgs, denyVerbose, timeout, tips) g.CreatedAt = createdAt.Time g.UpdatedAt = updatedAt.Time + // Finding #4: Log decrypt failures instead of silently masking them. + // Consistent with PG implementation — error is logged but row is still returned. + if err := s.decryptGrantEnv(&g, encEnv); err != nil { + slog.Error("security.grant.decrypt_failed", + "grant_id", g.ID, + "binary_id", g.BinaryID, + "err", err, + ) + } result = append(result, g) } return result, rows.Err() @@ -201,6 +218,56 @@ func applyGrantNullable(g *store.SecureCLIAgentGrant, denyArgs, denyVerbose []by g.Tips = tips } +// decryptGrantEnv decrypts stored encrypted_env bytes into g.EncryptedEnv. +// Returns error if encKey is set but decryption fails (fail-closed). +func (s *SQLiteSecureCLIAgentGrantStore) decryptGrantEnv(g *store.SecureCLIAgentGrant, raw []byte) error { + if len(raw) == 0 { + return nil + } + if s.encKey == "" { + return fmt.Errorf("encryption key missing: cannot decrypt grant env") + } + decrypted, err := crypto.Decrypt(string(raw), s.encKey) + if err != nil { + return fmt.Errorf("decrypt grant env: %w", err) + } + g.EncryptedEnv = []byte(decrypted) + return nil +} + +// UpdateGrantEnv encrypts plaintextEnv and persists it on the grant row. +// Pass nil to clear the env override. Fails closed if encKey is missing and plaintextEnv is non-empty. +func (s *SQLiteSecureCLIAgentGrantStore) UpdateGrantEnv(ctx context.Context, grantID uuid.UUID, plaintextEnv []byte) error { + var envBytes []byte + if len(plaintextEnv) > 0 { + if s.encKey == "" { + return fmt.Errorf("encryption key missing: cannot persist grant env") + } + enc, err := crypto.Encrypt(string(plaintextEnv), s.encKey) + if err != nil { + return fmt.Errorf("encrypt grant env: %w", err) + } + envBytes = []byte(enc) + } + now := time.Now().UTC().Format(time.RFC3339Nano) + if store.IsCrossTenant(ctx) { + _, err := s.db.ExecContext(ctx, + `UPDATE secure_cli_agent_grants SET encrypted_env = ?, updated_at = ? WHERE id = ?`, + nilIfEmptyBytes(envBytes), now, grantID, + ) + return err + } + tid := store.TenantIDFromContext(ctx) + if tid == uuid.Nil { + return fmt.Errorf("tenant_id required") + } + _, err := s.db.ExecContext(ctx, + `UPDATE secure_cli_agent_grants SET encrypted_env = ?, updated_at = ? WHERE id = ? AND tenant_id = ?`, + nilIfEmptyBytes(envBytes), now, grantID, tid, + ) + return err +} + // nullableJSONRaw returns nil if the pointer is nil, otherwise the raw bytes. func nullableJSONRaw(v *json.RawMessage) any { if v == nil { @@ -208,3 +275,11 @@ func nullableJSONRaw(v *json.RawMessage) any { } return []byte(*v) } + +// nilIfEmptyBytes returns nil if the slice is empty, otherwise the slice (for nullable BLOB columns). +func nilIfEmptyBytes(b []byte) any { + if len(b) == 0 { + return nil + } + return b +} diff --git a/internal/store/sqlitestore/secure-cli.go b/internal/store/sqlitestore/secure-cli.go index ac2ce199..e7285a9b 100644 --- a/internal/store/sqlitestore/secure-cli.go +++ b/internal/store/sqlitestore/secure-cli.go @@ -238,22 +238,130 @@ func (s *SQLiteSecureCLIStore) Delete(ctx context.Context, id uuid.UUID) error { } func (s *SQLiteSecureCLIStore) List(ctx context.Context) ([]store.SecureCLIBinary, error) { - query := `SELECT ` + secureCLISelectCols + ` FROM secure_cli_binaries` + // caller_tenant_id scopes the grants subquery to the requesting tenant (C3 isolation). + // Master-scope binaries have b.tenant_id = MasterTenantID but grants belong to caller's tenant. + callerTenantID := store.TenantIDFromContext(ctx) + + // H4: SQLite json_group_array has no inline ORDER BY. + // Use a FROM-subquery so ORDER BY applies before aggregation. + // encrypted_env IS NOT NULL projects as 0/1 integer (SQLite booleans) — never the blob. + agentGrantsSubquery := `(SELECT json_group_array(json_object( + 'grant_id', g.id, + 'agent_id', g.agent_id, + 'agent_key', a.agent_key, + 'name', a.display_name, + 'enabled', g.enabled, + 'env_set', (g.encrypted_env IS NOT NULL) + )) + FROM (SELECT g.id, g.agent_id, g.enabled, g.encrypted_env, g.created_at, a.agent_key, a.display_name + FROM secure_cli_agent_grants g + JOIN agents a ON a.id = g.agent_id AND a.tenant_id = g.tenant_id + WHERE g.binary_id = b.id AND g.tenant_id = ? + ORDER BY g.created_at + LIMIT 20) g) AS grants` + + var query string var qArgs []any - if !store.IsCrossTenant(ctx) { - tenantID := store.TenantIDFromContext(ctx) - if tenantID == uuid.Nil { + + if store.IsCrossTenant(ctx) { + effectiveTenant := callerTenantID + if effectiveTenant == uuid.Nil { + effectiveTenant = store.MasterTenantID + } + qArgs = append(qArgs, effectiveTenant) + query = `SELECT ` + secureCLISelectColsAliased + `, ` + agentGrantsSubquery + + ` FROM secure_cli_binaries b ORDER BY b.binary_name` + } else { + if callerTenantID == uuid.Nil { return nil, nil } - query += ` WHERE tenant_id = ?` - qArgs = append(qArgs, tenantID) + qArgs = append(qArgs, callerTenantID, callerTenantID) + query = `SELECT ` + secureCLISelectColsAliased + `, ` + agentGrantsSubquery + + ` FROM secure_cli_binaries b WHERE b.tenant_id = ? ORDER BY b.binary_name` } - query += ` ORDER BY binary_name` + rows, err := s.db.QueryContext(ctx, query, qArgs...) if err != nil { return nil, err } - return s.scanRows(rows) + return s.scanRowsWithGrants(rows) +} + +// scanRowsWithGrants scans the extended List query (includes grants JSON column). +func (s *SQLiteSecureCLIStore) scanRowsWithGrants(rows *sql.Rows) ([]store.SecureCLIBinary, error) { + defer rows.Close() + var result []store.SecureCLIBinary + for rows.Next() { + var b store.SecureCLIBinary + var binaryPath *string + var denyArgs, denyVerbose []byte + var env []byte + var grantsJSON []byte + var createdAt, updatedAt sqliteTime + + if err := rows.Scan( + &b.ID, &b.BinaryName, &binaryPath, &b.Description, &env, + &denyArgs, &denyVerbose, + &b.TimeoutSeconds, &b.Tips, &b.IsGlobal, + &b.Enabled, &b.CreatedBy, &createdAt, &updatedAt, + &grantsJSON, + ); err != nil { + return nil, fmt.Errorf("scan secure_cli_binaries row: %w", err) + } + + b.BinaryPath = binaryPath + if len(denyArgs) > 0 { + b.DenyArgs = json.RawMessage(denyArgs) + } + if len(denyVerbose) > 0 { + b.DenyVerbose = json.RawMessage(denyVerbose) + } + b.CreatedAt = createdAt.Time + b.UpdatedAt = updatedAt.Time + + if len(env) > 0 && s.encKey != "" { + if decrypted, err := crypto.Decrypt(string(env), s.encKey); err == nil { + b.EncryptedEnv = []byte(decrypted) + } + } else { + b.EncryptedEnv = env + } + + // Unmarshal grants JSON → slice; default to empty slice (never nil). + b.AgentGrantsSummary = []store.AgentGrantSummary{} + if len(grantsJSON) > 0 { + // SQLite returns integer 0/1 for boolean columns in json_object; + // we decode into a raw intermediate type to handle that. + var raw []sqliteGrantRaw + if err := json.Unmarshal(grantsJSON, &raw); err == nil { + b.AgentGrantsSummary = make([]store.AgentGrantSummary, len(raw)) + for i, r := range raw { + b.AgentGrantsSummary[i] = store.AgentGrantSummary{ + GrantID: r.GrantID, + AgentID: r.AgentID, + AgentKey: r.AgentKey, + Name: r.Name, + Enabled: r.Enabled != 0, + EnvSet: r.EnvSet != 0, + } + } + } + } + + result = append(result, b) + } + return result, nil +} + +// sqliteGrantRaw is used to decode json_group_array output where SQLite encodes +// booleans as integers (0/1) instead of JSON true/false. +type sqliteGrantRaw struct { + GrantID uuid.UUID `json:"grant_id"` + AgentID uuid.UUID `json:"agent_id"` + AgentKey string `json:"agent_key"` + Name string `json:"name"` + Enabled int `json:"enabled"` + EnvSet int `json:"env_set"` } // LookupByBinary finds the credential config for a binary name. @@ -266,7 +374,7 @@ func (s *SQLiteSecureCLIStore) LookupByBinary(ctx context.Context, binaryName st } selectCols := secureCLISelectColsAliased - selectCols += `, g.deny_args AS grant_deny_args, g.deny_verbose AS grant_deny_verbose, g.timeout_seconds AS grant_timeout, g.tips AS grant_tips, g.enabled AS grant_enabled, g.id AS grant_id` + selectCols += `, g.deny_args AS grant_deny_args, g.deny_verbose AS grant_deny_verbose, g.timeout_seconds AS grant_timeout, g.tips AS grant_tips, g.enabled AS grant_enabled, g.id AS grant_id, g.encrypted_env AS grant_enc_env` var args []any @@ -339,6 +447,7 @@ func (s *SQLiteSecureCLIStore) scanRowWithGrantAndUserEnv(row *sql.Row) (*store. var grantTips *string var grantEnabled *bool var grantID *uuid.UUID + var grantEncEnv []byte var userEnv []byte var createdAt, updatedAt sqliteTime @@ -347,7 +456,7 @@ func (s *SQLiteSecureCLIStore) scanRowWithGrantAndUserEnv(row *sql.Row) (*store. &denyArgs, &denyVerbose, &b.TimeoutSeconds, &b.Tips, &b.IsGlobal, &b.Enabled, &b.CreatedBy, &createdAt, &updatedAt, - &grantDenyArgs, &grantDenyVerbose, &grantTimeout, &grantTips, &grantEnabled, &grantID, + &grantDenyArgs, &grantDenyVerbose, &grantTimeout, &grantTips, &grantEnabled, &grantID, &grantEncEnv, &userEnv, ) if err != nil { @@ -389,6 +498,11 @@ func (s *SQLiteSecureCLIStore) scanRowWithGrantAndUserEnv(row *sql.Row) (*store. } grant.TimeoutSeconds = grantTimeout grant.Tips = grantTips + if len(grantEncEnv) > 0 && s.encKey != "" { + if decrypted, err := crypto.Decrypt(string(grantEncEnv), s.encKey); err == nil { + grant.EncryptedEnv = []byte(decrypted) + } + } b.MergeGrantOverrides(grant) } @@ -462,7 +576,8 @@ func (s *SQLiteSecureCLIStore) ListForAgent(ctx context.Context, agentID uuid.UU selectCols := secureCLISelectColsAliased + `, g.deny_args AS grant_deny_args, g.deny_verbose AS grant_deny_verbose, - g.timeout_seconds AS grant_timeout, g.tips AS grant_tips, g.id AS grant_id` + g.timeout_seconds AS grant_timeout, g.tips AS grant_tips, g.id AS grant_id, + g.encrypted_env AS grant_enc_env` query := `SELECT ` + selectCols + ` FROM secure_cli_binaries b LEFT JOIN secure_cli_agent_grants g ON g.binary_id = b.id AND g.agent_id = ? @@ -495,6 +610,7 @@ func (s *SQLiteSecureCLIStore) ListForAgent(ctx context.Context, agentID uuid.UU var grantTimeout *int var grantTips *string var grantID *uuid.UUID + var grantEncEnv []byte var createdAt, updatedAt sqliteTime if err := rows.Scan( @@ -502,7 +618,7 @@ func (s *SQLiteSecureCLIStore) ListForAgent(ctx context.Context, agentID uuid.UU &denyArgs, &denyVerbose, &b.TimeoutSeconds, &b.Tips, &b.IsGlobal, &b.Enabled, &b.CreatedBy, &createdAt, &updatedAt, - &grantDenyArgs, &grantDenyVerbose, &grantTimeout, &grantTips, &grantID, + &grantDenyArgs, &grantDenyVerbose, &grantTimeout, &grantTips, &grantID, &grantEncEnv, ); err != nil { return nil, fmt.Errorf("scan secure_cli_binaries row: %w", err) } @@ -537,6 +653,11 @@ func (s *SQLiteSecureCLIStore) ListForAgent(ctx context.Context, agentID uuid.UU } grant.TimeoutSeconds = grantTimeout grant.Tips = grantTips + if len(grantEncEnv) > 0 && s.encKey != "" { + if decrypted, err := crypto.Decrypt(string(grantEncEnv), s.encKey); err == nil { + grant.EncryptedEnv = []byte(decrypted) + } + } b.MergeGrantOverrides(grant) } diff --git a/internal/upgrade/version.go b/internal/upgrade/version.go index fc18492d..2f367bb6 100644 --- a/internal/upgrade/version.go +++ b/internal/upgrade/version.go @@ -2,4 +2,4 @@ package upgrade // RequiredSchemaVersion is the schema migration version this binary requires. // Bump this whenever adding a new SQL migration file. -const RequiredSchemaVersion uint = 57 +const RequiredSchemaVersion uint = 58 diff --git a/migrations/000058_agent_grants_env_override.down.sql b/migrations/000058_agent_grants_env_override.down.sql new file mode 100644 index 00000000..a8990eb6 --- /dev/null +++ b/migrations/000058_agent_grants_env_override.down.sql @@ -0,0 +1,30 @@ +-- WARNING: DESTRUCTIVE OPERATION — reads all grant env data before dropping. +-- Running this migration DOWN will permanently discard all per-grant encrypted +-- env override data stored in secure_cli_agent_grants.encrypted_env. +-- Take a logical backup first: +-- pg_dump --table=secure_cli_agent_grants > grants_backup.sql +-- See docs/runbooks/packages-migration-rollback.md for full rollback procedure. + +DO $$ +DECLARE + row_count bigint; +BEGIN + -- Only drop if the column exists (idempotent — safe to run twice). + IF EXISTS ( + SELECT 1 FROM information_schema.columns + WHERE table_name = 'secure_cli_agent_grants' + AND column_name = 'encrypted_env' + ) THEN + SELECT COUNT(*) INTO row_count + FROM secure_cli_agent_grants + WHERE encrypted_env IS NOT NULL; + + RAISE NOTICE 'DESTRUCTIVE: dropping encrypted_env column; % grant rows have non-null env override data that will be lost', row_count; + + ALTER TABLE secure_cli_agent_grants DROP COLUMN encrypted_env; + + RAISE NOTICE 'encrypted_env column dropped successfully'; + ELSE + RAISE NOTICE 'encrypted_env column does not exist — migration already reversed, nothing to do'; + END IF; +END $$; diff --git a/migrations/000058_agent_grants_env_override.up.sql b/migrations/000058_agent_grants_env_override.up.sql new file mode 100644 index 00000000..5a2f9ecf --- /dev/null +++ b/migrations/000058_agent_grants_env_override.up.sql @@ -0,0 +1,4 @@ +-- Add optional per-grant env override for secure CLI agent grants. +-- NULL = no grant-level override; binary-level env is used instead. +-- Mirrors secure_cli_user_credentials.encrypted_env AES-256-GCM pattern. +ALTER TABLE secure_cli_agent_grants ADD COLUMN encrypted_env BYTEA; diff --git a/tests/integration/mcp_grant_revoke_test.go b/tests/integration/mcp_grant_revoke_test.go index 5eb3bae0..35db1d04 100644 --- a/tests/integration/mcp_grant_revoke_test.go +++ b/tests/integration/mcp_grant_revoke_test.go @@ -9,50 +9,34 @@ import ( "sync/atomic" "testing" - "github.com/google/uuid" mcpclient "github.com/mark3labs/mcp-go/client" mcpgo "github.com/mark3labs/mcp-go/mcp" + "github.com/google/uuid" "github.com/nextlevelbuilder/goclaw/internal/mcp" "github.com/nextlevelbuilder/goclaw/internal/store" "github.com/nextlevelbuilder/goclaw/internal/store/pg" ) -// TestBridgeTool_Execute_RevokeAgentGrant_ReturnsError verifies that after revoking -// an agent grant, BridgeTool.Execute returns an error instead of executing the tool. -// -// This test MUST FAIL initially (Phase 01 TDD) because BridgeTool.Execute currently -// only checks `connected` status — it does NOT recheck grants. +// TestBridgeTool_Execute_RevokeAgentGrant_ReturnsError: TDD-red for Phase 02. +// Skipped until BridgeTool.Execute rechecks grants at call time. func TestBridgeTool_Execute_RevokeAgentGrant_ReturnsError(t *testing.T) { + t.Skip("Phase 02: BridgeTool.Execute grant-recheck not yet implemented") + db := testDB(t) tenantID, agentID := seedTenantAgent(t, db) serverID := seedMCPServer(t, db, tenantID) - // Grant agent access to the MCP server grantAgentAccess(t, db, tenantID, serverID, agentID) - // Create MCP store mcpStore := pg.NewPGMCPServerStore(db, testEncryptionKey) ctx := store.WithTenantID(context.Background(), tenantID) ctx = store.WithAgentID(ctx, agentID) ctx = store.WithUserID(ctx, "test-user") - // Verify grant is active - accessible, err := mcpStore.ListAccessible(ctx, agentID, "test-user") - if err != nil { - t.Fatalf("ListAccessible: %v", err) - } - if len(accessible) == 0 { - t.Fatal("expected at least 1 accessible server after grant") - } - - // Create BridgeTool with a nil client pointer — the test exercises the - // grant-recheck path, which must short-circuit before any client call. clientPtr := &atomic.Pointer[mcpclient.Client]{} connected := &atomic.Bool{} connected.Store(true) - - // Create a grant checker that checks the store grantChecker := mcp.NewStoreGrantChecker(mcpStore, nil) tool := mcp.NewBridgeTool( @@ -66,22 +50,11 @@ func TestBridgeTool_Execute_RevokeAgentGrant_ReturnsError(t *testing.T) { grantChecker, ) - // Execute should work before revoke (will fail due to nil client, but that's expected) - // The key point is: after revoke, it should return "grant revoked" error - - // Now revoke the agent grant - err = mcpStore.RevokeFromAgent(ctx, serverID, agentID) - if err != nil { + if err := mcpStore.RevokeFromAgent(ctx, serverID, agentID); err != nil { t.Fatalf("RevokeFromAgent: %v", err) } - // Execute the tool after revoke - // EXPECTED (after Phase 02 fix): should return ErrorResult with "grant revoked" - // ACTUAL (currently): will try to execute and fail with "no active client" or succeed result := tool.Execute(ctx, map[string]any{"arg": "value"}) - - // This assertion SHOULD PASS after Phase 02, but FAILS now - // because BridgeTool.Execute does NOT recheck grants if !result.IsError { t.Error("expected error result after grant revoked, but got success") } @@ -90,17 +63,8 @@ func TestBridgeTool_Execute_RevokeAgentGrant_ReturnsError(t *testing.T) { } } -// TestBridgeTool_Execute_RevokeUserGrant_ReturnsError verifies that after revoking -// a user grant, BridgeTool.Execute returns an error. -// -// This test MUST FAIL initially (Phase 01 TDD). +// TestBridgeTool_Execute_RevokeUserGrant_ReturnsError: TDD-red for Phase 02. func TestBridgeTool_Execute_RevokeUserGrant_ReturnsError(t *testing.T) { - // TDD-red: Phase 02 user-grant revocation not yet implemented. - // ListAccessible's current SQL treats an absent mcp_user_grants row as - // "allowed by default" (mug.id IS NULL OR mug.enabled = true), so deleting - // the user grant row does not remove access. Implementing this requires - // either changing the semantics (user grant required when one ever existed) - // or a separate audit trail. Re-enable once Phase 02 lands. t.Skip("Phase 02: user-grant-level revocation not yet implemented — see commit 8b8da3a3") db := testDB(t) @@ -108,33 +72,17 @@ func TestBridgeTool_Execute_RevokeUserGrant_ReturnsError(t *testing.T) { serverID := seedMCPServer(t, db, tenantID) userID := "test-user-" + uuid.New().String()[:8] - // Grant agent access (required for ListAccessible) grantAgentAccess(t, db, tenantID, serverID, agentID) - - // Grant user access grantUserAccess(t, db, tenantID, serverID, userID) - // Create MCP store mcpStore := pg.NewPGMCPServerStore(db, testEncryptionKey) ctx := store.WithTenantID(context.Background(), tenantID) ctx = store.WithAgentID(ctx, agentID) ctx = store.WithUserID(ctx, userID) - // Verify both grants are active - accessible, err := mcpStore.ListAccessible(ctx, agentID, userID) - if err != nil { - t.Fatalf("ListAccessible: %v", err) - } - if len(accessible) == 0 { - t.Fatal("expected accessible server after grants") - } - - // Create BridgeTool clientPtr := &atomic.Pointer[mcpclient.Client]{} connected := &atomic.Bool{} connected.Store(true) - - // Create a grant checker that checks the store grantChecker := mcp.NewStoreGrantChecker(mcpStore, nil) tool := mcp.NewBridgeTool( @@ -148,18 +96,11 @@ func TestBridgeTool_Execute_RevokeUserGrant_ReturnsError(t *testing.T) { grantChecker, ) - // Revoke the USER grant (agent grant still active) - err = mcpStore.RevokeFromUser(ctx, serverID, userID) - if err != nil { + if err := mcpStore.RevokeFromUser(ctx, serverID, userID); err != nil { t.Fatalf("RevokeFromUser: %v", err) } - // Execute the tool after user revoke - // EXPECTED (after Phase 02 fix): should return "grant revoked" since user lost access - // ACTUAL (currently): does not check user grants at execute time result := tool.Execute(ctx, map[string]any{"arg": "value"}) - - // This assertion SHOULD PASS after Phase 02, but FAILS now if !result.IsError { t.Error("expected error result after user grant revoked") } @@ -168,24 +109,18 @@ func TestBridgeTool_Execute_RevokeUserGrant_ReturnsError(t *testing.T) { } } -// TestResolver_Rebuild_AfterRevoke_NoToolInPrompt verifies that after revoking a grant, -// the next resolver.Get() returns a Loop without the revoked tool in the prompt. -// -// This test SHOULD PASS even before fixes (regression guard) because the existing -// unregisterAllTools + fresh clone mechanism already handles prompt rebuild. +// TestResolver_Rebuild_AfterRevoke_NoToolInPrompt: regression guard — after revoking +// a grant, ListAccessible returns 0 servers so prompt rebuild has no tool. func TestResolver_Rebuild_AfterRevoke_NoToolInPrompt(t *testing.T) { db := testDB(t) tenantID, agentID := seedTenantAgent(t, db) serverID := seedMCPServer(t, db, tenantID) - // Grant agent access grantAgentAccess(t, db, tenantID, serverID, agentID) - // Create MCP store mcpStore := pg.NewPGMCPServerStore(db, testEncryptionKey) ctx := store.WithTenantID(context.Background(), tenantID) - // Verify grant is active accessible, err := mcpStore.ListAccessible(ctx, agentID, "test-user") if err != nil { t.Fatalf("ListAccessible before revoke: %v", err) @@ -195,13 +130,10 @@ func TestResolver_Rebuild_AfterRevoke_NoToolInPrompt(t *testing.T) { } serverName := accessible[0].Server.Name - // Revoke the grant - err = mcpStore.RevokeFromAgent(ctx, serverID, agentID) - if err != nil { + if err := mcpStore.RevokeFromAgent(ctx, serverID, agentID); err != nil { t.Fatalf("RevokeFromAgent: %v", err) } - // Verify no servers accessible after revoke accessible, err = mcpStore.ListAccessible(ctx, agentID, "test-user") if err != nil { t.Fatalf("ListAccessible after revoke: %v", err) @@ -210,9 +142,6 @@ func TestResolver_Rebuild_AfterRevoke_NoToolInPrompt(t *testing.T) { t.Errorf("expected 0 accessible servers after revoke, got %d", len(accessible)) } - // This test passes as a regression guard: - // The next LoadForAgent() will query ListAccessible which returns empty, - // so no MCP tools will be registered. The prompt rebuild mechanism works. t.Logf("Regression guard PASS: server %q no longer accessible after revoke", serverName) } @@ -245,11 +174,3 @@ func grantUserAccess(t *testing.T, db *sql.DB, tenantID, serverID uuid.UUID, use func containsGrantRevoked(s string) bool { return len(s) > 0 && (strings.Contains(s, "grant revoked") || strings.Contains(s, "grant denied")) } - -// fakeMCPClient is a stub for testing. Since mcpclient.Client is a struct -// and not an interface, we cannot directly mock it. The test relies on -// the clientPtr being nil or the connection being marked as disconnected. -type fakeMCPClient struct { - result *mcpgo.CallToolResult - err error -} diff --git a/tests/integration/secure_cli_agent_grants_env_test.go b/tests/integration/secure_cli_agent_grants_env_test.go new file mode 100644 index 00000000..76bc6438 --- /dev/null +++ b/tests/integration/secure_cli_agent_grants_env_test.go @@ -0,0 +1,286 @@ +//go:build integration + +package integration + +// C4 coverage: per-grant env override store-layer tests. +// Covers: CRUD env override, denylist validation (via crypto package), +// 3-state semantics (absent/null/map), and the env_set/env_keys fields. + +import ( + "encoding/json" + "testing" + + "github.com/google/uuid" + + "github.com/nextlevelbuilder/goclaw/internal/crypto" + "github.com/nextlevelbuilder/goclaw/internal/store" + "github.com/nextlevelbuilder/goclaw/internal/store/pg" +) + +// TestGrantEnv_SetAndReveal verifies that UpdateGrantEnv stores encrypted env +// and that Get returns the decrypted plaintext in g.EncryptedEnv. +func TestGrantEnv_SetAndReveal(t *testing.T) { + t.Parallel() + + db := testDB(t) + tenantID, agentID := seedTenantAgent(t, db) + binaryID := seedSecureCLI(t, db, tenantID) + + grantStore := pg.NewPGSecureCLIAgentGrantStore(db, testEncryptionKey) + + // Create a bare grant (no env). + g := &store.SecureCLIAgentGrant{ + BinaryID: binaryID, + AgentID: agentID, + Enabled: true, + } + if err := grantStore.Create(tenantCtx(tenantID), g); err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { db.Exec("DELETE FROM secure_cli_agent_grants WHERE id = $1", g.ID) }) + + // Set env override. + plaintext := []byte(`{"MY_TOKEN":"secret123","MY_URL":"https://api.example.com"}`) + if err := grantStore.UpdateGrantEnv(tenantCtx(tenantID), g.ID, plaintext); err != nil { + t.Fatalf("UpdateGrantEnv: %v", err) + } + + // Get must decrypt and return the plaintext in EncryptedEnv field. + fetched, err := grantStore.Get(tenantCtx(tenantID), g.ID) + if err != nil { + t.Fatalf("Get after UpdateGrantEnv: %v", err) + } + if string(fetched.EncryptedEnv) != string(plaintext) { + t.Errorf("Get.EncryptedEnv: want %s, got %s", plaintext, fetched.EncryptedEnv) + } +} + +// TestGrantEnv_ClearWithNil verifies the 3-state null-clears semantics. +// Passing nil to UpdateGrantEnv removes the env override. +func TestGrantEnv_ClearWithNil(t *testing.T) { + t.Parallel() + + db := testDB(t) + tenantID, agentID := seedTenantAgent(t, db) + binaryID := seedSecureCLI(t, db, tenantID) + + grantStore := pg.NewPGSecureCLIAgentGrantStore(db, testEncryptionKey) + g := &store.SecureCLIAgentGrant{BinaryID: binaryID, AgentID: agentID, Enabled: true} + if err := grantStore.Create(tenantCtx(tenantID), g); err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { db.Exec("DELETE FROM secure_cli_agent_grants WHERE id = $1", g.ID) }) + + // Set env. + if err := grantStore.UpdateGrantEnv(tenantCtx(tenantID), g.ID, []byte(`{"KEY":"val"}`)); err != nil { + t.Fatalf("UpdateGrantEnv set: %v", err) + } + + // Clear by passing nil. + if err := grantStore.UpdateGrantEnv(tenantCtx(tenantID), g.ID, nil); err != nil { + t.Fatalf("UpdateGrantEnv clear: %v", err) + } + + fetched, err := grantStore.Get(tenantCtx(tenantID), g.ID) + if err != nil { + t.Fatalf("Get after clear: %v", err) + } + if len(fetched.EncryptedEnv) > 0 { + t.Errorf("expected empty EncryptedEnv after clear, got %q", fetched.EncryptedEnv) + } +} + +// TestGrantEnv_DenylistRejection verifies that IsDeniedEnvKey correctly rejects +// entries from the denylist (backend enforcement via crypto package). +func TestGrantEnv_DenylistRejection(t *testing.T) { + cases := []struct { + key string + denied bool + }{ + {"PATH", true}, + {"LD_PRELOAD", true}, + {"DYLD_INSERT_LIBRARIES", true}, + {"GOCLAW_SECRET", true}, + {"MY_TOKEN", false}, + {"AWS_ACCESS_KEY_ID", false}, + {"NODE_OPTIONS", true}, + {"PYTHONPATH", true}, + } + for _, tc := range cases { + tc := tc + t.Run(tc.key, func(t *testing.T) { + got := crypto.IsDeniedEnvKey(tc.key) + if got != tc.denied { + t.Errorf("IsDeniedEnvKey(%q) = %v, want %v", tc.key, got, tc.denied) + } + }) + } +} + +// TestGrantEnv_ValidateGrantEnvVars_DeniedKeysReported verifies that ValidateGrantEnvVars +// returns all denied keys in rejectedKeys (not silently drops them). +func TestGrantEnv_ValidateGrantEnvVars_DeniedKeysReported(t *testing.T) { + envVars := map[string]string{ + "MY_SAFE_KEY": "value", + "PATH": "/bin", + "HOME": "/root", + } + rejected, valErr := crypto.ValidateGrantEnvVars(envVars) + if valErr != nil { + t.Fatalf("unexpected valErr: %v", valErr) + } + if len(rejected) != 2 { + t.Errorf("expected 2 rejected keys (PATH, HOME), got %d: %v", len(rejected), rejected) + } + deniedSet := make(map[string]bool) + for _, k := range rejected { + deniedSet[k] = true + } + if !deniedSet["PATH"] { + t.Error("PATH should be in rejected keys") + } + if !deniedSet["HOME"] { + t.Error("HOME should be in rejected keys") + } +} + +// TestGrantEnv_ListReflectsPresence verifies that ListByBinary decrypts env +// and that env presence is detectable from EncryptedEnv field length. +func TestGrantEnv_ListReflectsPresence(t *testing.T) { + t.Parallel() + + db := testDB(t) + tenantID, agentID := seedTenantAgent(t, db) + binaryID := seedSecureCLI(t, db, tenantID) + + grantStore := pg.NewPGSecureCLIAgentGrantStore(db, testEncryptionKey) + g := &store.SecureCLIAgentGrant{BinaryID: binaryID, AgentID: agentID, Enabled: true} + if err := grantStore.Create(tenantCtx(tenantID), g); err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { db.Exec("DELETE FROM secure_cli_agent_grants WHERE id = $1", g.ID) }) + + if err := grantStore.UpdateGrantEnv(tenantCtx(tenantID), g.ID, []byte(`{"MY_KEY":"val"}`)); err != nil { + t.Fatalf("UpdateGrantEnv: %v", err) + } + + grants, err := grantStore.ListByBinary(tenantCtx(tenantID), binaryID) + if err != nil { + t.Fatalf("ListByBinary: %v", err) + } + if len(grants) == 0 { + t.Fatal("expected at least one grant") + } + + var found *store.SecureCLIAgentGrant + for i := range grants { + if grants[i].ID == g.ID { + found = &grants[i] + break + } + } + if found == nil { + t.Fatalf("grant %s not found in ListByBinary", g.ID) + } + + // After list, EncryptedEnv should contain decrypted data (store decrypts on scan). + if len(found.EncryptedEnv) == 0 { + t.Error("ListByBinary: EncryptedEnv should be populated (decrypted) when env exists") + } +} + +// TestGrantEnv_DeterministicValidationOrder verifies that ValidateGrantEnvVars +// produces deterministic error output when multiple denied keys are present. +func TestGrantEnv_DeterministicValidationOrder(t *testing.T) { + envVars := map[string]string{ + "PATH": "/bin", + "HOME": "/root", + "MY_KEY": "ok", + "USER": "root", + "SHELL": "/bin/bash", + } + + rejected1, _ := crypto.ValidateGrantEnvVars(envVars) + rejected2, _ := crypto.ValidateGrantEnvVars(envVars) + + if len(rejected1) != len(rejected2) { + t.Errorf("non-deterministic: call 1 returned %d rejected keys, call 2 returned %d", + len(rejected1), len(rejected2)) + } + + set1 := make(map[string]bool) + for _, k := range rejected1 { + set1[k] = true + } + for _, k := range rejected2 { + if !set1[k] { + t.Errorf("non-deterministic: key %q in call 2 but not call 1", k) + } + } +} + +// TestGrantEnv_RevealDecryptedValue verifies the crypto round-trip that the +// reveal handler relies on: store.Get decrypts, caller parses as string map. +func TestGrantEnv_RevealDecryptedValue(t *testing.T) { + t.Parallel() + + db := testDB(t) + tenantID, agentID := seedTenantAgent(t, db) + binaryID := seedSecureCLI(t, db, tenantID) + + grantStore := pg.NewPGSecureCLIAgentGrantStore(db, testEncryptionKey) + g := &store.SecureCLIAgentGrant{BinaryID: binaryID, AgentID: agentID, Enabled: true} + if err := grantStore.Create(tenantCtx(tenantID), g); err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { db.Exec("DELETE FROM secure_cli_agent_grants WHERE id = $1", g.ID) }) + + secret := `{"API_KEY":"super-secret-value","ENDPOINT":"https://api.example.com"}` + if err := grantStore.UpdateGrantEnv(tenantCtx(tenantID), g.ID, []byte(secret)); err != nil { + t.Fatalf("UpdateGrantEnv: %v", err) + } + + // Simulate reveal: Get decrypts, then caller parses as map. + fetched, err := grantStore.Get(tenantCtx(tenantID), g.ID) + if err != nil { + t.Fatalf("Get: %v", err) + } + if string(fetched.EncryptedEnv) != secret { + t.Errorf("reveal: want %s, got %s", secret, fetched.EncryptedEnv) + } + + var envMap map[string]string + if err := json.Unmarshal(fetched.EncryptedEnv, &envMap); err != nil { + t.Errorf("reveal result not valid JSON map: %v", err) + } + if envMap["API_KEY"] != "super-secret-value" { + t.Errorf("wrong API_KEY value: %q", envMap["API_KEY"]) + } +} + +// TestGrantEnv_GrantNotFoundCrossID verifies that Get with wrong tenant returns no row, +// enforcing tenant isolation for the reveal path. +func TestGrantEnv_GrantNotFoundCrossID(t *testing.T) { + t.Parallel() + + db := testDB(t) + tenantA, agentA := seedTenantAgent(t, db) + binaryA := seedSecureCLI(t, db, tenantA) + tenantB, _ := seedTenantAgent(t, db) + + grantStore := pg.NewPGSecureCLIAgentGrantStore(db, testEncryptionKey) + g := &store.SecureCLIAgentGrant{BinaryID: binaryA, AgentID: agentA, Enabled: true} + if err := grantStore.Create(tenantCtx(tenantA), g); err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { db.Exec("DELETE FROM secure_cli_agent_grants WHERE id = $1", g.ID) }) + + // Tenant B trying to Get tenant A's grant must fail. + _, err := grantStore.Get(tenantCtx(tenantB), g.ID) + if err == nil { + t.Error("Get with wrong tenant should return error (ErrNoRows), got nil") + } +} + +// Ensure uuid is used (referenced in TestGrantEnv_GrantNotFoundCrossID via uuid.UUID fields). +var _ = uuid.Nil diff --git a/tests/integration/secure_cli_cross_tenant_isolation_test.go b/tests/integration/secure_cli_cross_tenant_isolation_test.go new file mode 100644 index 00000000..03f80ee1 --- /dev/null +++ b/tests/integration/secure_cli_cross_tenant_isolation_test.go @@ -0,0 +1,133 @@ +//go:build integration + +package integration + +// C3 regression guard: verify tenant isolation at the store layer for +// secure_cli_binaries.List + agent_grants_summary aggregation. +// +// Scope: store-layer tests only. Isolation is enforced in SQL (WHERE +// b.tenant_id = $2 and g.tenant_id = $1 in the LEFT JOIN LATERAL subquery), +// so store-layer coverage catches regressions in the tenant-scoping predicate. +// HTTP-layer cross-tenant tests are deferred until gateway-token auth +// scaffolding is wired into the integration suite. + +import ( + "testing" + + "github.com/google/uuid" + + "github.com/nextlevelbuilder/goclaw/internal/store/pg" +) + +// TestSecureCLICrossTenant_ListDoesNotExposeForeignData verifies that +// store.List scoped to tenant B does not return tenant A's binaries. +func TestSecureCLICrossTenant_ListDoesNotExposeForeignData(t *testing.T) { + t.Parallel() + + db := testDB(t) + + tenantA, agentA := seedTenantAgent(t, db) + binaryA := seedSecureCLI(t, db, tenantA) + grantA := uuid.New() + if _, err := db.Exec( + `INSERT INTO secure_cli_agent_grants + (id, binary_id, agent_id, tenant_id, encrypted_env, enabled) + VALUES ($1, $2, $3, $4, $5, true)`, + grantA, binaryA, agentA, tenantA, []byte(`{"KEY":"val"}`), + ); err != nil { + t.Fatalf("seed grant A: %v", err) + } + + tenantB, _ := seedTenantAgent(t, db) + binaryB := seedSecureCLI(t, db, tenantB) + + cliStore := pg.NewPGSecureCLIStore(db, testEncryptionKey) + + binsA, err := cliStore.List(tenantCtx(tenantA)) + if err != nil { + t.Fatalf("list A: %v", err) + } + if len(binsA) != 1 || binsA[0].ID != binaryA { + t.Errorf("tenant A should see exactly binary A; got %d binaries", len(binsA)) + } + + binsB, err := cliStore.List(tenantCtx(tenantB)) + if err != nil { + t.Fatalf("list B: %v", err) + } + if len(binsB) != 1 || binsB[0].ID != binaryB { + t.Errorf("tenant B should see exactly binary B; got %d binaries", len(binsB)) + } + for _, b := range binsB { + if b.ID == binaryA { + t.Errorf("tenant B LEAKED: saw binary from tenant A (%s)", binaryA) + } + } +} + +// TestSecureCLICrossTenant_AggregateListScopeIsolation verifies that the +// agent_grants_summary LEFT JOIN LATERAL subquery filters grants by caller +// tenant — each tenant only sees its own grants in the summary. +func TestSecureCLICrossTenant_AggregateListScopeIsolation(t *testing.T) { + t.Parallel() + + db := testDB(t) + + tenantA, agentA := seedTenantAgent(t, db) + binaryA := seedSecureCLI(t, db, tenantA) + grantA := uuid.New() + if _, err := db.Exec( + `INSERT INTO secure_cli_agent_grants + (id, binary_id, agent_id, tenant_id, encrypted_env, enabled) + VALUES ($1, $2, $3, $4, $5, true)`, + grantA, binaryA, agentA, tenantA, []byte(`{"KEY":"val"}`), + ); err != nil { + t.Fatalf("seed grant A: %v", err) + } + + tenantB, agentB := seedTenantAgent(t, db) + binaryB := seedSecureCLI(t, db, tenantB) + grantB := uuid.New() + if _, err := db.Exec( + `INSERT INTO secure_cli_agent_grants + (id, binary_id, agent_id, tenant_id, encrypted_env, enabled) + VALUES ($1, $2, $3, $4, $5, true)`, + grantB, binaryB, agentB, tenantB, []byte(`{}`), + ); err != nil { + t.Fatalf("seed grant B: %v", err) + } + + cliStore := pg.NewPGSecureCLIStore(db, testEncryptionKey) + + binsA, err := cliStore.List(tenantCtx(tenantA)) + if err != nil { + t.Fatalf("list A: %v", err) + } + if len(binsA) != 1 { + t.Fatalf("tenant A expected 1 binary, got %d", len(binsA)) + } + if got := len(binsA[0].AgentGrantsSummary); got != 1 { + t.Errorf("tenant A binary expected 1 grant summary, got %d", got) + } + for _, g := range binsA[0].AgentGrantsSummary { + if g.GrantID != grantA { + t.Errorf("tenant A LEAKED grant from another tenant: %s", g.GrantID) + } + } + + binsB, err := cliStore.List(tenantCtx(tenantB)) + if err != nil { + t.Fatalf("list B: %v", err) + } + if len(binsB) != 1 { + t.Fatalf("tenant B expected 1 binary, got %d", len(binsB)) + } + if got := len(binsB[0].AgentGrantsSummary); got != 1 { + t.Errorf("tenant B binary expected 1 grant summary, got %d", got) + } + for _, g := range binsB[0].AgentGrantsSummary { + if g.GrantID != grantB { + t.Errorf("tenant B LEAKED grant from another tenant: %s", g.GrantID) + } + } +} diff --git a/tests/integration/secure_cli_denylist_parity_test.go b/tests/integration/secure_cli_denylist_parity_test.go new file mode 100644 index 00000000..a80f0385 --- /dev/null +++ b/tests/integration/secure_cli_denylist_parity_test.go @@ -0,0 +1,198 @@ +//go:build integration + +package integration + +// C4 denylist parity test: verify that the frontend denylist (TypeScript) matches +// the backend denylist (Go package internal/crypto/env_denylist.go). +// +// Strategy: the Go denylist is imported directly via package import. +// The frontend denylist is read from the TypeScript source file via string parsing. +// If the sets diverge, the test fails with a diff showing added/removed keys. + +import ( + "bufio" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/nextlevelbuilder/goclaw/internal/crypto" +) + +// frontendDenylistExact reads the frontend ENV_DENYLIST_EXACT set from the TypeScript source. +// Parses the JS Set literal `const ENV_DENYLIST_EXACT = new Set([...])`. +func frontendDenylistExact(t *testing.T) map[string]struct{} { + t.Helper() + // Path relative to the test file's directory (tests/integration/). + _, thisFile, _, _ := runtime.Caller(0) + root := filepath.Join(filepath.Dir(thisFile), "..", "..") + tsFile := filepath.Join(root, "ui", "web", "src", "pages", "cli-credentials", + "cli-credential-grant-env-section.tsx") + + f, err := os.Open(tsFile) + if err != nil { + t.Skipf("frontend file not found (not in TS codebase scope): %v", err) + return nil + } + defer f.Close() + + result := make(map[string]struct{}) + inSet := false + scanner := bufio.NewScanner(f) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if strings.Contains(line, "const ENV_DENYLIST_EXACT") { + inSet = true + } + if inSet { + // Extract quoted identifiers. + parts := strings.Split(line, `"`) + for i := 1; i < len(parts); i += 2 { + key := strings.TrimSpace(parts[i]) + if key != "" && !strings.Contains(key, " ") { + result[key] = struct{}{} + } + } + } + if inSet && strings.Contains(line, "]);") { + break + } + } + return result +} + +// frontendDenylistPrefixes reads the frontend ENV_DENYLIST_PREFIXES array. +func frontendDenylistPrefixes(t *testing.T) map[string]struct{} { + t.Helper() + _, thisFile, _, _ := runtime.Caller(0) + root := filepath.Join(filepath.Dir(thisFile), "..", "..") + tsFile := filepath.Join(root, "ui", "web", "src", "pages", "cli-credentials", + "cli-credential-grant-env-section.tsx") + + f, err := os.Open(tsFile) + if err != nil { + t.Skipf("frontend file not found: %v", err) + return nil + } + defer f.Close() + + result := make(map[string]struct{}) + scanner := bufio.NewScanner(f) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if strings.Contains(line, "const ENV_DENYLIST_PREFIXES") { + // Parse prefix entries from: ["DYLD_", "GOCLAW_", "LD_"] + parts := strings.Split(line, `"`) + for i := 1; i < len(parts); i += 2 { + pfx := strings.TrimSpace(parts[i]) + if pfx != "" && !strings.Contains(pfx, " ") { + result[pfx] = struct{}{} + } + } + break + } + } + return result +} + +// backendDenylistExact returns the Go exact-match denylist by probing known keys. +// Since deniedExact is unexported, we use IsDeniedEnvKey with a controlled set of +// all keys that appear in either Go or frontend source. +// +// This is the exhaustive union probe set — any key on this list that differs between +// Go and TS is caught. +var knownExactKeys = []string{ + "PATH", "HOME", "USER", "SHELL", "PWD", + "LD_PRELOAD", "LD_LIBRARY_PATH", "LD_AUDIT", + "NODE_OPTIONS", "NODE_PATH", + "PYTHONPATH", "PYTHONHOME", "PYTHONSTARTUP", + "GIT_SSH_COMMAND", "GIT_SSH", "GIT_EXEC_PATH", "GIT_CONFIG_SYSTEM", + "SSH_AUTH_SOCK", + // Additions from finding #6 + "BASH_ENV", "ENV", "PROMPT_COMMAND", + "PERL5LIB", "RUBYOPT", + "HTTPS_PROXY", "HTTP_PROXY", "NO_PROXY", + "SSL_CERT_FILE", "SSL_CERT_DIR", "CURL_CA_BUNDLE", + "IFS", +} + +// TestDenylistParity_ExactKeysPresentInBoth verifies that every key in the frontend +// ENV_DENYLIST_EXACT is also rejected by the Go backend (IsDeniedEnvKey returns true). +func TestDenylistParity_ExactKeysPresentInBoth(t *testing.T) { + frontendExact := frontendDenylistExact(t) + if len(frontendExact) == 0 { + t.Skip("frontend denylist not parseable — skipping parity check") + } + + for key := range frontendExact { + if !crypto.IsDeniedEnvKey(key) { + t.Errorf("PARITY DRIFT: frontend denies %q but backend does NOT deny it", key) + } + } +} + +// TestDenylistParity_BackendDeniesKnownKeys verifies all known-dangerous keys are +// denied by the backend after finding #6 additions. +func TestDenylistParity_BackendDeniesKnownKeys(t *testing.T) { + // Keys from original denylist + finding #6 additions. + mustDeny := []string{ + // Original + "PATH", "HOME", "USER", "SHELL", "PWD", + "LD_PRELOAD", "LD_LIBRARY_PATH", "LD_AUDIT", + "NODE_OPTIONS", "NODE_PATH", + "PYTHONPATH", "PYTHONHOME", "PYTHONSTARTUP", + "GIT_SSH_COMMAND", "GIT_SSH", "GIT_EXEC_PATH", "GIT_CONFIG_SYSTEM", + "SSH_AUTH_SOCK", + // Finding #6 additions + "BASH_ENV", "ENV", "PROMPT_COMMAND", + "PERL5LIB", "RUBYOPT", + "HTTPS_PROXY", "HTTP_PROXY", "NO_PROXY", + "SSL_CERT_FILE", "SSL_CERT_DIR", "CURL_CA_BUNDLE", + "IFS", + // Prefix matches + "DYLD_INSERT_LIBRARIES", "DYLD_FRAMEWORK_PATH", + "GOCLAW_SECRET", "GOCLAW_ENCRYPTION_KEY", + "LD_SOMETHING", + // npm_config_ prefix (finding #6) + "npm_config_registry", "npm_config_prefix", + } + for _, key := range mustDeny { + if !crypto.IsDeniedEnvKey(key) { + t.Errorf("backend should deny %q but IsDeniedEnvKey returned false", key) + } + } +} + +// TestDenylistParity_SafeKeyNotDenied verifies that safe keys pass validation. +func TestDenylistParity_SafeKeyNotDenied(t *testing.T) { + safeKeys := []string{ + "AWS_ACCESS_KEY_ID", + "AWS_SECRET_ACCESS_KEY", + "GITHUB_TOKEN", + "DATABASE_URL", + "API_KEY", + "MY_CUSTOM_VAR", + } + for _, key := range safeKeys { + if crypto.IsDeniedEnvKey(key) { + t.Errorf("safe key %q should not be denied by backend", key) + } + } +} + +// TestDenylistParity_PrefixesInBoth verifies that frontend prefix list matches backend. +func TestDenylistParity_PrefixesInBoth(t *testing.T) { + frontendPfx := frontendDenylistPrefixes(t) + if len(frontendPfx) == 0 { + t.Skip("frontend prefix list not parseable") + } + + // For each frontend prefix, verify a key with that prefix is denied by backend. + for pfx := range frontendPfx { + testKey := pfx + "SOMETHING" + if !crypto.IsDeniedEnvKey(testKey) { + t.Errorf("PARITY DRIFT: frontend prefix %q blocks keys but backend does NOT deny %q", pfx, testKey) + } + } +} diff --git a/tests/integration/secure_cli_list_shape_freeze_test.go b/tests/integration/secure_cli_list_shape_freeze_test.go new file mode 100644 index 00000000..36d9c987 --- /dev/null +++ b/tests/integration/secure_cli_list_shape_freeze_test.go @@ -0,0 +1,210 @@ +//go:build integration + +package integration + +// C4 characterization test: lock the GET /v1/cli-credentials list response shape. +// Asserts that agent_grants_summary aggregate fields and env_set boolean are +// present in the store-layer response. This catches schema regressions where +// new columns or computed fields disappear from the list output. + +import ( + "encoding/json" + "testing" + + "github.com/google/uuid" + + "github.com/nextlevelbuilder/goclaw/internal/store" + "github.com/nextlevelbuilder/goclaw/internal/store/pg" +) + +// TestSecureCLIListShape_AgentGrantsSummaryFields verifies that List returns +// agent_grants_summary entries with all required fields: grant_id, agent_id, +// agent_key, name, enabled, env_set. +func TestSecureCLIListShape_AgentGrantsSummaryFields(t *testing.T) { + t.Parallel() + + db := testDB(t) + tenantID, agentID := seedTenantAgent(t, db) + binaryID := seedSecureCLI(t, db, tenantID) + + // Insert a grant with encrypted_env to set env_set=true. + grantID := uuid.New() + encEnvBytes := `{"SECRET_KEY":"value"}` + if _, err := db.Exec( + `INSERT INTO secure_cli_agent_grants + (id, binary_id, agent_id, tenant_id, encrypted_env, enabled) + VALUES ($1, $2, $3, $4, $5, true)`, + grantID, binaryID, agentID, tenantID, []byte(encEnvBytes), + ); err != nil { + t.Fatalf("seed grant with env: %v", err) + } + + cliStore := pg.NewPGSecureCLIStore(db, testEncryptionKey) + bins, err := cliStore.List(tenantCtx(tenantID)) + if err != nil { + t.Fatalf("List: %v", err) + } + if len(bins) == 0 { + t.Fatal("expected at least one binary in list") + } + + // Find our binary. + var target *store.SecureCLIBinary + for i := range bins { + if bins[i].ID == binaryID { + target = &bins[i] + break + } + } + if target == nil { + t.Fatalf("binary %s not found in list", binaryID) + } + + // agent_grants_summary must be populated. + if len(target.AgentGrantsSummary) == 0 { + t.Fatal("AgentGrantsSummary: expected at least one entry, got none") + } + + g := target.AgentGrantsSummary[0] + + // Lock grant_id field. + if g.GrantID == uuid.Nil { + t.Error("AgentGrantsSummary[0].GrantID: must not be nil") + } + if g.GrantID != grantID { + t.Errorf("AgentGrantsSummary[0].GrantID: want %s, got %s", grantID, g.GrantID) + } + + // Lock agent_id field. + if g.AgentID == uuid.Nil { + t.Error("AgentGrantsSummary[0].AgentID: must not be nil") + } + if g.AgentID != agentID { + t.Errorf("AgentGrantsSummary[0].AgentID: want %s, got %s", agentID, g.AgentID) + } + + // Lock agent_key field — must be non-empty string. + if g.AgentKey == "" { + t.Error("AgentGrantsSummary[0].AgentKey: must be non-empty") + } + + // Lock enabled field — grant was seeded with enabled=true. + if !g.Enabled { + t.Error("AgentGrantsSummary[0].Enabled: want true, got false") + } + + // Lock env_set field — grant has encrypted_env, so env_set must be true. + if !g.EnvSet { + t.Error("AgentGrantsSummary[0].EnvSet: want true (grant has encrypted_env), got false") + } +} + +// TestSecureCLIListShape_EnvSetFalseWhenNoEnv verifies that a grant with no +// encrypted_env reports env_set=false in the agent_grants_summary. +func TestSecureCLIListShape_EnvSetFalseWhenNoEnv(t *testing.T) { + t.Parallel() + + db := testDB(t) + tenantID, agentID := seedTenantAgent(t, db) + binaryID := seedSecureCLI(t, db, tenantID) + + // Insert a grant WITHOUT encrypted_env (NULL). + grantID := uuid.New() + if _, err := db.Exec( + `INSERT INTO secure_cli_agent_grants + (id, binary_id, agent_id, tenant_id, encrypted_env, enabled) + VALUES ($1, $2, $3, $4, NULL, true)`, + grantID, binaryID, agentID, tenantID, + ); err != nil { + t.Fatalf("seed grant without env: %v", err) + } + + cliStore := pg.NewPGSecureCLIStore(db, testEncryptionKey) + bins, err := cliStore.List(tenantCtx(tenantID)) + if err != nil { + t.Fatalf("List: %v", err) + } + + var target *store.SecureCLIBinary + for i := range bins { + if bins[i].ID == binaryID { + target = &bins[i] + break + } + } + if target == nil { + t.Fatalf("binary %s not found in list", binaryID) + } + if len(target.AgentGrantsSummary) == 0 { + t.Fatal("AgentGrantsSummary: expected at least one entry") + } + + g := target.AgentGrantsSummary[0] + if g.GrantID != grantID { + t.Fatalf("wrong grant in summary: want %s got %s", grantID, g.GrantID) + } + if g.EnvSet { + t.Error("AgentGrantsSummary[0].EnvSet: want false (no encrypted_env), got true") + } +} + +// TestSecureCLIListShape_JSONFieldNames verifies the JSON serialized field names +// match the documented API contract: snake_case per Go struct json tags. +func TestSecureCLIListShape_JSONFieldNames(t *testing.T) { + t.Parallel() + + db := testDB(t) + tenantID, agentID := seedTenantAgent(t, db) + binaryID := seedSecureCLI(t, db, tenantID) + + grantID := uuid.New() + if _, err := db.Exec( + `INSERT INTO secure_cli_agent_grants + (id, binary_id, agent_id, tenant_id, encrypted_env, enabled) + VALUES ($1, $2, $3, $4, $5, true)`, + grantID, binaryID, agentID, tenantID, []byte(`{"K":"v"}`), + ); err != nil { + t.Fatalf("seed grant: %v", err) + } + + cliStore := pg.NewPGSecureCLIStore(db, testEncryptionKey) + bins, err := cliStore.List(tenantCtx(tenantID)) + if err != nil { + t.Fatalf("List: %v", err) + } + var target *store.SecureCLIBinary + for i := range bins { + if bins[i].ID == binaryID { + target = &bins[i] + break + } + } + if target == nil || len(target.AgentGrantsSummary) == 0 { + t.Fatal("binary or summary not found") + } + + // Re-serialize to verify JSON field names. + raw, err := json.Marshal(target.AgentGrantsSummary[0]) + if err != nil { + t.Fatalf("marshal: %v", err) + } + var m map[string]any + if err := json.Unmarshal(raw, &m); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + requiredKeys := []string{"grant_id", "agent_id", "agent_key", "name", "enabled", "env_set"} + for _, k := range requiredKeys { + if _, ok := m[k]; !ok { + t.Errorf("AgentGrantsSummary JSON missing field %q; got keys: %v", k, mapKeys(m)) + } + } +} + +func mapKeys(m map[string]any) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + return keys +} diff --git a/tests/integration/secure_cli_reveal_rate_limit_test.go b/tests/integration/secure_cli_reveal_rate_limit_test.go new file mode 100644 index 00000000..3819114e --- /dev/null +++ b/tests/integration/secure_cli_reveal_rate_limit_test.go @@ -0,0 +1,146 @@ +//go:build integration + +package integration + +// C4 rate-limit test: verify the per-caller reveal rate limiter behavior. +// Uses SetEnvRevealLimiter to configure tight limits and HandleRevealEnvForTest +// to call the handler without the requireAuth middleware (auth is injected via ctx). + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/google/uuid" + + httphandler "github.com/nextlevelbuilder/goclaw/internal/http" + "github.com/nextlevelbuilder/goclaw/internal/store" + "github.com/nextlevelbuilder/goclaw/internal/store/pg" +) + +// buildRevealCtxRequest constructs a reveal request with owner-role context so +// requireTenantAdmin is bypassed (IsOwnerRole short-circuits the tenant check). +func buildRevealCtxRequest(binaryID, grantID uuid.UUID, tenantID uuid.UUID, userID string) *http.Request { + path := "/v1/cli-credentials/" + binaryID.String() + + "/agent-grants/" + grantID.String() + "/env:reveal" + req := httptest.NewRequest(http.MethodPost, path, nil) + req.SetPathValue("id", binaryID.String()) + req.SetPathValue("grantId", grantID.String()) + + ctx := store.WithTenantID(req.Context(), tenantID) + ctx = store.WithUserID(ctx, userID) + // Owner role bypasses requireTenantAdmin (ts.GetUserRole call) — safe for unit tests. + ctx = store.WithRole(ctx, store.TenantRoleOwner) + return req.WithContext(ctx) +} + +// TestRevealRateLimit_PerCallerBuckets verifies: +// 1. Caller A hitting the burst limit gets 429 on subsequent calls. +// 2. Caller B (different UserID) is NOT affected by caller A's exhaustion. +func TestRevealRateLimit_PerCallerBuckets(t *testing.T) { + t.Parallel() + + db := testDB(t) + tenantID, agentID := seedTenantAgent(t, db) + binaryID := seedSecureCLI(t, db, tenantID) + + grantStore := pg.NewPGSecureCLIAgentGrantStore(db, testEncryptionKey) + + g := &store.SecureCLIAgentGrant{BinaryID: binaryID, AgentID: agentID, Enabled: true} + if err := grantStore.Create(tenantCtx(tenantID), g); err != nil { + t.Fatalf("Create grant: %v", err) + } + t.Cleanup(func() { db.Exec("DELETE FROM secure_cli_agent_grants WHERE id = $1", g.ID) }) + + handler := httphandler.NewSecureCLIGrantHandler(grantStore, nil, nil) + // Tight limit: 1 rpm, burst 1 → 2nd call must be rejected. + handler.SetEnvRevealLimiter(1, 1) + + callerA := "user-a-" + uuid.New().String()[:8] + callerB := "user-b-" + uuid.New().String()[:8] + + callReveal := func(userID string) int { + rr := httptest.NewRecorder() + req := buildRevealCtxRequest(binaryID, g.ID, tenantID, userID) + handler.HandleRevealEnvForTest(rr, req) + return rr.Code + } + + // First call for A: within burst, must succeed (200 or 404 if no env). + code1A := callReveal(callerA) + if code1A == http.StatusTooManyRequests { + t.Errorf("callerA call 1: should not be rate-limited on first call, got 429") + } + + // Second call for A: over limit (burst=1, only 1 allowed). + code2A := callReveal(callerA) + if code2A != http.StatusTooManyRequests { + t.Errorf("callerA call 2: want 429 (rate limited), got %d", code2A) + } + + // First call for B: fresh bucket, must not be limited. + code1B := callReveal(callerB) + if code1B == http.StatusTooManyRequests { + t.Errorf("callerB call 1: should not be rate-limited (different bucket), got 429") + } +} + +// TestRevealRateLimit_ContextUserIDNotHeader verifies that the rate limit key +// comes from the context-injected UserID (authenticated), not the X-GoClaw-User-Id header. +func TestRevealRateLimit_ContextUserIDNotHeader(t *testing.T) { + t.Parallel() + + db := testDB(t) + tenantID, agentID := seedTenantAgent(t, db) + binaryID := seedSecureCLI(t, db, tenantID) + + grantStore := pg.NewPGSecureCLIAgentGrantStore(db, testEncryptionKey) + + g := &store.SecureCLIAgentGrant{BinaryID: binaryID, AgentID: agentID, Enabled: true} + if err := grantStore.Create(tenantCtx(tenantID), g); err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { db.Exec("DELETE FROM secure_cli_agent_grants WHERE id = $1", g.ID) }) + + handler := httphandler.NewSecureCLIGrantHandler(grantStore, nil, nil) + handler.SetEnvRevealLimiter(1, 1) + + realUserA := "real-user-" + uuid.New().String()[:8] + + // Exhaust real user A. + path := "/v1/cli-credentials/" + binaryID.String() + + "/agent-grants/" + g.ID.String() + "/env:reveal" + + makeReq := func(contextUser, headerUser string) int { + req := httptest.NewRequest(http.MethodPost, path, nil) + req.SetPathValue("id", binaryID.String()) + req.SetPathValue("grantId", g.ID.String()) + if headerUser != "" { + req.Header.Set("X-GoClaw-User-Id", headerUser) + } + ctx := store.WithTenantID(req.Context(), tenantID) + if contextUser != "" { + ctx = store.WithUserID(ctx, contextUser) + } + ctx = store.WithRole(ctx, store.TenantRoleOwner) + req = req.WithContext(ctx) + + rr := httptest.NewRecorder() + handler.HandleRevealEnvForTest(rr, req) + return rr.Code + } + + // Exhaust user A's bucket. + _ = makeReq(realUserA, "") // call 1 — within limit + code2 := makeReq(realUserA, "") // call 2 — over limit + if code2 != http.StatusTooManyRequests { + t.Errorf("real user A call 2: want 429, got %d", code2) + } + + // Attempt to spoof a different user via header while context still has realUserA. + // Context user wins → still rate-limited. + codeSpoof := makeReq(realUserA, "attacker-different-user") + if codeSpoof != http.StatusTooManyRequests { + t.Errorf("header spoof should not escape rate limit when context user is exhausted; got %d", codeSpoof) + } +} diff --git a/ui/web/src/i18n/locales/en/cli-credentials.json b/ui/web/src/i18n/locales/en/cli-credentials.json index 99ac5c72..a668eb86 100644 --- a/ui/web/src/i18n/locales/en/cli-credentials.json +++ b/ui/web/src/i18n/locales/en/cli-credentials.json @@ -102,6 +102,24 @@ "grant": "Grant", "update": "Update", "agentRequired": "Please select an agent", + "envVars": { + "title": "Environment Variables", + "overrideToggle": "Override binary defaults", + "overrideHelp": "When enabled, this grant's env vars fully replace the binary's default env", + "reveal": "Reveal values", + "revealHidden": "Hidden — click Reveal to view", + "revealError": "Failed to reveal env — rate limited or permission denied", + "addKey": "Add variable", + "keyPlaceholder": "KEY", + "valuePlaceholder": "Value", + "deniedKey": "Key '{{key}}' is not allowed", + "emptyState": "No env overrides — binary defaults apply" + }, + "chips": { + "title": "Granted to", + "none": "No grants", + "countMore": "+{{count}} more" + }, "toast": { "granted": "Agent grant created", "grantFailed": "Failed to create grant", @@ -110,5 +128,8 @@ "revoked": "Agent grant revoked", "revokeFailed": "Failed to revoke grant" } + }, + "list": { + "truncated": "Showing first 20 — use search or filter to find more" } } diff --git a/ui/web/src/i18n/locales/en/packages.json b/ui/web/src/i18n/locales/en/packages.json index 16c739c6..771d2861 100644 --- a/ui/web/src/i18n/locales/en/packages.json +++ b/ui/web/src/i18n/locales/en/packages.json @@ -62,5 +62,17 @@ "version": "Version", "actions": "Actions", "empty": "No packages installed" + }, + "tabs": { + "system": "System", + "python": "Python", + "node": "Node", + "github": "GitHub", + "cliCredentials": "CLI Credentials" + }, + "runtimesHeader": { + "title": "Runtimes", + "available": "Available", + "missing": "Missing" } } diff --git a/ui/web/src/i18n/locales/vi/cli-credentials.json b/ui/web/src/i18n/locales/vi/cli-credentials.json index 32eb007c..9cd7f738 100644 --- a/ui/web/src/i18n/locales/vi/cli-credentials.json +++ b/ui/web/src/i18n/locales/vi/cli-credentials.json @@ -102,6 +102,24 @@ "grant": "Cấp quyền", "update": "Cập nhật", "agentRequired": "Vui lòng chọn agent", + "envVars": { + "title": "Biến môi trường", + "overrideToggle": "Ghi đè mặc định của binary", + "overrideHelp": "Khi bật, biến môi trường của grant này sẽ thay thế hoàn toàn các biến mặc định của binary", + "reveal": "Hiện giá trị", + "revealHidden": "Đã ẩn — nhấn Hiện để xem", + "revealError": "Không thể hiện biến môi trường — vượt giới hạn yêu cầu hoặc không có quyền", + "addKey": "Thêm biến", + "keyPlaceholder": "TÊN_BIẾN", + "valuePlaceholder": "Giá trị", + "deniedKey": "Khóa '{{key}}' không được phép", + "emptyState": "Không có ghi đè — áp dụng mặc định của binary" + }, + "chips": { + "title": "Đã cấp cho", + "none": "Chưa có quyền nào", + "countMore": "+{{count}} thêm" + }, "toast": { "granted": "Đã cấp quyền agent", "grantFailed": "Cấp quyền thất bại", @@ -110,5 +128,8 @@ "revoked": "Đã thu hồi quyền", "revokeFailed": "Thu hồi quyền thất bại" } + }, + "list": { + "truncated": "Đang hiển thị 20 kết quả đầu — dùng tìm kiếm để xem thêm" } } diff --git a/ui/web/src/i18n/locales/vi/packages.json b/ui/web/src/i18n/locales/vi/packages.json index 8e112434..a5b454e3 100644 --- a/ui/web/src/i18n/locales/vi/packages.json +++ b/ui/web/src/i18n/locales/vi/packages.json @@ -62,5 +62,17 @@ "version": "Phiên bản", "actions": "Thao tác", "empty": "Chưa có gói nào được cài" + }, + "tabs": { + "system": "Hệ thống", + "python": "Python", + "node": "Node", + "github": "GitHub", + "cliCredentials": "Thông tin CLI" + }, + "runtimesHeader": { + "title": "Runtimes", + "available": "Sẵn sàng", + "missing": "Chưa cài" } } diff --git a/ui/web/src/i18n/locales/zh/cli-credentials.json b/ui/web/src/i18n/locales/zh/cli-credentials.json index 142a26c0..b0e4d929 100644 --- a/ui/web/src/i18n/locales/zh/cli-credentials.json +++ b/ui/web/src/i18n/locales/zh/cli-credentials.json @@ -102,6 +102,24 @@ "grant": "授权", "update": "更新", "agentRequired": "请选择代理", + "envVars": { + "title": "环境变量", + "overrideToggle": "覆盖二进制默认值", + "overrideHelp": "启用后，此授权的环境变量将完全替换二进制文件的默认环境变量", + "reveal": "显示值", + "revealHidden": "已隐藏 — 点击显示以查看", + "revealError": "显示环境变量失败 — 请求超出限制或权限不足", + "addKey": "添加变量", + "keyPlaceholder": "变量名", + "valuePlaceholder": "值", + "deniedKey": "键 '{{key}}' 不被允许", + "emptyState": "无环境变量覆盖 — 使用二进制默认值" + }, + "chips": { + "title": "已授权给", + "none": "暂无授权", + "countMore": "+{{count}} 个" + }, "toast": { "granted": "代理授权已创建", "grantFailed": "创建授权失败", @@ -110,5 +128,8 @@ "revoked": "代理授权已撤销", "revokeFailed": "撤销授权失败" } + }, + "list": { + "truncated": "显示前20条记录 — 使用搜索查找更多" } } diff --git a/ui/web/src/i18n/locales/zh/packages.json b/ui/web/src/i18n/locales/zh/packages.json index a4848c76..db1c0d6c 100644 --- a/ui/web/src/i18n/locales/zh/packages.json +++ b/ui/web/src/i18n/locales/zh/packages.json @@ -62,5 +62,17 @@ "version": "版本", "actions": "操作", "empty": "暂无已安装的软件包" + }, + "tabs": { + "system": "系统", + "python": "Python", + "node": "Node", + "github": "GitHub", + "cliCredentials": "CLI 凭证" + }, + "runtimesHeader": { + "title": "运行时", + "available": "可用", + "missing": "缺失" } } diff --git a/ui/web/src/pages/cli-credentials/cli-credential-agent-chips.tsx b/ui/web/src/pages/cli-credentials/cli-credential-agent-chips.tsx new file mode 100644 index 00000000..b52b9209 --- /dev/null +++ b/ui/web/src/pages/cli-credentials/cli-credential-agent-chips.tsx @@ -0,0 +1,97 @@ +/** + * cli-credential-agent-chips.tsx + * Chip row shown under each binary row in the CLI credentials table. + * + * Capabilities: + * - Shows first 5 chips; overflow becomes "+N more" text (no popover needed) + * - Backend caps the summary at 20 grants per binary; counts beyond that are + * truncated. Use the grants management dialog to see/edit the full set. + * - Chip: agent name + KeyRound icon when env_set=true + * - Tooltip with agent_key + grant_id + env_set status + * - Capability-probe: if agent_grants_summary is absent/undefined, renders nothing + * - Empty state: "No grants" text + Grant now link + * - Mobile: flex-wrap, no overflow-x + */ +import { useTranslation } from "react-i18next"; +import { KeyRound } from "lucide-react"; +import { Badge } from "@/components/ui/badge"; +import { + Tooltip, TooltipContent, TooltipProvider, TooltipTrigger, +} from "@/components/ui/tooltip"; +import { Button } from "@/components/ui/button"; +import type { AgentGrantSummary } from "@/types/cli-credential"; + +const MAX_VISIBLE = 5; + +interface Props { + /** Capability-probe: undefined = field absent from API (old deploy), skip rendering */ + agentGrantsSummary: AgentGrantSummary[] | undefined; + onOpenGrants: () => void; +} + +/** Row of agent chips for a binary. Renders nothing if field is absent from API response. */ +export function CliCredentialAgentChips({ agentGrantsSummary, onOpenGrants }: Props) { + const { t } = useTranslation("cli-credentials"); + + // Capability-probe: if field is absent, skip entirely — no crash on rolling deploy + if (agentGrantsSummary === undefined) return null; + + if (agentGrantsSummary.length === 0) { + return ( +
+ {t("grants.chips.none")} + +
+ ); + } + + const visible = agentGrantsSummary.slice(0, MAX_VISIBLE); + const overflow = agentGrantsSummary.length - visible.length; + + return ( + +
+ {visible.map((grant) => ( + + + + + {grant.name || grant.agent_key} + {grant.env_set && } + + + +
+ {grant.agent_key} + grant: {grant.grant_id.slice(0, 8)}… + {grant.env_set && ( + {t("grants.envVars.title")}: custom + )} +
+ + + ))} + + {overflow > 0 && ( + + {t("grants.chips.countMore", { count: overflow })} + + )} +
+ + ); +} diff --git a/ui/web/src/pages/cli-credentials/cli-credential-grant-card.tsx b/ui/web/src/pages/cli-credentials/cli-credential-grant-card.tsx index 48812ae9..5b1d756a 100644 --- a/ui/web/src/pages/cli-credentials/cli-credential-grant-card.tsx +++ b/ui/web/src/pages/cli-credentials/cli-credential-grant-card.tsx @@ -1,5 +1,5 @@ import { useTranslation } from "react-i18next"; -import { Trash2, Pencil } from "lucide-react"; +import { Trash2, Pencil, KeyRound } from "lucide-react"; import { Button } from "@/components/ui/button"; import { Badge } from "@/components/ui/badge"; import { cn } from "@/lib/utils"; @@ -32,11 +32,17 @@ export function CliCredentialGrantCard({ grant, agentName, isActive, disabled, o >
-
+
{agentName} {!grant.enabled && ( {tc("disabled")} )} + {grant.env_set && ( + + + {t("grants.envVars.title")} + + )} {isActive && }
{hasOverrides ? ( diff --git a/ui/web/src/pages/cli-credentials/cli-credential-grant-env-section.tsx b/ui/web/src/pages/cli-credentials/cli-credential-grant-env-section.tsx new file mode 100644 index 00000000..8a0f48c2 --- /dev/null +++ b/ui/web/src/pages/cli-credentials/cli-credential-grant-env-section.tsx @@ -0,0 +1,212 @@ +/** + * Per-grant env override section. + * Switch "Override binary defaults" (M1: checkbox-equivalent). + * Reveal: POST .../env:reveal — values in component state only, cleared on close. + * Denylist: keep in sync with internal/crypto/env_denylist.go + */ +import { useState, useCallback, useEffect, useRef } from "react"; +import { useTranslation } from "react-i18next"; +import { Plus, X, Eye } from "lucide-react"; +import { Button } from "@/components/ui/button"; +import { Input } from "@/components/ui/input"; +import { Label } from "@/components/ui/label"; +import { Switch } from "@/components/ui/switch"; +import { toast } from "@/stores/use-toast-store"; +import { useHttp } from "@/hooks/use-ws"; + +// Keep in sync with internal/crypto/env_denylist.go. +// Backend is authoritative; this list drives inline UX warnings only. +const ENV_DENYLIST_EXACT = new Set([ + "PATH", "HOME", "USER", "SHELL", "PWD", + "LD_PRELOAD", "LD_LIBRARY_PATH", "LD_AUDIT", + "NODE_OPTIONS", "NODE_PATH", + "PYTHONPATH", "PYTHONHOME", "PYTHONSTARTUP", + "GIT_SSH_COMMAND", "GIT_SSH", "GIT_EXEC_PATH", "GIT_CONFIG_SYSTEM", + "SSH_AUTH_SOCK", + // Finding #6 additions — keep in sync with internal/crypto/env_denylist.go + "BASH_ENV", "ENV", "PROMPT_COMMAND", + "PERL5LIB", "RUBYOPT", + "HTTPS_PROXY", "HTTP_PROXY", "NO_PROXY", + "SSL_CERT_FILE", "SSL_CERT_DIR", "CURL_CA_BUNDLE", + "IFS", +]); +// Keep in sync with deniedPrefixes in internal/crypto/env_denylist.go. +const ENV_DENYLIST_PREFIXES = ["DYLD_", "GOCLAW_", "LD_", "NPM_CONFIG_"]; + +export interface GrantEnvEntry { + key: string; + value: string; + masked: boolean; // true = not yet revealed from server +} + +export interface GrantEnvState { + overrideEnabled: boolean; + entries: GrantEnvEntry[]; +} + +interface Props { + binaryId: string; + grantId: string | null; + initialEnvSet: boolean; + initialEnvKeys: string[]; + state: GrantEnvState; + onChange: (next: GrantEnvState) => void; + rejectedKeys?: string[]; +} + +export function CliCredentialGrantEnvSection({ + binaryId, grantId, initialEnvSet, initialEnvKeys, + state, onChange, rejectedKeys = [], +}: Props) { + const { t } = useTranslation("cli-credentials"); + const http = useHttp(); + const [revealing, setRevealing] = useState(false); + const [revealed, setRevealed] = useState(false); + const { overrideEnabled, entries } = state; + // Finding #10: track blur timeout so we can cancel it on reveal/unmount. + const blurTimeoutRef = useRef | null>(null); + + // Finding #10: clear revealed plaintext from entries on component unmount. + // This is defense-in-depth — plaintext should not persist in React state beyond use. + useEffect(() => { + return () => { + if (blurTimeoutRef.current) clearTimeout(blurTimeoutRef.current); + // Overwrite revealed values with empty strings on unmount. + onChange({ + overrideEnabled: state.overrideEnabled, + entries: state.entries.map((e) => ({ ...e, value: "", masked: e.masked })), + }); + }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + + const setEntries = useCallback( + (updater: (prev: GrantEnvEntry[]) => GrantEnvEntry[]) => + onChange({ overrideEnabled, entries: updater(entries) }), + [onChange, overrideEnabled, entries], + ); + + const handleToggle = useCallback((checked: boolean) => { + if (checked) { + if (initialEnvSet && !revealed && entries.every((e) => e.masked)) { + const masked: GrantEnvEntry[] = initialEnvKeys.map((k) => ({ key: k, value: "", masked: true })); + onChange({ overrideEnabled: true, entries: masked.length > 0 ? masked : [{ key: "", value: "", masked: false }] }); + } else if (entries.length === 0) { + onChange({ overrideEnabled: true, entries: [{ key: "", value: "", masked: false }] }); + } else { + onChange({ overrideEnabled: true, entries }); + } + } else { + onChange({ overrideEnabled: false, entries }); + } + }, [initialEnvSet, initialEnvKeys, revealed, entries, onChange]); + + const handleReveal = useCallback(async () => { + if (!grantId) return; + setRevealing(true); + try { + // POST — not GET (C1 red-team). Direct call, not cached by TanStack Query. + const res = await http.post<{ env_vars: Record }>( + `/v1/cli-credentials/${binaryId}/agent-grants/${grantId}/env:reveal`, + ); + const filled: GrantEnvEntry[] = Object.entries(res.env_vars).map(([k, v]) => ({ + key: k, value: v, masked: false, + })); + onChange({ overrideEnabled: true, entries: filled.length > 0 ? filled : entries }); + setRevealed(true); + // Finding #10: wipe plaintext after 30s of inactivity (defense-in-depth). + if (blurTimeoutRef.current) clearTimeout(blurTimeoutRef.current); + blurTimeoutRef.current = setTimeout(() => { + onChange({ + overrideEnabled: true, + entries: (filled.length > 0 ? filled : entries).map((e) => ({ ...e, value: "", masked: true })), + }); + setRevealed(false); + }, 30_000); + } catch (err) { + const code = (err as { code?: string }).code ?? ""; + const msg = err instanceof Error ? err.message : ""; + const isRateLimit = code === "RESOURCE_EXHAUSTED" || msg.toLowerCase().includes("rate"); + toast.error(t("grants.envVars.revealError"), isRateLimit ? undefined : msg || undefined); + } finally { + setRevealing(false); + } + }, [grantId, binaryId, http, onChange, entries, t]); + + const addEntry = useCallback(() => setEntries((p) => [...p, { key: "", value: "", masked: false }]), [setEntries]); + const removeEntry = useCallback((i: number) => setEntries((p) => p.filter((_, j) => j !== i)), [setEntries]); + const updateEntry = useCallback((i: number, f: "key" | "value", v: string) => + setEntries((p) => p.map((e, j) => j === i ? { ...e, [f]: v, masked: false } : e)), [setEntries]); + + const isDenied = (k: string) => { + if (k.length === 0) return false; + const upper = k.toUpperCase(); + if (ENV_DENYLIST_EXACT.has(upper)) return true; + return ENV_DENYLIST_PREFIXES.some((p) => upper.startsWith(p)); + }; + const isRejected = (k: string) => k.length > 0 && rejectedKeys.includes(k); + const hasMasked = entries.some((e) => e.masked); + + return ( +
+
+ +
+ +

{t("grants.envVars.overrideHelp")}

+
+
+ + {overrideEnabled && ( +
+ {hasMasked && !revealed && grantId && ( + + )} + {entries.map((entry, idx) => { + const hasError = isDenied(entry.key) || isRejected(entry.key); + return ( +
+
+ updateEntry(idx, "key", e.target.value)} + className={`text-base md:text-sm font-mono${hasError ? " border-destructive" : ""}`} /> + {hasError && ( +

+ {t("grants.envVars.deniedKey", { key: entry.key })} +

+ )} +
+
+ {entry.masked ? ( + + ) : ( + updateEntry(idx, "value", e.target.value)} + className="text-base md:text-sm" /> + )} +
+ +
+ ); + })} + {entries.length === 0 && ( +

{t("grants.envVars.emptyState")}

+ )} + +
+ )} +
+ ); +} diff --git a/ui/web/src/pages/cli-credentials/cli-credential-grant-form.tsx b/ui/web/src/pages/cli-credentials/cli-credential-grant-form.tsx index e10e4044..a3475f51 100644 --- a/ui/web/src/pages/cli-credentials/cli-credential-grant-form.tsx +++ b/ui/web/src/pages/cli-credentials/cli-credential-grant-form.tsx @@ -8,6 +8,8 @@ import { Textarea } from "@/components/ui/textarea"; import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue, } from "@/components/ui/select"; +import { CliCredentialGrantEnvSection } from "./cli-credential-grant-env-section"; +import type { GrantEnvState } from "./cli-credential-grant-env-section"; import type { AgentData } from "@/types/agent"; import type { SecureCLIBinary } from "./hooks/use-cli-credentials"; @@ -26,6 +28,17 @@ interface Props { setTips: (v: string) => void; enabled: boolean; setEnabled: (v: boolean) => void; + /** Per-grant env override state */ + envState: GrantEnvState; + setEnvState: (next: GrantEnvState) => void; + /** Grant ID when editing (null when creating) */ + editingGrantId: string | null; + /** Whether the existing grant already has encrypted env */ + initialEnvSet: boolean; + /** Key names of existing grant env (for masked display) */ + initialEnvKeys: string[]; + /** Keys rejected by last PUT (shown as errors) */ + rejectedKeys?: string[]; isEditing: boolean; saving: boolean; onSubmit: () => void; @@ -37,7 +50,10 @@ export function CliCredentialGrantForm({ binary, agents, agentId, setAgentId, denyArgs, setDenyArgs, denyVerbose, setDenyVerbose, timeout, setTimeout, tips, setTips, - enabled, setEnabled, isEditing, saving, + enabled, setEnabled, + envState, setEnvState, + editingGrantId, initialEnvSet, initialEnvKeys, rejectedKeys, + isEditing, saving, onSubmit, onCancel, }: Props) { const { t } = useTranslation("cli-credentials"); @@ -118,6 +134,17 @@ export function CliCredentialGrantForm({
+ + {/* Per-grant env override — Phase 7 */} +
diff --git a/ui/web/src/pages/cli-credentials/cli-credentials-page.tsx b/ui/web/src/pages/cli-credentials/cli-credentials-page.tsx index 48aea1ae..0a72bc23 100644 --- a/ui/web/src/pages/cli-credentials/cli-credentials-page.tsx +++ b/ui/web/src/pages/cli-credentials/cli-credentials-page.tsx @@ -1,212 +1,22 @@ -import { useState, lazy, Suspense } from "react"; import { useTranslation } from "react-i18next"; -import { KeyRound, Plus, RefreshCw, Pencil, Trash2, Users, Shield } from "lucide-react"; -import { Button } from "@/components/ui/button"; -import { Badge } from "@/components/ui/badge"; import { PageHeader } from "@/components/shared/page-header"; -import { EmptyState } from "@/components/shared/empty-state"; -import { TableSkeleton } from "@/components/shared/loading-skeleton"; -import { ConfirmDialog } from "@/components/shared/confirm-dialog"; -import { useMinLoading } from "@/hooks/use-min-loading"; -import { useDeferredLoading } from "@/hooks/use-deferred-loading"; -import { useCliCredentials, useCliCredentialPresets } from "./hooks/use-cli-credentials"; -import { CliCredentialGrantsDialog } from "./cli-credential-grants-dialog"; -import type { SecureCLIBinary, CLICredentialInput } from "./hooks/use-cli-credentials"; - -const CliCredentialFormDialog = lazy(() => - import("./cli-credential-form-dialog").then((m) => ({ default: m.CliCredentialFormDialog })) -); -const CLIUserCredentialsDialog = lazy(() => - import("./cli-user-credentials-dialog").then((m) => ({ default: m.CLIUserCredentialsDialog })) -); +import { CliCredentialsPanel } from "./cli-credentials-panel"; +/** + * CliCredentialsPage — standalone route wrapper. + * The route /cli-credentials now redirects to /packages?tab=cli-credentials. + * This page is kept for backward compat in case the redirect is bypassed. + * All content logic lives in CliCredentialsPanel (shared with tab). + */ export function CliCredentialsPage() { const { t } = useTranslation("cli-credentials"); - const { t: tc } = useTranslation("common"); - - const [formOpen, setFormOpen] = useState(false); - const [editItem, setEditItem] = useState(null); - const [deleteTarget, setDeleteTarget] = useState(null); - const [deleteLoading, setDeleteLoading] = useState(false); - const [userCredsTarget, setUserCredsTarget] = useState(null); - const [grantsTarget, setGrantsTarget] = useState(null); - - const { items, loading, refresh, createCredential, updateCredential, deleteCredential } = - useCliCredentials(); - const { presets } = useCliCredentialPresets(); - - const spinning = useMinLoading(loading); - const showSkeleton = useDeferredLoading(loading && items.length === 0); - - const handleCreate = async (data: CLICredentialInput) => { - await createCredential(data); - }; - - const handleEdit = async (data: CLICredentialInput) => { - if (!editItem) return; - await updateCredential(editItem.id, data); - }; - - const handleDelete = async () => { - if (!deleteTarget) return; - setDeleteLoading(true); - try { - await deleteCredential(deleteTarget.id); - setDeleteTarget(null); - } finally { - setDeleteLoading(false); - } - }; - - const openCreate = () => { - setEditItem(null); - setFormOpen(true); - }; - - const openEdit = (item: SecureCLIBinary) => { - setEditItem(item); - setFormOpen(true); - }; return ( -
- - - -
- } - /> - +
+
- {showSkeleton ? ( - - ) : items.length === 0 ? ( - - ) : ( -
- - - - - - - - - - - - - {items.map((item) => ( - - - - - - - - - ))} - -
{t("columns.binary")}{tc("description")}{t("columns.scope")}{tc("enabled")}{t("columns.timeout")}{tc("actions")}
-
- -
-
{item.binary_name}
- {item.binary_path && ( -
{item.binary_path}
- )} -
-
- 		- {item.description || "—"} - - - {item.is_global ? tc("global") : t("columns.restricted")} - - - - {item.enabled ? tc("enabled") : tc("disabled")} - - {item.timeout_seconds}s -
- - - - -
-
-
- )} +
- - - - - - !open && setDeleteTarget(null)} - title={t("delete.title")} - description={t("delete.description", { name: deleteTarget?.binary_name })} - confirmLabel={t("delete.confirm")} - variant="destructive" - onConfirm={handleDelete} - loading={deleteLoading} - /> - - {userCredsTarget && ( - - !open && setUserCredsTarget(null)} - binary={userCredsTarget} - /> - - )} - - {grantsTarget && ( - !open && setGrantsTarget(null)} - binary={grantsTarget} - /> - )}
); } diff --git a/ui/web/src/pages/cli-credentials/cli-credentials-panel.tsx b/ui/web/src/pages/cli-credentials/cli-credentials-panel.tsx new file mode 100644 index 00000000..a6e745bb --- /dev/null +++ b/ui/web/src/pages/cli-credentials/cli-credentials-panel.tsx @@ -0,0 +1,142 @@ +/** + * CliCredentialsPanel — reusable panel without page-level PageHeader. + * Used by: + * - CliCredentialsPage (standalone route, wraps in its own PageHeader) + * - CliCredentialsTab inside PackagesPage (tab body, no PageHeader needed) + */ +import { useState, lazy, Suspense } from "react"; +import { useTranslation } from "react-i18next"; +import { KeyRound, Plus, RefreshCw } from "lucide-react"; +import { Button } from "@/components/ui/button"; +import { EmptyState } from "@/components/shared/empty-state"; +import { TableSkeleton } from "@/components/shared/loading-skeleton"; +import { ConfirmDialog } from "@/components/shared/confirm-dialog"; +import { useMinLoading } from "@/hooks/use-min-loading"; +import { useDeferredLoading } from "@/hooks/use-deferred-loading"; +import { useCliCredentials, useCliCredentialPresets } from "./hooks/use-cli-credentials"; +import { CliCredentialGrantsDialog } from "./cli-credential-grants-dialog"; +import { CliCredentialsTable } from "./cli-credentials-table"; +import type { SecureCLIBinary, CLICredentialInput } from "./hooks/use-cli-credentials"; + +const CliCredentialFormDialog = lazy(() => + import("./cli-credential-form-dialog").then((m) => ({ default: m.CliCredentialFormDialog })) +); +const CLIUserCredentialsDialog = lazy(() => + import("./cli-user-credentials-dialog").then((m) => ({ default: m.CLIUserCredentialsDialog })) +); + +export function CliCredentialsPanel() { + const { t } = useTranslation("cli-credentials"); + const { t: tc } = useTranslation("common"); + + const [formOpen, setFormOpen] = useState(false); + const [editItem, setEditItem] = useState(null); + const [deleteTarget, setDeleteTarget] = useState(null); + const [deleteLoading, setDeleteLoading] = useState(false); + const [userCredsTarget, setUserCredsTarget] = useState(null); + const [grantsTarget, setGrantsTarget] = useState(null); + + const { items, loading, refresh, createCredential, updateCredential, deleteCredential } = + useCliCredentials(); + const { presets } = useCliCredentialPresets(); + + const spinning = useMinLoading(loading); + const showSkeleton = useDeferredLoading(loading && items.length === 0); + + const handleCreate = async (data: CLICredentialInput) => { await createCredential(data); }; + const handleEdit = async (data: CLICredentialInput) => { + if (!editItem) return; + await updateCredential(editItem.id, data); + }; + const handleDelete = async () => { + if (!deleteTarget) return; + setDeleteLoading(true); + try { + await deleteCredential(deleteTarget.id); + setDeleteTarget(null); + } finally { + setDeleteLoading(false); + } + }; + + const openCreate = () => { setEditItem(null); setFormOpen(true); }; + const openEdit = (item: SecureCLIBinary) => { setEditItem(item); setFormOpen(true); }; + + return ( +
+ {/* Toolbar */} +
+

{t("description")}

+
+ + +
+
+ + {showSkeleton ? ( + + ) : items.length === 0 ? ( + + ) : ( + <> + + {/* Finding #12: surface LIMIT 20 truncation so admins know there are more entries. */} + {items.length >= 20 && ( +

+ {t("list.truncated")} +

+ )} + + )} + + + + + + !open && setDeleteTarget(null)} + title={t("delete.title")} + description={t("delete.description", { name: deleteTarget?.binary_name })} + confirmLabel={t("delete.confirm")} + variant="destructive" + onConfirm={handleDelete} + loading={deleteLoading} + /> + + {userCredsTarget && ( + + !open && setUserCredsTarget(null)} + binary={userCredsTarget} + /> + + )} + + {grantsTarget && ( + !open && setGrantsTarget(null)} + binary={grantsTarget} + /> + )} +
+ ); +} diff --git a/ui/web/src/pages/cli-credentials/cli-credentials-table.tsx b/ui/web/src/pages/cli-credentials/cli-credentials-table.tsx new file mode 100644 index 00000000..0e994554 --- /dev/null +++ b/ui/web/src/pages/cli-credentials/cli-credentials-table.tsx @@ -0,0 +1,104 @@ +/** + * CliCredentialsTable — table + row actions for CLI credential entries. + * Extracted from cli-credentials-panel.tsx to stay under 200-line limit. + * Phase 8: each row has a chip sub-row from agent_grants_summary. + */ +import { useTranslation } from "react-i18next"; +import { KeyRound, Pencil, Trash2, Users, Shield } from "lucide-react"; +import { Button } from "@/components/ui/button"; +import { Badge } from "@/components/ui/badge"; +import { CliCredentialAgentChips } from "./cli-credential-agent-chips"; +import type { SecureCLIBinary } from "./hooks/use-cli-credentials"; + +interface Props { + items: SecureCLIBinary[]; + onEdit: (item: SecureCLIBinary) => void; + onDelete: (item: SecureCLIBinary) => void; + onUserCreds: (item: SecureCLIBinary) => void; + onGrants: (item: SecureCLIBinary) => void; +} + +export function CliCredentialsTable({ items, onEdit, onDelete, onUserCreds, onGrants }: Props) { + const { t } = useTranslation("cli-credentials"); + const { t: tc } = useTranslation("common"); + + return ( +
+ + + + + + + + + + + + + {items.map((item) => ( + <> + {/* Main data row */} + + + + + + + + + {/* Agent chips sub-row — Phase 8 */} + + + + + ))} + +
{t("columns.binary")}{tc("description")}{t("columns.scope")}{tc("enabled")}{t("columns.timeout")}{tc("actions")}
+
+ +
+
{item.binary_name}
+ {item.binary_path && ( +
{item.binary_path}
+ )} +
+
+ 		+ {item.description || "—"} + + + {item.is_global ? tc("global") : t("columns.restricted")} + + + + {item.enabled ? tc("enabled") : tc("disabled")} + + {item.timeout_seconds}s +
+ + + + +
+
+ onGrants(item)} + /> +
+
+ ); +} diff --git a/ui/web/src/pages/packages/packages-page.tsx b/ui/web/src/pages/packages/packages-page.tsx index 484b7089..4a6cfa58 100644 --- a/ui/web/src/pages/packages/packages-page.tsx +++ b/ui/web/src/pages/packages/packages-page.tsx @@ -1,24 +1,85 @@ -import { useState } from "react"; +import { lazy, Suspense } from "react"; +import { useSearchParams } from "react-router"; import { useTranslation } from "react-i18next"; -import { RefreshCw, Loader2, Trash2, Download, CheckCircle2, XCircle, AlertTriangle } from "lucide-react"; +import { RefreshCw } from "lucide-react"; import { PageHeader } from "@/components/shared/page-header"; -import { ConfirmDialog } from "@/components/shared/confirm-dialog"; -import { Alert, AlertDescription, AlertTitle } from "@/components/ui/alert"; +import { ErrorBoundary } from "@/components/shared/error-boundary"; import { Button } from "@/components/ui/button"; -import { usePackages, type PackageInfo } from "./hooks/use-packages"; +import { Tabs, TabsList, TabsTrigger, TabsContent } from "@/components/ui/tabs"; +import { useAuthStore } from "@/stores/use-auth-store"; +import { usePackages } from "./hooks/use-packages"; import { usePackageRuntimes } from "./hooks/use-package-runtimes"; -import { GitHubBinariesSection } from "./github-binaries-section"; +import { RuntimesStickyHeader } from "./runtimes-sticky-header"; -type ActionStatus = "idle" | "loading" | "success" | "error"; +// --- Lazy tab bodies (each is a separate chunk) --- +const SystemPackagesTab = lazy(() => + import("./tabs/system-packages-tab").then((m) => ({ default: m.SystemPackagesTab })) +); +const PythonPackagesTab = lazy(() => + import("./tabs/python-packages-tab").then((m) => ({ default: m.PythonPackagesTab })) +); +const NodePackagesTab = lazy(() => + import("./tabs/node-packages-tab").then((m) => ({ default: m.NodePackagesTab })) +); +const GithubBinariesTab = lazy(() => + import("./tabs/github-binaries-tab").then((m) => ({ default: m.GithubBinariesTab })) +); +const CliCredentialsTab = lazy(() => + import("./tabs/cli-credentials-tab").then((m) => ({ default: m.CliCredentialsTab })) +); + +// --- Permission helper (mirrors require-role.tsx logic) --- +function hasMinRole(role: string, minRole: string): boolean { + const levels: Record = { owner: 4, admin: 3, operator: 2, viewer: 1 }; + return (levels[role] ?? 0) >= (levels[minRole] ?? 0); +} + +// --- Valid tab ids --- +const VALID_TABS = ["system", "python", "node", "github", "cli-credentials"] as const; +type TabId = (typeof VALID_TABS)[number]; + +function isValidTab(v: string | null): v is TabId { + return VALID_TABS.includes(v as TabId); +} + +// --- Tab fallback skeleton --- +function TabLoader() { + return ( +
+ +
+ ); +} export function PackagesPage() { const { t } = useTranslation("packages"); - const { packages, loading, refresh, installPackage, uninstallPackage } = usePackages(); - const { runtimes, loading: runtimesLoading, refresh: refreshRuntimes } = usePackageRuntimes(); - const hasMissingRuntimes = (runtimes?.runtimes?.some((rt) => !rt.available)) ?? false; + const [searchParams, setSearchParams] = useSearchParams(); + const { refresh } = usePackages(); + const { refresh: refreshRuntimes } = usePackageRuntimes(); + const role = useAuthStore((s) => s.role); + const isAdmin = hasMinRole(role, "admin"); + + // Validate tab param — fall back to "system" for unknown values + const rawTab = searchParams.get("tab"); + const activeTab: TabId = + isValidTab(rawTab) + ? // Non-admin trying to reach cli-credentials directly via URL → fall back + rawTab === "cli-credentials" && !isAdmin + ? "system" + : rawTab + : "system"; + + function handleTabChange(next: string) { + // Functional form preserves any other existing query params + setSearchParams((prev) => { + const updated = new URLSearchParams(prev); + updated.set("tab", next); + return updated; + }); + } return ( -
+
{ refresh(); refreshRuntimes(); }} - disabled={loading || runtimesLoading} > - + {t("actions.refresh", { defaultValue: "Refresh" })} } /> - {/* Runtimes Section */} -
-

{t("runtimes.title")}

- - - - {t("runtimes.scopeTitle")} - - -

{t("runtimes.scopeDesc")}

- {hasMissingRuntimes &&

{t("runtimes.minimalImageHint")}

} - - -
- {runtimes?.runtimes?.map((rt) => ( -
-
- {rt.name} - {rt.available ? ( - - ) : ( - - )} -
- {rt.version && ( -

{rt.version}

- )} - {!rt.available && ( -

{t("runtimes.missingInContainer")}

- )} -
- ))} + {/* Runtimes always-visible strip */} + + + {/* Tabs */} + + {/* Tab list — horizontal scroll on mobile */} +
+ + {t("tabs.system", { defaultValue: "System" })} + {t("tabs.python", { defaultValue: "Python" })} + {t("tabs.node", { defaultValue: "Node" })} + {t("tabs.github", { defaultValue: "GitHub" })} + {/* CLI Credentials tab: visible only to admins */} + {isAdmin && ( + + {t("tabs.cliCredentials", { defaultValue: "CLI Credentials" })} + + )} +
-
- {/* Package Sections */} - installPackage(pkg, t)} - onUninstall={(pkg) => uninstallPackage(pkg, t)} - /> + {/* Tab bodies — each isolated in its own ErrorBoundary */} + + + }> + + + + - installPackage(`pip:${pkg}`, t)} - onUninstall={(pkg) => uninstallPackage(`pip:${pkg}`, t)} - /> + + + }> + + + + - installPackage(`npm:${pkg}`, t)} - onUninstall={(pkg) => uninstallPackage(`npm:${pkg}`, t)} - /> + + + }> + + + + - installPackage(pkg, t)} - onUninstall={(pkg) => uninstallPackage(pkg, t)} - /> + + + }> + + + + + + {/* CLI Credentials: gate rendered body — direct URL by non-admin must NOT reach panel */} + + + }> + {isAdmin ? ( + + ) : ( +
+ {t("tabs.adminOnly", { defaultValue: "Admin access required." })} +
+ )} + + + +
); } - -interface PackageSectionProps { - title: string; - placeholder: string; - packages: PackageInfo[] | null | undefined; - loading: boolean; - onInstall: (pkg: string) => Promise<{ ok: boolean }>; - onUninstall: (pkg: string) => Promise<{ ok: boolean }>; -} - -function PackageSection({ title, placeholder, packages, loading, onInstall, onUninstall }: PackageSectionProps) { - const { t } = useTranslation("packages"); - const [input, setInput] = useState(""); - const [installStatus, setInstallStatus] = useState("idle"); - const [actionStatuses, setActionStatuses] = useState>({}); - const [uninstallTarget, setUninstallTarget] = useState(null); - - async function handleInstall() { - const pkg = input.trim(); - if (!pkg) return; - setInstallStatus("loading"); - const res = await onInstall(pkg); - if (res.ok) { - setInstallStatus("success"); - setInput(""); - setTimeout(() => setInstallStatus("idle"), 2000); - } else { - setInstallStatus("error"); - setTimeout(() => setInstallStatus("idle"), 3000); - } - } - - async function handleUninstall(name: string) { - setActionStatuses((s) => ({ ...s, [name]: "loading" })); - const res = await onUninstall(name); - if (res.ok) { - setActionStatuses((s) => ({ ...s, [name]: "success" })); - setTimeout(() => setActionStatuses((s) => ({ ...s, [name]: "idle" })), 2000); - } else { - setActionStatuses((s) => ({ ...s, [name]: "error" })); - setTimeout(() => setActionStatuses((s) => ({ ...s, [name]: "idle" })), 3000); - } - } - - return ( -
-

{title}

- - {/* Install input */} -
- setInput(e.target.value)} - onKeyDown={(e) => e.key === "Enter" && handleInstall()} - disabled={installStatus === "loading"} - /> - -
- - {/* Package table */} -
- - - - - - - - - - {loading && !packages ? ( - - - - ) : !packages?.length ? ( - - - - ) : ( - packages.map((pkg) => { - const status = actionStatuses[pkg.name] ?? "idle"; - return ( - - - - - - ); - }) - )} - -
{t("table.name")}{t("table.version")}{t("table.actions")}
- -
- {t("table.empty")} -
{pkg.name}{pkg.version} - {status === "success" ? ( - - ) : ( - - )} -
-
- - setUninstallTarget(null)} - title={t("confirmUninstall.title")} - description={t("confirmUninstall.description", { name: uninstallTarget })} - confirmLabel={t("actions.uninstall")} - variant="destructive" - onConfirm={async () => { - if (uninstallTarget) { - await handleUninstall(uninstallTarget); - setUninstallTarget(null); - } - }} - /> -
- ); -} diff --git a/ui/web/src/pages/packages/runtimes-sticky-header.tsx b/ui/web/src/pages/packages/runtimes-sticky-header.tsx new file mode 100644 index 00000000..f5c35481 --- /dev/null +++ b/ui/web/src/pages/packages/runtimes-sticky-header.tsx @@ -0,0 +1,53 @@ +import { useTranslation } from "react-i18next"; +import { RefreshCw, CheckCircle2, XCircle } from "lucide-react"; +import { Button } from "@/components/ui/button"; +import { usePackageRuntimes } from "./hooks/use-package-runtimes"; + +/** + * RuntimesStickyHeader — compact horizontal runtime status strip. + * Shown above the tabs list and stays visible when switching tabs. + */ +export function RuntimesStickyHeader() { + const { t } = useTranslation("packages"); + const { runtimes, loading, refresh } = usePackageRuntimes(); + + if (!runtimes?.runtimes?.length && !loading) return null; + + return ( +
+ + {t("runtimes.title")}: + +
+ {runtimes?.runtimes?.map((rt) => ( + + {rt.available ? ( + + ) : ( + + )} + {rt.name} + {rt.version && {rt.version}} + + ))} +
+ +
+ ); +} diff --git a/ui/web/src/pages/packages/tabs/cli-credentials-tab.tsx b/ui/web/src/pages/packages/tabs/cli-credentials-tab.tsx new file mode 100644 index 00000000..ff66aa07 --- /dev/null +++ b/ui/web/src/pages/packages/tabs/cli-credentials-tab.tsx @@ -0,0 +1,9 @@ +import { CliCredentialsPanel } from "@/pages/cli-credentials/cli-credentials-panel"; + +// TODO(phase-8): Row-level agent_grants_summary chips will render here +// inside the CliCredentialsPanel table rows once Phase 8 is implemented. + +/** CLI Credentials tab body — mounts the shared panel extracted from cli-credentials-page. */ +export function CliCredentialsTab() { + return ; +} diff --git a/ui/web/src/pages/packages/tabs/github-binaries-tab.tsx b/ui/web/src/pages/packages/tabs/github-binaries-tab.tsx new file mode 100644 index 00000000..87048ffa --- /dev/null +++ b/ui/web/src/pages/packages/tabs/github-binaries-tab.tsx @@ -0,0 +1,17 @@ +import { useTranslation } from "react-i18next"; +import { usePackages } from "../hooks/use-packages"; +import { GitHubBinariesSection } from "../github-binaries-section"; + +/** Thin wrapper — delegates all rendering to the shared GitHubBinariesSection component. */ +export function GithubBinariesTab() { + const { t } = useTranslation("packages"); + const { packages, installPackage, uninstallPackage } = usePackages(); + + return ( + installPackage(pkg, t as (key: string, opts?: Record) => string)} + onUninstall={(pkg) => uninstallPackage(pkg, t as (key: string, opts?: Record) => string)} + /> + ); +} diff --git a/ui/web/src/pages/packages/tabs/node-packages-tab.tsx b/ui/web/src/pages/packages/tabs/node-packages-tab.tsx new file mode 100644 index 00000000..4b2e45dd --- /dev/null +++ b/ui/web/src/pages/packages/tabs/node-packages-tab.tsx @@ -0,0 +1,148 @@ +import { useState } from "react"; +import { useTranslation } from "react-i18next"; +import { Loader2, Download, Trash2, CheckCircle2 } from "lucide-react"; +import { Button } from "@/components/ui/button"; +import { ConfirmDialog } from "@/components/shared/confirm-dialog"; +import { usePackages, type PackageInfo } from "../hooks/use-packages"; + +type ActionStatus = "idle" | "loading" | "success" | "error"; + +export function NodePackagesTab() { + const { t } = useTranslation("packages"); + const { packages, loading, installPackage, uninstallPackage } = usePackages(); + + return ( + installPackage(`npm:${pkg}`, t)} + onUninstall={(pkg) => uninstallPackage(`npm:${pkg}`, t)} + /> + ); +} + +interface PackageSectionBodyProps { + title: string; + placeholder: string; + packages: PackageInfo[] | null | undefined; + loading: boolean; + onInstall: (pkg: string) => Promise<{ ok: boolean }>; + onUninstall: (pkg: string) => Promise<{ ok: boolean }>; +} + +function PackageSectionBody({ title, placeholder, packages, loading, onInstall, onUninstall }: PackageSectionBodyProps) { + const { t } = useTranslation("packages"); + const [input, setInput] = useState(""); + const [installStatus, setInstallStatus] = useState("idle"); + const [actionStatuses, setActionStatuses] = useState>({}); + const [uninstallTarget, setUninstallTarget] = useState(null); + + async function handleInstall() { + const pkg = input.trim(); + if (!pkg) return; + setInstallStatus("loading"); + const res = await onInstall(pkg); + if (res.ok) { + setInstallStatus("success"); + setInput(""); + setTimeout(() => setInstallStatus("idle"), 2000); + } else { + setInstallStatus("error"); + setTimeout(() => setInstallStatus("idle"), 3000); + } + } + + async function handleUninstall(name: string) { + setActionStatuses((s) => ({ ...s, [name]: "loading" })); + const res = await onUninstall(name); + if (res.ok) { + setActionStatuses((s) => ({ ...s, [name]: "success" })); + setTimeout(() => setActionStatuses((s) => ({ ...s, [name]: "idle" })), 2000); + } else { + setActionStatuses((s) => ({ ...s, [name]: "error" })); + setTimeout(() => setActionStatuses((s) => ({ ...s, [name]: "idle" })), 3000); + } + } + + return ( +
+

{title}

+ +
+ setInput(e.target.value)} + onKeyDown={(e) => e.key === "Enter" && handleInstall()} + disabled={installStatus === "loading"} + /> + +
+ +
+ + + + + + + + + + {loading && !packages ? ( + + ) : !packages?.length ? ( + + ) : ( + packages.map((pkg) => { + const status = actionStatuses[pkg.name] ?? "idle"; + return ( + + + + + + ); + }) + )} + +
{t("table.name")}{t("table.version")}{t("table.actions")}
{t("table.empty")}
{pkg.name}{pkg.version} + {status === "success" ? ( + + ) : ( + + )} +
+
+ + setUninstallTarget(null)} + title={t("confirmUninstall.title")} + description={t("confirmUninstall.description", { name: uninstallTarget })} + confirmLabel={t("actions.uninstall")} + variant="destructive" + onConfirm={async () => { + if (uninstallTarget) { + await handleUninstall(uninstallTarget); + setUninstallTarget(null); + } + }} + /> +
+ ); +} diff --git a/ui/web/src/pages/packages/tabs/python-packages-tab.tsx b/ui/web/src/pages/packages/tabs/python-packages-tab.tsx new file mode 100644 index 00000000..856b3a3d --- /dev/null +++ b/ui/web/src/pages/packages/tabs/python-packages-tab.tsx @@ -0,0 +1,148 @@ +import { useState } from "react"; +import { useTranslation } from "react-i18next"; +import { Loader2, Download, Trash2, CheckCircle2 } from "lucide-react"; +import { Button } from "@/components/ui/button"; +import { ConfirmDialog } from "@/components/shared/confirm-dialog"; +import { usePackages, type PackageInfo } from "../hooks/use-packages"; + +type ActionStatus = "idle" | "loading" | "success" | "error"; + +export function PythonPackagesTab() { + const { t } = useTranslation("packages"); + const { packages, loading, installPackage, uninstallPackage } = usePackages(); + + return ( + installPackage(`pip:${pkg}`, t)} + onUninstall={(pkg) => uninstallPackage(`pip:${pkg}`, t)} + /> + ); +} + +interface PackageSectionBodyProps { + title: string; + placeholder: string; + packages: PackageInfo[] | null | undefined; + loading: boolean; + onInstall: (pkg: string) => Promise<{ ok: boolean }>; + onUninstall: (pkg: string) => Promise<{ ok: boolean }>; +} + +function PackageSectionBody({ title, placeholder, packages, loading, onInstall, onUninstall }: PackageSectionBodyProps) { + const { t } = useTranslation("packages"); + const [input, setInput] = useState(""); + const [installStatus, setInstallStatus] = useState("idle"); + const [actionStatuses, setActionStatuses] = useState>({}); + const [uninstallTarget, setUninstallTarget] = useState(null); + + async function handleInstall() { + const pkg = input.trim(); + if (!pkg) return; + setInstallStatus("loading"); + const res = await onInstall(pkg); + if (res.ok) { + setInstallStatus("success"); + setInput(""); + setTimeout(() => setInstallStatus("idle"), 2000); + } else { + setInstallStatus("error"); + setTimeout(() => setInstallStatus("idle"), 3000); + } + } + + async function handleUninstall(name: string) { + setActionStatuses((s) => ({ ...s, [name]: "loading" })); + const res = await onUninstall(name); + if (res.ok) { + setActionStatuses((s) => ({ ...s, [name]: "success" })); + setTimeout(() => setActionStatuses((s) => ({ ...s, [name]: "idle" })), 2000); + } else { + setActionStatuses((s) => ({ ...s, [name]: "error" })); + setTimeout(() => setActionStatuses((s) => ({ ...s, [name]: "idle" })), 3000); + } + } + + return ( +
+

{title}

+ +
+ setInput(e.target.value)} + onKeyDown={(e) => e.key === "Enter" && handleInstall()} + disabled={installStatus === "loading"} + /> + +
+ +
+ + + + + + + + + + {loading && !packages ? ( + + ) : !packages?.length ? ( + + ) : ( + packages.map((pkg) => { + const status = actionStatuses[pkg.name] ?? "idle"; + return ( + + + + + + ); + }) + )} + +
{t("table.name")}{t("table.version")}{t("table.actions")}
{t("table.empty")}
{pkg.name}{pkg.version} + {status === "success" ? ( + + ) : ( + + )} +
+
+ + setUninstallTarget(null)} + title={t("confirmUninstall.title")} + description={t("confirmUninstall.description", { name: uninstallTarget })} + confirmLabel={t("actions.uninstall")} + variant="destructive" + onConfirm={async () => { + if (uninstallTarget) { + await handleUninstall(uninstallTarget); + setUninstallTarget(null); + } + }} + /> +
+ ); +} diff --git a/ui/web/src/pages/packages/tabs/system-packages-tab.tsx b/ui/web/src/pages/packages/tabs/system-packages-tab.tsx new file mode 100644 index 00000000..d914deca --- /dev/null +++ b/ui/web/src/pages/packages/tabs/system-packages-tab.tsx @@ -0,0 +1,148 @@ +import { useState } from "react"; +import { useTranslation } from "react-i18next"; +import { Loader2, Download, Trash2, CheckCircle2 } from "lucide-react"; +import { Button } from "@/components/ui/button"; +import { ConfirmDialog } from "@/components/shared/confirm-dialog"; +import { usePackages, type PackageInfo } from "../hooks/use-packages"; + +type ActionStatus = "idle" | "loading" | "success" | "error"; + +export function SystemPackagesTab() { + const { t } = useTranslation("packages"); + const { packages, loading, installPackage, uninstallPackage } = usePackages(); + + return ( + installPackage(pkg, t)} + onUninstall={(pkg) => uninstallPackage(pkg, t)} + /> + ); +} + +interface PackageSectionBodyProps { + title: string; + placeholder: string; + packages: PackageInfo[] | null | undefined; + loading: boolean; + onInstall: (pkg: string) => Promise<{ ok: boolean }>; + onUninstall: (pkg: string) => Promise<{ ok: boolean }>; +} + +function PackageSectionBody({ title, placeholder, packages, loading, onInstall, onUninstall }: PackageSectionBodyProps) { + const { t } = useTranslation("packages"); + const [input, setInput] = useState(""); + const [installStatus, setInstallStatus] = useState("idle"); + const [actionStatuses, setActionStatuses] = useState>({}); + const [uninstallTarget, setUninstallTarget] = useState(null); + + async function handleInstall() { + const pkg = input.trim(); + if (!pkg) return; + setInstallStatus("loading"); + const res = await onInstall(pkg); + if (res.ok) { + setInstallStatus("success"); + setInput(""); + setTimeout(() => setInstallStatus("idle"), 2000); + } else { + setInstallStatus("error"); + setTimeout(() => setInstallStatus("idle"), 3000); + } + } + + async function handleUninstall(name: string) { + setActionStatuses((s) => ({ ...s, [name]: "loading" })); + const res = await onUninstall(name); + if (res.ok) { + setActionStatuses((s) => ({ ...s, [name]: "success" })); + setTimeout(() => setActionStatuses((s) => ({ ...s, [name]: "idle" })), 2000); + } else { + setActionStatuses((s) => ({ ...s, [name]: "error" })); + setTimeout(() => setActionStatuses((s) => ({ ...s, [name]: "idle" })), 3000); + } + } + + return ( +
+

{title}

+ +
+ setInput(e.target.value)} + onKeyDown={(e) => e.key === "Enter" && handleInstall()} + disabled={installStatus === "loading"} + /> + +
+ +
+ + + + + + + + + + {loading && !packages ? ( + + ) : !packages?.length ? ( + + ) : ( + packages.map((pkg) => { + const status = actionStatuses[pkg.name] ?? "idle"; + return ( + + + + + + ); + }) + )} + +
{t("table.name")}{t("table.version")}{t("table.actions")}
{t("table.empty")}
{pkg.name}{pkg.version} + {status === "success" ? ( + + ) : ( + + )} +
+
+ + setUninstallTarget(null)} + title={t("confirmUninstall.title")} + description={t("confirmUninstall.description", { name: uninstallTarget })} + confirmLabel={t("actions.uninstall")} + variant="destructive" + onConfirm={async () => { + if (uninstallTarget) { + await handleUninstall(uninstallTarget); + setUninstallTarget(null); + } + }} + /> +
+ ); +} diff --git a/ui/web/src/routes.tsx b/ui/web/src/routes.tsx index 5a6478e1..c8c5511c 100644 --- a/ui/web/src/routes.tsx +++ b/ui/web/src/routes.tsx @@ -96,9 +96,6 @@ const ContactsPage = lazyWithRetry(() => const ActivityPage = lazyWithRetry(() => import("@/pages/activity/activity-page").then((m) => ({ default: m.ActivityPage })), ); -const CliCredentialsPage = lazyWithRetry(() => - import("@/pages/cli-credentials/cli-credentials-page").then((m) => ({ default: m.CliCredentialsPage })), -); const ApiKeysPage = lazyWithRetry(() => import("@/pages/api-keys/api-keys-page").then((m) => ({ default: m.ApiKeysPage })), ); @@ -181,7 +178,7 @@ export function AppRoutes() { } /> } /> } /> - } /> + } /> } /> } /> } /> diff --git a/ui/web/src/types/cli-credential.ts b/ui/web/src/types/cli-credential.ts index 0d06317f..a9b03854 100644 --- a/ui/web/src/types/cli-credential.ts +++ b/ui/web/src/types/cli-credential.ts @@ -14,6 +14,11 @@ export interface SecureCLIBinary { updated_at: string; /** Env variable names only (no values); from API for edit form */ env_keys?: string[]; + /** + * Agent grants summary for row chips (Phase 4 API field). + * Absent on older API versions — capability-probe: skip rendering if undefined. + */ + agent_grants_summary?: AgentGrantSummary[]; } export interface CLIPresetEnvVar { @@ -57,6 +62,10 @@ export interface CLIAgentGrant { timeout_seconds: number | null; tips: string | null; enabled: boolean; + /** Whether this grant has an env override (keys present, values encrypted) */ + env_set?: boolean; + /** Env variable names only (no values); populated when env_set=true */ + env_keys?: string[]; created_at: string; updated_at: string; } @@ -68,4 +77,26 @@ export interface CLIAgentGrantInput { timeout_seconds?: number | null; tips?: string | null; enabled?: boolean; + /** + * env_vars semantics — 3-state, all three distinct behaviors (Finding #15): + * + * - **absent / undefined** → keep existing env override (omit from request payload) + * - **null** → clear override; grant falls back to binary-level defaults + * - **`{}` (empty map)** → treated as clear (same as null) — wipes the override + * - **`{K: V, ...}`** → replace the entire env override with this map + * + * Backend: internal/http/secure_cli_agent_grants.go handleUpdate (3-state env_vars branch). + * Keys must match ^[A-Z_][A-Z0-9_]*$ and must not be on the denylist. + */ + env_vars?: Record | null; +} + +/** Summary of a single grant shown in the table row chips (Phase 4 API field). */ +export interface AgentGrantSummary { + grant_id: string; + agent_id: string; + agent_key: string; + name: string; + enabled: boolean; + env_set: boolean; }