Skip to content

Commit 9a2d22f

Browse files
authored
fixes to operator restore flow (#296)
* fix(nexus): reject operator recover/restore explicitly in memory mode 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 <volkan.ozcelik@broadcom.com> * feat(pilot): read restore shard from stdin when not on a terminal `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 <volkan.ozcelik@broadcom.com> * chore(context): close stale tasks, file the recovery-drill task 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 <volkan.ozcelik@broadcom.com> --------- Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com>
1 parent 01cf4a9 commit 9a2d22f

7 files changed

Lines changed: 224 additions & 12 deletions

File tree

.context/TASKS.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,9 @@ the name-based policy work.
3535
-->
3636

3737
### Phase 1: Correctness & Broken Things `#priority:high`
38-
- [ ] Fix broken CI integration test #source:jira.xml #added:2026-07-14
39-
- [ ] Fix broken recovery/restore flow #source:jira.xml #added:2026-07-14
40-
- [ ] 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
38+
- [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)
39+
- [ ] 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)
40+
- [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)
4141
- [ ] 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
4242
- [ ] 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
4343
- [ ] 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.
5050
- [ ] 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
5151
- [ ] 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
5252
- [ ] `start.sh` should exercise recovery/restore and encryption/decryption #source:jira.xml #added:2026-07-14
53+
- [ ] 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
5354

5455
### Phase 4: Policy & Secrets `#priority:medium`
5556
- [ ] 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

app/nexus/internal/route/operator/recover.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/spiffe/spike-sdk-go/config/env"
1313
sdkErrors "github.com/spiffe/spike-sdk-go/errors"
1414
"github.com/spiffe/spike-sdk-go/journal"
15+
"github.com/spiffe/spike-sdk-go/log"
1516
"github.com/spiffe/spike-sdk-go/net"
1617
"github.com/spiffe/spike-sdk-go/security/mem"
1718

@@ -48,6 +49,22 @@ func RouteRecover(
4849
const fName = "routeRecover"
4950
journal.AuditRequest(fName, r, audit, journal.AuditCreate)
5051

52+
// The in-memory backend keeps no persistent state, so recovery
53+
// shards would be useless after a restart. Reject the request
54+
// explicitly instead of failing later with a misleading "not
55+
// enough shards" internal error.
56+
if env.BackendStoreTypeVal() == env.Memory {
57+
log.Warn(fName, "message", "rejecting recover: in-memory backend")
58+
failErr := sdkErrors.ErrDataInvalidInput.Clone()
59+
failErr.Msg = "recovery is not applicable to the in-memory backend"
60+
if respondErr := net.Fail(
61+
reqres.RecoverResponse{}.BadRequest(), w, http.StatusBadRequest,
62+
); respondErr != nil {
63+
return failErr.Wrap(respondErr)
64+
}
65+
return failErr
66+
}
67+
5168
_, err := net.ReadParseAndGuard[
5269
reqres.RecoverRequest, reqres.RecoverResponse](
5370
w, r, reqres.RecoverResponse{}.BadRequest(), guardRecoverRequest,
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// \\ SPIKE: Secure your secrets with SPIFFE. — https://spike.ist/
2+
// \\\\\ Copyright 2024-present SPIKE contributors.
3+
// \\\\\\\ SPDX-License-Identifier: Apache-2.0
4+
5+
package operator
6+
7+
import (
8+
"net/http"
9+
"net/http/httptest"
10+
"os"
11+
"testing"
12+
13+
"github.com/spiffe/spike-sdk-go/config/env"
14+
sdkErrors "github.com/spiffe/spike-sdk-go/errors"
15+
"github.com/spiffe/spike-sdk-go/journal"
16+
)
17+
18+
func TestRouteRecover_MemoryMode(t *testing.T) {
19+
// Save original environment variables
20+
originalStore := os.Getenv(env.NexusBackendStore)
21+
defer func() {
22+
if originalStore != "" {
23+
_ = os.Setenv(env.NexusBackendStore, originalStore)
24+
} else {
25+
_ = os.Unsetenv(env.NexusBackendStore)
26+
}
27+
}()
28+
29+
// Set to memory mode
30+
_ = os.Setenv(env.NexusBackendStore, "memory")
31+
32+
// Verify the environment is set correctly
33+
if env.BackendStoreTypeVal() != env.Memory {
34+
t.Fatal("Expected Memory backend store type")
35+
}
36+
37+
// Create a test request
38+
req := httptest.NewRequest(http.MethodPost, "/recover", nil)
39+
w := httptest.NewRecorder()
40+
audit := &journal.AuditEntry{}
41+
42+
// Call function
43+
err := RouteRecover(w, req, audit)
44+
45+
// Recovery shards are useless for the in-memory backend; the route
46+
// must reject the request explicitly rather than fail later with a
47+
// misleading "not enough shards" internal error.
48+
if err == nil {
49+
t.Error("Expected an error in memory mode")
50+
return
51+
}
52+
if !err.Is(sdkErrors.ErrDataInvalidInput) {
53+
t.Errorf("Expected ErrDataInvalidInput, got: %v", err)
54+
}
55+
if w.Code != http.StatusBadRequest {
56+
t.Errorf("Expected status %d, got: %d",
57+
http.StatusBadRequest, w.Code)
58+
}
59+
}

app/nexus/internal/route/operator/restore.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,20 @@ func RouteRestore(
6363
const fName = "routeRestore"
6464
journal.AuditRequest(fName, r, audit, journal.AuditCreate)
6565

66+
// The in-memory backend keeps no persistent state, so there is
67+
// nothing to restore. Reject the request explicitly: returning
68+
// without a response body reads as "failed to communicate with
69+
// SPIKE Nexus" on the Pilot side.
6670
if env.BackendStoreTypeVal() == env.Memory {
67-
log.Info(fName, "message", "skipping restoration: in-memory mode")
68-
return nil
71+
log.Warn(fName, "message", "rejecting restore: in-memory backend")
72+
failErr := sdkErrors.ErrDataInvalidInput.Clone()
73+
failErr.Msg = "restore is not applicable to the in-memory backend"
74+
if respondErr := net.Fail(
75+
reqres.RestoreResponse{}.BadRequest(), w, http.StatusBadRequest,
76+
); respondErr != nil {
77+
return failErr.Wrap(respondErr)
78+
}
79+
return failErr
6980
}
7081

7182
request, err := net.ReadParseAndGuard[

app/nexus/internal/route/operator/restore_test.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/spiffe/spike-sdk-go/api/entity/v1/reqres"
1717
"github.com/spiffe/spike-sdk-go/config/env"
1818
"github.com/spiffe/spike-sdk-go/crypto"
19+
sdkErrors "github.com/spiffe/spike-sdk-go/errors"
1920

2021
"github.com/spiffe/spike-sdk-go/journal"
2122
)
@@ -47,9 +48,18 @@ func TestRouteRestore_MemoryMode(t *testing.T) {
4748
// Call function
4849
err := RouteRestore(w, req, audit)
4950

50-
// Should return nil (no error) and skip processing in memory mode
51-
if err != nil {
52-
t.Errorf("Expected no error in memory mode, got: %v", err)
51+
// Restore does not apply to the in-memory backend; the route must
52+
// reject the request explicitly rather than return an empty 200.
53+
if err == nil {
54+
t.Error("Expected an error in memory mode")
55+
return
56+
}
57+
if !err.Is(sdkErrors.ErrDataInvalidInput) {
58+
t.Errorf("Expected ErrDataInvalidInput, got: %v", err)
59+
}
60+
if w.Code != http.StatusBadRequest {
61+
t.Errorf("Expected status %d, got: %d",
62+
http.StatusBadRequest, w.Code)
5363
}
5464
}
5565

app/spike/internal/cmd/operator/restore.go

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55
package operator
66

77
import (
8+
"bytes"
89
"context"
910
"encoding/hex"
11+
"io"
1012
"os"
1113
"strconv"
1214
"strings"
@@ -63,11 +65,8 @@ func newOperatorRestoreCommand(
6365
Run: func(cmd *cobra.Command, args []string) {
6466
spiffeid.IsPilotRestoreOrDie(SPIFFEID)
6567

66-
cmd.Println("(your input will be hidden as you paste/type it)")
67-
cmd.Print("Enter recovery shard: ")
68-
shard, readErr := term.ReadPassword(int(os.Stdin.Fd()))
68+
shard, readErr := readShardInput(cmd)
6969
if readErr != nil {
70-
cmd.Println("") // newline after hidden input
7170
cmd.PrintErrf("Error: %v\n", readErr)
7271
return
7372
}
@@ -167,3 +166,39 @@ func newOperatorRestoreCommand(
167166

168167
return restoreCmd
169168
}
169+
170+
// readShardInput reads a recovery shard from standard input. When stdin
171+
// is a terminal, the input is hidden while typed. Otherwise, the shard
172+
// is read until EOF, which lets scripts (such as the bare-metal recovery
173+
// drill) drive `spike operator restore` non-interactively.
174+
//
175+
// In the non-interactive mode, the process supplying the shard holds a
176+
// copy of it too, so scripted restore should be reserved for development
177+
// environments and recovery drills.
178+
//
179+
// Parameters:
180+
// - cmd: The Cobra command used for prompting.
181+
//
182+
// Returns:
183+
// - []byte: The shard bytes with surrounding whitespace removed.
184+
// - error: An error if reading standard input fails.
185+
func readShardInput(cmd *cobra.Command) ([]byte, error) {
186+
fd := int(os.Stdin.Fd())
187+
188+
if term.IsTerminal(fd) {
189+
cmd.Println("(your input will be hidden as you paste/type it)")
190+
cmd.Print("Enter recovery shard: ")
191+
shard, readErr := term.ReadPassword(fd)
192+
cmd.Println("") // newline after hidden input
193+
return shard, readErr
194+
}
195+
196+
// A shard line is well under 4KB; the limit only guards against
197+
// unbounded input.
198+
data, readErr := io.ReadAll(io.LimitReader(os.Stdin, 4096))
199+
if readErr != nil {
200+
return nil, readErr
201+
}
202+
203+
return bytes.TrimSpace(data), nil
204+
}
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// \\ SPIKE: Secure your secrets with SPIFFE. — https://spike.ist/
2+
// \\\\\ Copyright 2024-present SPIKE contributors.
3+
// \\\\\\\ SPDX-License-Identifier: Apache-2.0
4+
5+
package operator
6+
7+
import (
8+
"os"
9+
"strings"
10+
"testing"
11+
12+
"github.com/spf13/cobra"
13+
)
14+
15+
// TestReadShardInput_NonInteractive verifies that readShardInput reads a
16+
// shard from a non-terminal stdin (a pipe), which is what allows scripts
17+
// to drive `spike operator restore`.
18+
func TestReadShardInput_NonInteractive(t *testing.T) {
19+
tests := []struct {
20+
name string
21+
input string
22+
want string
23+
}{
24+
{
25+
name: "plain shard line",
26+
input: "spike:1:" + strings.Repeat("ab", 32) + "\n",
27+
want: "spike:1:" + strings.Repeat("ab", 32),
28+
},
29+
{
30+
name: "surrounding whitespace is trimmed",
31+
input: " spike:2:" + strings.Repeat("cd", 32) + " \n\n",
32+
want: "spike:2:" + strings.Repeat("cd", 32),
33+
},
34+
{
35+
name: "no trailing newline",
36+
input: "spike:3:" + strings.Repeat("ef", 32),
37+
want: "spike:3:" + strings.Repeat("ef", 32),
38+
},
39+
}
40+
41+
for _, tt := range tests {
42+
t.Run(tt.name, func(t *testing.T) {
43+
r, w, pipeErr := os.Pipe()
44+
if pipeErr != nil {
45+
t.Fatalf("failed to create a pipe: %v", pipeErr)
46+
return
47+
}
48+
49+
original := os.Stdin
50+
os.Stdin = r
51+
t.Cleanup(func() {
52+
os.Stdin = original
53+
_ = r.Close()
54+
})
55+
56+
if _, writeErr := w.WriteString(tt.input); writeErr != nil {
57+
t.Fatalf("failed to write to the pipe: %v", writeErr)
58+
return
59+
}
60+
if closeErr := w.Close(); closeErr != nil {
61+
t.Fatalf("failed to close the pipe writer: %v", closeErr)
62+
return
63+
}
64+
65+
cmd := &cobra.Command{Use: "test"}
66+
67+
got, readErr := readShardInput(cmd)
68+
if readErr != nil {
69+
t.Fatalf("readShardInput() error = %v", readErr)
70+
return
71+
}
72+
73+
if string(got) != tt.want {
74+
t.Errorf("readShardInput() = %q, want %q",
75+
string(got), tt.want)
76+
}
77+
})
78+
}
79+
}

0 commit comments

Comments
 (0)