From ec05c80e90fd5ad4c59858c4a24472a2ac38f29d Mon Sep 17 00:00:00 2001 From: viettranx Date: Thu, 2 Apr 2026 14:56:49 +0700 Subject: [PATCH] fix(secure-cli): resolve ambiguous column in LookupByBinary JOIN query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LookupByBinary uses LEFT JOIN with secure_cli_user_credentials but SELECT columns lacked table alias prefix, causing PostgreSQL error: "column reference 'id' is ambiguous (SQLSTATE 42702)" This silently broke ALL credentialed CLI exec — commands fell through to regular shell exec without injected env vars. Fix: use b.-prefixed column names for JOIN queries. Also add diagnostic logging to lookupCredentialedBinary for future debugging. --- internal/store/pg/secure_cli.go | 8 +++++++- internal/tools/credentialed_exec.go | 9 ++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/internal/store/pg/secure_cli.go b/internal/store/pg/secure_cli.go index 1d88d4b2..96c882d9 100644 --- a/internal/store/pg/secure_cli.go +++ b/internal/store/pg/secure_cli.go @@ -26,6 +26,11 @@ func NewPGSecureCLIStore(db *sql.DB, encryptionKey string) *PGSecureCLIStore { const secureCLISelectCols = `id, binary_name, binary_path, description, encrypted_env, deny_args, deny_verbose, timeout_seconds, tips, agent_id, enabled, created_by, created_at, updated_at` +// secureCLISelectColsAliased is the same as secureCLISelectCols but prefixed with table alias "b." +// Required for LookupByBinary which uses LEFT JOIN (ambiguous column names without prefix). +const secureCLISelectColsAliased = `b.id, b.binary_name, b.binary_path, b.description, b.encrypted_env, + b.deny_args, b.deny_verbose, b.timeout_seconds, b.tips, b.agent_id, b.enabled, b.created_by, b.created_at, b.updated_at` + func (s *PGSecureCLIStore) Create(ctx context.Context, b *store.SecureCLIBinary) error { if err := store.ValidateUserID(b.CreatedBy); err != nil { return err @@ -271,7 +276,8 @@ func (s *PGSecureCLIStore) LookupByBinary(ctx context.Context, binaryName string } // Build query with optional LEFT JOIN for per-user credentials. - selectCols := secureCLISelectCols + // Use aliased columns (b.) to avoid ambiguous column reference with JOIN. + selectCols := secureCLISelectColsAliased joinClause := "" if userID != "" { selectCols += ", uc.encrypted_env AS user_env" diff --git a/internal/tools/credentialed_exec.go b/internal/tools/credentialed_exec.go index 1fb41363..7d818d48 100644 --- a/internal/tools/credentialed_exec.go +++ b/internal/tools/credentialed_exec.go @@ -281,6 +281,7 @@ func formatCredentialedResult(binary string, args []string, // Returns the credential config and parsed args, or nil if not credentialed. func (t *ExecTool) lookupCredentialedBinary(ctx context.Context, command string) (*store.SecureCLIBinary, string, []string) { if t.secureCLIStore == nil { + slog.Warn("secure_cli.lookup: store is nil, skipping credentialed exec", "command", command) return nil, "", nil } binary, args, err := parseCommandBinary(command) @@ -296,9 +297,15 @@ func (t *ExecTool) lookupCredentialedBinary(ctx context.Context, command string) // Pass userID for per-user credential resolution (LEFT JOIN, zero extra queries). userID := store.UserIDFromContext(ctx) cred, err := t.secureCLIStore.LookupByBinary(ctx, binary, agentIDPtr, userID) - if err != nil || cred == nil { + if err != nil { + slog.Warn("secure_cli.lookup: query failed", "binary", binary, "agent_id", agentID, "error", err) return nil, "", nil } + if cred == nil { + slog.Warn("secure_cli.lookup: no credential found", "binary", binary, "agent_id", agentID) + return nil, "", nil + } + slog.Info("secure_cli.lookup: found credential", "binary", binary, "cred_id", cred.ID, "env_size", len(cred.EncryptedEnv)) return cred, binary, args }