From cf770fbb3c5510c103a5c7acef2d72ff510951bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20=C3=96z=C3=A7elik?= Date: Thu, 16 Jul 2026 22:17:53 -0700 Subject: [PATCH 1/2] fix(pilot): route normal CLI output to stdout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cobra's Print family writes to OutOrStderr, and the root command never set an output writer, so every piece of normal output, secret values included, went to stderr. Shell redirection silently misbehaved: `spike secret get db/creds > creds.txt` produced an empty file while the secret splashed onto the terminal, and piping data into another tool required merging the streams first. Set the root command's output writer to stdout; subcommands inherit it. The PrintErr family keeps writing to stderr, so errors remain separable from data, which is what shell users and scripts expect. The bare-metal harness scripts already capture both streams (2>&1), so their behavior does not change. Spec: TBD Signed-off-by: Volkan Özçelik --- app/spike/internal/cmd/cmd.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/spike/internal/cmd/cmd.go b/app/spike/internal/cmd/cmd.go index 05dbc21f..1c3271a5 100644 --- a/app/spike/internal/cmd/cmd.go +++ b/app/spike/internal/cmd/cmd.go @@ -44,6 +44,14 @@ import ( // Initialize(source, "spiffe://example.org/pilot") // Execute() func Initialize(source *workloadapi.X509Source, SPIFFEID string) { + // Cobra's Print family writes to OutOrStderr, so without an explicit + // output writer every piece of normal output (secret values included) + // lands on stderr, and shell redirection such as + // `spike secret get db/creds > creds.txt` yields an empty file. + // Route data output to stdout; the PrintErr family keeps writing to + // stderr, so errors stay separable. Subcommands inherit this writer. + rootCmd.SetOut(os.Stdout) + rootCmd.AddCommand(policy.NewCommand(source, SPIFFEID)) rootCmd.AddCommand(secret.NewCommand(source, SPIFFEID)) rootCmd.AddCommand(cipher.NewCommand(source, SPIFFEID)) From 0c767efd3d63d8d1a0aa95a0d59fbce1b0995fa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20=C3=96z=C3=A7elik?= Date: Thu, 16 Jul 2026 22:17:53 -0700 Subject: [PATCH 2/2] test(nexus): isolate sqlite state tests from the real data directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sqlite-backed tests in state/base, state/persist, and backend/sqlite/persist operated on the real ~/.spike/data/spike.db and deleted it as part of their setup, which destroyed the database out from under a live bare-metal dev environment whenever `make test` ran alongside one. It bit three gate runs this week. fs.NexusDataFolder memoizes its result with sync.Once, so a per-test t.Setenv cannot redirect it once the first test has resolved the path. Instead, each affected package now sets SPIKE_NEXUS_DATA_DIR to a per-run temporary directory in TestMain, before anything resolves the folder, and removes the directory afterward. A full run of the three packages leaves ~/.spike untouched, and the temp-dir removal also cleans up the spike_test_*.db files the suite used to leave behind. TASKS.md: closes this task and the Pilot stdout task delivered by the previous commit. Spec: TBD Signed-off-by: Volkan Özçelik --- .context/TASKS.md | 4 +- .../backend/sqlite/persist/testmain_test.go | 39 +++++++++++++++++++ .../internal/state/base/testmain_test.go | 39 +++++++++++++++++++ app/nexus/internal/state/persist/init_test.go | 22 +++++++++++ 4 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 app/nexus/internal/state/backend/sqlite/persist/testmain_test.go create mode 100644 app/nexus/internal/state/base/testmain_test.go diff --git a/.context/TASKS.md b/.context/TASKS.md index 1cce02aa..29268d83 100644 --- a/.context/TASKS.md +++ b/.context/TASKS.md @@ -41,14 +41,14 @@ the name-based policy work. - [x] Retry sqlite operations with exponential backoff on transient locks → ideas/research-db-resilience.md #source:jira.xml #added:2026-07-14 #done:2026-07-16 (withSerializableTx retries SQLITE_BUSY/SQLITE_LOCKED with exponential backoff at the single choke point every write flows through; reads rely on WAL plus the busy_timeout DSN parameter and honor the operation deadline; note the DB ops live under state/backend/sqlite/persist these days, not state/persist) - [x] Bound the Bootstrap keeper-wait loop with a configurable timeout/max-attempts instead of looping forever → ideas/research-db-resilience.md #source:jira.xml #added:2026-07-14 #done:2026-07-16 (stale: superseded by the SDK retry migration; broadcastToKeeper bounds each keeper with retry.WithMaxAttempts, a per-keeper context timeout, and configurable backoff intervals — app/bootstrap/internal/net/dispatch.go — and broadcast.go bounds init verification with WithMaxElapsedTime) - [ ] Make `env` accessors return sentinel errors instead of calling `log.FatalLn` (removes env→log circular dep, makes them testable) → ideas/research-env-error-handling.md #source:jira.xml #added:2026-07-14 (2026-07-16: the offending accessors now live in spike-sdk-go config/env, so the sentinel-error refactor is an upstream SDK change; the in-repo share is adapting callers once the SDK ships it) -- [ ] Fix Pilot printing normal output to stderr: cobra Print* writes to OutOrStderr and the root command never calls SetOut, so data output (secrets included) lands on stderr and `spike secret get x > file.txt` yields an empty file; every harness script compensates with 2>&1. Set the root command output writer to stdout and audit scripts/docs for reliance on the old behavior. #added:2026-07-16 +- [x] Fix Pilot printing normal output to stderr: cobra Print* writes to OutOrStderr and the root command never calls SetOut, so data output (secrets included) lands on stderr and `spike secret get x > file.txt` yields an empty file; every harness script compensates with 2>&1. #added:2026-07-16 #done:2026-07-16 (rootCmd.SetOut(os.Stdout) in cmd.Initialize; PrintErr still goes to stderr, and the harness scripts that merge streams keep working) ### Phase 2: SDK Extraction `#priority:medium` - [ ] Graduate generic internal helpers to spike-sdk-go (nonce/crypto, Shamir verify, permission (de)serialize, validation, trust/spiffeid, URL builders, `GCMNonceSize`, `Id()`, canonical permission set) → ideas/research-sdk-extraction.md #source:jira.xml #added:2026-07-14 ### Phase 3: Testing `#priority:medium` - [ ] Make `make test` concurrent again (currently serialized by env setup) → ideas/research-cli-testing.md #source:jira.xml #added:2026-07-14 -- [ ] Move the sqlite state tests off the real ~/.spike/data/spike.db to t.TempDir(): make test deletes the live dev environment database mid-run (bit us twice on 2026-07-15 and 2026-07-16), and shared global state is part of why tests are serialized with -p 1. #added:2026-07-16 +- [x] Move the sqlite state tests off the real ~/.spike/data/spike.db: make test deleted the live dev environment database mid-run (bit us three times this week). #added:2026-07-16 #done:2026-07-16 (fs.NexusDataFolder is sync.Once-memoized, so per-test t.Setenv cannot work; instead each affected package sets SPIKE_NEXUS_DATA_DIR to a per-run temp dir in TestMain before the first resolution — state/base, state/persist, and backend/sqlite/persist — verified: a full package run leaves ~/.spike untouched) - [ ] Add integration tests: root key cached/recovered/not-re-initialized; secret & policy CRUD; Pilot denies when Nexus uninitialized / warns when unreachable → ideas/research-cli-testing.md #source:jira.xml #added:2026-07-14 - [ ] Raise CLI command coverage to 60%+ via unit + HTTP-mock tests; fix `t.Skip()`ed tests; DI-refactor `sendShardsToKeepers` → ideas/research-cli-testing.md #source:jira.xml #added:2026-07-14 - [x] `start.sh` should exercise recovery/restore and encryption/decryption #source:jira.xml #added:2026-07-14 #done:2026-07-16 (encryption/decryption checks live in start.sh since the policy-validation rework; recovery/restore is exercised by make drill-recovery, kept as a separate second-terminal script deliberately so the crash simulation never runs inside the normal startup path) diff --git a/app/nexus/internal/state/backend/sqlite/persist/testmain_test.go b/app/nexus/internal/state/backend/sqlite/persist/testmain_test.go new file mode 100644 index 00000000..1ae5d952 --- /dev/null +++ b/app/nexus/internal/state/backend/sqlite/persist/testmain_test.go @@ -0,0 +1,39 @@ +// \\ SPIKE: Secure your secrets with SPIFFE. — https://spike.ist/ +// \\\\\ Copyright 2024-present SPIKE contributors. +// \\\\\\\ SPDX-License-Identifier: Apache-2.0 + +package persist + +import ( + "fmt" + "os" + "testing" + + "github.com/spiffe/spike-sdk-go/config/env" +) + +// TestMain points SPIKE_NEXUS_DATA_DIR at a per-run temporary directory +// before any test resolves the Nexus data folder. fs.NexusDataFolder +// memoizes its result with sync.Once, so the override must happen before +// the first call; doing it here isolates the whole package run from the +// real ~/.spike/data directory, whose database these tests used to +// delete out from under a live dev environment. +func TestMain(m *testing.M) { + dir, mkErr := os.MkdirTemp("", "spike-sqlite-persist-test-*") + if mkErr != nil { + fmt.Fprintln(os.Stderr, + "failed to create a temporary data directory:", mkErr) + os.Exit(1) + } + + if setErr := os.Setenv(env.NexusDataDir, dir); setErr != nil { + _ = os.RemoveAll(dir) + fmt.Fprintln(os.Stderr, "failed to set "+env.NexusDataDir+":", setErr) + os.Exit(1) + } + + code := m.Run() + + _ = os.RemoveAll(dir) + os.Exit(code) +} diff --git a/app/nexus/internal/state/base/testmain_test.go b/app/nexus/internal/state/base/testmain_test.go new file mode 100644 index 00000000..071cd0a2 --- /dev/null +++ b/app/nexus/internal/state/base/testmain_test.go @@ -0,0 +1,39 @@ +// \\ SPIKE: Secure your secrets with SPIFFE. — https://spike.ist/ +// \\\\\ Copyright 2024-present SPIKE contributors. +// \\\\\\\ SPDX-License-Identifier: Apache-2.0 + +package base + +import ( + "fmt" + "os" + "testing" + + "github.com/spiffe/spike-sdk-go/config/env" +) + +// TestMain points SPIKE_NEXUS_DATA_DIR at a per-run temporary directory +// before any test resolves the Nexus data folder. fs.NexusDataFolder +// memoizes its result with sync.Once, so the override must happen before +// the first call; doing it here isolates the whole package run from the +// real ~/.spike/data directory, whose database these tests used to +// delete out from under a live dev environment. +func TestMain(m *testing.M) { + dir, mkErr := os.MkdirTemp("", "spike-state-base-test-*") + if mkErr != nil { + fmt.Fprintln(os.Stderr, + "failed to create a temporary data directory:", mkErr) + os.Exit(1) + } + + if setErr := os.Setenv(env.NexusDataDir, dir); setErr != nil { + _ = os.RemoveAll(dir) + fmt.Fprintln(os.Stderr, "failed to set "+env.NexusDataDir+":", setErr) + os.Exit(1) + } + + code := m.Run() + + _ = os.RemoveAll(dir) + os.Exit(code) +} diff --git a/app/nexus/internal/state/persist/init_test.go b/app/nexus/internal/state/persist/init_test.go index c60e9bf1..c7d277f6 100644 --- a/app/nexus/internal/state/persist/init_test.go +++ b/app/nexus/internal/state/persist/init_test.go @@ -5,9 +5,11 @@ package persist import ( + "fmt" "os" "testing" + "github.com/spiffe/spike-sdk-go/config/env" "github.com/spiffe/spike-sdk-go/crypto" "github.com/spiffe/spike/app/nexus/internal/state/backend/memory" @@ -311,6 +313,25 @@ func BenchmarkBackend_Access(b *testing.B) { // Helper to clean environment between tests func TestMain(m *testing.M) { + // Point SPIKE_NEXUS_DATA_DIR at a per-run temporary directory before + // any test resolves the Nexus data folder. fs.NexusDataFolder + // memoizes its result with sync.Once, so the override must happen + // before the first call; doing it here isolates the whole package + // run from the real ~/.spike/data directory, whose database these + // tests used to delete out from under a live dev environment. + dir, mkErr := os.MkdirTemp("", "spike-state-persist-test-*") + if mkErr != nil { + fmt.Fprintln(os.Stderr, + "failed to create a temporary data directory:", mkErr) + os.Exit(1) + } + + if setErr := os.Setenv(env.NexusDataDir, dir); setErr != nil { + _ = os.RemoveAll(dir) + fmt.Fprintln(os.Stderr, "failed to set "+env.NexusDataDir+":", setErr) + os.Exit(1) + } + // Run tests code := m.Run() @@ -318,5 +339,6 @@ func TestMain(m *testing.M) { _ = os.Setenv("SPIKE_NEXUS_BACKEND_STORE", "memory") InitializeBackend(nil) + _ = os.RemoveAll(dir) os.Exit(code) }