From 0578d408e6e95e003408ffb87749aa4bf3147d41 Mon Sep 17 00:00:00 2001 From: Brandon Carlson Date: Wed, 19 Aug 2026 12:35:28 -0500 Subject: [PATCH 01/15] fix: require a real passphrase for the file keyring backend (#1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The file backend previously encrypted tokens and client credentials with an empty passphrase, making the on-disk encryption a no-op. Now the passphrase comes from QBO_KEYRING_FILE_PASSWORD or an interactive prompt (memoized per process so multi-open commands prompt once), and headless invocations fail loudly when neither is available. --no-input suppresses the prompt. A set-but-empty env var is rejected explicitly, and a wrong passphrase surfaces as a config error naming the variable instead of 'not authenticated'. BREAKING: entries stored by older versions used the empty passphrase and can no longer be decrypted — re-run qbo auth login (and qbo auth set-client) once after upgrading. Co-authored-by: Claude Fable 5 --- go.mod | 2 +- internal/auth/clientcreds_test.go | 15 +++- internal/auth/token.go | 69 ++++++++++++++++-- internal/auth/token_test.go | 115 ++++++++++++++++++++++++++++++ internal/cmd/root.go | 3 +- skills/qbo/SKILL.md | 2 +- skills/qbo/references/COMMANDS.md | 1 + 7 files changed, 197 insertions(+), 10 deletions(-) create mode 100644 internal/auth/token_test.go diff --git a/go.mod b/go.mod index ffd0fd1..34ed492 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/alecthomas/kong v1.15.0 github.com/muesli/termenv v0.16.0 golang.org/x/oauth2 v0.36.0 + golang.org/x/term v0.44.0 ) require ( @@ -21,5 +22,4 @@ require ( github.com/mtibben/percent v0.2.1 // indirect github.com/rivo/uniseg v0.4.7 // indirect golang.org/x/sys v0.46.0 // indirect - golang.org/x/term v0.44.0 // indirect ) diff --git a/internal/auth/clientcreds_test.go b/internal/auth/clientcreds_test.go index 6a94512..629674e 100644 --- a/internal/auth/clientcreds_test.go +++ b/internal/auth/clientcreds_test.go @@ -6,12 +6,23 @@ import ( "golang.org/x/oauth2" ) -// hermeticKeyring forces the encrypted-file backend in a temp dir so tests -// never touch the real OS keychain. +// hermeticKeyring forces the file backend in a temp dir so tests never touch +// the real OS keychain. func hermeticKeyring(t *testing.T) { t.Helper() t.Setenv("QBO_KEYRING_BACKEND", "file") + t.Setenv("QBO_KEYRING_FILE_PASSWORD", "test-passphrase") t.Setenv("QBO_CONFIG_DIR", t.TempDir()) + resetFilePassCache(t) +} + +// resetFilePassCache clears the process-wide memoized passphrase so each test +// resolves it fresh from its own env. +func resetFilePassCache(t *testing.T) { + t.Helper() + filePassMu.Lock() + filePassCached = "" + filePassMu.Unlock() } func TestClientCredsRoundTrip(t *testing.T) { diff --git a/internal/auth/token.go b/internal/auth/token.go index 2c969d6..dd26d9c 100644 --- a/internal/auth/token.go +++ b/internal/auth/token.go @@ -2,13 +2,16 @@ package auth import ( "encoding/json" + "fmt" "os" "path/filepath" + "sync" "time" "github.com/99designs/keyring" "github.com/voska/qbo-cli/internal/errfmt" "golang.org/x/oauth2" + "golang.org/x/term" ) const keyringService = "qbo-cli" @@ -39,17 +42,66 @@ func openKeyring() (keyring.Keyring, error) { KeychainTrustApplication: true, KeychainAccessibleWhenUnlocked: true, FileDir: dir, - FilePasswordFunc: func(_ string) (string, error) { return "", nil }, + FilePasswordFunc: filePassword, } - // QBO_KEYRING_BACKEND=file forces the encrypted-file backend instead of the - // OS keychain — useful on headless hosts (e.g. a background daemon that - // can't reach the login keychain) and for hermetic tests. + // QBO_KEYRING_BACKEND=file forces the file backend instead of the OS + // keychain — useful on headless hosts (e.g. a background daemon that + // can't reach the login keychain, with QBO_KEYRING_FILE_PASSWORD set) + // and for hermetic tests. The files are only as protected as the + // passphrase supplied via filePassword. if os.Getenv("QBO_KEYRING_BACKEND") == "file" { cfg.AllowedBackends = []keyring.BackendType{keyring.FileBackend} } return keyring.Open(cfg) } +// NonInteractive disables the file-backend passphrase prompt. Set from the +// global --no-input flag: never prompt; fail if input would be needed. +var NonInteractive bool + +var ( + filePassMu sync.Mutex + filePassCached string +) + +// filePassword supplies the passphrase for the file backend. It is only +// invoked when that backend is actually selected — forced via +// QBO_KEYRING_BACKEND=file, or the fallback on hosts with no OS keychain. +// An empty passphrase would make the on-disk encryption a no-op, so a real +// one is required: from QBO_KEYRING_FILE_PASSWORD, or an interactive prompt. +// The result is cached for the process so a command that opens the keyring +// several times (load token, load creds, store refreshed token) prompts at +// most once — repeated prompts also risk a mistyped later entry silently +// re-encrypting an entry under a different passphrase. +func filePassword(prompt string) (string, error) { + filePassMu.Lock() + defer filePassMu.Unlock() + if filePassCached != "" { + return filePassCached, nil + } + if pw, set := os.LookupEnv("QBO_KEYRING_FILE_PASSWORD"); set { + if pw == "" { + return "", errfmt.Config("QBO_KEYRING_FILE_PASSWORD is set but empty — the file keyring passphrase must not be empty") + } + filePassCached = pw + return pw, nil + } + if !NonInteractive && term.IsTerminal(int(os.Stdin.Fd())) { + fmt.Fprintf(os.Stderr, "%s: ", prompt) + b, err := term.ReadPassword(int(os.Stdin.Fd())) + fmt.Fprintln(os.Stderr) + if err != nil { + return "", err + } + if len(b) == 0 { + return "", errfmt.Config("file keyring passphrase must not be empty") + } + filePassCached = string(b) + return filePassCached, nil + } + return "", errfmt.Config("file keyring backend requires a passphrase — set QBO_KEYRING_FILE_PASSWORD") +} + func StoreToken(realmID string, token *oauth2.Token) error { kr, err := openKeyring() if err != nil { @@ -71,9 +123,16 @@ func LoadToken(realmID string) (*oauth2.Token, error) { return nil, errfmt.Wrap(errfmt.ExitConfig, "cannot open keyring", err) } item, err := kr.Get(realmID) - if err != nil { + if errNotFound(err) { return nil, errfmt.Auth("not authenticated — run: qbo auth login") } + if err != nil { + // A decode failure means the entry exists but can't be decrypted: + // wrong QBO_KEYRING_FILE_PASSWORD, or the entry predates the + // passphrase requirement (older versions encrypted with an empty + // passphrase). Don't misreport it as "not authenticated". + return nil, errfmt.Wrap(errfmt.ExitConfig, "cannot read token from keyring — check QBO_KEYRING_FILE_PASSWORD; tokens stored by older versions must be recreated with: qbo auth login", err) + } var token oauth2.Token if err := json.Unmarshal(item.Data, &token); err != nil { return nil, errfmt.Wrap(errfmt.ExitConfig, "corrupt token data", err) diff --git a/internal/auth/token_test.go b/internal/auth/token_test.go new file mode 100644 index 0000000..de040b9 --- /dev/null +++ b/internal/auth/token_test.go @@ -0,0 +1,115 @@ +package auth + +import ( + "os" + "strings" + "testing" + + "golang.org/x/oauth2" +) + +func TestFilePasswordFromEnv(t *testing.T) { + resetFilePassCache(t) + t.Setenv("QBO_KEYRING_FILE_PASSWORD", "hunter2") + pw, err := filePassword("passphrase") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if pw != "hunter2" { + t.Fatalf("got %q, want %q", pw, "hunter2") + } +} + +func TestFilePasswordRejectsSetButEmptyEnv(t *testing.T) { + resetFilePassCache(t) + t.Setenv("QBO_KEYRING_FILE_PASSWORD", "") + _, err := filePassword("passphrase") + if err == nil || !strings.Contains(err.Error(), "set but empty") { + t.Fatalf("expected set-but-empty error, got: %v", err) + } +} + +func TestFilePasswordFailsHeadlessWithoutEnv(t *testing.T) { + resetFilePassCache(t) + t.Setenv("QBO_KEYRING_FILE_PASSWORD", "x") // register restore, then unset + os.Unsetenv("QBO_KEYRING_FILE_PASSWORD") + // Under `go test`, stdin is not a terminal, so the prompt path is + // unreachable and the missing passphrase must fail, never silently + // fall back to an empty one. + if _, err := filePassword("passphrase"); err == nil { + t.Fatal("expected error when no passphrase source is available") + } +} + +func TestFilePasswordIsCachedPerProcess(t *testing.T) { + resetFilePassCache(t) + t.Setenv("QBO_KEYRING_FILE_PASSWORD", "first") + if _, err := filePassword("passphrase"); err != nil { + t.Fatal(err) + } + t.Setenv("QBO_KEYRING_FILE_PASSWORD", "second") + pw, err := filePassword("passphrase") + if err != nil { + t.Fatal(err) + } + if pw != "first" { + t.Fatalf("expected cached passphrase %q, got %q", "first", pw) + } +} + +func TestStoreTokenFailsWithoutFilePassphrase(t *testing.T) { + hermeticKeyring(t) + t.Setenv("QBO_KEYRING_FILE_PASSWORD", "") + + err := StoreToken("realm", &oauth2.Token{AccessToken: "x"}) + if err == nil { + t.Fatal("expected store to fail without a file passphrase") + } +} + +func TestTokenRoundTripWithFilePassphrase(t *testing.T) { + hermeticKeyring(t) + + want := &oauth2.Token{AccessToken: "access", RefreshToken: "refresh"} + if err := StoreToken("realm", want); err != nil { + t.Fatalf("store: %v", err) + } + got, err := LoadToken("realm") + if err != nil { + t.Fatalf("load: %v", err) + } + if got.AccessToken != want.AccessToken || got.RefreshToken != want.RefreshToken { + t.Fatalf("round-trip mismatch: got %+v want %+v", got, want) + } +} + +func TestLoadTokenWrongPassphraseIsConfigErrorNotAuth(t *testing.T) { + hermeticKeyring(t) + + if err := StoreToken("realm", &oauth2.Token{AccessToken: "x"}); err != nil { + t.Fatalf("store: %v", err) + } + + t.Setenv("QBO_KEYRING_FILE_PASSWORD", "wrong-passphrase") + resetFilePassCache(t) + + _, err := LoadToken("realm") + if err == nil { + t.Fatal("expected load with wrong passphrase to fail") + } + if strings.Contains(err.Error(), "not authenticated") { + t.Fatalf("wrong passphrase misreported as not authenticated: %v", err) + } + if !strings.Contains(err.Error(), "QBO_KEYRING_FILE_PASSWORD") { + t.Fatalf("error should point at the passphrase: %v", err) + } +} + +func TestLoadTokenMissingEntryIsAuthError(t *testing.T) { + hermeticKeyring(t) + + _, err := LoadToken("no-such-realm") + if err == nil || !strings.Contains(err.Error(), "not authenticated") { + t.Fatalf("missing token should be an auth error, got: %v", err) + } +} diff --git a/internal/cmd/root.go b/internal/cmd/root.go index e6002fa..032ec2b 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -63,7 +63,7 @@ func (g *Globals) keyringCreds() auth.ClientCreds { c, ok, err := auth.LoadClientCreds() switch { case err != nil: - output.Warn("could not read client credentials from keyring: %v", err) + output.Warn("could not read client credentials from keyring (check QBO_KEYRING_FILE_PASSWORD before re-running qbo auth set-client): %v", err) case ok: g.cc = c } @@ -92,6 +92,7 @@ func NewGlobals(ctx context.Context, cli *CLI) (*Globals, error) { return nil, err } opts := output.NewOptions(cli.JSON, cli.Plain, cli.ResultsOnly, cli.Select) + auth.NonInteractive = cli.NoInput return &Globals{ Ctx: output.WithOptions(ctx, opts), Config: cfg, diff --git a/skills/qbo/SKILL.md b/skills/qbo/SKILL.md index 4df905a..c48cd79 100644 --- a/skills/qbo/SKILL.md +++ b/skills/qbo/SKILL.md @@ -65,7 +65,7 @@ qbo auth login --manual For non-localhost redirect URIs, `qbo auth login` prints the auth URL and prompts you to paste the callback URL after authorizing. -Tokens **and** client credentials (once `qbo auth set-client` runs, or after the first `qbo auth login`) are stored in the system keychain (macOS Keychain, Windows Credential Manager) with file-based fallback at `~/.config/qbo/tokens/`. Credentials resolve in priority order: env var (`QBO_CLIENT_ID`/`QBO_CLIENT_SECRET`) → keychain → `config.json`, so env vars still work and override the keychain when set. On headless hosts that can't reach the login keychain, set `QBO_KEYRING_BACKEND=file` to use the encrypted-file backend instead. +Tokens **and** client credentials (once `qbo auth set-client` runs, or after the first `qbo auth login`) are stored in the system keychain (macOS Keychain, Windows Credential Manager) with file-based fallback at `~/.config/qbo/tokens/`. Credentials resolve in priority order: env var (`QBO_CLIENT_ID`/`QBO_CLIENT_SECRET`) → keychain → `config.json`, so env vars still work and override the keychain when set. On headless hosts that can't reach the login keychain, set `QBO_KEYRING_BACKEND=file` to use the encrypted-file backend instead — this also requires `QBO_KEYRING_FILE_PASSWORD` (the passphrase encrypting the token files; non-interactive runs fail without it). Tokens stored by versions that predate the passphrase requirement can't be decrypted; re-run `qbo auth login` once after upgrading. ### Verify diff --git a/skills/qbo/references/COMMANDS.md b/skills/qbo/references/COMMANDS.md index 088db26..739ad4c 100644 --- a/skills/qbo/references/COMMANDS.md +++ b/skills/qbo/references/COMMANDS.md @@ -205,3 +205,4 @@ and auto-posts on a schedule. Use the standard `list`/`get`/`create`/`update`/`d | `QBO_AUTO_JSON` | Set to `1` for auto-JSON when stdout is piped | | `QBO_CONFIG_DIR` | Override config directory (default: `~/.config/qbo/`) | | `QBO_KEYRING_BACKEND` | Set to `file` to force the encrypted-file backend (headless hosts) | +| `QBO_KEYRING_FILE_PASSWORD` | Passphrase for the file backend; required when no terminal is available to prompt | From e4504a1e27b3bc396c9a0a6320d54a2cc989581a Mon Sep 17 00:00:00 2001 From: Brandon Carlson Date: Wed, 19 Aug 2026 12:35:31 -0500 Subject: [PATCH 02/15] fix: never silently overwrite files in qbo download (#2) The save path for qbo download without -o comes from the API response, which anyone with write access to the QBO company controls. Previously os.Create silently truncated whatever that name pointed at in the working directory. Now the destination is created with O_EXCL and an existing file (or planted symlink) is a usage error unless --force is passed; the existence check runs before any API calls are spent. API-supplied names are reduced to a safe basename (both separator styles) and hidden-file names like .envrc are refused outright. --force writes to a temp file and renames into place, so a mid-stream failure preserves the original and a symlinked name is replaced rather than written through. Close errors now fail the command instead of reporting a corrupt file as saved. Co-authored-by: Claude Fable 5 --- internal/cmd/download.go | 113 ++++++++++++++++++---- internal/cmd/download_test.go | 170 ++++++++++++++++++++++++++++++++++ internal/cmd/root.go | 2 +- internal/cmd/schema.go | 4 +- skills/qbo/SKILL.md | 3 +- 5 files changed, 272 insertions(+), 20 deletions(-) create mode 100644 internal/cmd/download_test.go diff --git a/internal/cmd/download.go b/internal/cmd/download.go index 132ebf1..849be8a 100644 --- a/internal/cmd/download.go +++ b/internal/cmd/download.go @@ -5,6 +5,7 @@ import ( "io" "os" "path/filepath" + "strings" "github.com/voska/qbo-cli/internal/errfmt" "github.com/voska/qbo-cli/internal/output" @@ -41,10 +42,24 @@ func (c *DownloadCmd) Run(g *Globals) error { if err != nil { return err } - savePath = extractFileName(meta) - if savePath == "" { + name, _ := extractFileName(meta) + if name == "" { return errfmt.Usage("attachable " + c.ID + " has no file name — pass -o to choose an output path") } + savePath, err = safeBaseName(name) + if err != nil { + return err + } + } + + // Check before spending API calls: the attachable read above, the + // download-URL fetch, and the body stream are all wasted if the + // destination is already taken. Lstat (not Stat) so a planted symlink + // counts as "exists" instead of being probed through. + if !g.CLI.Force { + if _, err := os.Lstat(savePath); err == nil { + return errfmt.Usage(savePath + " already exists — pass --force to overwrite") + } } body, err := client.Download(g.Ctx, c.ID) @@ -53,31 +68,97 @@ func (c *DownloadCmd) Run(g *Globals) error { } defer func() { _ = body.Close() }() - f, err := os.Create(savePath) + n, err := writeDest(savePath, body, g.CLI.Force) if err != nil { - return errfmt.Wrap(errfmt.ExitError, "cannot create file", err) + return err } + output.Hint("saved %s (%d bytes)", savePath, n) + return WriteOutput(g.Ctx, map[string]any{"id": c.ID, "path": savePath, "bytes": n}) +} + +// writeDest saves the stream to path without ever damaging an existing file. +// Without force, the destination is created with O_EXCL, so nothing existing +// is truncated and a pre-planted symlink is never followed. With force, the +// stream lands in a temp file first and is renamed over the destination only +// after a complete, flushed write — the original survives a mid-stream +// failure, and the destination name is replaced (never written through, so a +// symlink there redirects nothing). +func writeDest(path string, body io.Reader, force bool) (int64, error) { + if force { + tmp, err := os.CreateTemp(filepath.Dir(path), filepath.Base(path)+".*.partial") + if err != nil { + return 0, errfmt.Wrap(errfmt.ExitError, "cannot create temp file", err) + } + n, err := writeAndClose(tmp, body) + if err != nil { + return 0, err + } + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + _ = os.Remove(tmp.Name()) + return 0, errfmt.Wrap(errfmt.ExitError, "cannot replace existing file", err) + } + if err := os.Rename(tmp.Name(), path); err != nil { + _ = os.Remove(tmp.Name()) + return 0, errfmt.Wrap(errfmt.ExitError, "cannot move file into place", err) + } + return n, nil + } + f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644) + if err != nil { + if os.IsExist(err) { + return 0, errfmt.Usage(path + " already exists — pass --force to overwrite") + } + return 0, errfmt.Wrap(errfmt.ExitError, "cannot create file", err) + } + return writeAndClose(f, body) +} + +// writeAndClose copies the body into f, enforcing the size cap and surfacing +// close/flush errors. On any failure the partial file is removed — it is +// either an O_EXCL-created destination (cannot have pre-existed) or a temp +// file, so an original is never deleted. +func writeAndClose(f *os.File, body io.Reader) (int64, error) { n, err := io.Copy(f, io.LimitReader(body, maxUploadSize+1)) - _ = f.Close() + closeErr := f.Close() + if err == nil { + err = closeErr + } if err != nil { - _ = os.Remove(savePath) - return errfmt.Wrap(errfmt.ExitError, "cannot write file", err) + _ = os.Remove(f.Name()) + return 0, errfmt.Wrap(errfmt.ExitError, "cannot write file", err) } if n > maxUploadSize { - _ = os.Remove(savePath) - return errfmt.New(errfmt.ExitError, fmt.Sprintf("download exceeds %d byte limit", maxUploadSize)) + _ = os.Remove(f.Name()) + return 0, errfmt.New(errfmt.ExitError, fmt.Sprintf("download exceeds %d byte limit", maxUploadSize)) } - - output.Hint("saved %s (%d bytes)", savePath, n) - return WriteOutput(g.Ctx, map[string]any{"id": c.ID, "path": savePath, "bytes": n}) + return n, nil } -func extractFileName(meta map[string]any) string { +// extractFileName pulls the original file name from an attachable read +// response. The bool reports whether the field was present. +func extractFileName(meta map[string]any) (string, bool) { att, _ := meta["Attachable"].(map[string]any) name, _ := att["FileName"].(string) - if name == "" { - return "" + return name, name != "" +} + +// safeBaseName reduces an API-supplied file name to a bare name that is safe +// to create in the working directory. The name comes from the QBO response, +// which anyone with write access to the company controls: directory parts +// are stripped (both separator styles — filepath.Base ignores backslashes on +// Unix), and hidden files are refused because a dropped .envrc or .zshrc is +// an attack, not a download. +func safeBaseName(name string) (string, error) { + if i := strings.LastIndexByte(name, '\\'); i >= 0 { + name = name[i+1:] + } + name = filepath.Base(name) + if name == "" || name == "." || name == string(filepath.Separator) { + return "", errfmt.Usage("attachable file name is unusable — pass -o to choose an output path") + } + if strings.HasPrefix(name, ".") { + return "", errfmt.Usage(fmt.Sprintf("refusing to write hidden file %q from API-supplied name — pass -o to choose an output path", name)) } - return filepath.Base(name) + return name, nil } diff --git a/internal/cmd/download_test.go b/internal/cmd/download_test.go new file mode 100644 index 0000000..8f629ee --- /dev/null +++ b/internal/cmd/download_test.go @@ -0,0 +1,170 @@ +package cmd + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/voska/qbo-cli/internal/errfmt" +) + +func TestSafeBaseNameStripsDirectories(t *testing.T) { + got, err := safeBaseName("../../etc/passwd") + if err != nil || got != "passwd" { + t.Fatalf("got %q, %v; want %q", got, err, "passwd") + } +} + +func TestSafeBaseNameStripsBackslashDirectories(t *testing.T) { + got, err := safeBaseName(`C:\tmp\receipt.pdf`) + if err != nil || got != "receipt.pdf" { + t.Fatalf("got %q, %v; want %q", got, err, "receipt.pdf") + } +} + +func TestSafeBaseNameRefusesHiddenFiles(t *testing.T) { + for _, name := range []string{".envrc", "dir/.zshrc", `C:\tmp\.bashrc`} { + if _, err := safeBaseName(name); exitCode(err) != errfmt.ExitUsage { + t.Fatalf("%q: expected usage error, got: %v", name, err) + } + } +} + +func TestSafeBaseNameRefusesUnusableNames(t *testing.T) { + for _, name := range []string{"/", "", "."} { + if _, err := safeBaseName(name); err == nil { + t.Fatalf("%q: expected error", name) + } + } +} + +func TestExtractFileName(t *testing.T) { + name, ok := extractFileName(map[string]any{"Attachable": map[string]any{"FileName": "receipt.pdf"}}) + if !ok || name != "receipt.pdf" { + t.Fatalf("got %q, %v", name, ok) + } + if _, ok := extractFileName(map[string]any{}); ok { + t.Fatal("expected no file name") + } +} + +func TestWriteDestRefusesExistingWithoutForce(t *testing.T) { + path := filepath.Join(t.TempDir(), "receipt.pdf") + if err := os.WriteFile(path, []byte("precious"), 0o644); err != nil { + t.Fatal(err) + } + + _, err := writeDest(path, strings.NewReader("new"), false) + if exitCode(err) != errfmt.ExitUsage { + t.Fatalf("expected usage error, got: %v", err) + } + + got, _ := os.ReadFile(path) + if string(got) != "precious" { + t.Fatalf("existing file was modified: %q", got) + } +} + +func TestWriteDestOverwritesWithForce(t *testing.T) { + path := filepath.Join(t.TempDir(), "receipt.pdf") + if err := os.WriteFile(path, []byte("old"), 0o644); err != nil { + t.Fatal(err) + } + + n, err := writeDest(path, strings.NewReader("new"), true) + if err != nil || n != 3 { + t.Fatalf("force write: n=%d err=%v", n, err) + } + got, _ := os.ReadFile(path) + if string(got) != "new" { + t.Fatalf("got %q, want %q", got, "new") + } +} + +func TestWriteDestForceReplacesSymlinkNameNotTarget(t *testing.T) { + dir := t.TempDir() + target := filepath.Join(dir, "target") + if err := os.WriteFile(target, []byte("target"), 0o644); err != nil { + t.Fatal(err) + } + link := filepath.Join(dir, "link") + if err := os.Symlink(target, link); err != nil { + t.Skipf("cannot create symlink: %v", err) + } + + if _, err := writeDest(link, strings.NewReader("payload"), true); err != nil { + t.Fatalf("force over symlink: %v", err) + } + + got, _ := os.ReadFile(target) + if string(got) != "target" { + t.Fatalf("symlink target was written through: %q", got) + } + if fi, err := os.Lstat(link); err != nil || fi.Mode()&os.ModeSymlink != 0 { + t.Fatalf("link should now be a regular file: %v %v", fi, err) + } + if got, _ := os.ReadFile(link); string(got) != "payload" { + t.Fatalf("destination content: %q", got) + } +} + +func TestWriteDestRefusesSymlinkWithoutForce(t *testing.T) { + dir := t.TempDir() + target := filepath.Join(dir, "target") + if err := os.WriteFile(target, []byte("target"), 0o644); err != nil { + t.Fatal(err) + } + link := filepath.Join(dir, "link") + if err := os.Symlink(target, link); err != nil { + t.Skipf("cannot create symlink: %v", err) + } + + if _, err := writeDest(link, strings.NewReader("payload"), false); err == nil { + t.Fatal("expected error writing over symlink without force") + } + got, _ := os.ReadFile(target) + if string(got) != "target" { + t.Fatalf("symlink target was written through: %q", got) + } +} + +func TestWriteDestForcePreservesOriginalOnFailure(t *testing.T) { + path := filepath.Join(t.TempDir(), "receipt.pdf") + if err := os.WriteFile(path, []byte("precious"), 0o644); err != nil { + t.Fatal(err) + } + + // An over-limit body fails after the copy; the original must survive + // because the write went to a temp file, and the temp must be gone. + _, err := writeDest(path, &infiniteReader{}, true) + if err == nil { + t.Fatal("expected size-limit error") + } + got, _ := os.ReadFile(path) + if string(got) != "precious" { + t.Fatalf("original destroyed on failed force download: %q", got) + } + leftovers, _ := filepath.Glob(filepath.Join(filepath.Dir(path), "*.partial")) + if len(leftovers) != 0 { + t.Fatalf("temp files left behind: %v", leftovers) + } +} + +func TestWriteDestNewFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "new.pdf") + n, err := writeDest(path, strings.NewReader("data"), false) + if err != nil || n != 4 { + t.Fatalf("n=%d err=%v", n, err) + } +} + +// infiniteReader never ends, driving writes past maxUploadSize. +type infiniteReader struct{} + +func (infiniteReader) Read(p []byte) (int, error) { + for i := range p { + p[i] = 'x' + } + return len(p), nil +} diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 032ec2b..303f1ea 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -22,7 +22,7 @@ type CLI struct { Sandbox bool `help:"Use sandbox environment."` DryRun bool `short:"n" name:"dry-run" help:"Show what would happen." aliases:"noop,preview"` NoInput bool `name:"no-input" help:"Never prompt; fail if confirmation needed."` - Force bool `help:"Skip confirmations." aliases:"yes,assume-yes"` + Force bool `help:"Skip confirmations; overwrite existing files." aliases:"yes,assume-yes"` Verbose bool `short:"v" help:"Verbose output to stderr."` MinorVer int `name:"minor-version" default:"75" help:"QBO API minor version."` Auth AuthCmd `cmd:"" help:"Authentication (login, logout, status, refresh)."` diff --git a/internal/cmd/schema.go b/internal/cmd/schema.go index d9a1463..2ebad6d 100644 --- a/internal/cmd/schema.go +++ b/internal/cmd/schema.go @@ -120,9 +120,9 @@ func fullSchema(version string) map[string]any { }, { "name": "download", - "help": "Download an attachment file", + "help": "Download an attachment file (refuses to overwrite existing files unless --force)", "args": []string{"id"}, - "flags": []string{"--output", "--url"}, + "flags": []string{"--output", "--url", "--force"}, }, { "name": "schema", diff --git a/skills/qbo/SKILL.md b/skills/qbo/SKILL.md index c48cd79..7b15c98 100644 --- a/skills/qbo/SKILL.md +++ b/skills/qbo/SKILL.md @@ -155,7 +155,8 @@ echo '{"CustomerRef":{"value":"CUSTOMER_ID"},"TotalAmt":1500,"Line":[{"Amount":1 ```bash qbo attach invoice 123 receipt.pdf --json qbo list attachable --where "AttachableRef.EntityRef.value = '123'" --json --results-only -qbo download +qbo download # saves under the attachment's own name; add -o to choose the path +qbo download --force # required to overwrite a file that already exists ``` ## Recurring Transactions From 0aa33ab5ba91b828d3c36c14f589bae79b88434d Mon Sep 17 00:00:00 2001 From: Brandon Carlson Date: Wed, 19 Aug 2026 12:35:35 -0500 Subject: [PATCH 03/15] fix: strip terminal escape sequences from untrusted output (#3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QBO string fields (customer names, memos, line descriptions) are often populated by external parties and were rendered verbatim in human and plain modes, letting embedded ANSI/OSC sequences rewrite or spoof terminal output — the same class git, gh, and kubectl have patched. Table cells and headers now drop C0/C1 controls and DEL, strip bidi overrides/isolates and zero-width characters, and flatten CR/LF/tab to spaces so hostile values can't forge rows or TSV columns. The stderr helpers (Hint/Success/Warn/ErrorMsg) sanitize too, covering API error bodies and API-supplied file names; message newlines are preserved. JSON mode is unchanged — encoding/json already escapes control chars. Co-authored-by: Claude Fable 5 --- internal/output/human.go | 12 ++-- internal/output/write.go | 79 ++++++++++++++++++++++++-- internal/output/write_test.go | 104 ++++++++++++++++++++++++++++++++++ 3 files changed, 185 insertions(+), 10 deletions(-) create mode 100644 internal/output/write_test.go diff --git a/internal/output/human.go b/internal/output/human.go index ae4576f..c43efad 100644 --- a/internal/output/human.go +++ b/internal/output/human.go @@ -66,26 +66,30 @@ func padRight(s string, width int) string { return s + strings.Repeat(" ", width-len(s)) } +// The stderr helpers sanitize the formatted message: interpolated values can +// carry untrusted API data (file names, error response bodies) whose control +// characters could rewrite or spoof terminal output. + func Hint(format string, args ...any) { - msg := fmt.Sprintf(format, args...) + msg := SanitizeMessage(fmt.Sprintf(format, args...)) styled := termenv.String(msg).Foreground(profile.Color("8")) fmt.Fprintln(os.Stderr, styled) } func Success(format string, args ...any) { - msg := fmt.Sprintf(format, args...) + msg := SanitizeMessage(fmt.Sprintf(format, args...)) styled := termenv.String("✓ " + msg).Foreground(profile.Color("2")) fmt.Fprintln(os.Stderr, styled) } func Warn(format string, args ...any) { - msg := fmt.Sprintf(format, args...) + msg := SanitizeMessage(fmt.Sprintf(format, args...)) styled := termenv.String("⚠ " + msg).Foreground(profile.Color("3")) fmt.Fprintln(os.Stderr, styled) } func ErrorMsg(format string, args ...any) { - msg := fmt.Sprintf(format, args...) + msg := SanitizeMessage(fmt.Sprintf(format, args...)) styled := termenv.String("✗ " + msg).Foreground(profile.Color("1")) fmt.Fprintln(os.Stderr, styled) } diff --git a/internal/output/write.go b/internal/output/write.go index debf793..ad7cc4c 100644 --- a/internal/output/write.go +++ b/internal/output/write.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "sort" + "strings" ) func Write(ctx context.Context, data any) error { @@ -60,30 +61,96 @@ func toTable(data any) ([]string, [][]string) { for i, item := range v { m, ok := item.(map[string]any) if !ok { - rows[i] = []string{fmt.Sprint(item)} + rows[i] = []string{cell(item)} continue } row := make([]string, len(headers)) for j, h := range headers { if val, ok := m[h]; ok { - row[j] = fmt.Sprint(val) + row[j] = cell(val) } } rows[i] = row } - return headers, rows + return sanitizeHeaders(headers), rows case map[string]any: headers := sortedKeys(v) row := make([]string, len(headers)) for i, h := range headers { - row[i] = fmt.Sprint(v[h]) + row[i] = cell(v[h]) } - return headers, [][]string{row} + return sanitizeHeaders(headers), [][]string{row} default: - return nil, [][]string{{fmt.Sprint(data)}} + return nil, [][]string{{cell(data)}} } } +// cell renders a value for human/plain output. API-sourced strings are +// untrusted (customer names, memos, and line descriptions are routinely +// populated by external parties), so control characters are removed: an +// embedded ESC/CSI/OSC sequence could rewrite or spoof terminal output, and +// an embedded newline or tab would forge extra table rows or TSV columns. +// JSON mode is unaffected — encoding/json escapes control characters itself. +func cell(val any) string { + return sanitizeCell(fmt.Sprint(val)) +} + +// sanitizeHeaders applies cell sanitization to header labels for display. +// Row values are looked up under the original (unsanitized) keys before this +// runs, so a hostile key only changes how the column is labeled, never which +// values land in it. +func sanitizeHeaders(headers []string) []string { + out := make([]string, len(headers)) + for i, h := range headers { + out[i] = sanitizeCell(h) + } + return out +} + +func sanitizeCell(s string) string { + return strings.Map(func(r rune) rune { + if r == '\n' || r == '\r' || r == '\t' { + return ' ' + } + return sanitizeRune(r) + }, s) +} + +// SanitizeMessage cleans untrusted text destined for stderr status lines +// (hints, warnings, error details that may embed raw API response bodies). +// Unlike table cells, newlines and tabs are legitimate message formatting +// and pass through. +func SanitizeMessage(s string) string { + return strings.Map(func(r rune) rune { + if r == '\n' || r == '\t' { + return r + } + return sanitizeRune(r) + }, s) +} + +// sanitizeRune drops characters that can rewrite, reorder, or invisibly +// alter terminal output; everything else passes through unchanged. +func sanitizeRune(r rune) rune { + switch { + case r < 0x20 || r == 0x7f: // C0 controls + DEL (includes ESC) + return -1 + case r >= 0x80 && r <= 0x9f: // C1 controls (8-bit CSI/OSC forms) + return -1 + case r == 0x2028 || r == 0x2029: // line/paragraph separators + return ' ' + case r == 0x200b || r == 0x200c || r == 0x200d || r == 0xfeff: // zero-width + return -1 + case r == 0x200e || r == 0x200f || r == 0x061c: // bidi marks + return -1 + case r >= 0x202a && r <= 0x202e: // bidi embedding/override + return -1 + case r >= 0x2066 && r <= 0x2069: // bidi isolates + return -1 + } + return r +} + func collectHeaders(items []any) []string { seen := map[string]bool{} var headers []string diff --git a/internal/output/write_test.go b/internal/output/write_test.go new file mode 100644 index 0000000..ebdbe93 --- /dev/null +++ b/internal/output/write_test.go @@ -0,0 +1,104 @@ +package output + +import ( + "strings" + "testing" +) + +func TestSanitizeCellStripsANSIEscapes(t *testing.T) { + got := sanitizeCell("Acme\x1b[2K\x1b[1A Corp") + if got != "Acme[2K[1A Corp" { + t.Fatalf("got %q", got) + } +} + +func TestSanitizeCellStripsOSC(t *testing.T) { + got := sanitizeCell("evil\x1b]0;spoofed title\x07vendor") + if got != "evil]0;spoofed titlevendor" { + t.Fatalf("got %q", got) + } +} + +func TestSanitizeCellStripsC1Controls(t *testing.T) { + // U+009B is the single-rune C1 CSI: it acts like ESC-[ on terminals + // that honor 8-bit controls. + got := sanitizeCell("ven\u009b31mdor") + if got != "ven31mdor" { + t.Fatalf("got %q", got) + } +} + +func TestSanitizeCellFlattensLineBreaksAndTabs(t *testing.T) { + got := sanitizeCell("line1\nline2\tcol\rmore
sep") + if got != "line1 line2 col more sep" { + t.Fatalf("got %q", got) + } +} + +func TestSanitizeCellStripsBidiAndZeroWidth(t *testing.T) { + in := "Acme \u202e00.9$ dnufeR\u202c end\u200bzw\ufeff before\u2028after" + got := sanitizeCell(in) + if got != "Acme 00.9$ dnufeR endzw before after" { + t.Fatalf("got %q", got) + } +} + +func TestSanitizeCellPassesPlainText(t *testing.T) { + in := "Åcme & Söns — 100% legit café ✓" + if got := sanitizeCell(in); got != in { + t.Fatalf("mangled clean text: %q -> %q", in, got) + } +} + +func TestSanitizeMessageKeepsNewlinesStripsEscapes(t *testing.T) { + got := SanitizeMessage("line1\nline2\t\x1b[2Jrest›x") + if got != "line1\nline2\t[2Jrestx" { + t.Fatalf("got %q", got) + } +} + +func TestToTableSanitizesValues(t *testing.T) { + headers, rows := toTable([]any{ + map[string]any{"DisplayName": "Acme\x1b[8m hidden", "Balance": 100.5}, + }) + if len(headers) != 2 || len(rows) != 1 { + t.Fatalf("unexpected shape: %v %v", headers, rows) + } + for _, cell := range rows[0] { + if strings.ContainsRune(cell, 0x1b) { + t.Fatalf("ESC reached table cell: %q", cell) + } + } +} + +func TestToTableSanitizesHeaders(t *testing.T) { + headers, rows := toTable([]any{ + map[string]any{"Name\x1b[2J": "a", "Memo\tX": "b"}, + }) + for _, h := range headers { + if strings.ContainsRune(h, 0x1b) || strings.ContainsRune(h, '\t') { + t.Fatalf("unsanitized header: %q", h) + } + } + // Row values must still be found under the original hostile keys. + joined := strings.Join(rows[0], "|") + if !strings.Contains(joined, "a") || !strings.Contains(joined, "b") { + t.Fatalf("row lookup broke under sanitized headers: %v", rows) + } + + _, rows2 := toTable(map[string]any{"Key\x1b[1A": "val"}) + if len(rows2) != 1 || rows2[0][0] != "val" { + t.Fatalf("single-object row lookup broke: %v", rows2) + } +} + +func TestToTableSanitizesSingleObjectAndScalar(t *testing.T) { + _, rows := toTable(map[string]any{"Memo": "pay\x1b[1A me"}) + if strings.ContainsRune(rows[0][0], 0x1b) { + t.Fatalf("ESC in object row: %q", rows[0][0]) + } + _, rows = toTable("raw\x1b[2Jstring") + if strings.ContainsRune(rows[0][0], 0x1b) { + t.Fatalf("ESC in scalar row: %q", rows[0][0]) + } +} From 02fab0648290466b91a6d0835e6276a296ed4111 Mon Sep 17 00:00:00 2001 From: Brandon Carlson Date: Wed, 19 Aug 2026 22:03:22 -0500 Subject: [PATCH 04/15] feat: add machine-scope bootstrap.json as lowest client-creds tier Read /Library/Application Support/qbo/bootstrap.json (installed by device management) carrying vault URL, secret name, and Entra tenant/client ID so a first login can fetch Intuit client creds from Azure Key Vault without the binary carrying org-specific values. auth status reports the tier (bootstrap-pending/bootstrap/bootstrap-error) and now emits credential diagnostics even before a company exists; auth login names the vault on provisioned machines and fails loudly (exit 10) on a malformed bootstrap file. Keyring creds gain an origin marker for the upcoming vault self-heal. Co-Authored-By: Claude Fable 5 --- internal/auth/clientcreds.go | 10 ++++ internal/cmd/auth.go | 73 ++++++++++++++++++++++------ internal/cmd/auth_test.go | 79 ++++++++++++++++++++++++++++++ internal/cmd/root.go | 24 ++++++++++ internal/config/bootstrap.go | 80 +++++++++++++++++++++++++++++++ internal/config/bootstrap_test.go | 80 +++++++++++++++++++++++++++++++ 6 files changed, 331 insertions(+), 15 deletions(-) create mode 100644 internal/cmd/auth_test.go create mode 100644 internal/config/bootstrap.go create mode 100644 internal/config/bootstrap_test.go diff --git a/internal/auth/clientcreds.go b/internal/auth/clientcreds.go index 142b0b8..6107e44 100644 --- a/internal/auth/clientcreds.go +++ b/internal/auth/clientcreds.go @@ -28,8 +28,18 @@ type ClientCreds struct { ClientID string `json:"client_id"` ClientSecret string `json:"client_secret"` RedirectURI string `json:"redirect_uri,omitempty"` + // Origin records which tier the credentials came from when they were + // stored: CredsOriginBootstrap for creds fetched via the machine-scope + // bootstrap flow, empty for user-supplied ones (set-client, env). The + // vault self-heal on refresh failure will key off this marker; nothing + // writes CredsOriginBootstrap until that flow lands. + Origin string `json:"origin,omitempty"` } +// CredsOriginBootstrap marks credentials fetched from an org vault via the +// machine-scope bootstrap config, as opposed to entered by the user. +const CredsOriginBootstrap = "bootstrap" + func StoreClientCreds(c ClientCreds) error { kr, err := openKeyring() if err != nil { diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go index 2a1888f..4de75ca 100644 --- a/internal/cmd/auth.go +++ b/internal/cmd/auth.go @@ -27,6 +27,15 @@ func (c *AuthLoginCmd) Run(g *Globals) error { clientID := g.ClientID() clientSecret := g.ClientSecret() if clientID == "" || clientSecret == "" { + // No creds in env/keyring/config: the bootstrap tier is next. A broken + // bootstrap file must fail loudly here, not read as "unprovisioned". + b, err := g.Bootstrap() + if err != nil { + return err + } + if b != nil { + return errfmt.Config("this machine is provisioned to fetch client credentials from " + b.VaultURL + ", but this qbo build does not support vault bootstrap yet — run: qbo auth set-client (or set QBO_CLIENT_ID and QBO_CLIENT_SECRET)") + } return errfmt.Config("set client credentials first — run: qbo auth set-client (or set QBO_CLIENT_ID and QBO_CLIENT_SECRET)") } @@ -68,11 +77,19 @@ func (c *AuthLoginCmd) Run(g *Globals) error { } // Persist the client credentials used so future invocations resolve them - // from the keyring without QBO_CLIENT_ID/SECRET in the environment. + // from the keyring without QBO_CLIENT_ID/SECRET in the environment. Keep + // the origin marker only when re-storing the exact keyring creds — env + // creds with a matching ID but rotated secret are user-supplied and must + // not be stamped as vault-owned. + origin := "" + if kc := g.keyringCreds(); kc.ClientID == clientID && kc.ClientSecret == clientSecret { + origin = kc.Origin + } if err := auth.StoreClientCreds(auth.ClientCreds{ ClientID: clientID, ClientSecret: clientSecret, RedirectURI: storedRedirect, + Origin: origin, }); err != nil { output.Warn("authenticated, but could not save client credentials to keyring: %v", err) } @@ -98,42 +115,68 @@ func (c *AuthLogoutCmd) Run(g *Globals) error { type AuthStatusCmd struct{} func (c *AuthStatusCmd) Run(g *Globals) error { - realmID, err := g.ResolveCompanyID() - if err != nil { - return err + // Credential/bootstrap diagnostics are built first so they are reported + // even when no company or token exists yet — that unauthenticated state + // is exactly when "why can't this machine log in" gets debugged. The + // original error still decides the exit code. + status := map[string]any{ + "authenticated": false, + "client_credentials": map[string]any{ + "configured": g.ClientID() != "" && g.ClientSecret() != "", + "source": clientCredsSource(g), + }, } - token, err := auth.LoadToken(realmID) + if b, err := g.Bootstrap(); err != nil { + status["bootstrap"] = map[string]any{"error": err.Error()} + } else if b != nil { + status["bootstrap"] = map[string]any{ + "vault_url": b.VaultURL, + "entra_tenant_id": b.EntraTenantID, + } + } + realmID, err := g.ResolveCompanyID() if err != nil { + _ = WriteOutput(g.Ctx, status) return err } - status := map[string]any{ - "company_id": realmID, - "authenticated": true, - "token_expiry": token.Expiry.Format(time.RFC3339), - "expired": auth.IsTokenExpired(token), - } + status["company_id"] = realmID co := g.Config.FindCompany(realmID) if co != nil { status["company_name"] = co.CompanyName status["environment"] = co.Environment } - status["client_credentials"] = map[string]any{ - "configured": g.ClientID() != "" && g.ClientSecret() != "", - "source": clientCredsSource(g), + token, err := auth.LoadToken(realmID) + if err != nil { + _ = WriteOutput(g.Ctx, status) + return err } + status["authenticated"] = true + status["token_expiry"] = token.Expiry.Format(time.RFC3339) + status["expired"] = auth.IsTokenExpired(token) return WriteOutput(g.Ctx, status) } // clientCredsSource reports which tier supplied the client credentials, for -// debugging "why can't this agent see QBO" situations. +// debugging "why can't this agent see QBO" situations. Keyring creds fetched +// via the machine-scope bootstrap flow report "bootstrap" rather than +// "keyring" so provisioned machines are distinguishable from set-client ones; +// a provisioned machine that hasn't fetched yet reports "bootstrap-pending". func clientCredsSource(g *Globals) string { + b, bErr := g.Bootstrap() switch { case os.Getenv("QBO_CLIENT_ID") != "": return "env" case g.keyringCreds().ClientID != "": + if g.keyringCreds().Origin == auth.CredsOriginBootstrap { + return "bootstrap" + } return "keyring" case g.Config.ClientID != "": return "config" + case bErr != nil: + return "bootstrap-error" + case b != nil: + return "bootstrap-pending" default: return "none" } diff --git a/internal/cmd/auth_test.go b/internal/cmd/auth_test.go new file mode 100644 index 0000000..44dcfe3 --- /dev/null +++ b/internal/cmd/auth_test.go @@ -0,0 +1,79 @@ +package cmd + +import ( + "context" + "strings" + "testing" + + "github.com/voska/qbo-cli/internal/auth" + "github.com/voska/qbo-cli/internal/config" + "github.com/voska/qbo-cli/internal/errfmt" +) + +// credsGlobals builds a Globals with the lazy keyring and bootstrap loads +// pre-resolved to the given values, so tests never touch the real keychain or +// filesystem. +func credsGlobals(cfg *config.Config, cc auth.ClientCreds, bs *config.Bootstrap, bsErr error) *Globals { + g := &Globals{Ctx: context.Background(), CLI: &CLI{}, Config: cfg} + g.ccOnce.Do(func() { g.cc = cc }) + g.bsOnce.Do(func() { g.bs = bs; g.bsErr = bsErr }) + return g +} + +func TestClientCredsSourceTiers(t *testing.T) { + bs := &config.Bootstrap{VaultURL: "https://v.vault.azure.net"} + cases := []struct { + name string + env string + cc auth.ClientCreds + cfg config.Config + bs *config.Bootstrap + bsErr error + want string + }{ + {name: "none", want: "none"}, + {name: "env wins", env: "env-id", cc: auth.ClientCreds{ClientID: "k"}, bs: bs, want: "env"}, + {name: "keyring", cc: auth.ClientCreds{ClientID: "k"}, cfg: config.Config{ClientID: "c"}, want: "keyring"}, + {name: "bootstrap-fetched keyring creds", cc: auth.ClientCreds{ClientID: "k", Origin: auth.CredsOriginBootstrap}, want: "bootstrap"}, + {name: "config file", cfg: config.Config{ClientID: "c"}, bs: bs, want: "config"}, + {name: "provisioned but not yet fetched", bs: bs, want: "bootstrap-pending"}, + {name: "broken bootstrap file", bsErr: errfmt.Config("boom"), want: "bootstrap-error"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Setenv("QBO_CLIENT_ID", tc.env) + cfg := tc.cfg + if got := clientCredsSource(credsGlobals(&cfg, tc.cc, tc.bs, tc.bsErr)); got != tc.want { + t.Fatalf("clientCredsSource() = %q, want %q", got, tc.want) + } + }) + } +} + +// A provisioned machine without creds must say so, not tell the user to go +// hand-paste org secrets as if the machine were unprovisioned. +func TestLoginWithoutCredsOnProvisionedMachine(t *testing.T) { + t.Setenv("QBO_CLIENT_ID", "") + t.Setenv("QBO_CLIENT_SECRET", "") + g := credsGlobals(&config.Config{}, auth.ClientCreds{}, &config.Bootstrap{VaultURL: "https://v.vault.azure.net"}, nil) + err := (&AuthLoginCmd{}).Run(g) + if got := exitCode(err); got != errfmt.ExitConfig { + t.Fatalf("exit = %d, want %d (%v)", got, errfmt.ExitConfig, err) + } + if !strings.Contains(err.Error(), "https://v.vault.azure.net") { + t.Fatalf("error should name the vault: %v", err) + } +} + +// A malformed bootstrap file must fail login loudly (exit 10), never read as +// an unprovisioned machine. +func TestLoginSurfacesBrokenBootstrap(t *testing.T) { + t.Setenv("QBO_CLIENT_ID", "") + t.Setenv("QBO_CLIENT_SECRET", "") + want := errfmt.Config("bootstrap config is missing required fields: vault_url") + g := credsGlobals(&config.Config{}, auth.ClientCreds{}, nil, want) + err := (&AuthLoginCmd{}).Run(g) + if err == nil || err.Error() != want.Error() { + t.Fatalf("err = %v, want %v", err, want) + } +} diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 303f1ea..e700d40 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -51,6 +51,30 @@ type Globals struct { ccOnce sync.Once cc auth.ClientCreds + + bsOnce sync.Once + bs *config.Bootstrap + bsErr error +} + +// Bootstrap lazily loads the machine-scope bootstrap config (installed by an +// org's device management), once per process. (nil, nil) means the machine is +// not org-provisioned. A malformed or incomplete file returns its config +// error so auth commands fail loudly (exit 10) instead of treating a +// half-provisioned machine as unprovisioned. Commands that never consult the +// bootstrap tier are unaffected. +func (g *Globals) Bootstrap() (*config.Bootstrap, error) { + g.bsOnce.Do(func() { + b, ok, err := config.LoadBootstrap() + if err != nil { + g.bsErr = err + return + } + if ok { + g.bs = b + } + }) + return g.bs, g.bsErr } // keyringCreds lazily loads the app-level OAuth client credentials from the diff --git a/internal/config/bootstrap.go b/internal/config/bootstrap.go new file mode 100644 index 0000000..31c35a8 --- /dev/null +++ b/internal/config/bootstrap.go @@ -0,0 +1,80 @@ +package config + +import ( + "encoding/json" + "os" + "runtime" + "strings" + + "github.com/voska/qbo-cli/internal/errfmt" +) + +// Bootstrap is machine-scope provisioning config installed by an org's device +// management (e.g. an Intune .pkg). It carries no secrets — only the pointers +// a first `qbo auth login` needs to fetch the OAuth client credentials from +// Azure Key Vault after an Entra ID sign-in. Its presence forms the lowest +// tier of client-credential resolution, below env, keyring, and config file, +// so the binary itself stays org-agnostic. +type Bootstrap struct { + // VaultURL is the Azure Key Vault base URL, e.g. https://myvault.vault.azure.net. + VaultURL string `json:"vault_url"` + // VaultSecretName names the Key Vault secret holding the client + // credentials JSON ({"client_id": ..., "client_secret": ...}). + VaultSecretName string `json:"vault_secret_name"` + // EntraTenantID is the Entra ID (Azure AD) tenant to authenticate against. + EntraTenantID string `json:"entra_tenant_id"` + // EntraClientID is the Entra app registration used for the sign-in that + // authorizes the Key Vault read. + EntraClientID string `json:"entra_client_id"` +} + +// BootstrapPath returns the machine-scope bootstrap file location, or the +// QBO_BOOTSTRAP_PATH override when set. +func BootstrapPath() string { + if p := os.Getenv("QBO_BOOTSTRAP_PATH"); p != "" { + return p + } + switch runtime.GOOS { + case "darwin": + return "/Library/Application Support/qbo/bootstrap.json" + case "windows": + return `C:\ProgramData\qbo\bootstrap.json` + default: + return "/etc/qbo/bootstrap.json" + } +} + +// LoadBootstrap reads the machine-scope bootstrap config. The bool reports +// whether the file existed; a missing file is not an error since most +// installs are not org-provisioned. A file that exists but is malformed or +// incomplete is a config error — a half-provisioned machine should fail +// loudly, not silently degrade to "no credentials". +func LoadBootstrap() (*Bootstrap, bool, error) { + path := BootstrapPath() + data, err := os.ReadFile(path) + if os.IsNotExist(err) { + return nil, false, nil + } + if err != nil { + return nil, false, errfmt.Wrap(errfmt.ExitConfig, "cannot read bootstrap config "+path, err) + } + var b Bootstrap + if err := json.Unmarshal(data, &b); err != nil { + return nil, false, errfmt.Wrap(errfmt.ExitConfig, "invalid bootstrap config "+path, err) + } + var missing []string + for _, f := range []struct{ name, val string }{ + {"vault_url", b.VaultURL}, + {"vault_secret_name", b.VaultSecretName}, + {"entra_tenant_id", b.EntraTenantID}, + {"entra_client_id", b.EntraClientID}, + } { + if f.val == "" { + missing = append(missing, f.name) + } + } + if len(missing) > 0 { + return nil, false, errfmt.Config("bootstrap config " + path + " is missing required fields: " + strings.Join(missing, ", ")) + } + return &b, true, nil +} diff --git a/internal/config/bootstrap_test.go b/internal/config/bootstrap_test.go new file mode 100644 index 0000000..8156b2a --- /dev/null +++ b/internal/config/bootstrap_test.go @@ -0,0 +1,80 @@ +package config + +import ( + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/voska/qbo-cli/internal/errfmt" +) + +func writeBootstrap(t *testing.T, content string) { + t.Helper() + path := filepath.Join(t.TempDir(), "bootstrap.json") + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + t.Setenv("QBO_BOOTSTRAP_PATH", path) +} + +func TestLoadBootstrapMissing(t *testing.T) { + t.Setenv("QBO_BOOTSTRAP_PATH", filepath.Join(t.TempDir(), "nope.json")) + b, ok, err := LoadBootstrap() + if err != nil || ok || b != nil { + t.Fatalf("missing file: got b=%v ok=%v err=%v, want nil false nil", b, ok, err) + } +} + +func TestLoadBootstrapValid(t *testing.T) { + writeBootstrap(t, `{ + "vault_url": "https://v.vault.azure.net", + "vault_secret_name": "qbo-client-creds", + "entra_tenant_id": "tenant", + "entra_client_id": "client" + }`) + b, ok, err := LoadBootstrap() + if err != nil || !ok { + t.Fatalf("valid file: ok=%v err=%v", ok, err) + } + if b.VaultURL != "https://v.vault.azure.net" || b.VaultSecretName != "qbo-client-creds" || + b.EntraTenantID != "tenant" || b.EntraClientID != "client" { + t.Fatalf("unexpected bootstrap: %+v", b) + } +} + +func TestLoadBootstrapInvalidJSON(t *testing.T) { + writeBootstrap(t, `{not json`) + _, ok, err := LoadBootstrap() + if ok || err == nil { + t.Fatalf("invalid JSON: ok=%v err=%v, want error", ok, err) + } + var e *errfmt.Error + if !errors.As(err, &e) || e.Code != errfmt.ExitConfig { + t.Fatalf("want ExitConfig error, got %v", err) + } +} + +func TestLoadBootstrapMissingFields(t *testing.T) { + writeBootstrap(t, `{"vault_url": "https://v.vault.azure.net", "entra_client_id": "client"}`) + _, ok, err := LoadBootstrap() + if ok || err == nil { + t.Fatalf("incomplete file: ok=%v err=%v, want error", ok, err) + } + for _, want := range []string{"vault_secret_name", "entra_tenant_id"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q should name missing field %q", err, want) + } + } + if strings.Contains(err.Error(), "vault_url,") || strings.Contains(err.Error(), "entra_client_id") { + t.Errorf("error %q names fields that were present", err) + } +} + +func TestBootstrapPathOverride(t *testing.T) { + t.Setenv("QBO_BOOTSTRAP_PATH", "/custom/bootstrap.json") + if got := BootstrapPath(); got != "/custom/bootstrap.json" { + t.Fatalf("BootstrapPath() = %q", got) + } +} From 4c5bce0cf209fd9504c980091c2a32d5876cc4e8 Mon Sep 17 00:00:00 2001 From: Brandon Carlson Date: Wed, 19 Aug 2026 22:03:44 -0500 Subject: [PATCH 05/15] chore: fix golangci-lint findings in test files Check the os.Unsetenv return in token_test.go and replace a raw invisible U+009B control character in write_test.go with its Unicode escape sequence. Co-Authored-By: Claude Fable 5 --- internal/auth/token_test.go | 4 +++- internal/output/write_test.go | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/auth/token_test.go b/internal/auth/token_test.go index de040b9..ee8f070 100644 --- a/internal/auth/token_test.go +++ b/internal/auth/token_test.go @@ -32,7 +32,9 @@ func TestFilePasswordRejectsSetButEmptyEnv(t *testing.T) { func TestFilePasswordFailsHeadlessWithoutEnv(t *testing.T) { resetFilePassCache(t) t.Setenv("QBO_KEYRING_FILE_PASSWORD", "x") // register restore, then unset - os.Unsetenv("QBO_KEYRING_FILE_PASSWORD") + if err := os.Unsetenv("QBO_KEYRING_FILE_PASSWORD"); err != nil { + t.Fatal(err) + } // Under `go test`, stdin is not a terminal, so the prompt path is // unreachable and the missing passphrase must fail, never silently // fall back to an empty one. diff --git a/internal/output/write_test.go b/internal/output/write_test.go index ebdbe93..60fc295 100644 --- a/internal/output/write_test.go +++ b/internal/output/write_test.go @@ -51,7 +51,7 @@ func TestSanitizeCellPassesPlainText(t *testing.T) { } func TestSanitizeMessageKeepsNewlinesStripsEscapes(t *testing.T) { - got := SanitizeMessage("line1\nline2\t\x1b[2Jrest›x") + got := SanitizeMessage("line1\nline2\t\x1b[2Jrest\u009bx") if got != "line1\nline2\t[2Jrestx" { t.Fatalf("got %q", got) } From b3f5e39ffffd79fe4fc3696dd8af23f6fcb69542 Mon Sep 17 00:00:00 2001 From: Brandon Carlson Date: Wed, 19 Aug 2026 22:08:46 -0500 Subject: [PATCH 06/15] chore: add beads tooling config Ignore local Dolt/beads state and add the bd init workflow block to AGENTS.md. The duplicate Codex-flavored block from bd setup codex was removed along with the .codex and .agents artifacts. Co-Authored-By: Claude Fable 5 --- .gitignore | 6 ++++++ AGENTS.md | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/.gitignore b/.gitignore index 9ef7d70..da3c655 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,9 @@ dist/ *.out .DS_Store mise.toml + +# Beads / Dolt files (added by bd init) +.dolt/ +*.db +.beads-credential-key +.beads/proxieddb/ diff --git a/AGENTS.md b/AGENTS.md index b9099be..09b4f0f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -49,3 +49,60 @@ make fmt # gofmt ## Commits Conventional Commits (`feat:`, `fix:`, `chore:`) with imperative summary. Keep changesets focused. + + +## Beads Issue Tracker + +This project uses **bd (beads)** for issue tracking. Run `bd prime` to see full workflow context and commands. + +### Quick Reference + +```bash +bd ready # Find available work +bd show # View issue details +bd update --claim # Claim work +bd close # Complete work +``` + +### Rules + +- Use `bd` for ALL task tracking — do NOT use TodoWrite, TaskCreate, or markdown TODO lists +- Run `bd prime` for detailed command reference and session close protocol +- Use `bd remember` for persistent knowledge — do NOT use MEMORY.md files + +**Architecture in one line:** issues live in a local Dolt DB; sync uses `refs/dolt/data` on your git remote; `.beads/issues.jsonl` is a passive export. See https://github.com/gastownhall/beads/blob/main/docs/SYNC_CONCEPTS.md for details and anti-patterns. + +## Agent Context Profiles + +The managed Beads block is task-tracking guidance, not permission to override repository, user, or orchestrator instructions. + +- **Conservative (default)**: Use `bd` for task tracking. Do not run git commits, git pushes, or Dolt remote sync unless explicitly asked. At handoff, report changed files, validation, and suggested next commands. +- **Minimal**: Keep tool instruction files as pointers to `bd prime`; use the same conservative git policy unless active instructions say otherwise. +- **Team-maintainer**: Only when the repository explicitly opts in, agents may close beads, run quality gates, commit, and push as part of session close. A current "do not commit" or "do not push" instruction still wins. + +## Session Completion + +This protocol applies when ending a Beads implementation workflow. It is subordinate to explicit user, repository, and orchestrator instructions. + +1. **File issues for remaining work** - Create beads for anything that needs follow-up +2. **Run quality gates** (if code changed) - Tests, linters, builds +3. **Update issue status** - Close finished work, update in-progress items +4. **Handle git/sync by active profile**: + ```bash + # Conservative/minimal/default: report status and proposed commands; wait for approval. + git status + + # Team-maintainer opt-in only, unless current instructions forbid it: + git pull --rebase + bd dolt push + git push + git status + ``` +5. **Hand off** - Summarize changes, validation, issue status, and any blocked sync/commit/push step + +**Critical rules:** +- Explicit user or orchestrator instructions override this Beads block. +- Do not commit or push without clear authority from the active profile or the current user request. +- If a required sync or push is blocked, stop and report the exact command and error. + + From d5be8a32cdf13b229a13debbe9f4e8659bc1af36 Mon Sep 17 00:00:00 2001 From: Brandon Carlson Date: Wed, 19 Aug 2026 22:29:56 -0500 Subject: [PATCH 07/15] feat: fetch client credentials from Azure Key Vault on first login On a provisioned machine with no configured client creds, qbo auth login now signs in to Entra ID (pure-Go MSAL, browser or --device-code, token cache in the qbo keyring) and fetches the Intuit client credentials from the org's Key Vault, storing them with origin=bootstrap before the Intuit OAuth flow. --no-input allows only the cached silent path (exit 4). The bearer token is only ever sent to https Azure Key Vault hosts; sign-in failures map onto the exit-code contract (6 for Conditional Access blocks and vault 403s, 4 for cancellations, 8 for network, 10 for misprovisioned secrets). set-client --clear also removes the cached Entra session. Closes qbo-cli-iyj.2. Co-Authored-By: Claude Fable 5 --- go.mod | 6 ++ go.sum | 13 +++ internal/auth/entracache.go | 50 ++++++++++++ internal/cmd/auth.go | 63 ++++++++++++++- internal/cmd/auth_test.go | 83 +++++++++++++++++-- internal/entra/entra.go | 141 +++++++++++++++++++++++++++++++++ internal/entra/entra_test.go | 76 ++++++++++++++++++ internal/vault/vault.go | 112 ++++++++++++++++++++++++++ internal/vault/vault_test.go | 149 +++++++++++++++++++++++++++++++++++ 9 files changed, 684 insertions(+), 9 deletions(-) create mode 100644 internal/auth/entracache.go create mode 100644 internal/entra/entra.go create mode 100644 internal/entra/entra_test.go create mode 100644 internal/vault/vault.go create mode 100644 internal/vault/vault_test.go diff --git a/go.mod b/go.mod index 34ed492..a129f1b 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.25.0 require ( github.com/99designs/keyring v1.2.2 + github.com/AzureAD/microsoft-authentication-library-for-go v1.8.0 github.com/alecthomas/kong v1.15.0 github.com/muesli/termenv v0.16.0 golang.org/x/oauth2 v0.36.0 @@ -16,10 +17,15 @@ require ( github.com/danieljoos/wincred v1.2.3 // indirect github.com/dvsekhvalnov/jose2go v1.8.0 // indirect github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2 // indirect + github.com/golang-jwt/jwt/v5 v5.2.2 // indirect + github.com/google/uuid v1.3.0 // indirect github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c // indirect + github.com/kylelemons/godebug v1.1.0 // indirect github.com/lucasb-eyer/go-colorful v1.4.0 // indirect github.com/mattn/go-isatty v0.0.22 // indirect github.com/mtibben/percent v0.2.1 // indirect + github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 // indirect github.com/rivo/uniseg v0.4.7 // indirect + golang.org/x/sync v0.10.0 // indirect golang.org/x/sys v0.46.0 // indirect ) diff --git a/go.sum b/go.sum index d877b1f..71377fa 100644 --- a/go.sum +++ b/go.sum @@ -2,6 +2,8 @@ github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 h1:/vQbFIOMb github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4/go.mod h1:hN7oaIRCjzsZ2dE+yG5k+rsdt3qcwykqK6HVGcKwsw4= github.com/99designs/keyring v1.2.2 h1:pZd3neh/EmUzWONb35LxQfvuY7kiSXAq3HQd97+XBn0= github.com/99designs/keyring v1.2.2/go.mod h1:wes/FrByc8j7lFOAGLGSNEg8f/PaI3cgTBqhFkHUrPk= +github.com/AzureAD/microsoft-authentication-library-for-go v1.8.0 h1:Nljr4q1GRA/5vCrMONS+g4u4LRHNgOXVSh3O43J2CnI= +github.com/AzureAD/microsoft-authentication-library-for-go v1.8.0/go.mod h1:Y33QHnf0FfdVewFFISOGe20mkZbxX4H839o955/PoeI= github.com/alecthomas/assert/v2 v2.11.0 h1:2Q9r3ki8+JYXvGsDyBXwH3LcJ+WK5D0gc5E8vS6K3D0= github.com/alecthomas/assert/v2 v2.11.0/go.mod h1:Bze95FyfUr7x34QZrjL+XP+0qgp/zg8yS+TtBj1WA3k= github.com/alecthomas/kong v1.15.0 h1:BVJstKbpO73zKpmIu+m/aLRrNmWwxXPIGTNin9VmLVI= @@ -18,6 +20,10 @@ github.com/dvsekhvalnov/jose2go v1.8.0 h1:LqkkVKAlHFfH9LOEl5fe4p/zL02OhWE7pCufMB github.com/dvsekhvalnov/jose2go v1.8.0/go.mod h1:QsHjhyTlD/lAVqn/NSbVZmSCGeDehTB/mPZadG+mhXU= github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2 h1:ZpnhV/YsD2/4cESfV5+Hoeu/iUR3ruzNvZ+yQfO03a0= github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2/go.mod h1:bBOAhwG1umN6/6ZUMtDFBMQR8jRg9O75tm9K00oMsK4= +github.com/golang-jwt/jwt/v5 v5.2.2 h1:Rl4B7itRWVtYIHFrSNd7vhTiz9UpLdi6gZhZ3wEeDy8= +github.com/golang-jwt/jwt/v5 v5.2.2/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= +github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I= +github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c h1:6rhixN/i8ZofjG1Y75iExal34USq5p+wiN1tpie8IrU= github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c/go.mod h1:NMPJylDgVpX0MLRlPy15sqSwOFv/U1GZ2m21JhFfek0= github.com/hexops/gotextdiff v1.0.3 h1:gitA9+qJrrTCsiCl7+kh75nPqQt1cx4ZkudSTLoUqJM= @@ -25,6 +31,8 @@ github.com/hexops/gotextdiff v1.0.3/go.mod h1:pSWU5MAI3yDq+fZBTazCSJysOMbxWL1BSo github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= +github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc= +github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/lucasb-eyer/go-colorful v1.4.0 h1:UtrWVfLdarDgc44HcS7pYloGHJUjHV/4FwW4TvVgFr4= github.com/lucasb-eyer/go-colorful v1.4.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= github.com/mattn/go-isatty v0.0.22 h1:j8l17JJ9i6VGPUFUYoTUKPSgKe/83EYU2zBC7YNKMw4= @@ -35,6 +43,8 @@ github.com/muesli/termenv v0.16.0 h1:S5AlUN9dENB57rsbnkPyfdGuWIlkmzJjbFf0Tf5FWUc github.com/muesli/termenv v0.16.0/go.mod h1:ZRfOIKPFDYQoDFF4Olj7/QJbW60Ol/kL1pU3VfY/Cnk= github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWbfPhv4DMiApHyliiK5xCTNVSPiaAs= github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= +github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 h1:KoWmjvw+nsYOo29YJK9vDA65RGE3NrOnUtO7a+RF9HU= +github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8/go.mod h1:HKlIX3XHQyzLZPlr7++PzdhaXEj94dEiJgZDTsxEqUI= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ= @@ -45,6 +55,9 @@ github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= +golang.org/x/sync v0.10.0 h1:3NQrjDixjgGwUOCaF8w2+VYHv0Ve/vGYSbdkTa98gmQ= +golang.org/x/sync v0.10.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= +golang.org/x/sys v0.0.0-20210616045830-e2b7044e8c71/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.46.0 h1:noSf2Fq6F8DBgS+LysIkx7rIExoNHJsxOAtPp4rthXw= golang.org/x/sys v0.46.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.44.0 h1:0rLvDRCtNj0gZkyIXhCyOb2OAzEhLVqc4B+hrsBhrmc= diff --git a/internal/auth/entracache.go b/internal/auth/entracache.go new file mode 100644 index 0000000..a741a7e --- /dev/null +++ b/internal/auth/entracache.go @@ -0,0 +1,50 @@ +package auth + +import ( + "github.com/99designs/keyring" + "github.com/voska/qbo-cli/internal/errfmt" +) + +// entraCacheKey is the reserved keyring entry holding the opaque MSAL token +// cache for the Entra ID sign-in used by the vault bootstrap flow. Like +// clientCredsKey, the hyphenated name never collides with numeric realm IDs. +const entraCacheKey = "entra-msal-cache" + +// StoreEntraCache persists the serialized MSAL token cache. +func StoreEntraCache(data []byte) error { + kr, err := openKeyring() + if err != nil { + return errfmt.Wrap(errfmt.ExitConfig, "cannot open keyring", err) + } + return kr.Set(keyring.Item{Key: entraCacheKey, Data: data}) +} + +// LoadEntraCache returns the serialized MSAL token cache. The bool reports +// whether an entry existed; a missing entry is not an error — it just means +// no Entra sign-in has been cached yet. +func LoadEntraCache() ([]byte, bool, error) { + kr, err := openKeyring() + if err != nil { + return nil, false, errfmt.Wrap(errfmt.ExitConfig, "cannot open keyring", err) + } + item, err := kr.Get(entraCacheKey) + if errNotFound(err) { + return nil, false, nil + } + if err != nil { + return nil, false, errfmt.Wrap(errfmt.ExitConfig, "cannot read Entra token cache", err) + } + return item.Data, true, nil +} + +// DeleteEntraCache removes the cached Entra sign-in, if any. +func DeleteEntraCache() error { + kr, err := openKeyring() + if err != nil { + return errfmt.Wrap(errfmt.ExitConfig, "cannot open keyring", err) + } + if err := kr.Remove(entraCacheKey); err != nil && !errNotFound(err) { + return errfmt.Wrap(errfmt.ExitConfig, "cannot remove Entra token cache", err) + } + return nil +} diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go index 4de75ca..654d0cb 100644 --- a/internal/cmd/auth.go +++ b/internal/cmd/auth.go @@ -1,13 +1,16 @@ package cmd import ( + "net/http" "os" "time" "github.com/voska/qbo-cli/internal/auth" "github.com/voska/qbo-cli/internal/config" + "github.com/voska/qbo-cli/internal/entra" "github.com/voska/qbo-cli/internal/errfmt" "github.com/voska/qbo-cli/internal/output" + "github.com/voska/qbo-cli/internal/vault" ) type AuthCmd struct { @@ -21,6 +24,44 @@ type AuthCmd struct { type AuthLoginCmd struct { Manual bool `help:"Print URL for manual copy instead of opening browser."` RedirectURI string `name:"redirect-uri" help:"OAuth redirect URI. Required for production (non-localhost). Set via flag, QBO_REDIRECT_URI, or config." env:"QBO_REDIRECT_URI"` + DeviceCode bool `name:"device-code" help:"Use a device code instead of a browser for the Entra sign-in (headless bootstrap)."` +} + +// fetchBootstrapCreds signs in to Entra ID and fetches the org's client +// credentials from Key Vault. A package var so tests can stub the network. +var fetchBootstrapCreds = func(g *Globals, b *config.Bootstrap, deviceCode bool) (auth.ClientCreds, error) { + token, err := entra.AcquireVaultToken(g.Ctx, entra.Options{ + TenantID: b.EntraTenantID, + ClientID: b.EntraClientID, + DeviceCode: deviceCode, + NoInput: g.CLI.NoInput, + Notify: func(msg string) { output.Hint("%s", msg) }, + }) + if err != nil { + return auth.ClientCreds{}, err + } + // The vault GET gets its own bounded client: the process context is only + // signal-cancelable, and a stalled connection must not hang login. + httpClient := &http.Client{Timeout: 60 * time.Second} + return vault.FetchClientCreds(g.Ctx, httpClient, b.VaultURL, b.VaultSecretName, token) +} + +// bootstrapCreds runs the vault bootstrap when no client credentials are +// configured on a provisioned machine, storing the fetched creds so a later +// failure in the Intuit flow doesn't force a second Entra round trip. +func bootstrapCreds(g *Globals, b *config.Bootstrap, deviceCode bool) (auth.ClientCreds, error) { + output.Hint("no client credentials configured — fetching from %s via Entra ID sign-in", b.VaultURL) + creds, err := fetchBootstrapCreds(g, b, deviceCode) + if err != nil { + return auth.ClientCreds{}, err + } + if err := auth.StoreClientCreds(creds); err != nil { + output.Warn("fetched credentials but could not save them to the keyring: %v", err) + } + // Refresh the memoized keyring creds (already resolved as empty above) so + // the post-login re-store sees these creds and preserves their origin. + g.cc = creds + return creds, nil } func (c *AuthLoginCmd) Run(g *Globals) error { @@ -33,10 +74,18 @@ func (c *AuthLoginCmd) Run(g *Globals) error { if err != nil { return err } - if b != nil { - return errfmt.Config("this machine is provisioned to fetch client credentials from " + b.VaultURL + ", but this qbo build does not support vault bootstrap yet — run: qbo auth set-client (or set QBO_CLIENT_ID and QBO_CLIENT_SECRET)") + if b == nil { + return errfmt.Config("set client credentials first — run: qbo auth set-client (or set QBO_CLIENT_ID and QBO_CLIENT_SECRET)") } - return errfmt.Config("set client credentials first — run: qbo auth set-client (or set QBO_CLIENT_ID and QBO_CLIENT_SECRET)") + if g.CLI.DryRun { + output.Hint("[dry-run] would sign in to Entra tenant %s and fetch client credentials from %s/secrets/%s", b.EntraTenantID, b.VaultURL, b.VaultSecretName) + return nil + } + creds, err := bootstrapCreds(g, b, c.DeviceCode) + if err != nil { + return err + } + clientID, clientSecret = creds.ClientID, creds.ClientSecret } redirectURI := c.RedirectURI @@ -226,12 +275,18 @@ type AuthSetClientCmd struct { func (c *AuthSetClientCmd) Run(g *Globals) error { if c.Clear { if g.CLI.DryRun { - output.Hint("[dry-run] would remove client credentials from keyring") + output.Hint("[dry-run] would remove client credentials and any cached Entra sign-in from keyring") return nil } if err := auth.DeleteClientCreds(); err != nil { return err } + // Clearing creds resets the bootstrap state too: the cached Entra + // session could silently re-fetch vault secrets and must not outlive + // the credentials it was used to obtain. + if err := auth.DeleteEntraCache(); err != nil { + return err + } output.Success("client credentials removed from keyring") return nil } diff --git a/internal/cmd/auth_test.go b/internal/cmd/auth_test.go index 44dcfe3..b895eb4 100644 --- a/internal/cmd/auth_test.go +++ b/internal/cmd/auth_test.go @@ -50,21 +50,94 @@ func TestClientCredsSourceTiers(t *testing.T) { } } -// A provisioned machine without creds must say so, not tell the user to go -// hand-paste org secrets as if the machine were unprovisioned. -func TestLoginWithoutCredsOnProvisionedMachine(t *testing.T) { +// A provisioned machine without creds must enter the bootstrap flow, and a +// bootstrap failure must surface through login with its exit code intact. +func TestLoginWithoutCredsOnProvisionedMachineBootstraps(t *testing.T) { t.Setenv("QBO_CLIENT_ID", "") t.Setenv("QBO_CLIENT_SECRET", "") + orig := fetchBootstrapCreds + t.Cleanup(func() { fetchBootstrapCreds = orig }) + fetchBootstrapCreds = func(_ *Globals, b *config.Bootstrap, _ bool) (auth.ClientCreds, error) { + return auth.ClientCreds{}, errfmt.New(errfmt.ExitForbidden, "access denied to "+b.VaultURL) + } g := credsGlobals(&config.Config{}, auth.ClientCreds{}, &config.Bootstrap{VaultURL: "https://v.vault.azure.net"}, nil) err := (&AuthLoginCmd{}).Run(g) - if got := exitCode(err); got != errfmt.ExitConfig { - t.Fatalf("exit = %d, want %d (%v)", got, errfmt.ExitConfig, err) + if got := exitCode(err); got != errfmt.ExitForbidden { + t.Fatalf("exit = %d, want %d (%v)", got, errfmt.ExitForbidden, err) } if !strings.Contains(err.Error(), "https://v.vault.azure.net") { t.Fatalf("error should name the vault: %v", err) } } +// --dry-run on a provisioned machine must return before any Entra or vault +// network activity. +func TestLoginBootstrapDryRunNoNetwork(t *testing.T) { + t.Setenv("QBO_CLIENT_ID", "") + t.Setenv("QBO_CLIENT_SECRET", "") + orig := fetchBootstrapCreds + t.Cleanup(func() { fetchBootstrapCreds = orig }) + fetchBootstrapCreds = func(*Globals, *config.Bootstrap, bool) (auth.ClientCreds, error) { + t.Fatal("dry-run must not fetch") + return auth.ClientCreds{}, nil + } + g := credsGlobals(&config.Config{}, auth.ClientCreds{}, &config.Bootstrap{VaultURL: "https://v", VaultSecretName: "s", EntraTenantID: "t", EntraClientID: "c"}, nil) + g.CLI.DryRun = true + if err := (&AuthLoginCmd{}).Run(g); err != nil { + t.Fatalf("dry-run login: %v", err) + } +} + +// A successful vault fetch must persist the creds (marked bootstrap) and +// refresh the memoized keyring creds before the Intuit flow runs, so a later +// Intuit failure or re-store can't lose them or their origin. +func TestBootstrapCredsStoresAndMarksOrigin(t *testing.T) { + t.Setenv("QBO_KEYRING_BACKEND", "file") + t.Setenv("QBO_KEYRING_FILE_PASSWORD", "test-passphrase") + t.Setenv("QBO_CONFIG_DIR", t.TempDir()) + orig := fetchBootstrapCreds + t.Cleanup(func() { fetchBootstrapCreds = orig }) + want := auth.ClientCreds{ClientID: "id", ClientSecret: "sec", Origin: auth.CredsOriginBootstrap} + fetchBootstrapCreds = func(*Globals, *config.Bootstrap, bool) (auth.ClientCreds, error) { + return want, nil + } + g := credsGlobals(&config.Config{}, auth.ClientCreds{}, nil, nil) + got, err := bootstrapCreds(g, &config.Bootstrap{VaultURL: "https://v"}, false) + if err != nil || got != want { + t.Fatalf("bootstrapCreds = %+v, %v", got, err) + } + if g.keyringCreds() != want { + t.Fatalf("memoized creds not refreshed: %+v", g.keyringCreds()) + } + stored, ok, err := auth.LoadClientCreds() + if err != nil || !ok || stored != want { + t.Fatalf("stored = %+v ok=%v err=%v", stored, ok, err) + } + if clientCredsSource(g) != "bootstrap" { + t.Fatalf("source = %q, want bootstrap", clientCredsSource(g)) + } +} + +// A fetch failure must propagate its exit code and leave nothing stored. +func TestBootstrapCredsFetchFailure(t *testing.T) { + t.Setenv("QBO_KEYRING_BACKEND", "file") + t.Setenv("QBO_KEYRING_FILE_PASSWORD", "test-passphrase") + t.Setenv("QBO_CONFIG_DIR", t.TempDir()) + orig := fetchBootstrapCreds + t.Cleanup(func() { fetchBootstrapCreds = orig }) + fetchBootstrapCreds = func(*Globals, *config.Bootstrap, bool) (auth.ClientCreds, error) { + return auth.ClientCreds{}, errfmt.New(errfmt.ExitForbidden, "denied") + } + g := credsGlobals(&config.Config{}, auth.ClientCreds{}, nil, nil) + _, err := bootstrapCreds(g, &config.Bootstrap{VaultURL: "https://v"}, false) + if got := exitCode(err); got != errfmt.ExitForbidden { + t.Fatalf("exit = %d, want %d (%v)", got, errfmt.ExitForbidden, err) + } + if _, ok, _ := auth.LoadClientCreds(); ok { + t.Fatal("failed fetch must not store creds") + } +} + // A malformed bootstrap file must fail login loudly (exit 10), never read as // an unprovisioned machine. func TestLoginSurfacesBrokenBootstrap(t *testing.T) { diff --git a/internal/entra/entra.go b/internal/entra/entra.go new file mode 100644 index 0000000..886cd12 --- /dev/null +++ b/internal/entra/entra.go @@ -0,0 +1,141 @@ +// Package entra acquires Azure Key Vault access tokens via an Entra ID +// sign-in, for the machine-scope bootstrap flow. It uses the pure-Go MSAL +// library (the Azure SDK's persistent cache needs cgo on macOS) with the +// token cache stored in the qbo keyring. +package entra + +import ( + "context" + "errors" + "net" + "net/url" + "strings" + + "github.com/AzureAD/microsoft-authentication-library-for-go/apps/cache" + "github.com/AzureAD/microsoft-authentication-library-for-go/apps/public" + "github.com/voska/qbo-cli/internal/auth" + "github.com/voska/qbo-cli/internal/errfmt" + "github.com/voska/qbo-cli/internal/output" +) + +// vaultScope requests a token usable against any Azure Key Vault; RBAC on the +// vault side decides what it can actually read. +const vaultScope = "https://vault.azure.net/.default" + +type Options struct { + TenantID string + ClientID string + // DeviceCode prints a code + URL instead of opening a local browser. + DeviceCode bool + // NoInput allows only the cached (silent) path; if interaction would be + // required the sign-in fails with ExitAuth. + NoInput bool + // Notify receives user-facing progress lines (e.g. the device-code + // instructions). Required when DeviceCode is set. + Notify func(string) +} + +// keyringCache adapts the qbo keyring to MSAL's cache interface. Read +// failures degrade to an empty cache with a warning — a corrupt cache should +// cost one extra sign-in, not block login. +type keyringCache struct{} + +func (keyringCache) Replace(_ context.Context, u cache.Unmarshaler, _ cache.ReplaceHints) error { + data, ok, err := auth.LoadEntraCache() + if err != nil { + output.Warn("ignoring unreadable Entra token cache: %v", err) + return nil + } + if !ok { + return nil + } + if err := u.Unmarshal(data); err != nil { + output.Warn("ignoring corrupt Entra token cache: %v", err) + } + return nil +} + +// Export must never return an error: MSAL aborts the whole acquisition on +// Export failure, which would discard a token that was already successfully +// acquired. A failed cache write costs one extra sign-in next time. +func (keyringCache) Export(_ context.Context, m cache.Marshaler, _ cache.ExportHints) error { + data, err := m.Marshal() + if err != nil { + output.Warn("could not serialize Entra token cache: %v", err) + return nil + } + if err := auth.StoreEntraCache(data); err != nil { + output.Warn("could not persist Entra token cache (next login will re-prompt): %v", err) + } + return nil +} + +// AcquireVaultToken signs in to Entra ID and returns a bearer token for Key +// Vault. It tries the cached account silently first, then falls back to an +// interactive browser or device-code flow per opts. +func AcquireVaultToken(ctx context.Context, opts Options) (string, error) { + client, err := public.New(opts.ClientID, + public.WithAuthority("https://login.microsoftonline.com/"+opts.TenantID), + public.WithCache(keyringCache{}), + ) + if err != nil { + return "", errfmt.Wrap(errfmt.ExitError, "cannot initialize Entra client", err) + } + scopes := []string{vaultScope} + + if accounts, err := client.Accounts(ctx); err == nil && len(accounts) > 0 { + result, err := client.AcquireTokenSilent(ctx, scopes, public.WithSilentAccount(accounts[0])) + if err == nil { + return result.AccessToken, nil + } + // Silent failure (expired refresh token, revoked session) falls + // through to interactive below. + } + + if opts.NoInput { + return "", errfmt.New(errfmt.ExitAuth, "Entra sign-in required but --no-input given — run qbo auth login interactively once to cache the sign-in") + } + + if opts.DeviceCode { + dc, err := client.AcquireTokenByDeviceCode(ctx, scopes) + if err != nil { + return "", classify(err) + } + opts.Notify(dc.Result.Message) + result, err := dc.AuthenticationResult(ctx) + if err != nil { + return "", classify(err) + } + return result.AccessToken, nil + } + + result, err := client.AcquireTokenInteractive(ctx, scopes) + if err != nil { + return "", classify(err) + } + return result.AccessToken, nil +} + +// classify maps Entra sign-in failures onto the CLI exit-code contract: +// Conditional Access blocks are permission problems (6), cancellations need a +// re-login (4), transport problems are retryable (8). +func classify(err error) error { + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + return errfmt.Wrap(errfmt.ExitAuth, "Entra sign-in canceled", err) + } + // AADSTS530xx are Conditional Access blocks (unmanaged/non-compliant + // device); AADSTS50005/50131 are related device-state denials. + msg := err.Error() + if strings.Contains(msg, "AADSTS53") || strings.Contains(msg, "AADSTS50131") || strings.Contains(msg, "AADSTS50005") { + return errfmt.Wrap(errfmt.ExitForbidden, "Entra sign-in blocked by policy: this device must be Intune-enrolled and compliant — contact IT", err) + } + if strings.Contains(msg, "AADSTS65004") { // user declined consent + return errfmt.Wrap(errfmt.ExitAuth, "Entra sign-in declined", err) + } + var ne net.Error + var ue *url.Error + if errors.As(err, &ne) || errors.As(err, &ue) { + return errfmt.Wrap(errfmt.ExitRetryable, "cannot reach Entra ID", err) + } + return errfmt.Wrap(errfmt.ExitAuth, "Entra sign-in failed", err) +} diff --git a/internal/entra/entra_test.go b/internal/entra/entra_test.go new file mode 100644 index 0000000..6baceef --- /dev/null +++ b/internal/entra/entra_test.go @@ -0,0 +1,76 @@ +package entra + +import ( + "context" + "errors" + "net/url" + "testing" + + "github.com/AzureAD/microsoft-authentication-library-for-go/apps/cache" + "github.com/voska/qbo-cli/internal/errfmt" +) + +func classifiedCode(t *testing.T, err error) int { + t.Helper() + var e *errfmt.Error + if !errors.As(classify(err), &e) { + t.Fatalf("classify returned non-errfmt error: %v", err) + } + return e.Code +} + +func TestClassify(t *testing.T) { + cases := []struct { + name string + err error + want int + }{ + {"conditional access block", errors.New(`AADSTS530003: Your device is required to be managed`), errfmt.ExitForbidden}, + {"device state denial", errors.New(`AADSTS50131: Device is not in required state`), errfmt.ExitForbidden}, + {"platform blocked", errors.New(`AADSTS50005: Device platform is not supported by policy`), errfmt.ExitForbidden}, + {"declined consent", errors.New(`AADSTS65004: User declined to consent`), errfmt.ExitAuth}, + {"canceled", context.Canceled, errfmt.ExitAuth}, + {"network", &url.Error{Op: "Get", URL: "https://login.microsoftonline.com", Err: errors.New("connection refused")}, errfmt.ExitRetryable}, + {"generic", errors.New("something else"), errfmt.ExitAuth}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := classifiedCode(t, tc.err); got != tc.want { + t.Fatalf("classify(%v) exit = %d, want %d", tc.err, got, tc.want) + } + }) + } +} + +type fakeCacheData struct{ data []byte } + +func (f *fakeCacheData) Marshal() ([]byte, error) { return f.data, nil } +func (f *fakeCacheData) Unmarshal(b []byte) error { f.data = append([]byte(nil), b...); return nil } + +// The keyring-backed MSAL cache must round-trip bytes and treat a missing +// entry as an empty cache. +func TestKeyringCacheRoundTrip(t *testing.T) { + t.Setenv("QBO_KEYRING_BACKEND", "file") + t.Setenv("QBO_KEYRING_FILE_PASSWORD", "test-passphrase") + t.Setenv("QBO_CONFIG_DIR", t.TempDir()) + + c := keyringCache{} + empty := &fakeCacheData{data: []byte("sentinel")} + if err := c.Replace(context.Background(), empty, cache.ReplaceHints{}); err != nil { + t.Fatalf("Replace on empty cache: %v", err) + } + if string(empty.data) != "sentinel" { + t.Fatalf("Replace overwrote cache despite no stored entry: %q", empty.data) + } + + if err := c.Export(context.Background(), &fakeCacheData{data: []byte("msal-cache-bytes")}, cache.ExportHints{}); err != nil { + t.Fatalf("Export: %v", err) + } + loaded := &fakeCacheData{} + if err := c.Replace(context.Background(), loaded, cache.ReplaceHints{}); err != nil { + t.Fatalf("Replace: %v", err) + } + if string(loaded.data) != "msal-cache-bytes" { + t.Fatalf("round-trip = %q", loaded.data) + } +} diff --git a/internal/vault/vault.go b/internal/vault/vault.go new file mode 100644 index 0000000..8bf6b8b --- /dev/null +++ b/internal/vault/vault.go @@ -0,0 +1,112 @@ +// Package vault fetches the org's OAuth client credentials from an Azure Key +// Vault secret. It is a single authenticated REST GET rather than the Azure +// SDK: the SDK's credential chain and persistent cache assume cgo on macOS, +// and one secret read doesn't justify the dependency tree. +package vault + +import ( + "context" + "encoding/json" + "errors" + "io" + "net/http" + "net/url" + "strings" + + "github.com/voska/qbo-cli/internal/auth" + "github.com/voska/qbo-cli/internal/errfmt" +) + +const apiVersion = "7.4" + +// checkVaultURL guards where the bearer token goes: bootstrap.json is +// machine-scope but not beyond tampering, and the token is valid for every +// vault the user can access — it must never be attached to a request leaving +// for an arbitrary (or cleartext) host. A package var so tests can point at a +// local fake vault. +var checkVaultURL = validateVaultURL + +// vaultHostSuffixes are the Key Vault DNS suffixes across Azure clouds. +var vaultHostSuffixes = []string{ + ".vault.azure.net", + ".vault.azure.cn", + ".vault.usgovcloudapi.net", + ".vault.microsoftazure.de", + ".managedhsm.azure.net", +} + +func validateVaultURL(vaultURL string) error { + u, err := url.Parse(vaultURL) + if err != nil || u.Scheme != "https" || u.Hostname() == "" { + return errfmt.New(errfmt.ExitConfig, "vault_url must be an https Azure Key Vault URL, got "+vaultURL) + } + host := strings.ToLower(u.Hostname()) + for _, suffix := range vaultHostSuffixes { + if strings.HasSuffix(host, suffix) { + return nil + } + } + return errfmt.New(errfmt.ExitConfig, "vault_url host "+host+" is not an Azure Key Vault — refusing to send credentials to it") +} + +// FetchClientCreds reads the named secret and parses it as client-credential +// JSON ({"client_id": ..., "client_secret": ..., "redirect_uri"?: ...}). +// The returned creds are marked Origin=bootstrap. +func FetchClientCreds(ctx context.Context, client *http.Client, vaultURL, secretName, bearer string) (auth.ClientCreds, error) { + if err := checkVaultURL(vaultURL); err != nil { + return auth.ClientCreds{}, err + } + secretURL := strings.TrimSuffix(vaultURL, "/") + "/secrets/" + url.PathEscape(secretName) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, secretURL+"?api-version="+apiVersion, nil) + if err != nil { + return auth.ClientCreds{}, errfmt.Wrap(errfmt.ExitError, "cannot build vault request", err) + } + req.Header.Set("Authorization", "Bearer "+bearer) + + resp, err := client.Do(req) + if err != nil { + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + return auth.ClientCreds{}, errfmt.Wrap(errfmt.ExitAuth, "vault fetch canceled", err) + } + return auth.ClientCreds{}, errfmt.Wrap(errfmt.ExitRetryable, "cannot reach key vault "+vaultURL, err) + } + defer func() { _ = resp.Body.Close() }() + body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + if err != nil { + return auth.ClientCreds{}, errfmt.Wrap(errfmt.ExitRetryable, "cannot read vault response", err) + } + + switch { + case resp.StatusCode == http.StatusOK: + case resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden: + return auth.ClientCreds{}, errfmt.New(errfmt.ExitForbidden, "access denied to "+secretURL+" — ask IT to grant your account access to the qbo credentials secret") + case resp.StatusCode == http.StatusNotFound: + return auth.ClientCreds{}, errfmt.New(errfmt.ExitConfig, "secret not found at "+secretURL+" — the vault is misprovisioned, contact IT") + case resp.StatusCode == http.StatusTooManyRequests || resp.StatusCode >= 500: + return auth.ClientCreds{}, errfmt.New(errfmt.ExitRetryable, "key vault returned "+resp.Status+" for "+secretURL) + default: + return auth.ClientCreds{}, errfmt.New(errfmt.ExitError, "key vault returned "+resp.Status+" for "+secretURL) + } + + var payload struct { + Value string `json:"value"` + } + if err := json.Unmarshal(body, &payload); err != nil { + return auth.ClientCreds{}, errfmt.Wrap(errfmt.ExitError, "unexpected vault response for "+secretURL, err) + } + return parseClientCreds(payload.Value, secretURL) +} + +// parseClientCreds validates the secret's content. Malformed content is a +// provisioning error (exit 10) and must never be stored. +func parseClientCreds(value, secretURL string) (auth.ClientCreds, error) { + var c auth.ClientCreds + if err := json.Unmarshal([]byte(value), &c); err != nil { + return auth.ClientCreds{}, errfmt.Wrap(errfmt.ExitConfig, "secret at "+secretURL+" is not client-credential JSON — the vault is misprovisioned, contact IT", err) + } + if c.ClientID == "" || c.ClientSecret == "" { + return auth.ClientCreds{}, errfmt.New(errfmt.ExitConfig, "secret at "+secretURL+" is missing client_id or client_secret — the vault is misprovisioned, contact IT") + } + c.Origin = auth.CredsOriginBootstrap + return c, nil +} diff --git a/internal/vault/vault_test.go b/internal/vault/vault_test.go new file mode 100644 index 0000000..9bdae3f --- /dev/null +++ b/internal/vault/vault_test.go @@ -0,0 +1,149 @@ +package vault + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/voska/qbo-cli/internal/auth" + "github.com/voska/qbo-cli/internal/errfmt" +) + +func exitCode(t *testing.T, err error) int { + t.Helper() + var e *errfmt.Error + if !errors.As(err, &e) { + t.Fatalf("not an errfmt error: %v", err) + } + return e.Code +} + +func fakeVault(t *testing.T, status int, body any) *httptest.Server { + t.Helper() + // httptest serves plain http on 127.0.0.1, which the real URL guard + // rightly rejects — bypass it for fetch tests; it has its own test. + orig := checkVaultURL + checkVaultURL = func(string) error { return nil } + t.Cleanup(func() { checkVaultURL = orig }) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got := r.Header.Get("Authorization"); got != "Bearer tok" { + t.Errorf("Authorization = %q", got) + } + if r.URL.Query().Get("api-version") == "" { + t.Error("missing api-version") + } + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(body) + })) + t.Cleanup(srv.Close) + return srv +} + +// The bearer token must never be attached to a request bound for a non-Azure +// or cleartext host — bootstrap.json is machine-scope but not tamper-proof. +func TestValidateVaultURL(t *testing.T) { + valid := []string{ + "https://myvault.vault.azure.net", + "https://MyVault.VAULT.AZURE.NET/", + "https://gov.vault.usgovcloudapi.net", + } + for _, u := range valid { + if err := validateVaultURL(u); err != nil { + t.Errorf("validateVaultURL(%q) = %v, want nil", u, err) + } + } + invalid := []string{ + "http://myvault.vault.azure.net", // cleartext + "https://attacker.example", // arbitrary host + "https://vault.azure.net.evil.example", // suffix spoof + "not a url", + "", + } + for _, u := range invalid { + err := validateVaultURL(u) + if err == nil { + t.Errorf("validateVaultURL(%q) = nil, want error", u) + continue + } + if got := exitCode(t, err); got != errfmt.ExitConfig { + t.Errorf("validateVaultURL(%q) exit = %d, want %d", u, got, errfmt.ExitConfig) + } + } +} + +// The guard runs before any request is built. +func TestFetchClientCredsRejectsBadVaultURL(t *testing.T) { + _, err := FetchClientCreds(context.Background(), http.DefaultClient, "https://attacker.example", "s", "tok") + if got := exitCode(t, err); got != errfmt.ExitConfig { + t.Fatalf("exit = %d, want %d (%v)", got, errfmt.ExitConfig, err) + } +} + +func TestFetchClientCredsOK(t *testing.T) { + srv := fakeVault(t, 200, map[string]string{ + "value": `{"client_id":"id","client_secret":"sec","redirect_uri":"https://cb"}`, + }) + got, err := FetchClientCreds(context.Background(), srv.Client(), srv.URL, "qbo-creds", "tok") + if err != nil { + t.Fatal(err) + } + want := auth.ClientCreds{ClientID: "id", ClientSecret: "sec", RedirectURI: "https://cb", Origin: auth.CredsOriginBootstrap} + if got != want { + t.Fatalf("got %+v, want %+v", got, want) + } +} + +func TestFetchClientCredsForbidden(t *testing.T) { + srv := fakeVault(t, 403, map[string]string{"error": "denied"}) + _, err := FetchClientCreds(context.Background(), srv.Client(), srv.URL, "qbo-creds", "tok") + if got := exitCode(t, err); got != errfmt.ExitForbidden { + t.Fatalf("exit = %d, want %d (%v)", got, errfmt.ExitForbidden, err) + } + if !strings.Contains(err.Error(), srv.URL+"/secrets/qbo-creds") { + t.Fatalf("error should name the secret URL: %v", err) + } +} + +func TestFetchClientCredsNotFound(t *testing.T) { + srv := fakeVault(t, 404, map[string]string{"error": "nope"}) + _, err := FetchClientCreds(context.Background(), srv.Client(), srv.URL, "qbo-creds", "tok") + if got := exitCode(t, err); got != errfmt.ExitConfig { + t.Fatalf("exit = %d, want %d (%v)", got, errfmt.ExitConfig, err) + } +} + +func TestFetchClientCredsServerError(t *testing.T) { + srv := fakeVault(t, 503, map[string]string{"error": "down"}) + _, err := FetchClientCreds(context.Background(), srv.Client(), srv.URL, "qbo-creds", "tok") + if got := exitCode(t, err); got != errfmt.ExitRetryable { + t.Fatalf("exit = %d, want %d (%v)", got, errfmt.ExitRetryable, err) + } +} + +func TestFetchClientCredsUnreachable(t *testing.T) { + srv := fakeVault(t, 200, nil) + srv.Close() // connection refused from here on + _, err := FetchClientCreds(context.Background(), http.DefaultClient, srv.URL, "qbo-creds", "tok") + if got := exitCode(t, err); got != errfmt.ExitRetryable { + t.Fatalf("exit = %d, want %d (%v)", got, errfmt.ExitRetryable, err) + } +} + +func TestFetchClientCredsMalformedSecret(t *testing.T) { + for name, value := range map[string]string{ + "not json": "hunter2", + "missing fields": `{"client_id":"id"}`, + } { + t.Run(name, func(t *testing.T) { + srv := fakeVault(t, 200, map[string]string{"value": value}) + _, err := FetchClientCreds(context.Background(), srv.Client(), srv.URL, "qbo-creds", "tok") + if got := exitCode(t, err); got != errfmt.ExitConfig { + t.Fatalf("exit = %d, want %d (%v)", got, errfmt.ExitConfig, err) + } + }) + } +} From af0c457904b4f9fa076cb3636195c3db97850973 Mon Sep 17 00:00:00 2001 From: Brandon Carlson Date: Wed, 19 Aug 2026 22:49:29 -0500 Subject: [PATCH 08/15] feat: self-heal rotated vault credentials and bound sign-in waits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a token refresh fails with invalid_client (typed RFC 6749 check, not string matching) and the credentials are vault-owned — bootstrap origin in the keyring with no env overrides — silently re-fetch them from Key Vault using the cached Entra session and retry once per invocation, preserving the stored redirect URI. User-supplied credentials are never auto-replaced; any other failure guides re-login (exit 4). The Entra browser wait now times out after 5 minutes like the Intuit flow, every MSAL network leg is bounded by a 30s HTTP client so headless self-heal can't hang, the silent path picks the cached account matching the configured tenant, and a stalled vault fetch is retryable (exit 8). Closes qbo-cli-iyj.3, qbo-cli-iyj.4. Co-Authored-By: Claude Fable 5 --- internal/auth/oauth.go | 25 ++++- internal/cmd/auth.go | 17 ++- internal/cmd/auth_test.go | 8 +- internal/cmd/refresh_test.go | 207 +++++++++++++++++++++++++++++++++++ internal/cmd/root.go | 91 ++++++++++++++- internal/entra/entra.go | 44 ++++++-- internal/entra/entra_test.go | 1 + internal/vault/vault.go | 5 +- 8 files changed, 367 insertions(+), 31 deletions(-) create mode 100644 internal/cmd/refresh_test.go diff --git a/internal/auth/oauth.go b/internal/auth/oauth.go index c22343f..0c5193d 100644 --- a/internal/auth/oauth.go +++ b/internal/auth/oauth.go @@ -5,6 +5,7 @@ import ( "context" "crypto/rand" "encoding/hex" + "errors" "fmt" "net" "net/http" @@ -55,11 +56,33 @@ func RefreshAccessToken(ctx context.Context, clientID, clientSecret string, toke src := cfg.TokenSource(ctx, token) newToken, err := src.Token() if err != nil { - return nil, errfmt.Wrap(errfmt.ExitAuth, "token refresh failed", err) + wrapped := errfmt.Wrap(errfmt.ExitAuth, "token refresh failed", err) + // Classify while the typed oauth2 error is still in hand — errfmt + // flattens the chain to a string. + var re *oauth2.RetrieveError + if errors.As(err, &re) && re.ErrorCode == "invalid_client" { + return nil, &InvalidClientError{Err: wrapped} + } + return nil, wrapped } return newToken, nil } +// InvalidClientError marks a refresh that Intuit rejected because the client +// credentials themselves are bad (rotated or revoked app secret), as opposed +// to a dead refresh token. The vault self-heal keys off this type. +type InvalidClientError struct{ Err *errfmt.Error } + +func (e *InvalidClientError) Error() string { return e.Err.Error() } +func (e *InvalidClientError) Unwrap() error { return e.Err } + +// IsInvalidClient reports whether err (anywhere in its chain) is an +// InvalidClientError. +func IsInvalidClient(err error) bool { + var ice *InvalidClientError + return errors.As(err, &ice) +} + const DefaultCallbackPort = 8844 func DefaultRedirectURI() string { diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go index 654d0cb..38ee584 100644 --- a/internal/cmd/auth.go +++ b/internal/cmd/auth.go @@ -28,13 +28,15 @@ type AuthLoginCmd struct { } // fetchBootstrapCreds signs in to Entra ID and fetches the org's client -// credentials from Key Vault. A package var so tests can stub the network. -var fetchBootstrapCreds = func(g *Globals, b *config.Bootstrap, deviceCode bool) (auth.ClientCreds, error) { +// credentials from Key Vault. silentOnly restricts it to the cached Entra +// token (no browser or device code) — required for self-heal, which runs +// inside arbitrary commands. A package var so tests can stub the network. +var fetchBootstrapCreds = func(g *Globals, b *config.Bootstrap, deviceCode, silentOnly bool) (auth.ClientCreds, error) { token, err := entra.AcquireVaultToken(g.Ctx, entra.Options{ TenantID: b.EntraTenantID, ClientID: b.EntraClientID, DeviceCode: deviceCode, - NoInput: g.CLI.NoInput, + NoInput: silentOnly, Notify: func(msg string) { output.Hint("%s", msg) }, }) if err != nil { @@ -51,7 +53,7 @@ var fetchBootstrapCreds = func(g *Globals, b *config.Bootstrap, deviceCode bool) // failure in the Intuit flow doesn't force a second Entra round trip. func bootstrapCreds(g *Globals, b *config.Bootstrap, deviceCode bool) (auth.ClientCreds, error) { output.Hint("no client credentials configured — fetching from %s via Entra ID sign-in", b.VaultURL) - creds, err := fetchBootstrapCreds(g, b, deviceCode) + creds, err := fetchBootstrapCreds(g, b, deviceCode, g.CLI.NoInput) if err != nil { return auth.ClientCreds{}, err } @@ -242,12 +244,7 @@ func (c *AuthRefreshCmd) Run(g *Globals) error { if err != nil { return err } - clientID := g.ClientID() - clientSecret := g.ClientSecret() - if clientID == "" || clientSecret == "" { - return errfmt.Config("client credentials required — run: qbo auth set-client (or set QBO_CLIENT_ID and QBO_CLIENT_SECRET)") - } - newToken, err := auth.RefreshAccessToken(g.Ctx, clientID, clientSecret, token) + newToken, err := g.refreshToken(token) if err != nil { return err } diff --git a/internal/cmd/auth_test.go b/internal/cmd/auth_test.go index b895eb4..0ca688d 100644 --- a/internal/cmd/auth_test.go +++ b/internal/cmd/auth_test.go @@ -57,7 +57,7 @@ func TestLoginWithoutCredsOnProvisionedMachineBootstraps(t *testing.T) { t.Setenv("QBO_CLIENT_SECRET", "") orig := fetchBootstrapCreds t.Cleanup(func() { fetchBootstrapCreds = orig }) - fetchBootstrapCreds = func(_ *Globals, b *config.Bootstrap, _ bool) (auth.ClientCreds, error) { + fetchBootstrapCreds = func(_ *Globals, b *config.Bootstrap, _, _ bool) (auth.ClientCreds, error) { return auth.ClientCreds{}, errfmt.New(errfmt.ExitForbidden, "access denied to "+b.VaultURL) } g := credsGlobals(&config.Config{}, auth.ClientCreds{}, &config.Bootstrap{VaultURL: "https://v.vault.azure.net"}, nil) @@ -77,7 +77,7 @@ func TestLoginBootstrapDryRunNoNetwork(t *testing.T) { t.Setenv("QBO_CLIENT_SECRET", "") orig := fetchBootstrapCreds t.Cleanup(func() { fetchBootstrapCreds = orig }) - fetchBootstrapCreds = func(*Globals, *config.Bootstrap, bool) (auth.ClientCreds, error) { + fetchBootstrapCreds = func(*Globals, *config.Bootstrap, bool, bool) (auth.ClientCreds, error) { t.Fatal("dry-run must not fetch") return auth.ClientCreds{}, nil } @@ -98,7 +98,7 @@ func TestBootstrapCredsStoresAndMarksOrigin(t *testing.T) { orig := fetchBootstrapCreds t.Cleanup(func() { fetchBootstrapCreds = orig }) want := auth.ClientCreds{ClientID: "id", ClientSecret: "sec", Origin: auth.CredsOriginBootstrap} - fetchBootstrapCreds = func(*Globals, *config.Bootstrap, bool) (auth.ClientCreds, error) { + fetchBootstrapCreds = func(*Globals, *config.Bootstrap, bool, bool) (auth.ClientCreds, error) { return want, nil } g := credsGlobals(&config.Config{}, auth.ClientCreds{}, nil, nil) @@ -125,7 +125,7 @@ func TestBootstrapCredsFetchFailure(t *testing.T) { t.Setenv("QBO_CONFIG_DIR", t.TempDir()) orig := fetchBootstrapCreds t.Cleanup(func() { fetchBootstrapCreds = orig }) - fetchBootstrapCreds = func(*Globals, *config.Bootstrap, bool) (auth.ClientCreds, error) { + fetchBootstrapCreds = func(*Globals, *config.Bootstrap, bool, bool) (auth.ClientCreds, error) { return auth.ClientCreds{}, errfmt.New(errfmt.ExitForbidden, "denied") } g := credsGlobals(&config.Config{}, auth.ClientCreds{}, nil, nil) diff --git a/internal/cmd/refresh_test.go b/internal/cmd/refresh_test.go new file mode 100644 index 0000000..8d5c583 --- /dev/null +++ b/internal/cmd/refresh_test.go @@ -0,0 +1,207 @@ +package cmd + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/voska/qbo-cli/internal/auth" + "github.com/voska/qbo-cli/internal/config" + "github.com/voska/qbo-cli/internal/errfmt" + "golang.org/x/oauth2" +) + +// stubRefresh replaces the Intuit refresh call; each element of results is +// consumed in order, and calls are recorded with the creds used. +type refreshCall struct{ clientID, clientSecret string } + +func stubRefresh(t *testing.T, errs ...error) *[]refreshCall { + t.Helper() + orig := refreshAccessToken + t.Cleanup(func() { refreshAccessToken = orig }) + var calls []refreshCall + refreshAccessToken = func(_ context.Context, id, secret string, _ *oauth2.Token) (*oauth2.Token, error) { + calls = append(calls, refreshCall{id, secret}) + if len(calls) <= len(errs) && errs[len(calls)-1] != nil { + return nil, errs[len(calls)-1] + } + return &oauth2.Token{AccessToken: "fresh", Expiry: time.Now().Add(time.Hour)}, nil + } + return &calls +} + +func stubFetch(t *testing.T, creds auth.ClientCreds, err error) *int { + t.Helper() + orig := fetchBootstrapCreds + t.Cleanup(func() { fetchBootstrapCreds = orig }) + var calls int + fetchBootstrapCreds = func(_ *Globals, _ *config.Bootstrap, deviceCode, silentOnly bool) (auth.ClientCreds, error) { + calls++ + if deviceCode || !silentOnly { + t.Error("self-heal must be silent: no device code, no interaction") + } + return creds, err + } + return &calls +} + +func hermeticCmdKeyring(t *testing.T) { + t.Helper() + t.Setenv("QBO_KEYRING_BACKEND", "file") + t.Setenv("QBO_KEYRING_FILE_PASSWORD", "test-passphrase") + t.Setenv("QBO_CONFIG_DIR", t.TempDir()) + t.Setenv("QBO_CLIENT_ID", "") + t.Setenv("QBO_CLIENT_SECRET", "") +} + +var invalidClientErr error = &auth.InvalidClientError{Err: errfmt.Wrap(errfmt.ExitAuth, "token refresh failed", errfmt.New(errfmt.ExitAuth, `oauth2: "invalid_client"`))} + +// Rotated org secret: refresh fails invalid_client, creds are re-fetched +// silently from the vault, and the retry succeeds with the new secret. +func TestRefreshSelfHealsRotatedBootstrapCreds(t *testing.T) { + hermeticCmdKeyring(t) + refreshes := stubRefresh(t, invalidClientErr, nil) + rotated := auth.ClientCreds{ClientID: "id", ClientSecret: "new-sec", Origin: auth.CredsOriginBootstrap} + fetches := stubFetch(t, rotated, nil) + + g := credsGlobals(&config.Config{}, + auth.ClientCreds{ClientID: "id", ClientSecret: "old-sec", Origin: auth.CredsOriginBootstrap}, + &config.Bootstrap{VaultURL: "https://v.vault.azure.net"}, nil) + tok, err := g.refreshToken(&oauth2.Token{RefreshToken: "rt"}) + if err != nil || tok.AccessToken != "fresh" { + t.Fatalf("refreshToken = %v, %v", tok, err) + } + if *fetches != 1 { + t.Fatalf("fetches = %d, want 1", *fetches) + } + if got := *refreshes; len(got) != 2 || got[0].clientSecret != "old-sec" || got[1].clientSecret != "new-sec" { + t.Fatalf("refresh calls = %+v", got) + } + if stored, ok, _ := auth.LoadClientCreds(); !ok || stored != rotated { + t.Fatalf("rotated creds not persisted: %+v ok=%v", stored, ok) + } +} + +// User-supplied creds are never auto-replaced, whatever the error says. +func TestRefreshNeverHealsUserSuppliedCreds(t *testing.T) { + hermeticCmdKeyring(t) + stubRefresh(t, invalidClientErr) + fetches := stubFetch(t, auth.ClientCreds{}, nil) + + g := credsGlobals(&config.Config{}, + auth.ClientCreds{ClientID: "id", ClientSecret: "sec"}, // set-client, no origin + &config.Bootstrap{VaultURL: "https://v.vault.azure.net"}, nil) + _, err := g.refreshToken(&oauth2.Token{RefreshToken: "rt"}) + if got := exitCode(err); got != errfmt.ExitAuth { + t.Fatalf("exit = %d, want %d (%v)", got, errfmt.ExitAuth, err) + } + if !strings.Contains(err.Error(), "qbo auth login") { + t.Fatalf("error should guide re-login: %v", err) + } + if *fetches != 0 { + t.Fatalf("fetches = %d, want 0", *fetches) + } +} + +// A QBO_CLIENT_SECRET env override means the user supplied part of the +// credentials — never auto-replace, even with bootstrap-origin keyring creds. +func TestRefreshNeverHealsWithEnvSecretOverride(t *testing.T) { + hermeticCmdKeyring(t) + t.Setenv("QBO_CLIENT_SECRET", "user-rotated") + stubRefresh(t, invalidClientErr) + fetches := stubFetch(t, auth.ClientCreds{}, nil) + + g := credsGlobals(&config.Config{}, + auth.ClientCreds{ClientID: "id", ClientSecret: "old", Origin: auth.CredsOriginBootstrap}, + &config.Bootstrap{VaultURL: "https://v.vault.azure.net"}, nil) + if _, err := g.refreshToken(&oauth2.Token{RefreshToken: "rt"}); err == nil { + t.Fatal("want error") + } + if *fetches != 0 { + t.Fatalf("fetches = %d, want 0", *fetches) + } +} + +// The vault secret's redirect_uri is optional: healing must not drop the +// redirect URI that login stored. +func TestSelfHealPreservesStoredRedirectURI(t *testing.T) { + hermeticCmdKeyring(t) + stubRefresh(t, invalidClientErr, nil) + stubFetch(t, auth.ClientCreds{ClientID: "id", ClientSecret: "new", Origin: auth.CredsOriginBootstrap}, nil) + + g := credsGlobals(&config.Config{}, + auth.ClientCreds{ClientID: "id", ClientSecret: "old", RedirectURI: "https://prod/cb", Origin: auth.CredsOriginBootstrap}, + &config.Bootstrap{VaultURL: "https://v.vault.azure.net"}, nil) + if _, err := g.refreshToken(&oauth2.Token{RefreshToken: "rt"}); err != nil { + t.Fatalf("refreshToken: %v", err) + } + if stored, ok, _ := auth.LoadClientCreds(); !ok || stored.RedirectURI != "https://prod/cb" { + t.Fatalf("redirect not preserved: %+v ok=%v", stored, ok) + } +} + +// A dead refresh token (non-invalid_client failure) guides re-login without +// touching the vault. +func TestRefreshDeadTokenGuidesRelogin(t *testing.T) { + hermeticCmdKeyring(t) + stubRefresh(t, errfmt.Wrap(errfmt.ExitAuth, "token refresh failed", errfmt.New(errfmt.ExitAuth, `oauth2: "invalid_grant"`))) + fetches := stubFetch(t, auth.ClientCreds{}, nil) + + g := credsGlobals(&config.Config{}, + auth.ClientCreds{ClientID: "id", ClientSecret: "sec", Origin: auth.CredsOriginBootstrap}, + &config.Bootstrap{VaultURL: "https://v.vault.azure.net"}, nil) + _, err := g.refreshToken(&oauth2.Token{RefreshToken: "rt"}) + if got := exitCode(err); got != errfmt.ExitAuth { + t.Fatalf("exit = %d, want %d (%v)", got, errfmt.ExitAuth, err) + } + if !strings.Contains(err.Error(), "qbo auth login") { + t.Fatalf("error should guide re-login: %v", err) + } + if *fetches != 0 { + t.Fatalf("fetches = %d, want 0", *fetches) + } +} + +// The heal runs at most once per invocation, even if the retried refresh +// fails with invalid_client again. +func TestRefreshHealsAtMostOnce(t *testing.T) { + hermeticCmdKeyring(t) + refreshes := stubRefresh(t, invalidClientErr, invalidClientErr, invalidClientErr) + fetches := stubFetch(t, auth.ClientCreds{ClientID: "id", ClientSecret: "s", Origin: auth.CredsOriginBootstrap}, nil) + + g := credsGlobals(&config.Config{}, + auth.ClientCreds{ClientID: "id", ClientSecret: "s", Origin: auth.CredsOriginBootstrap}, + &config.Bootstrap{VaultURL: "https://v.vault.azure.net"}, nil) + if _, err := g.refreshToken(&oauth2.Token{RefreshToken: "rt"}); err == nil { + t.Fatal("want error") + } + // A second refresh attempt in the same invocation must not re-heal. + if _, err := g.refreshToken(&oauth2.Token{RefreshToken: "rt"}); err == nil { + t.Fatal("want error") + } + if *fetches != 1 { + t.Fatalf("fetches = %d, want 1", *fetches) + } + if len(*refreshes) != 3 { + t.Fatalf("refresh calls = %d, want 3", len(*refreshes)) + } +} + +// A failed vault re-fetch degrades to the guided re-login error. +func TestRefreshHealFetchFailureFallsBack(t *testing.T) { + hermeticCmdKeyring(t) + stubRefresh(t, invalidClientErr) + fetches := stubFetch(t, auth.ClientCreds{}, errfmt.New(errfmt.ExitAuth, "no cached sign-in")) + + g := credsGlobals(&config.Config{}, + auth.ClientCreds{ClientID: "id", ClientSecret: "s", Origin: auth.CredsOriginBootstrap}, + &config.Bootstrap{VaultURL: "https://v.vault.azure.net"}, nil) + _, err := g.refreshToken(&oauth2.Token{RefreshToken: "rt"}) + if got := exitCode(err); got != errfmt.ExitAuth { + t.Fatalf("exit = %d, want %d (%v)", got, errfmt.ExitAuth, err) + } + if *fetches != 1 { + t.Fatalf("fetches = %d, want 1", *fetches) + } +} diff --git a/internal/cmd/root.go b/internal/cmd/root.go index e700d40..32725d3 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -2,6 +2,7 @@ package cmd import ( "context" + "errors" "os" "sync" @@ -55,6 +56,9 @@ type Globals struct { bsOnce sync.Once bs *config.Bootstrap bsErr error + + // healAttempted caps the vault self-heal at one attempt per invocation. + healAttempted bool } // Bootstrap lazily loads the machine-scope bootstrap config (installed by an @@ -151,12 +155,7 @@ func (g *Globals) loadAndRefreshToken(realmID string) (*oauth2.Token, error) { return nil, err } if auth.IsTokenExpired(token) { - clientID := g.ClientID() - clientSecret := g.ClientSecret() - if clientID == "" || clientSecret == "" { - return nil, errfmt.Config("client credentials required for token refresh — run: qbo auth set-client (or set QBO_CLIENT_ID and QBO_CLIENT_SECRET)") - } - newToken, err := auth.RefreshAccessToken(g.Ctx, clientID, clientSecret, token) + newToken, err := g.refreshToken(token) if err != nil { return nil, err } @@ -168,6 +167,86 @@ func (g *Globals) loadAndRefreshToken(realmID string) (*oauth2.Token, error) { return token, nil } +// refreshAccessToken is a stub point for tests; production always hits Intuit. +var refreshAccessToken = auth.RefreshAccessToken + +// refreshToken exchanges the refresh token for a new access token. When the +// credentials came from the vault bootstrap and Intuit rejects them +// (invalid_client — the org rotated the app secret), it silently re-fetches +// from the vault using the cached Entra token and retries once. User-supplied +// credentials (env, set-client, config) are never auto-replaced. Persisting +// the new token is the caller's job. +func (g *Globals) refreshToken(token *oauth2.Token) (*oauth2.Token, error) { + clientID := g.ClientID() + clientSecret := g.ClientSecret() + if clientID == "" || clientSecret == "" { + return nil, errfmt.Config("client credentials required for token refresh — run: qbo auth set-client (or set QBO_CLIENT_ID and QBO_CLIENT_SECRET)") + } + newToken, err := refreshAccessToken(g.Ctx, clientID, clientSecret, token) + if err != nil && auth.IsInvalidClient(err) { + if creds, ok := g.selfHealCreds(); ok { + newToken, err = refreshAccessToken(g.Ctx, creds.ClientID, creds.ClientSecret, token) + } + } + if err != nil { + return nil, withReloginHint(err) + } + return newToken, nil +} + +// withReloginHint appends re-login guidance to an already-shaped errfmt error +// without re-wrapping it (Wrap flattens the message into the detail, which +// would stutter "token refresh failed" twice). +func withReloginHint(err error) error { + var e *errfmt.Error + if errors.As(err, &e) { + hinted := *e + hinted.Message += " — run: qbo auth login" + return &hinted + } + return errfmt.Wrap(errfmt.ExitAuth, "token refresh failed — run: qbo auth login", err) +} + +// credsAreBootstrapOwned reports whether the credentials the refresh actually +// used are the vault-fetched keyring entry — the only ones self-heal may +// replace. Any env override (either variable) means the user supplied at +// least part of the credentials, so they are never auto-replaced. +func (g *Globals) credsAreBootstrapOwned() bool { + return os.Getenv("QBO_CLIENT_ID") == "" && + os.Getenv("QBO_CLIENT_SECRET") == "" && + g.keyringCreds().Origin == auth.CredsOriginBootstrap +} + +// selfHealCreds re-fetches rotated client credentials from the vault, at most +// once per invocation, and only for credentials the bootstrap flow owns. +// It is silent by design: only the cached Entra token is used, and failure +// just falls back to the caller's guided re-login error. +func (g *Globals) selfHealCreds() (auth.ClientCreds, bool) { + if g.healAttempted || !g.credsAreBootstrapOwned() { + return auth.ClientCreds{}, false + } + g.healAttempted = true + b, err := g.Bootstrap() + if err != nil || b == nil { + return auth.ClientCreds{}, false + } + creds, err := fetchBootstrapCreds(g, b, false, true) + if err != nil { + output.Warn("could not refresh rotated credentials from vault: %v", err) + return auth.ClientCreds{}, false + } + // The vault secret's redirect_uri is optional; keep the one login stored + // so a production (non-localhost) setup survives the heal. + if creds.RedirectURI == "" { + creds.RedirectURI = g.keyringCreds().RedirectURI + } + if err := auth.StoreClientCreds(creds); err != nil { + output.Warn("fetched rotated credentials but could not save them to the keyring: %v", err) + } + g.cc = creds + return creds, true +} + func WriteOutput(ctx context.Context, data any) error { return output.Write(ctx, data) } diff --git a/internal/entra/entra.go b/internal/entra/entra.go index 886cd12..f0fdd78 100644 --- a/internal/entra/entra.go +++ b/internal/entra/entra.go @@ -8,8 +8,10 @@ import ( "context" "errors" "net" + "net/http" "net/url" "strings" + "time" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/cache" "github.com/AzureAD/microsoft-authentication-library-for-go/apps/public" @@ -22,6 +24,12 @@ import ( // vault side decides what it can actually read. const vaultScope = "https://vault.azure.net/.default" +// interactiveTimeout bounds the browser sign-in wait so an abandoned login — +// especially one driven by an agent — doesn't hang forever. It matches the +// Intuit flow's loginTimeout. The device-code flow is exempt: it carries its +// own server-issued expiry. +const interactiveTimeout = 5 * time.Minute + type Options struct { TenantID string ClientID string @@ -77,23 +85,36 @@ func AcquireVaultToken(ctx context.Context, opts Options) (string, error) { client, err := public.New(opts.ClientID, public.WithAuthority("https://login.microsoftonline.com/"+opts.TenantID), public.WithCache(keyringCache{}), + // Bound every network leg (silent refresh, device-code polling, code + // exchange): the process context is only signal-cancelable, and the + // silent path also runs headless inside arbitrary commands via + // self-heal, where a stalled connection must not hang forever. + public.WithHTTPClient(&http.Client{Timeout: 30 * time.Second}), ) if err != nil { return "", errfmt.Wrap(errfmt.ExitError, "cannot initialize Entra client", err) } scopes := []string{vaultScope} - if accounts, err := client.Accounts(ctx); err == nil && len(accounts) > 0 { - result, err := client.AcquireTokenSilent(ctx, scopes, public.WithSilentAccount(accounts[0])) - if err == nil { - return result.AccessToken, nil + // Pick the cached account for this tenant — the cache can hold sign-ins + // to other tenants, and accounts[0] order is not guaranteed. + if accounts, err := client.Accounts(ctx); err == nil { + for _, account := range accounts { + if !strings.EqualFold(account.Realm, opts.TenantID) { + continue + } + result, err := client.AcquireTokenSilent(ctx, scopes, public.WithSilentAccount(account)) + if err == nil { + return result.AccessToken, nil + } + // Silent failure (expired refresh token, revoked session) falls + // through to interactive below. + break } - // Silent failure (expired refresh token, revoked session) falls - // through to interactive below. } if opts.NoInput { - return "", errfmt.New(errfmt.ExitAuth, "Entra sign-in required but --no-input given — run qbo auth login interactively once to cache the sign-in") + return "", errfmt.New(errfmt.ExitAuth, "a cached Entra sign-in is required here but none is available — run qbo auth login interactively once to cache it") } if opts.DeviceCode { @@ -109,7 +130,9 @@ func AcquireVaultToken(ctx context.Context, opts Options) (string, error) { return result.AccessToken, nil } - result, err := client.AcquireTokenInteractive(ctx, scopes) + ictx, cancel := context.WithTimeout(ctx, interactiveTimeout) + defer cancel() + result, err := client.AcquireTokenInteractive(ictx, scopes) if err != nil { return "", classify(err) } @@ -120,7 +143,10 @@ func AcquireVaultToken(ctx context.Context, opts Options) (string, error) { // Conditional Access blocks are permission problems (6), cancellations need a // re-login (4), transport problems are retryable (8). func classify(err error) error { - if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + if errors.Is(err, context.DeadlineExceeded) { + return errfmt.Wrap(errfmt.ExitAuth, "Entra sign-in timed out waiting for the browser — run qbo auth login again", err) + } + if errors.Is(err, context.Canceled) { return errfmt.Wrap(errfmt.ExitAuth, "Entra sign-in canceled", err) } // AADSTS530xx are Conditional Access blocks (unmanaged/non-compliant diff --git a/internal/entra/entra_test.go b/internal/entra/entra_test.go index 6baceef..4615d4e 100644 --- a/internal/entra/entra_test.go +++ b/internal/entra/entra_test.go @@ -30,6 +30,7 @@ func TestClassify(t *testing.T) { {"platform blocked", errors.New(`AADSTS50005: Device platform is not supported by policy`), errfmt.ExitForbidden}, {"declined consent", errors.New(`AADSTS65004: User declined to consent`), errfmt.ExitAuth}, {"canceled", context.Canceled, errfmt.ExitAuth}, + {"timed out", context.DeadlineExceeded, errfmt.ExitAuth}, {"network", &url.Error{Op: "Get", URL: "https://login.microsoftonline.com", Err: errors.New("connection refused")}, errfmt.ExitRetryable}, {"generic", errors.New("something else"), errfmt.ExitAuth}, } diff --git a/internal/vault/vault.go b/internal/vault/vault.go index 8bf6b8b..98187b2 100644 --- a/internal/vault/vault.go +++ b/internal/vault/vault.go @@ -65,9 +65,12 @@ func FetchClientCreds(ctx context.Context, client *http.Client, vaultURL, secret resp, err := client.Do(req) if err != nil { - if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + if errors.Is(err, context.Canceled) { return auth.ClientCreds{}, errfmt.Wrap(errfmt.ExitAuth, "vault fetch canceled", err) } + // DeadlineExceeded here is the bounded HTTP client tripping on a + // stalled connection (the process context has no deadline) — a + // transport failure, retryable like the path below. return auth.ClientCreds{}, errfmt.Wrap(errfmt.ExitRetryable, "cannot reach key vault "+vaultURL, err) } defer func() { _ = resp.Body.Close() }() From 28981a8da523d9bcbbc79b399927aaeb54592c5d Mon Sep 17 00:00:00 2001 From: Brandon Carlson Date: Thu, 20 Aug 2026 14:07:05 -0500 Subject: [PATCH 09/15] feat: bouncer-mode login for hosted redirect URIs Intuit production apps reject http://localhost redirect URIs, so a hosted HTTPS page must receive the callback and forward it to the CLI. Route all redirect URIs through the local listener: a non-local redirect_uri is sent to Intuit (auth URL and token exchange) while the callback is still caught on localhost:8844. The --manual flag, previously declared but unused, now explicitly selects the paste-the-callback-URL flow. Skill docs updated to match. Also fix a latent race (callback handler signaled the result/error channels before writing the HTTP response, letting server.Close() kill the connection mid-response) and a listener leak on the GenerateState error path. Co-Authored-By: Claude Fable 5 --- internal/auth/config.go | 5 +- internal/auth/oauth.go | 42 ++++++-- internal/auth/oauth_test.go | 158 ++++++++++++++++++++++++++++++ internal/cmd/auth.go | 4 +- skills/qbo/SKILL.md | 14 +-- skills/qbo/references/COMMANDS.md | 2 +- 6 files changed, 206 insertions(+), 19 deletions(-) create mode 100644 internal/auth/oauth_test.go diff --git a/internal/auth/config.go b/internal/auth/config.go index b96555c..8f8a1a0 100644 --- a/internal/auth/config.go +++ b/internal/auth/config.go @@ -10,6 +10,9 @@ const ( var Scopes = []string{"com.intuit.quickbooks.accounting"} +// tokenURL is a var so tests can point the code exchange at a stub server. +var tokenURL = TokenURLProduction + func OAuthConfig(clientID, clientSecret, redirectURL string) *oauth2.Config { return &oauth2.Config{ ClientID: clientID, @@ -18,7 +21,7 @@ func OAuthConfig(clientID, clientSecret, redirectURL string) *oauth2.Config { Scopes: Scopes, Endpoint: oauth2.Endpoint{ AuthURL: AuthURLProduction, - TokenURL: TokenURLProduction, + TokenURL: tokenURL, }, } } diff --git a/internal/auth/oauth.go b/internal/auth/oauth.go index 0c5193d..0186122 100644 --- a/internal/auth/oauth.go +++ b/internal/auth/oauth.go @@ -98,18 +98,24 @@ func isLocalRedirect(redirectURL string) bool { return host == "localhost" || host == "127.0.0.1" || host == "::1" } -func LoginInteractive(ctx context.Context, clientID, clientSecret, redirectURL string) (*AuthResult, error) { +// LoginInteractive runs the browser OAuth flow, listening on +// localhost:8844 for the callback regardless of the registered redirect URI. +// A non-local redirectURL is "bouncer mode": Intuit requires HTTPS redirect +// URIs on production apps, so a hosted static page receives the callback and +// forwards it (query string intact) to the local listener. manual selects the +// paste-the-callback-URL flow instead, which requires an interactive stdin. +func LoginInteractive(ctx context.Context, clientID, clientSecret, redirectURL string, manual bool) (*AuthResult, error) { if redirectURL == "" { redirectURL = DefaultRedirectURI() } - if isLocalRedirect(redirectURL) { - return loginLocal(ctx, clientID, clientSecret, redirectURL) + if manual { + return loginManual(ctx, clientID, clientSecret, redirectURL) } - return loginManual(ctx, clientID, clientSecret, redirectURL) + return loginListen(ctx, clientID, clientSecret, redirectURL) } -func loginLocal(ctx context.Context, clientID, clientSecret, redirectURL string) (*AuthResult, error) { +func loginListen(ctx context.Context, clientID, clientSecret, redirectURL string) (*AuthResult, error) { ctx, cancel := context.WithTimeout(ctx, loginTimeout) defer cancel() @@ -122,33 +128,46 @@ func loginLocal(ctx context.Context, clientID, clientSecret, redirectURL string) cfg := OAuthConfig(clientID, clientSecret, redirectURL) state, err := GenerateState() if err != nil { + _ = listener.Close() return nil, err } resultCh := make(chan *AuthResult, 1) errCh := make(chan error, 1) + // The response must be written and flushed before signaling the channels: + // a signal makes loginListen return and Close the server, which kills the + // connection before an unflushed response reaches the browser. + flush := func(w http.ResponseWriter) { + if f, ok := w.(http.Flusher); ok { + f.Flush() + } + } mux := http.NewServeMux() mux.HandleFunc("/callback", func(w http.ResponseWriter, r *http.Request) { if r.URL.Query().Get("state") != state { - errCh <- errfmt.New(errfmt.ExitAuth, "state mismatch") http.Error(w, "state mismatch", http.StatusBadRequest) + flush(w) + errCh <- errfmt.New(errfmt.ExitAuth, "state mismatch") return } code := r.URL.Query().Get("code") realmID := r.URL.Query().Get("realmId") if code == "" || realmID == "" { - errCh <- errfmt.New(errfmt.ExitAuth, "missing code or realmId") http.Error(w, "missing parameters", http.StatusBadRequest) + flush(w) + errCh <- errfmt.New(errfmt.ExitAuth, "missing code or realmId") return } token, terr := cfg.Exchange(ctx, code) if terr != nil { - errCh <- errfmt.Wrap(errfmt.ExitAuth, "token exchange failed", terr) http.Error(w, "exchange failed", http.StatusInternalServerError) + flush(w) + errCh <- errfmt.Wrap(errfmt.ExitAuth, "token exchange failed", terr) return } _, _ = fmt.Fprint(w, "

Authenticated!

You can close this window.

") + flush(w) resultCh <- &AuthResult{Token: token, RealmID: realmID} }) @@ -162,6 +181,9 @@ func loginLocal(ctx context.Context, clientID, clientSecret, redirectURL string) authURL := cfg.AuthCodeURL(state, oauth2.AccessTypeOffline) fmt.Fprintf(os.Stderr, "Open this URL in your browser:\n\n %s\n\n", authURL) + if !isLocalRedirect(redirectURL) { + fmt.Fprintf(os.Stderr, "The callback will bounce through %s back to this machine.\n", redirectURL) + } openBrowser(authURL) select { @@ -218,7 +240,9 @@ func loginManual(ctx context.Context, clientID, clientSecret, redirectURL string return &AuthResult{Token: token, RealmID: realmID}, nil } -func openBrowser(url string) { +// openBrowser is a var so tests can capture the auth URL instead of opening +// a real browser. +var openBrowser = func(url string) { var cmd *exec.Cmd switch runtime.GOOS { case "darwin": diff --git a/internal/auth/oauth_test.go b/internal/auth/oauth_test.go new file mode 100644 index 0000000..a77dc2d --- /dev/null +++ b/internal/auth/oauth_test.go @@ -0,0 +1,158 @@ +package auth + +import ( + "context" + "fmt" + "io" + "net/http" + "net/http/httptest" + "net/url" + "testing" + "time" +) + +func TestIsLocalRedirect(t *testing.T) { + cases := []struct { + uri string + want bool + }{ + {"http://localhost:8844/callback", true}, + {"http://127.0.0.1:8844/callback", true}, + {"http://[::1]:8844/callback", true}, + {"https://qbo-auth.leantechniques.com/callback", false}, + {"https://developer.intuit.com/v2/OAuth2Playground/RedirectUrl", false}, + {"://not a url", false}, + } + for _, c := range cases { + if got := isLocalRedirect(c.uri); got != c.want { + t.Errorf("isLocalRedirect(%q) = %v, want %v", c.uri, got, c.want) + } + } +} + +// TestLoginInteractiveBouncer exercises bouncer mode end to end: a hosted +// (non-local) redirect URI goes to Intuit in the auth URL and the token +// exchange, while the callback itself lands on the local listener, as the +// hosted bouncer page would forward it. +func TestLoginInteractiveBouncer(t *testing.T) { + const hosted = "https://qbo-auth.example.com/callback" + + tokenSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if err := r.ParseForm(); err != nil { + t.Errorf("token request parse: %v", err) + } + if got := r.Form.Get("redirect_uri"); got != hosted { + t.Errorf("token exchange redirect_uri = %q, want %q", got, hosted) + } + if got := r.Form.Get("code"); got != "test-code" { + t.Errorf("token exchange code = %q, want %q", got, "test-code") + } + w.Header().Set("Content-Type", "application/json") + _, _ = fmt.Fprint(w, `{"access_token":"at","refresh_token":"rt","token_type":"bearer","expires_in":3600}`) + })) + defer tokenSrv.Close() + + origTokenURL := tokenURL + tokenURL = tokenSrv.URL + defer func() { tokenURL = origTokenURL }() + + authURLCh := make(chan string, 1) + origOpen := openBrowser + openBrowser = func(u string) { authURLCh <- u } + defer func() { openBrowser = origOpen }() + + type loginResult struct { + res *AuthResult + err error + } + done := make(chan loginResult, 1) + go func() { + res, err := LoginInteractive(context.Background(), "client-id", "client-secret", hosted, false) + done <- loginResult{res, err} + }() + + var authURL string + select { + case authURL = <-authURLCh: + case <-time.After(5 * time.Second): + t.Fatal("login never produced an auth URL") + } + + u, err := url.Parse(authURL) + if err != nil { + t.Fatalf("auth URL unparseable: %v", err) + } + q := u.Query() + if got := q.Get("redirect_uri"); got != hosted { + t.Errorf("auth URL redirect_uri = %q, want %q", got, hosted) + } + state := q.Get("state") + if state == "" { + t.Fatal("auth URL missing state") + } + + callback := fmt.Sprintf("http://127.0.0.1:%d/callback?state=%s&code=test-code&realmId=42", DefaultCallbackPort, url.QueryEscape(state)) + resp, err := http.Get(callback) + if err != nil { + t.Fatalf("callback request failed — bouncer mode did not start the local listener: %v", err) + } + body, _ := io.ReadAll(resp.Body) + _ = resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("callback returned %d: %s", resp.StatusCode, body) + } + + select { + case lr := <-done: + if lr.err != nil { + t.Fatalf("LoginInteractive: %v", lr.err) + } + if lr.res.RealmID != "42" { + t.Errorf("RealmID = %q, want %q", lr.res.RealmID, "42") + } + if lr.res.Token.AccessToken != "at" { + t.Errorf("AccessToken = %q, want %q", lr.res.Token.AccessToken, "at") + } + case <-time.After(5 * time.Second): + t.Fatal("login did not complete after callback") + } +} + +// TestLoginInteractiveStateMismatch confirms a forged callback is rejected. +func TestLoginInteractiveStateMismatch(t *testing.T) { + authURLCh := make(chan string, 1) + origOpen := openBrowser + openBrowser = func(u string) { authURLCh <- u } + defer func() { openBrowser = origOpen }() + + done := make(chan error, 1) + go func() { + _, err := LoginInteractive(context.Background(), "client-id", "client-secret", "", false) + done <- err + }() + + select { + case <-authURLCh: + case <-time.After(5 * time.Second): + t.Fatal("login never produced an auth URL") + } + + callback := fmt.Sprintf("http://127.0.0.1:%d/callback?state=wrong&code=x&realmId=1", DefaultCallbackPort) + resp, err := http.Get(callback) + if err != nil { + t.Fatalf("callback request failed: %v", err) + } + _ = resp.Body.Close() + if resp.StatusCode != http.StatusBadRequest { + t.Errorf("forged callback returned %d, want %d", resp.StatusCode, http.StatusBadRequest) + } + + select { + case lerr := <-done: + if lerr == nil { + t.Fatal("LoginInteractive accepted a state mismatch") + } + case <-time.After(5 * time.Second): + t.Fatal("login did not fail after state mismatch") + } +} diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go index 38ee584..efeb26a 100644 --- a/internal/cmd/auth.go +++ b/internal/cmd/auth.go @@ -22,7 +22,7 @@ type AuthCmd struct { } type AuthLoginCmd struct { - Manual bool `help:"Print URL for manual copy instead of opening browser."` + Manual bool `help:"Paste-the-callback-URL flow instead of the local listener (requires an interactive terminal)."` RedirectURI string `name:"redirect-uri" help:"OAuth redirect URI. Required for production (non-localhost). Set via flag, QBO_REDIRECT_URI, or config." env:"QBO_REDIRECT_URI"` DeviceCode bool `name:"device-code" help:"Use a device code instead of a browser for the Entra sign-in (headless bootstrap)."` } @@ -107,7 +107,7 @@ func (c *AuthLoginCmd) Run(g *Globals) error { return nil } - result, err := auth.LoginInteractive(g.Ctx, clientID, clientSecret, redirectURI) + result, err := auth.LoginInteractive(g.Ctx, clientID, clientSecret, redirectURI, c.Manual) if err != nil { return err } diff --git a/skills/qbo/SKILL.md b/skills/qbo/SKILL.md index 7b15c98..c4b6023 100644 --- a/skills/qbo/SKILL.md +++ b/skills/qbo/SKILL.md @@ -36,11 +36,10 @@ See [Intuit's OAuth 2.0 guide](https://developer.intuit.com/app/developer/qbo/do **Sandbox** allows `http://localhost:8844/callback` — just register it in the Intuit portal and `qbo auth login` handles everything automatically. -**Production** does not allow localhost. Three options: +**Production** does not allow localhost — Intuit requires an HTTPS redirect URI on a real host. Two options: -1. **Tunnel/funnel address** — Route a domain (e.g. `https://auth.yourdomain.com`) back to your machine. Register it as the redirect URI. Use `--redirect-uri` or set `QBO_REDIRECT_URI`. -2. **Login on the same machine** — If the agent runs on a machine with a browser, use a tunnel so localhost callbacks work. -3. **Non-resolving domain** — Register any domain you own (e.g. `https://yourdomain.com`) as the redirect URI. After authorizing, the browser redirects there with `?code=...&realmId=...&state=...` in the URL. Copy the full URL from the browser and paste it back into `qbo auth login` (or provide it to the agent to exchange manually). +1. **Bouncer page (recommended)** — Host a static HTTPS page that forwards the callback query string to the local listener (`location.replace('http://localhost:8844/callback' + location.search)`). Register its URL as the redirect URI and pass it via `--redirect-uri` or `QBO_REDIRECT_URI`. `qbo auth login` sends Intuit the hosted URI but still catches the forwarded callback locally — the login completes automatically, no pasting. +2. **Manual paste (`--manual`)** — Register any HTTPS URL you own as the redirect URI. After authorizing, the browser lands there with `?code=...&realmId=...&state=...`; copy the full URL and paste it back into `qbo auth login --manual`. Requires an interactive terminal. ## Setup @@ -59,11 +58,14 @@ qbo auth login --redirect-uri https://yourdomain.com export QBO_REDIRECT_URI=https://yourdomain.com qbo auth login -# Print the URL without opening a browser (useful for agents/remote) +# Paste-the-callback-URL flow instead of the local listener (needs a TTY) qbo auth login --manual ``` -For non-localhost redirect URIs, `qbo auth login` prints the auth URL and prompts you to paste the callback URL after authorizing. +`qbo auth login` always prints the auth URL and listens on `localhost:8844` +for the callback — for a non-localhost redirect URI the hosted page must +forward the callback there (bouncer mode, above). Use `--manual` only when +the local listener can't work (e.g. logging in from a different machine). Tokens **and** client credentials (once `qbo auth set-client` runs, or after the first `qbo auth login`) are stored in the system keychain (macOS Keychain, Windows Credential Manager) with file-based fallback at `~/.config/qbo/tokens/`. Credentials resolve in priority order: env var (`QBO_CLIENT_ID`/`QBO_CLIENT_SECRET`) → keychain → `config.json`, so env vars still work and override the keychain when set. On headless hosts that can't reach the login keychain, set `QBO_KEYRING_BACKEND=file` to use the encrypted-file backend instead — this also requires `QBO_KEYRING_FILE_PASSWORD` (the passphrase encrypting the token files; non-interactive runs fail without it). Tokens stored by versions that predate the passphrase requirement can't be decrypted; re-run `qbo auth login` once after upgrading. diff --git a/skills/qbo/references/COMMANDS.md b/skills/qbo/references/COMMANDS.md index 739ad4c..89e0633 100644 --- a/skills/qbo/references/COMMANDS.md +++ b/skills/qbo/references/COMMANDS.md @@ -49,7 +49,7 @@ qbo auth status # Show token status, company, and client-cred sour qbo auth refresh # Force token refresh ``` -For sandbox, the default `http://localhost:8844/callback` redirect works automatically. For production, pass `--redirect-uri` (or set `QBO_REDIRECT_URI`) to a registered public URI. Non-localhost URIs use a manual flow: paste the callback URL after authorizing in the browser. +For sandbox, the default `http://localhost:8844/callback` redirect works automatically. For production, pass `--redirect-uri` (or set `QBO_REDIRECT_URI`) to a registered HTTPS URI whose page forwards the callback query string to `http://localhost:8844/callback` (bouncer mode) — the login then completes with no pasting. `--manual` selects the paste-the-callback-URL flow instead (requires an interactive terminal). ### list From f9568f6826c4ec99b6f78f08b7d143681393ff6a Mon Sep 17 00:00:00 2001 From: Brandon Carlson Date: Thu, 20 Aug 2026 15:17:05 -0500 Subject: [PATCH 10/15] =?UTF-8?q?docs:=20correct=20CGO=20note=20=E2=80=94?= =?UTF-8?q?=20keychain=20backend=20requires=20cgo?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 09b4f0f..29277e9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -5,7 +5,7 @@ CLI for humans and AI agents. Data goes to stdout (parseable), hints/progress to ## Build & Test ```bash -make build # Build to bin/qbo (uses CGO_ENABLED=0 for pure Go) +make build # Build to bin/qbo (needs cgo: macOS Keychain backend is cgo-based) make test # Run tests with race detector make lint # golangci-lint make vet # go vet From ab558ba3e29175e17572eafd67d48fadafeb97ac Mon Sep 17 00:00:00 2001 From: Brandon Carlson Date: Thu, 20 Aug 2026 15:45:47 -0500 Subject: [PATCH 11/15] fix: scope skill allowed-tools to read-only qbo commands Bash(qbo *) pre-authorized mutating commands (create/update/delete/batch) whenever the skill was active, bypassing the permission prompt. Reads stay frictionless; writes now require explicit user approval. Co-Authored-By: Claude Fable 5 --- skills/qbo/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/qbo/SKILL.md b/skills/qbo/SKILL.md index c4b6023..cec352e 100644 --- a/skills/qbo/SKILL.md +++ b/skills/qbo/SKILL.md @@ -1,7 +1,7 @@ --- name: qbo description: Queries, creates, updates, and manages QuickBooks Online data via the qbo CLI. Use when working with QBO entities (invoices, customers, bills, payments, vendors, accounts, items, estimates, attachables, recurring transactions), running reports, or setting up install, auth, sandbox, and company switching. -allowed-tools: Bash(qbo *) +allowed-tools: Bash(qbo query:*), Bash(qbo get:*), Bash(qbo list:*), Bash(qbo report:*), Bash(qbo cdc:*), Bash(qbo schema), Bash(qbo schema:*), Bash(qbo exit-codes), Bash(qbo company info), Bash(qbo company info:*), Bash(qbo company list), Bash(qbo auth status) --- # qbo — QuickBooks Online CLI From 362a6c7cf2749b216d019dd702f729ac98a74119 Mon Sep 17 00:00:00 2001 From: Brandon Carlson Date: Mon, 24 Aug 2026 19:39:52 -0500 Subject: [PATCH 12/15] docs: add TimeActivity examples to qbo skill Co-Authored-By: Claude Fable 5 --- skills/qbo/SKILL.md | 2 + .../qbo/references/TIMEACTIVITY-EXAMPLES.md | 53 +++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 skills/qbo/references/TIMEACTIVITY-EXAMPLES.md diff --git a/skills/qbo/SKILL.md b/skills/qbo/SKILL.md index cec352e..cf2b652 100644 --- a/skills/qbo/SKILL.md +++ b/skills/qbo/SKILL.md @@ -189,3 +189,5 @@ qbo exit-codes --json # Exit codes as JSON ## Reference See [references/COMMANDS.md](references/COMMANDS.md) for full command reference. + +See [references/TIMEACTIVITY-EXAMPLES.md](references/TIMEACTIVITY-EXAMPLES.md) for time-tracking workflows. diff --git a/skills/qbo/references/TIMEACTIVITY-EXAMPLES.md b/skills/qbo/references/TIMEACTIVITY-EXAMPLES.md new file mode 100644 index 0000000..47d234c --- /dev/null +++ b/skills/qbo/references/TIMEACTIVITY-EXAMPLES.md @@ -0,0 +1,53 @@ +# TimeActivity Examples + +TimeActivity is registered as a generic entity (queryable, creatable, updatable, deletable), so it works through the standard verbs — no dedicated command. + +## Read + +```bash +# List time activities for a date range +qbo list timeactivity --where "TxnDate >= '2026-08-01' AND TxnDate <= '2026-08-24'" --json + +# Fetch one by ID +qbo get timeactivity 145 + +# Filter by employee, project useful fields +qbo query "SELECT * FROM TimeActivity WHERE EmployeeRef = '55'" --select Id,TxnDate,Hours,Minutes,Description +``` + +## Create + +JSON via file or stdin (`-f -`): + +```bash +echo '{ + "TxnDate": "2026-08-24", + "NameOf": "Employee", + "EmployeeRef": {"value": "55"}, + "CustomerRef": {"value": "102"}, + "ItemRef": {"value": "3"}, + "BillableStatus": "Billable", + "Hours": 4, + "Minutes": 30, + "Description": "Sprint planning and backlog grooming" +}' | qbo create timeactivity -f - + +# Dry-run first to see what would be sent +qbo create timeactivity -f entry.json --dry-run +``` + +Key fields: + +- `NameOf` — `"Employee"` or `"Vendor"`; pair with `EmployeeRef` or `VendorRef` accordingly. +- Duration — either `Hours`/`Minutes`, or `StartTime`/`EndTime` (ISO 8601). +- `BillableStatus` — `Billable`, `NotBillable`, or `HasBeenBilled`. Billable entries need `CustomerRef` and typically `HourlyRate`. +- `ItemRef` — service item; required when billable to a customer. + +## Update / Delete + +```bash +qbo update timeactivity 145 -f patch.json --sparse +qbo delete timeactivity 145 +``` + +Always include `Id` and `SyncToken` in the update payload. `qbo update` defaults to a FULL update — any writable field omitted from the payload gets cleared. Pass `--sparse` to update only the fields present in the payload. From f12eea9f46263119124d0463ba940c61c1583895 Mon Sep 17 00:00:00 2001 From: Brandon Carlson Date: Tue, 25 Aug 2026 08:57:27 -0500 Subject: [PATCH 13/15] feat: scheduled upstream sync with Claude conflict resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Weekday-cron workflow that merges voska/qbo-cli upstream changes into a SHA-keyed sync branch, runs the quality gates (build/test/vet/lint), and opens a review-gated PR. Dirty merges are resolved by claude-code-action authenticated via workload identity federation (no repo secrets), running without ambient git credentials, then re-verified deterministically — including that fork history and LT customizations survived — before the PR opens. Co-Authored-By: Claude Fable 5 --- .github/workflows/upstream-sync.yml | 239 ++++++++++++++++++++++++++++ 1 file changed, 239 insertions(+) create mode 100644 .github/workflows/upstream-sync.yml diff --git a/.github/workflows/upstream-sync.yml b/.github/workflows/upstream-sync.yml new file mode 100644 index 0000000..0beecb1 --- /dev/null +++ b/.github/workflows/upstream-sync.yml @@ -0,0 +1,239 @@ +name: Upstream Sync + +# Software factory: watch voska/qbo-cli for new commits, merge them into a +# sync branch, run the quality gates, and open a PR for human review. +# Clean merges that pass the gates open a ready PR directly. Conflicts or +# test failures are handed to Claude Code in CI to resolve; its result is +# re-verified deterministically before a PR is opened. Human merge is the +# ship gate — nothing lands on main automatically. + +on: + schedule: + - cron: "0 12 * * 1-5" # weekdays ~06:00 America/Chicago + workflow_dispatch: + +permissions: + contents: write + pull-requests: write + issues: write + +concurrency: + group: upstream-sync + cancel-in-progress: false + +env: + UPSTREAM_REPO: https://github.com/voska/qbo-cli.git + REVIEWER: bcarlso-lt + +jobs: + sync: + runs-on: ubuntu-latest + outputs: + status: ${{ steps.status.outputs.status }} + branch: ${{ steps.detect.outputs.branch }} + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 0 + + - name: Detect upstream changes + id: detect + run: | + git remote add upstream "$UPSTREAM_REPO" + git fetch upstream main + count=$(git rev-list --count HEAD..upstream/main) + echo "count=$count" >> "$GITHUB_OUTPUT" + # key the branch on the upstream SHA, not the date: idempotent per + # upstream state and immune to same-day rerun collisions + echo "branch=sync/upstream-$(git rev-parse --short upstream/main)" >> "$GITHUB_OUTPUT" + echo "Upstream commits not in main: $count" + + - name: Check for existing sync PR + id: existing + if: steps.detect.outputs.count != '0' + env: + GH_TOKEN: ${{ github.token }} + run: | + open=$(gh pr list --state open --json headRefName \ + --jq '[.[] | select(.headRefName | startswith("sync/upstream-"))] | length') + echo "open=$open" >> "$GITHUB_OUTPUT" + [ "$open" != "0" ] && echo "A sync PR is already open; skipping." || true + + - name: Attempt merge + id: merge + if: steps.detect.outputs.count != '0' && steps.existing.outputs.open == '0' + run: | + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git checkout -b "${{ steps.detect.outputs.branch }}" + if git merge --no-edit upstream/main; then + echo "merged=true" >> "$GITHUB_OUTPUT" + else + git merge --abort + echo "merged=false" >> "$GITHUB_OUTPUT" + fi + + - uses: actions/setup-go@v6 + if: steps.merge.outputs.merged == 'true' + with: + go-version: "1.25" + + - name: Quality gates + id: gates + if: steps.merge.outputs.merged == 'true' + continue-on-error: true + run: | + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh | sh -s -- -b /usr/local/bin v2.11.4 + make build + make test + make vet + make lint + + - name: Resolve status + id: status + run: | + if [ "${{ steps.detect.outputs.count }}" = "0" ]; then + echo "status=up-to-date" >> "$GITHUB_OUTPUT" + elif [ "${{ steps.existing.outputs.open }}" != "0" ]; then + echo "status=pr-exists" >> "$GITHUB_OUTPUT" + elif [ "${{ steps.merge.outputs.merged }}" = "true" ] && [ "${{ steps.gates.outcome }}" = "success" ]; then + echo "status=clean-green" >> "$GITHUB_OUTPUT" + else + echo "status=needs-claude" >> "$GITHUB_OUTPUT" + fi + + - name: Push branch and open PR + if: steps.status.outputs.status == 'clean-green' + env: + GH_TOKEN: ${{ github.token }} + run: | + # --force: the SHA-keyed branch may linger from an earlier failed run + git push --force origin "${{ steps.detect.outputs.branch }}" + { + echo "Automated upstream sync: merges \`voska/qbo-cli\` main into ours." + echo + echo "Merge was clean and \`make build\`, \`make test\`, \`make vet\`, \`make lint\` all passed in this workflow." + echo + echo "## Upstream commits" + echo + git log --oneline origin/main..upstream/main | sed 's/^/- /' + } > /tmp/pr-body.md + gh pr create \ + --title "sync: merge upstream $(date -u +%Y-%m-%d)" \ + --body-file /tmp/pr-body.md \ + --base main \ + --head "${{ steps.detect.outputs.branch }}" \ + --reviewer "$REVIEWER" + + claude-resolve: + needs: sync + if: needs.sync.outputs.status == 'needs-claude' + runs-on: ubuntu-latest + permissions: + contents: write + pull-requests: write + issues: write + id-token: write # fetch GitHub OIDC token for Anthropic identity federation + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 0 + # No ambient credentials while Claude runs: "do not push" is enforced + # by mechanism, not just prompt. The push step re-authenticates itself. + persist-credentials: false + + - uses: actions/setup-go@v6 + with: + go-version: "1.25" + + - name: Prepare sync branch + run: | + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh | sh -s -- -b /usr/local/bin v2.11.4 + git remote add upstream "$UPSTREAM_REPO" + git fetch upstream main + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git checkout -b "${{ needs.sync.outputs.branch }}" + + - name: Claude resolves the merge + uses: anthropics/claude-code-action@v1 + with: + # Anthropic workload identity federation — no API key secret needed. + # These are non-sensitive identifiers from the Anthropic Console + # (Organization settings → identity federation). + anthropic_organization_id: 63cc5434-820d-401a-b6c7-9a1e65133921 + anthropic_service_account_id: svac_01VsbTypvfaU2JjdxqTALKQG + anthropic_federation_rule_id: fdrl_013FbzWdgBJhvuJutsQhDSXW + claude_args: '--allowedTools "Bash(git:*),Bash(make:*),Bash(go:*),Read,Edit,Write,Grep,Glob"' + prompt: | + You are on branch ${{ needs.sync.outputs.branch }} of a fork of voska/qbo-cli. + The remote `upstream` is already fetched. Merge `upstream/main` into this + branch. A plain `git merge` either conflicts or breaks the quality gates — + your job is to produce a correct, green merge. + + Rules for resolving conflicts: + - This fork carries Lean Techniques customizations that MUST be preserved: + `internal/entra/`, `internal/vault/`, the bootstrap.json client-creds + tier in `internal/auth/` and `internal/config/`, and the bouncer-mode + login flow for hosted redirect URIs. + - For everything else, prefer upstream's version unless it breaks the + customizations above. + - After resolving, run `make build`, `make test`, `make vet`, and + `make lint` and fix any failures until all four pass. + - Commit the merge (and any follow-up fixes) with clear messages that list + the files you resolved by hand. + - Do NOT push; the workflow pushes after verifying your work. + + - name: Verify Claude's result + id: verify + continue-on-error: true + run: | + [ -z "$(git status --porcelain)" ] # fail on any uncommitted or untracked file + [ "$(git rev-list --count HEAD..upstream/main)" = "0" ] # upstream fully merged + git merge-base --is-ancestor origin/main HEAD # fork history preserved, not reset to upstream + test -d internal/entra && test -d internal/vault # LT customizations still present + make build + make test + make vet + make lint + + - name: Push branch and open PR + if: steps.verify.outcome == 'success' + env: + GH_TOKEN: ${{ github.token }} + run: | + # checkout ran with persist-credentials:false — authenticate this push + # explicitly; --force covers a leftover branch from an earlier run + git push --force \ + "https://x-access-token:${GH_TOKEN}@github.com/${{ github.repository }}.git" \ + "HEAD:refs/heads/${{ needs.sync.outputs.branch }}" + { + echo "Automated upstream sync: merges \`voska/qbo-cli\` main into ours." + echo + echo "> [!IMPORTANT]" + echo "> The plain merge conflicted or failed the quality gates, so **Claude" + echo "> Code resolved it in CI**. Gates were re-verified deterministically" + echo "> afterward, but review the resolution commits carefully." + echo + echo "## Merged upstream commits" + echo + git log --oneline origin/main..upstream/main | sed 's/^/- /' + } > /tmp/pr-body.md + gh pr create \ + --title "sync: merge upstream $(date -u +%Y-%m-%d) (Claude-resolved)" \ + --body-file /tmp/pr-body.md \ + --base main \ + --head "${{ needs.sync.outputs.branch }}" \ + --reviewer "$REVIEWER" + + - name: File issue on failure + # always(): must fire even when the Claude step itself failed (which + # would otherwise skip this step via the implicit success() guard) + if: always() && steps.verify.outcome != 'success' + env: + GH_TOKEN: ${{ github.token }} + run: | + gh issue create \ + --title "Upstream sync needs manual resolution ($(date -u +%Y-%m-%d))" \ + --body "The scheduled upstream sync could not be completed automatically: the plain merge of \`upstream/main\` was dirty and Claude's resolution did not verify green. Resolve manually: \`git fetch upstream && git merge upstream/main\`, fix conflicts/tests, and open a PR. Workflow run: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" \ + --assignee "$REVIEWER" From ce3efe7c859281fc6e492bc2d0f1b76e755ebc77 Mon Sep 17 00:00:00 2001 From: Brandon Carlson Date: Mon, 31 Aug 2026 12:24:55 -0500 Subject: [PATCH 14/15] docs: drop TimeActivity examples reference from qbo skill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes skills/qbo/references/TIMEACTIVITY-EXAMPLES.md and its link in SKILL.md — content was specific to our own use case and doesn't belong in the general-purpose skill. Co-Authored-By: Claude Sonnet 5 --- skills/qbo/SKILL.md | 2 - .../qbo/references/TIMEACTIVITY-EXAMPLES.md | 53 ------------------- 2 files changed, 55 deletions(-) delete mode 100644 skills/qbo/references/TIMEACTIVITY-EXAMPLES.md diff --git a/skills/qbo/SKILL.md b/skills/qbo/SKILL.md index cf2b652..cec352e 100644 --- a/skills/qbo/SKILL.md +++ b/skills/qbo/SKILL.md @@ -189,5 +189,3 @@ qbo exit-codes --json # Exit codes as JSON ## Reference See [references/COMMANDS.md](references/COMMANDS.md) for full command reference. - -See [references/TIMEACTIVITY-EXAMPLES.md](references/TIMEACTIVITY-EXAMPLES.md) for time-tracking workflows. diff --git a/skills/qbo/references/TIMEACTIVITY-EXAMPLES.md b/skills/qbo/references/TIMEACTIVITY-EXAMPLES.md deleted file mode 100644 index 47d234c..0000000 --- a/skills/qbo/references/TIMEACTIVITY-EXAMPLES.md +++ /dev/null @@ -1,53 +0,0 @@ -# TimeActivity Examples - -TimeActivity is registered as a generic entity (queryable, creatable, updatable, deletable), so it works through the standard verbs — no dedicated command. - -## Read - -```bash -# List time activities for a date range -qbo list timeactivity --where "TxnDate >= '2026-08-01' AND TxnDate <= '2026-08-24'" --json - -# Fetch one by ID -qbo get timeactivity 145 - -# Filter by employee, project useful fields -qbo query "SELECT * FROM TimeActivity WHERE EmployeeRef = '55'" --select Id,TxnDate,Hours,Minutes,Description -``` - -## Create - -JSON via file or stdin (`-f -`): - -```bash -echo '{ - "TxnDate": "2026-08-24", - "NameOf": "Employee", - "EmployeeRef": {"value": "55"}, - "CustomerRef": {"value": "102"}, - "ItemRef": {"value": "3"}, - "BillableStatus": "Billable", - "Hours": 4, - "Minutes": 30, - "Description": "Sprint planning and backlog grooming" -}' | qbo create timeactivity -f - - -# Dry-run first to see what would be sent -qbo create timeactivity -f entry.json --dry-run -``` - -Key fields: - -- `NameOf` — `"Employee"` or `"Vendor"`; pair with `EmployeeRef` or `VendorRef` accordingly. -- Duration — either `Hours`/`Minutes`, or `StartTime`/`EndTime` (ISO 8601). -- `BillableStatus` — `Billable`, `NotBillable`, or `HasBeenBilled`. Billable entries need `CustomerRef` and typically `HourlyRate`. -- `ItemRef` — service item; required when billable to a customer. - -## Update / Delete - -```bash -qbo update timeactivity 145 -f patch.json --sparse -qbo delete timeactivity 145 -``` - -Always include `Id` and `SyncToken` in the update payload. `qbo update` defaults to a FULL update — any writable field omitted from the payload gets cleared. Pass `--sparse` to update only the fields present in the payload. From 86e22d0a95b821877f3cb75d71e6d5c5d9dbf7f6 Mon Sep 17 00:00:00 2001 From: Brandon Carlson Date: Mon, 31 Aug 2026 12:24:59 -0500 Subject: [PATCH 15/15] chore: track shared .claude settings, ignore local/runtime state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit the project-level settings.json (beads plugin + bd prime hook) so it's shared across the team. Ignore settings.local.json (personal preference) and scheduled_tasks.lock (runtime state) — neither belongs in version control. Co-Authored-By: Claude Sonnet 5 --- .claude/settings.json | 18 ++++++++++++++++++ .gitignore | 4 ++++ 2 files changed, 22 insertions(+) create mode 100644 .claude/settings.json diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..7d804ce --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,18 @@ +{ + "enabledPlugins": { + "beads@beads-marketplace": true + }, + "hooks": { + "SessionStart": [ + { + "hooks": [ + { + "command": "bd prime --hook-json", + "type": "command" + } + ], + "matcher": "" + } + ] + } +} \ No newline at end of file diff --git a/.gitignore b/.gitignore index da3c655..242ff70 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,7 @@ mise.toml *.db .beads-credential-key .beads/proxieddb/ + +# Claude Code local/runtime state +.claude/settings.local.json +.claude/scheduled_tasks.lock