Skip to content

Commit 0abc56b

Browse files
committed
chore(agents): Add warnings on running config creation commands without proper precautions, update vulnerable Go version
1 parent b77ff90 commit 0abc56b

7 files changed

Lines changed: 292 additions & 15 deletions

File tree

‎AGENTS.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ Skipping these steps leads to pattern violations, broken dual-mode, and pricing
3434
- **Interactive hint bar** — every direct `prompter.Select(...)` outside the wizard engine must pass `tui.WithShowHints(true)` (and the equivalent option on `MultiSelect`) so the prompt renders its key hints below the choices. Wizard steps are exempt — the composite already renders the hint bar
3535
- **Ctrl+C exits immediately, no confirmation** — use `cmdutil.IsPromptCancel(err)` to detect either Esc or Ctrl+C and return cleanly. When a flow needs different behavior per key (e.g. a "Back to list / Exit" gate where Esc means back), split with `IsPromptInterrupt(err)` (Ctrl+C) and `IsPromptBack(err)` (Esc). Never show an "Exit?" confirmation dialog — Unix users expect Ctrl+C to be terminal
3636
- **`pkg/` is in-tree** — the TUI core (`pkg/tui*`), `pkg/log`, `pkg/version` are part of this repo; edit them directly
37+
- **Never run the binary against the real config dir** — every manual, scripted, or pty-driven `./bin/verda` run sets `VERDA_HOME=$(mktemp -d)` (or uses `make run.sandbox`). `VERDA_SHARED_CREDENTIALS_FILE` is not enough; it leaves `config.yaml` and `EnsureVerdaDir` pointing at the real `~/.verda`. Driving `auth login` to completion once overwrote a developer's real credentials, and a clobbered client secret cannot be recovered from the API. See CLAUDE.md § "NEVER run the binary against the real config dir"
3738
- **Commit only when asked** — don't auto-commit
3839

3940
## Risky Areas — Slow Down
@@ -44,6 +45,7 @@ Skipping these steps leads to pattern violations, broken dual-mode, and pricing
4445
| `options/credentials.go` | Break auth = break everything | Test all profiles, expired tokens |
4546
| Agent mode (`--agent`) | JSON contract change = break downstream | Check structured error format |
4647
| Wizard steps | Step ordering, cache invalidation | Map dependencies before coding |
48+
| Running `auth login` / any binary run | Overwrites the real `~/.verda`; lost secrets are unrecoverable | Set `VERDA_HOME=$(mktemp -d)` first, always |
4749

4850
## Done Checklist
4951

@@ -54,5 +56,6 @@ Skipping these steps leads to pattern violations, broken dual-mode, and pricing
5456
- [ ] Interactive and non-interactive modes both work
5557
- [ ] Interactive Selects pass `tui.WithShowHints(true)` so the hint bar renders
5658
- [ ] No leftover debug code, TODOs, or commented-out blocks
59+
- [ ] Every manual/pty run of the binary set `VERDA_HOME` to a temp dir — the real `~/.verda` is untouched
5760

5861
If `make lint` reports issues, fix them *before* announcing completion. See `CLAUDE.md` § "Go House Style" for the patterns that prevent the common hits (http.NoBody, American spelling, reused constants, rangeValCopy, nilerr annotations, etc.).

‎CLAUDE.md‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,31 @@ If you modified a command, also verify:
174174
- `--agent -o json` mode works (structured output, no TUI)
175175
- `--debug` shows request/response payloads
176176

177+
### NEVER run the binary against the real config dir
178+
179+
Any manual, scripted, or pty-driven run of `./bin/verda` MUST set `VERDA_HOME` to a
180+
throwaway directory:
181+
182+
```bash
183+
VERDA_HOME=$(mktemp -d) ./bin/verda <command> # or: make run.sandbox ARGS="<command>"
184+
```
185+
186+
`VERDA_HOME` (see `options.VerdaDir`) redirects the whole config dir — credentials *and*
187+
`config.yaml`. `VERDA_SHARED_CREDENTIALS_FILE` covers only the credentials file, so
188+
`auth use`, `settings`, and `EnsureVerdaDir` still hit the real `~/.verda`. Use
189+
`VERDA_HOME`.
190+
191+
This is not hypothetical: driving the `auth login` wizard to completion to verify a TUI
192+
fix overwrote a developer's real `~/.verda/credentials` with test values.
193+
`auth login` replaces an existing profile with no warning — the documented re-auth
194+
behavior — and **a client secret cannot be read back from the API, so a clobber is
195+
unrecoverable**. Assume any command may write to the config dir, not just the obviously
196+
auth-shaped ones.
197+
198+
The repo's own suites already do this — copy them, don't hand-roll a harness:
199+
`tests/contract/main_test.go` (`cliEnv` strips every inherited `VERDA_*`, then sets
200+
`VERDA_HOME=t.TempDir()`) and `options/registry_credentials_test.go:168`.
201+
177202
## Other Agents
178203

179204
This repo targets Claude Code and OpenAI Codex. Claude auto-loads this file; Codex auto-loads `AGENTS.md` (execution contract). A `.cursor/rules/main.mdc` pointer exists for Cursor users but is not a primary target — if Cursor drops out of the stack, delete it rather than letting it drift.

‎Makefile‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
OUTPUT_DIR ?= bin
22

3-
.PHONY: all build clean lint lint.fix security test test.integration test-s3-integration fmt changelog changelog.unreleased hooks.install pre-commit help
3+
.PHONY: all build clean run.sandbox lint lint.fix security test test.integration test-s3-integration fmt changelog changelog.unreleased hooks.install pre-commit help
44

55
## Build -------------------------------------------------------------------
66

@@ -14,6 +14,12 @@ build: ## Build the binary into bin/
1414
clean: ## Remove build artifacts
1515
@rm -rf $(OUTPUT_DIR)
1616

17+
# Never drive the binary against the real ~/.verda: auth login replaces a profile
18+
# with no warning, and a clobbered client secret cannot be read back from the API.
19+
# VERDA_HOME redirects the whole config dir; VERDA_SHARED_CREDENTIALS_FILE does not.
20+
run.sandbox: build ## Run the binary against a throwaway config dir, e.g. make run.sandbox ARGS="auth login"
21+
@dir=$$(mktemp -d) && echo "VERDA_HOME=$$dir" && VERDA_HOME=$$dir $(OUTPUT_DIR)/verda $(ARGS)
22+
1723
## Quality -----------------------------------------------------------------
1824

1925
lint: ## Run golangci-lint on all packages

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module github.com/verda-cloud/verda-cli
22

3-
go 1.25.12
3+
go 1.25.13
44

55
require (
66
charm.land/lipgloss/v2 v2.0.2

‎internal/verda-cli/cmd/auth/CLAUDE.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
- `path.go` -- Helpers: `resolveCredentialsFile`, `defaultConfigFilePath`
1313
- `auth_test.go` -- Tests for `writeActiveProfile`, `resolveCredentialsFile`
1414
- `wizard_test.go` -- Wizard flow tests with mock prompter
15+
- `login_test.go` -- Flag-driven write path: new/named profile, merge, re-auth overwrite, 0600, flag-over-env
1516

1617
## Domain-Specific Logic
1718
- Credentials file resolution order: explicit flag > `VERDA_SHARED_CREDENTIALS_FILE` env var > `options.DefaultCredentialsFilePath()`
@@ -28,6 +29,18 @@
2829
- The `selectThemeWizard` pattern of returning `nil` on wizard error (user cancel) is NOT used here -- login returns the wizard error directly.
2930
- `writeActiveProfile` in `use.go` merges into existing config YAML rather than overwriting the whole file.
3031
- `login` creates the `~/.verda/` directory via `options.EnsureVerdaDir()` before saving.
32+
- **Never run `auth login` against the real config dir.** Set `VERDA_HOME` to a temp dir for
33+
any manual or pty-driven run (`make run.sandbox ARGS="auth login"`). Re-running login
34+
replaces an existing profile with no warning -- intentional, it is the re-auth path --
35+
and a client secret cannot be read back from the API, so a clobber is unrecoverable.
36+
`VERDA_SHARED_CREDENTIALS_FILE` alone is insufficient: `EnsureVerdaDir()` resolves
37+
through `VerdaDir()` and would still mkdir the real `~/.verda`. Tests must set
38+
`VERDA_HOME` for the same reason.
39+
- `login_test.go` covers only the flag-driven path. Supplying both `--client-id` and
40+
`--client-secret` is what skips the wizard, so the post-wizard validation gate is
41+
unreachable from a test -- the engine is constructed inline in `RunE`, and a wizard in a
42+
test would start a real `tea.Program` against the developer's stdin. Covering that gate
43+
means injecting the engine.
3144

3245
## Relationships
3346
- `cmdutil.Factory` / `cmdutil.IOStreams` -- standard dependency injection
Lines changed: 216 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,216 @@
1+
// Copyright 2026 Verda Cloud Oy
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package auth
16+
17+
import (
18+
"bytes"
19+
"os"
20+
"path/filepath"
21+
"runtime"
22+
"strings"
23+
"testing"
24+
25+
cmdutil "github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/util"
26+
"github.com/verda-cloud/verda-cli/internal/verda-cli/options"
27+
)
28+
29+
// These tests cover the flag-driven write path only. Supplying both
30+
// --client-id and --client-secret is what skips the wizard (login.go checks
31+
// them with OR), and a wizard here would start a real tea.Program against the
32+
// developer's stdin — a hang when `go test` runs from a terminal. The
33+
// post-wizard "still empty" validation gate is therefore unreachable from a
34+
// test; covering it needs the engine injected rather than constructed inline.
35+
36+
// sandboxHome points the whole config dir at a temp tree. VERDA_HOME, not
37+
// VERDA_SHARED_CREDENTIALS_FILE: login calls options.EnsureVerdaDir, which
38+
// resolves through VerdaDir and would mkdir the developer's real ~/.verda even
39+
// with the credentials path redirected elsewhere.
40+
func sandboxHome(t *testing.T) string {
41+
t.Helper()
42+
dir := t.TempDir()
43+
t.Setenv("VERDA_HOME", dir)
44+
t.Setenv("VERDA_SHARED_CREDENTIALS_FILE", filepath.Join(dir, "credentials"))
45+
return dir
46+
}
47+
48+
func runAuthLoginForTest(t *testing.T, args ...string) error {
49+
t.Helper()
50+
streams := cmdutil.IOStreams{Out: &bytes.Buffer{}, ErrOut: &bytes.Buffer{}}
51+
cmd := NewCmdLogin(cmdutil.NewTestFactory(nil), streams)
52+
cmd.SetArgs(args)
53+
cmd.SetOut(streams.Out)
54+
cmd.SetErr(streams.ErrOut)
55+
cmd.SilenceUsage = true
56+
cmd.SilenceErrors = true
57+
return cmd.Execute()
58+
}
59+
60+
func loadProfile(t *testing.T, path, profile string) *options.SharedCredentials {
61+
t.Helper()
62+
creds, err := options.LoadSharedCredentialsForProfile(path, profile)
63+
if err != nil {
64+
t.Fatalf("LoadSharedCredentialsForProfile(%q, %q): %v", path, profile, err)
65+
}
66+
return creds
67+
}
68+
69+
func TestLoginWritesNewProfile(t *testing.T) {
70+
dir := sandboxHome(t)
71+
path := filepath.Join(dir, "credentials")
72+
73+
if err := runAuthLoginForTest(t, "--client-id", "id-1", "--client-secret", "secret-1"); err != nil {
74+
t.Fatalf("login: %v", err)
75+
}
76+
77+
got := loadProfile(t, path, "default")
78+
if got.ClientID != "id-1" {
79+
t.Errorf("ClientID = %q, want id-1", got.ClientID)
80+
}
81+
if got.ClientSecret != "secret-1" {
82+
t.Errorf("ClientSecret = %q, want secret-1", got.ClientSecret)
83+
}
84+
if got.BaseURL != defaultBaseURL {
85+
t.Errorf("BaseURL = %q, want %q", got.BaseURL, defaultBaseURL)
86+
}
87+
}
88+
89+
// A leaked secret is not recoverable, so the 0600 is load-bearing rather than
90+
// cosmetic. Windows has no mode bits to assert.
91+
func TestLoginRestrictsFilePermissions(t *testing.T) {
92+
if runtime.GOOS == "windows" {
93+
t.Skip("no Unix mode bits on Windows")
94+
}
95+
dir := sandboxHome(t)
96+
path := filepath.Join(dir, "credentials")
97+
98+
if err := runAuthLoginForTest(t, "--client-id", "id", "--client-secret", "secret"); err != nil {
99+
t.Fatalf("login: %v", err)
100+
}
101+
102+
info, err := os.Stat(path)
103+
if err != nil {
104+
t.Fatalf("stat: %v", err)
105+
}
106+
if perm := info.Mode().Perm(); perm != 0o600 {
107+
t.Errorf("mode = %#o, want 0600", perm)
108+
}
109+
}
110+
111+
func TestLoginWritesNamedProfileAndBaseURL(t *testing.T) {
112+
dir := sandboxHome(t)
113+
path := filepath.Join(dir, "credentials")
114+
115+
err := runAuthLoginForTest(t,
116+
"--profile", "staging",
117+
"--base-url", "https://staging-api.verda.com/v1",
118+
"--client-id", "stg-id",
119+
"--client-secret", "stg-secret",
120+
)
121+
if err != nil {
122+
t.Fatalf("login: %v", err)
123+
}
124+
125+
got := loadProfile(t, path, "staging")
126+
if got.BaseURL != "https://staging-api.verda.com/v1" {
127+
t.Errorf("BaseURL = %q", got.BaseURL)
128+
}
129+
if got.ClientID != "stg-id" {
130+
t.Errorf("ClientID = %q, want stg-id", got.ClientID)
131+
}
132+
133+
if _, err := options.LoadSharedCredentialsForProfile(path, "default"); err == nil {
134+
t.Error("a [default] section appeared; --profile must write only the named section")
135+
}
136+
}
137+
138+
// The writer merges into the existing INI. Dropping unrelated profiles would
139+
// destroy credentials the user cannot recover from the API.
140+
func TestLoginPreservesOtherProfiles(t *testing.T) {
141+
dir := sandboxHome(t)
142+
path := filepath.Join(dir, "credentials")
143+
144+
seed := "[other]\n" +
145+
"verda_base_url = https://other.verda.com/v1\n" +
146+
"verda_client_id = other-id\n" +
147+
"verda_client_secret = other-secret\n"
148+
if err := os.WriteFile(path, []byte(seed), 0o600); err != nil {
149+
t.Fatalf("seed: %v", err)
150+
}
151+
152+
if err := runAuthLoginForTest(t, "--client-id", "new-id", "--client-secret", "new-secret"); err != nil {
153+
t.Fatalf("login: %v", err)
154+
}
155+
156+
other := loadProfile(t, path, "other")
157+
if other.ClientID != "other-id" || other.ClientSecret != "other-secret" {
158+
t.Errorf("[other] was modified: %+v", other)
159+
}
160+
if added := loadProfile(t, path, "default"); added.ClientID != "new-id" {
161+
t.Errorf("[default] ClientID = %q, want new-id", added.ClientID)
162+
}
163+
}
164+
165+
// Re-running login against a profile is the documented re-auth path: rotating a
166+
// secret must replace the stored one, not append or refuse.
167+
func TestLoginOverwritesSameProfile(t *testing.T) {
168+
dir := sandboxHome(t)
169+
path := filepath.Join(dir, "credentials")
170+
171+
if err := runAuthLoginForTest(t, "--client-id", "old", "--client-secret", "old-secret"); err != nil {
172+
t.Fatalf("first login: %v", err)
173+
}
174+
if err := runAuthLoginForTest(t, "--client-id", "new", "--client-secret", "new-secret"); err != nil {
175+
t.Fatalf("second login: %v", err)
176+
}
177+
178+
got := loadProfile(t, path, "default")
179+
if got.ClientID != "new" || got.ClientSecret != "new-secret" {
180+
t.Errorf("re-login did not replace credentials: %+v", got)
181+
}
182+
183+
data, err := os.ReadFile(path) //nolint:gosec // test-owned temp file
184+
if err != nil {
185+
t.Fatalf("read: %v", err)
186+
}
187+
if n := strings.Count(string(data), "[default]"); n != 1 {
188+
t.Errorf("found %d [default] sections, want 1", n)
189+
}
190+
if strings.Contains(string(data), "old-secret") {
191+
t.Error("the replaced secret is still present in the file")
192+
}
193+
}
194+
195+
// --credentials-file outranks VERDA_SHARED_CREDENTIALS_FILE; sandboxHome sets
196+
// the env var, so a write landing at the flag path proves the precedence.
197+
func TestLoginCredentialsFileFlagWinsOverEnv(t *testing.T) {
198+
dir := sandboxHome(t)
199+
flagPath := filepath.Join(dir, "explicit-credentials")
200+
201+
err := runAuthLoginForTest(t,
202+
"--credentials-file", flagPath,
203+
"--client-id", "flag-id",
204+
"--client-secret", "flag-secret",
205+
)
206+
if err != nil {
207+
t.Fatalf("login: %v", err)
208+
}
209+
210+
if got := loadProfile(t, flagPath, "default"); got.ClientID != "flag-id" {
211+
t.Errorf("ClientID = %q, want flag-id", got.ClientID)
212+
}
213+
if _, err := os.Stat(filepath.Join(dir, "credentials")); !os.IsNotExist(err) {
214+
t.Error("the env-var path was written despite --credentials-file")
215+
}
216+
}

0 commit comments

Comments
 (0)