internal/creds/creds.go at b663803169, reviewed by codex:gpt-6-astra, verified by claude-code:claude-fable-5-1.
On Linux with HOME, XDG_CONFIG_HOME and CORRAL_CREDS_DIR unset, Open rejects an explicitly configured environment credential because it requires the fallback file directory before constructing the environment backend.
#!/bin/sh
set -eu
d=$(mktemp -d ./creds-review-XXXXXX)
trap 'rm -rf "$d"' EXIT HUP INT TERM
cat > "$d/main.go" <<'EOF'
package main
import (
"fmt"
"os"
"runtime"
"github.com/pdbethke/corralai/internal/creds"
"github.com/zalando/go-keyring"
)
func main() {
if runtime.GOOS != "linux" { fmt.Println("reproduction requires Linux"); os.Exit(1) }
keyring.MockInit()
for _, name := range []string{"HOME", "XDG_CONFIG_HOME", "CORRAL_CREDS_DIR"} {
if err := os.Unsetenv(name); err != nil { panic(err) }
}
if err := os.Setenv("OPENAI_API_KEY", "fake-env-credential"); err != nil { panic(err) }
_, configErr := os.UserConfigDir()
if configErr == nil { fmt.Println("configuration precondition not met"); os.Exit(1) }
store, err := creds.Open()
if store != nil || err == nil || err.Error() != configErr.Error() {
fmt.Printf("defect not demonstrated: store=%v error=%v\n", store, err)
os.Exit(1)
}
fmt.Printf("OPENAI_API_KEY is present: %t\n", os.Getenv("OPENAI_API_KEY") != "")
fmt.Printf("Environment credential cannot be resolved: Open returned nil store and error: %v\n", err)
}
EOF
GOPROXY=off GOSUMDB=off GOTOOLCHAIN=local go run "$d/main.go"OPENAI_API_KEY is present: true Environment credential cannot be resolved: Open returned nil store and error: neither $XDG_CONFIG_HOME nor $HOME are defined
I tried to refute on three fronts and failed on each. (1) Ordering: credsDir() is called first (line 213) and any error is returned immediately (lines 214-216); envBackend{} is only appended at line 217, so there is no path by which an env-only credential survives a credsDir failure. (2) Fallback inside credsDir: lines 184-193 consult only CORRAL_CREDS_DIR and os.UserConfigDir, which on Linux returns the exact error the reviewer recorded when both XDG_CONFIG_HOME and HOME are unset; there is no third fallback (e.g. os.TempDir or an empty-dir sentinel). (3) Intent: the comment on Open (line 211) states a missing store is not an error, and newAgeFileLazy defers identity resolution precisely so the file tier need not exist at Open time, which means the directory could have been resolved lazily too; the eager failure is therefore inconsistent with the package's own stated contract rather than a documented tradeoff. The reviewer's script targets the named input and its output follows necessarily from this code, so the finding stands as written.
confirmed by pdbethke — real, reproduced, recheck on main b6638031: still-reproduces. Open calls credsDir first and returns its error before envBackend is in the chain (creds.go:213-217), so with HOME, XDG_CONFIG_HOME and CORRAL_CREDS_DIR unset an env-only key cannot be resolved — contrary to Open's own 'a missing store is not an error'. Uncommon environment (HOME-less containers/CI), hence real but narrow.
ref b3e2ec973c76be118e706bbb8d371b45f17ec94f53859159e0b6d8bbb9f5f736#R1
Recorded so that "found nothing here" can be told apart from "never looked here".
Finding 1 identifies an unnecessary dependency on filesystem configuration that blocks environment-only credentials. Review was read-only; the reproduction script is supplied for execution by the reviewer harness.
The single finding is accurate and I could not refute it. Open at internal/creds/creds.go:213-216 calls credsDir and returns its error before the environment backend is ever placed in the chain (line 217), so with CORRAL_CREDS_DIR, XDG_CONFIG_HOME and HOME all unset, os.UserConfigDir fails and Open returns nil even though envBackend.get would have resolved OPENAI_API_KEY from the process environment without touching the filesystem. The reviewer's script exercises exactly the input the claim names and its recorded output matches what the code must produce. The only mitigating points are that CORRAL_CREDS_DIR is an explicit escape hatch and that a HOME-less process is uncommon; neither narrows the claim, and the doc comment on Open (line 211, 'a missing store is not an error') argues the reviewer's way, since the intended contract is that an unconfigured file store should not block resolution. The items the reviewer listed as sound (Get, PresentProviders, Set, Remove, List, Redact, lazy age identity) also read correctly against the source.