fix(security): hard-deny ungranted exec for registered CLI binaries

Closes credential-scope bypass where an agent without a `secure_cli_agent_grants` row could still invoke a registered binary via shell fallthrough and pick up inherited env (`$GH_TOKEN`) or on-disk OAuth state. Registration is now an authorization boundary, not only a credential-injection hint.

- Add `SecureCLIStore.IsRegisteredBinary` on PG + SQLite backends — case-insensitive, tenant-scoped.
- New gate branch in `internal/tools/shell.go` (after normalization, before exec approval) with shell-wrapper unwrap depth 3 (sh/bash/zsh/dash -c, env K=V, nohup, stdbuf, timeout), 2s DB-lookup timeout, fail-CLOSED on error. New log events: `security.credentialed_binary_denied`, `security.credentialed_binary_gate_error`, `security.credentialed_binary_wrapper_too_deep`.
- Env scrubbing on host fall-through (`internal/tools/env_scrub.go`) strips static credential keys (GH_TOKEN, AWS_*, OPENAI_API_KEY, …) plus dynamic keys from tenant's registered binaries; preserves HOME/PATH/TERM/LANG/USER/TZ.
- Subagent ExecTool registration (`cmd/gateway_agents.go` → `buildSubagentToolsRegistry`) now receives the same `SecureCLIStore` — parent can't delegate to a child to bypass the gate.
- Binary names lowercased on Create/Update for symmetry with case-insensitive lookup.
- Thread `pgStores.SecureCLI` into `setupSubagents` in `cmd/gateway.go`.
- 25+ unit tests (gate, env-scrub, SQLite IsRegisteredBinary) + 5 PG integration tests (deny-ungranted, allow-granted, unregistered-unchanged, is_global-not-denied regression, shell-wrapper-bypass denied). All green.
This commit is contained in:
viettranx committed 2026-04-17 19:02:53 +07:00
1 parent 92dbb12c84
commit 7e2e66c095
16 files changed
+1588 -16

No files matched your search

+41
View File
@@ -9,6 +9,7 @@ import (
"errors"
"fmt"
"log/slog"
"strings"
"time"
"github.com/google/uuid"
@@ -43,6 +44,10 @@ func (s *SQLiteSecureCLIStore) Create(ctx context.Context, b *store.SecureCLIBin
b.ID = store.GenNewID()
}
// Normalize binary_name to lowercase so IsRegisteredBinary (which lowercases
// the candidate) can match. Admin entering "Gh" becomes "gh".
b.BinaryName = strings.ToLower(strings.TrimSpace(b.BinaryName))
var envBytes []byte
if len(b.EncryptedEnv) > 0 && s.encKey != "" {
encrypted, err := crypto.Encrypt(string(b.EncryptedEnv), s.encKey)
@@ -191,6 +196,13 @@ func (s *SQLiteSecureCLIStore) Update(ctx context.Context, id uuid.UUID, updates
}
}
// Normalize binary_name to lowercase if updated (parity with Create).
if nameVal, ok := updates["binary_name"]; ok {
if nameStr, isStr := nameVal.(string); isStr {
updates["binary_name"] = strings.ToLower(strings.TrimSpace(nameStr))
}
}
// Encrypt env if present in updates
if envVal, ok := updates["encrypted_env"]; ok {
if envStr, isStr := envVal.(string); isStr && envStr != "" && s.encKey != "" {
@@ -410,6 +422,35 @@ func (s *SQLiteSecureCLIStore) ListEnabled(ctx context.Context) ([]store.SecureC
return s.scanRows(rows)
}
// IsRegisteredBinary reports whether a binary requires a grant (is_global=0)
// and is enabled for the current tenant. See interface godoc for rationale.
func (s *SQLiteSecureCLIStore) IsRegisteredBinary(ctx context.Context, binaryName string) (bool, error) {
name := strings.ToLower(strings.TrimSpace(binaryName))
if name == "" {
return false, nil
}
query := `SELECT EXISTS(
SELECT 1 FROM secure_cli_binaries
WHERE LOWER(binary_name) = ?
AND enabled = 1
AND is_global = 0`
args := []any{name}
if !store.IsCrossTenant(ctx) {
tid := store.TenantIDFromContext(ctx)
if tid == uuid.Nil {
return false, nil
}
query += ` AND tenant_id = ?`
args = append(args, tid)
}
query += `)`
var exists bool
if err := s.db.QueryRowContext(ctx, query, args...).Scan(&exists); err != nil {
return false, err
}
return exists, nil
}
// ListForAgent returns all CLIs accessible by an agent (global + granted),
// with grant overrides merged into the returned configs.
func (s *SQLiteSecureCLIStore) ListForAgent(ctx context.Context, agentID uuid.UUID) ([]store.SecureCLIBinary, error) {