agent: secrets fail-loud on corrupt blob + small polish

Save and SaveAdmin now propagate loadBundle errors instead of silently
overwriting a corrupt file (data-loss fix). Tests added for both paths.
reportStats logs a Debug on RunStats failure; r in runJob gets a comment
explaining the prune-runner asymmetry; runner_test comment tightened.
This commit is contained in:
2026-05-03 22:49:12 +01:00
parent 22adde36b3
commit a110e3c00c
5 changed files with 93 additions and 3 deletions
+8
View File
@@ -288,6 +288,14 @@ func (d *dispatcher) runJob(ctx context.Context, p api.CommandRunPayload, tx wsc
if creds.Empty() {
return fmt.Errorf("repo credentials not configured (waiting for server config.update push)")
}
// r is the everyday runner — bound to the host's repo
// (append-only) credentials. Reused by every kind except
// JobPrune, which builds its own runner against the
// admin-credentials slot when p.RequiresAdminCreds is set
// (admin creds are not loaded for any other kind, so they're
// not on r). If you find yourself adding a new JobKind that
// needs delete authority, mirror the JobPrune pattern below
// — don't try to overload r.
r := runner.New(runner.Config{
ResticBin: d.resticBin,
RepoURL: creds.URL,
+2
View File
@@ -348,6 +348,8 @@ func (r *Runner) reportStats(ctx context.Context, env restic.Env, patch api.Repo
patch.RawSizeBytes = &raw
patch.UniqueFiles = &files
patch.SnapshotCount = &snaps
} else {
slog.Debug("runner: stats refresh failed (non-fatal)", "err", err)
}
}
envOut, err := api.Marshal(api.MsgRepoStats, "", patch)
+3 -1
View File
@@ -207,7 +207,9 @@ esac
}
// Assert envelope ordering: job.started → repo.stats → job.finished.
// (No log.stream expected here because check exits immediately.)
// (No log.stream expected because the fake script produces no
// output before exit 1 — a real restic check would emit log lines
// before exiting non-zero.)
order := envelopeOrder(tx.envs)
wantTypes := []api.MessageType{api.MsgJobStarted, api.MsgRepoStats, api.MsgJobFinished}
positions := map[api.MessageType]int{}
+8 -2
View File
@@ -161,7 +161,10 @@ func (s *Store) Load() (Repo, error) {
// Save replaces the repo slot on disk atomically, preserving the
// admin slot. Mode is 0600. Parent directory must already exist.
func (s *Store) Save(r Repo) error {
b, _ := s.loadBundle() // ignore read errors; we overwrite repo slot
b, err := s.loadBundle()
if err != nil {
return fmt.Errorf("secrets: load before save: %w", err)
}
b.Repo = r
return s.saveBundle(b)
}
@@ -182,7 +185,10 @@ func (s *Store) LoadAdmin() (Repo, error) {
// SaveAdmin replaces the admin slot on disk atomically, preserving
// the repo slot. Mode is 0600.
func (s *Store) SaveAdmin(r Repo) error {
b, _ := s.loadBundle() // ignore read errors; we overwrite admin slot
b, err := s.loadBundle()
if err != nil {
return fmt.Errorf("secrets: load before save: %w", err)
}
b.Admin = &r
return s.saveBundle(b)
}
+72
View File
@@ -173,6 +173,78 @@ func TestSecretsAdminSlotIndependent(t *testing.T) {
}
}
func TestSecretsSaveRefusesCorruptFile(t *testing.T) {
t.Parallel()
dir := t.TempDir()
path := filepath.Join(dir, "secrets.enc")
st, err := New(path, freshKey(t))
if err != nil {
t.Fatalf("new: %v", err)
}
// Lay down a valid file first.
if err := st.Save(Repo{URL: "rest:https://r/host", Password: "pw"}); err != nil {
t.Fatalf("initial save: %v", err)
}
// Corrupt the file.
garbage := []byte("not encrypted")
if err := os.WriteFile(path, garbage, 0o600); err != nil {
t.Fatalf("write garbage: %v", err)
}
// Save must refuse to overwrite: decrypt will fail.
saveErr := st.Save(Repo{URL: "rest:https://r/host", Password: "new"})
if saveErr == nil {
t.Fatal("Save over corrupt file must return an error; got nil")
}
// File must NOT have been replaced — still contains the garbage bytes.
got, err := os.ReadFile(path)
if err != nil {
t.Fatalf("re-read: %v", err)
}
if string(got) != string(garbage) {
t.Errorf("corrupt file was overwritten; file size now %d (was %d)", len(got), len(garbage))
}
}
func TestSecretsSaveAdminRefusesCorruptFile(t *testing.T) {
t.Parallel()
dir := t.TempDir()
path := filepath.Join(dir, "secrets.enc")
st, err := New(path, freshKey(t))
if err != nil {
t.Fatalf("new: %v", err)
}
// Lay down a valid file first.
if err := st.SaveAdmin(Repo{URL: "rest:https://r/host", Password: "adminpw"}); err != nil {
t.Fatalf("initial save admin: %v", err)
}
// Corrupt the file.
garbage := []byte("not encrypted admin")
if err := os.WriteFile(path, garbage, 0o600); err != nil {
t.Fatalf("write garbage: %v", err)
}
// SaveAdmin must refuse to overwrite: decrypt will fail.
saveErr := st.SaveAdmin(Repo{URL: "rest:https://r/host", Password: "new"})
if saveErr == nil {
t.Fatal("SaveAdmin over corrupt file must return an error; got nil")
}
// File must NOT have been replaced.
got, err := os.ReadFile(path)
if err != nil {
t.Fatalf("re-read: %v", err)
}
if string(got) != string(garbage) {
t.Errorf("corrupt file was overwritten; file size now %d (was %d)", len(got), len(garbage))
}
}
func TestSecretsLegacyFlatBlobMigrates(t *testing.T) {
t.Parallel()
dir := t.TempDir()