From 378c9a823de1233eb33cedf9dd693e940e683748 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20=C3=96z=C3=A7elik?= Date: Thu, 16 Jul 2026 08:44:02 -0700 Subject: [PATCH 1/3] fix(nexus): reject operator recover/restore explicitly in memory mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RouteRestore short-circuited in in-memory mode by returning nil without writing a response, so the Pilot received an empty 200 and reported the misleading "Failed to communicate with SPIKE Nexus." RouteRecover had no memory-mode check at all and failed later with an equally misleading "not enough shards" internal error. Both routes now reject the request up front with a 400 and a message stating that recovery does not apply to the in-memory backend, which keeps no persistent state to recover or restore. The memory-mode test for restore is updated to the new contract, and recover gains the symmetric test. Spec: TBD Signed-off-by: Volkan Özçelik --- app/nexus/internal/route/operator/recover.go | 17 ++++++ .../internal/route/operator/recover_test.go | 59 +++++++++++++++++++ app/nexus/internal/route/operator/restore.go | 15 ++++- .../internal/route/operator/restore_test.go | 16 ++++- 4 files changed, 102 insertions(+), 5 deletions(-) create mode 100644 app/nexus/internal/route/operator/recover_test.go diff --git a/app/nexus/internal/route/operator/recover.go b/app/nexus/internal/route/operator/recover.go index 8b22feef..b3d3d032 100644 --- a/app/nexus/internal/route/operator/recover.go +++ b/app/nexus/internal/route/operator/recover.go @@ -12,6 +12,7 @@ import ( "github.com/spiffe/spike-sdk-go/config/env" sdkErrors "github.com/spiffe/spike-sdk-go/errors" "github.com/spiffe/spike-sdk-go/journal" + "github.com/spiffe/spike-sdk-go/log" "github.com/spiffe/spike-sdk-go/net" "github.com/spiffe/spike-sdk-go/security/mem" @@ -48,6 +49,22 @@ func RouteRecover( const fName = "routeRecover" journal.AuditRequest(fName, r, audit, journal.AuditCreate) + // The in-memory backend keeps no persistent state, so recovery + // shards would be useless after a restart. Reject the request + // explicitly instead of failing later with a misleading "not + // enough shards" internal error. + if env.BackendStoreTypeVal() == env.Memory { + log.Warn(fName, "message", "rejecting recover: in-memory backend") + failErr := sdkErrors.ErrDataInvalidInput.Clone() + failErr.Msg = "recovery is not applicable to the in-memory backend" + if respondErr := net.Fail( + reqres.RecoverResponse{}.BadRequest(), w, http.StatusBadRequest, + ); respondErr != nil { + return failErr.Wrap(respondErr) + } + return failErr + } + _, err := net.ReadParseAndGuard[ reqres.RecoverRequest, reqres.RecoverResponse]( w, r, reqres.RecoverResponse{}.BadRequest(), guardRecoverRequest, diff --git a/app/nexus/internal/route/operator/recover_test.go b/app/nexus/internal/route/operator/recover_test.go new file mode 100644 index 00000000..a7d732d4 --- /dev/null +++ b/app/nexus/internal/route/operator/recover_test.go @@ -0,0 +1,59 @@ +// \\ SPIKE: Secure your secrets with SPIFFE. — https://spike.ist/ +// \\\\\ Copyright 2024-present SPIKE contributors. +// \\\\\\\ SPDX-License-Identifier: Apache-2.0 + +package operator + +import ( + "net/http" + "net/http/httptest" + "os" + "testing" + + "github.com/spiffe/spike-sdk-go/config/env" + sdkErrors "github.com/spiffe/spike-sdk-go/errors" + "github.com/spiffe/spike-sdk-go/journal" +) + +func TestRouteRecover_MemoryMode(t *testing.T) { + // Save original environment variables + originalStore := os.Getenv(env.NexusBackendStore) + defer func() { + if originalStore != "" { + _ = os.Setenv(env.NexusBackendStore, originalStore) + } else { + _ = os.Unsetenv(env.NexusBackendStore) + } + }() + + // Set to memory mode + _ = os.Setenv(env.NexusBackendStore, "memory") + + // Verify the environment is set correctly + if env.BackendStoreTypeVal() != env.Memory { + t.Fatal("Expected Memory backend store type") + } + + // Create a test request + req := httptest.NewRequest(http.MethodPost, "/recover", nil) + w := httptest.NewRecorder() + audit := &journal.AuditEntry{} + + // Call function + err := RouteRecover(w, req, audit) + + // Recovery shards are useless for the in-memory backend; the route + // must reject the request explicitly rather than fail later with a + // misleading "not enough shards" internal error. + if err == nil { + t.Error("Expected an error in memory mode") + return + } + if !err.Is(sdkErrors.ErrDataInvalidInput) { + t.Errorf("Expected ErrDataInvalidInput, got: %v", err) + } + if w.Code != http.StatusBadRequest { + t.Errorf("Expected status %d, got: %d", + http.StatusBadRequest, w.Code) + } +} diff --git a/app/nexus/internal/route/operator/restore.go b/app/nexus/internal/route/operator/restore.go index f1ece31f..dfb93c6e 100644 --- a/app/nexus/internal/route/operator/restore.go +++ b/app/nexus/internal/route/operator/restore.go @@ -63,9 +63,20 @@ func RouteRestore( const fName = "routeRestore" journal.AuditRequest(fName, r, audit, journal.AuditCreate) + // The in-memory backend keeps no persistent state, so there is + // nothing to restore. Reject the request explicitly: returning + // without a response body reads as "failed to communicate with + // SPIKE Nexus" on the Pilot side. if env.BackendStoreTypeVal() == env.Memory { - log.Info(fName, "message", "skipping restoration: in-memory mode") - return nil + log.Warn(fName, "message", "rejecting restore: in-memory backend") + failErr := sdkErrors.ErrDataInvalidInput.Clone() + failErr.Msg = "restore is not applicable to the in-memory backend" + if respondErr := net.Fail( + reqres.RestoreResponse{}.BadRequest(), w, http.StatusBadRequest, + ); respondErr != nil { + return failErr.Wrap(respondErr) + } + return failErr } request, err := net.ReadParseAndGuard[ diff --git a/app/nexus/internal/route/operator/restore_test.go b/app/nexus/internal/route/operator/restore_test.go index 45cbb0d8..42c85d0a 100644 --- a/app/nexus/internal/route/operator/restore_test.go +++ b/app/nexus/internal/route/operator/restore_test.go @@ -16,6 +16,7 @@ import ( "github.com/spiffe/spike-sdk-go/api/entity/v1/reqres" "github.com/spiffe/spike-sdk-go/config/env" "github.com/spiffe/spike-sdk-go/crypto" + sdkErrors "github.com/spiffe/spike-sdk-go/errors" "github.com/spiffe/spike-sdk-go/journal" ) @@ -47,9 +48,18 @@ func TestRouteRestore_MemoryMode(t *testing.T) { // Call function err := RouteRestore(w, req, audit) - // Should return nil (no error) and skip processing in memory mode - if err != nil { - t.Errorf("Expected no error in memory mode, got: %v", err) + // Restore does not apply to the in-memory backend; the route must + // reject the request explicitly rather than return an empty 200. + if err == nil { + t.Error("Expected an error in memory mode") + return + } + if !err.Is(sdkErrors.ErrDataInvalidInput) { + t.Errorf("Expected ErrDataInvalidInput, got: %v", err) + } + if w.Code != http.StatusBadRequest { + t.Errorf("Expected status %d, got: %d", + http.StatusBadRequest, w.Code) } } From 752763fb9661c5d1e03a8725aeb57b45025c52f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20=C3=96z=C3=A7elik?= Date: Thu, 16 Jul 2026 08:44:02 -0700 Subject: [PATCH 2/3] feat(pilot): read restore shard from stdin when not on a terminal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `spike operator restore` read the recovery shard exclusively through term.ReadPassword, which fails on a non-TTY stdin. That made the restore flow impossible to script, blocking the planned bare-metal recovery drill (TASKS.md, Phase 3). Extract the prompt into readShardInput: interactive callers keep the hidden-input behavior, while piped or redirected stdin is read to EOF and trimmed. The non-interactive path is covered by a unit test; the doc comment notes that scripted restore leaves a shard copy with the calling process and is meant for development and drills. Spec: TBD Signed-off-by: Volkan Özçelik --- app/spike/internal/cmd/operator/restore.go | 43 +++++++++- .../internal/cmd/operator/restore_test.go | 79 +++++++++++++++++++ 2 files changed, 118 insertions(+), 4 deletions(-) create mode 100644 app/spike/internal/cmd/operator/restore_test.go diff --git a/app/spike/internal/cmd/operator/restore.go b/app/spike/internal/cmd/operator/restore.go index ca87566e..381ac595 100644 --- a/app/spike/internal/cmd/operator/restore.go +++ b/app/spike/internal/cmd/operator/restore.go @@ -5,8 +5,10 @@ package operator import ( + "bytes" "context" "encoding/hex" + "io" "os" "strconv" "strings" @@ -63,11 +65,8 @@ func newOperatorRestoreCommand( Run: func(cmd *cobra.Command, args []string) { spiffeid.IsPilotRestoreOrDie(SPIFFEID) - cmd.Println("(your input will be hidden as you paste/type it)") - cmd.Print("Enter recovery shard: ") - shard, readErr := term.ReadPassword(int(os.Stdin.Fd())) + shard, readErr := readShardInput(cmd) if readErr != nil { - cmd.Println("") // newline after hidden input cmd.PrintErrf("Error: %v\n", readErr) return } @@ -167,3 +166,39 @@ func newOperatorRestoreCommand( return restoreCmd } + +// readShardInput reads a recovery shard from standard input. When stdin +// is a terminal, the input is hidden while typed. Otherwise, the shard +// is read until EOF, which lets scripts (such as the bare-metal recovery +// drill) drive `spike operator restore` non-interactively. +// +// In the non-interactive mode, the process supplying the shard holds a +// copy of it too, so scripted restore should be reserved for development +// environments and recovery drills. +// +// Parameters: +// - cmd: The Cobra command used for prompting. +// +// Returns: +// - []byte: The shard bytes with surrounding whitespace removed. +// - error: An error if reading standard input fails. +func readShardInput(cmd *cobra.Command) ([]byte, error) { + fd := int(os.Stdin.Fd()) + + if term.IsTerminal(fd) { + cmd.Println("(your input will be hidden as you paste/type it)") + cmd.Print("Enter recovery shard: ") + shard, readErr := term.ReadPassword(fd) + cmd.Println("") // newline after hidden input + return shard, readErr + } + + // A shard line is well under 4KB; the limit only guards against + // unbounded input. + data, readErr := io.ReadAll(io.LimitReader(os.Stdin, 4096)) + if readErr != nil { + return nil, readErr + } + + return bytes.TrimSpace(data), nil +} diff --git a/app/spike/internal/cmd/operator/restore_test.go b/app/spike/internal/cmd/operator/restore_test.go new file mode 100644 index 00000000..6fcc19d7 --- /dev/null +++ b/app/spike/internal/cmd/operator/restore_test.go @@ -0,0 +1,79 @@ +// \\ SPIKE: Secure your secrets with SPIFFE. — https://spike.ist/ +// \\\\\ Copyright 2024-present SPIKE contributors. +// \\\\\\\ SPDX-License-Identifier: Apache-2.0 + +package operator + +import ( + "os" + "strings" + "testing" + + "github.com/spf13/cobra" +) + +// TestReadShardInput_NonInteractive verifies that readShardInput reads a +// shard from a non-terminal stdin (a pipe), which is what allows scripts +// to drive `spike operator restore`. +func TestReadShardInput_NonInteractive(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + { + name: "plain shard line", + input: "spike:1:" + strings.Repeat("ab", 32) + "\n", + want: "spike:1:" + strings.Repeat("ab", 32), + }, + { + name: "surrounding whitespace is trimmed", + input: " spike:2:" + strings.Repeat("cd", 32) + " \n\n", + want: "spike:2:" + strings.Repeat("cd", 32), + }, + { + name: "no trailing newline", + input: "spike:3:" + strings.Repeat("ef", 32), + want: "spike:3:" + strings.Repeat("ef", 32), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r, w, pipeErr := os.Pipe() + if pipeErr != nil { + t.Fatalf("failed to create a pipe: %v", pipeErr) + return + } + + original := os.Stdin + os.Stdin = r + t.Cleanup(func() { + os.Stdin = original + _ = r.Close() + }) + + if _, writeErr := w.WriteString(tt.input); writeErr != nil { + t.Fatalf("failed to write to the pipe: %v", writeErr) + return + } + if closeErr := w.Close(); closeErr != nil { + t.Fatalf("failed to close the pipe writer: %v", closeErr) + return + } + + cmd := &cobra.Command{Use: "test"} + + got, readErr := readShardInput(cmd) + if readErr != nil { + t.Fatalf("readShardInput() error = %v", readErr) + return + } + + if string(got) != tt.want { + t.Errorf("readShardInput() = %q, want %q", + string(got), tt.want) + } + }) + } +} From 324af9db801664a834fa3418c2068b751c74704d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20=C3=96z=C3=A7elik?= Date: Thu, 16 Jul 2026 08:50:45 -0700 Subject: [PATCH 3/3] chore(context): close stale tasks, file the recovery-drill task MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Close the cipher stream-mode task (both cipher modes verified passing via the make start checks on 2026-07-15) and the CI integration-test task (CI has been green for several weeks). File the scripted live recovery/restore drill under Phase 3 with the rationale from the 2026-07-16 code review of the flow, and annotate the Phase 1 recovery/restore task to point at it. Spec: TBD Signed-off-by: Volkan Özçelik --- .context/TASKS.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.context/TASKS.md b/.context/TASKS.md index c83cc4e9..37fde5d0 100644 --- a/.context/TASKS.md +++ b/.context/TASKS.md @@ -35,9 +35,9 @@ the name-based policy work. --> ### Phase 1: Correctness & Broken Things `#priority:high` -- [ ] Fix broken CI integration test #source:jira.xml #added:2026-07-14 -- [ ] Fix broken recovery/restore flow #source:jira.xml #added:2026-07-14 -- [ ] Fix `spike cipher` stream mode (broken; owner: Murat); JSON mode fix unblocks encryption-as-a-service demo/docs #source:jira.xml #added:2026-07-14 +- [x] Fix broken CI integration test #source:jira.xml #added:2026-07-14 #done:2026-07-16 (stale: CI has been green for several weeks; closed on user confirmation) +- [ ] Fix broken recovery/restore flow #source:jira.xml #added:2026-07-14 (code review 2026-07-16: no live breakage found; awaiting the scripted drill in Phase 3 to confirm and close) +- [x] Fix `spike cipher` stream mode (broken; owner: Murat); JSON mode fix unblocks encryption-as-a-service demo/docs #source:jira.xml #added:2026-07-14 #done:2026-07-15 (stale: both cipher streaming and file modes verified passing via the make start checks on 2026-07-15) - [ ] Retry sqlite operations with exponential backoff on transient locks (all of `app/nexus/internal/state/persist`) → ideas/research-db-resilience.md #source:jira.xml #added:2026-07-14 - [ ] 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 - [ ] 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 @@ -50,6 +50,7 @@ the name-based policy work. - [ ] 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 - [ ] `start.sh` should exercise recovery/restore and encryption/decryption #source:jira.xml #added:2026-07-14 +- [ ] Scripted live recovery/restore drill: once `make start` completes cleanly, run `spike operator recover`, kill Nexus and the Keepers, restart Nexus alone, feed the shards back via `spike operator restore` (scriptable via stdin since fix/operator-restore), and verify a pre-crash secret reads back. Rationale: the 2026-07-16 code review found no live breakage (shard-index fidelity intact end to end; guards use exact SPIFFE role matching, unaffected by the policy-name migration), so only a drill can prove the Phase 1 "recovery/restore is broken" claim stale and close both tasks. Needs the recover/restore role entries (spire-server-entry-recover-register.sh / -restore-register.sh), which make start does not register by default. #added:2026-07-16 ### Phase 4: Policy & Secrets `#priority:medium` - [ ] Add a `list` permission type; scope `spike secret list` to the caller's allowed path patterns → ideas/research-list-permission.md #source:jira.xml #added:2026-07-14