From 96eaf0dba13fb01195755040e410118de0a09d81 Mon Sep 17 00:00:00 2001 From: barelyworkingcode Date: Fri, 25 Sep 2026 23:52:15 -0700 Subject: [PATCH 1/2] test(projects): a refused or missing-project update must not rewrite settings.json Checkpoint before the fix; --no-verify because the tests are red by design. Refs #46 Co-Authored-By: Claude Opus 5.5 (1M context) --- cmd/relay/project_update_declined_test.go | 89 +++++++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 cmd/relay/project_update_declined_test.go diff --git a/cmd/relay/project_update_declined_test.go b/cmd/relay/project_update_declined_test.go new file mode 100644 index 00000000..2f8d284c --- /dev/null +++ b/cmd/relay/project_update_declined_test.go @@ -0,0 +1,89 @@ +package main + +import ( + "context" + "errors" + "testing" + + "github.com/barelyworkingcode/relay/internal/config" + "github.com/barelyworkingcode/relay/internal/project" +) + +func pudOps(t *testing.T, store config.SettingsStore) *ProjectOps { + t.Helper() + return &ProjectOps{Store: store, Gate: allowGate(t), Issuance: enabledIssuanceRecorder(t)} +} + +func pudNoSurfaces() project.McpSurfaces { return project.McpSurfaces{} } + +// A refusal from the update's own validation must decline the settings write, +// not commit the unchanged settings back over the file. +func TestProjectUpdateThatIsRefusedWritesNothing(t *testing.T) { + badMode := config.ProjectMode("sometimes") + badAccess := map[string]string{"keeper": "admin"} + cases := []struct { + name string + fields project.UpdateFields + }{ + {name: "invalid mode", fields: project.UpdateFields{Mode: &badMode}}, + {name: "invalid access", fields: project.UpdateFields{Access: &badAccess}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dir, store := odwSandbox(t) + stored := mkStoreProject(t, store, config.ProjectKindLocal, "Acme", t.TempDir()) + before := odwSnap(t, dir) + + _, found, err := pudOps(t, store).Update(context.Background(), stored.ID, tc.fields, pudNoSurfaces, auditViaIPC, "") + if err == nil { + t.Fatal("Update accepted an invalid field; want a refusal") + } + if !found { + t.Fatalf("Update reported the stored project as missing (err = %v); want found with a refusal", err) + } + if errors.Is(err, errProjectSaveFailed) { + t.Fatalf("Update reported a validation refusal as a save failure: %v", err) + } + before.assertUntouched(t, dir, "ProjectOps.Update ("+tc.name+")") + }) + } +} + +func TestProjectUpdateOfAMissingProjectWritesNothing(t *testing.T) { + dir, store := odwSandbox(t) + mkStoreProject(t, store, config.ProjectKindLocal, "Acme", t.TempDir()) + before := odwSnap(t, dir) + + name := "Renamed" + got, found, err := pudOps(t, store).Update(context.Background(), "ghost", project.UpdateFields{Name: &name}, pudNoSurfaces, auditViaIPC, "") + if err != nil { + t.Fatalf("Update of an unknown id returned error %v; want none", err) + } + if found { + t.Fatalf("Update of an unknown id reported found (returned %+v)", got) + } + before.assertUntouched(t, dir, "ProjectOps.Update (unknown id)") +} + +// Declining is for refusals only: an accepted update still reaches the file. +func TestProjectUpdateThatIsValidStillWrites(t *testing.T) { + dir, store := odwSandbox(t) + stored := mkStoreProject(t, store, config.ProjectKindLocal, "Acme", t.TempDir()) + before := odwSnap(t, dir) + + name := "Acme Renamed" + got, found, err := pudOps(t, store).Update(context.Background(), stored.ID, project.UpdateFields{Name: &name}, pudNoSurfaces, auditViaIPC, "") + if err != nil || !found { + t.Fatalf("Update = (found %v, err %v); want found with no error", found, err) + } + if got.ID != stored.ID || got.Name != name { + t.Fatalf("Update returned project %q named %q; want %q named %q", got.ID, got.Name, stored.ID, name) + } + if string(sdRead(t, dir)) == string(before.bytes) { + t.Fatal("a valid Update left settings.json unchanged") + } + onDisk, _ := config.FindProjectByID(sealedSettingsStoreAt(dir).Get(), stored.ID) + if onDisk == nil || onDisk.Name != name { + t.Fatalf("settings.json after a valid Update holds %+v; want the project named %q", onDisk, name) + } +} From 707d8b3d97ee44e0603869791f230f4e55c14eba Mon Sep 17 00:00:00 2001 From: barelyworkingcode Date: Fri, 25 Sep 2026 23:53:28 -0700 Subject: [PATCH 2/2] fix(projects): decline the settings write when a project update is refused A refused update, or one naming a missing project, returned nil from the declinable callback, so settings.json was rewritten with identical content. The callback now returns the refusal and the store declines. Refs #46 Co-Authored-By: Claude Opus 5.5 (1M context) --- cmd/relay/project_ops.go | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/cmd/relay/project_ops.go b/cmd/relay/project_ops.go index 9aff87de..865f609f 100644 --- a/cmd/relay/project_ops.go +++ b/cmd/relay/project_ops.go @@ -77,6 +77,10 @@ var errProjectSaveFailed = errors.New("failed to save settings") // retries against the current record. var errProjectChangedDuringApproval = errors.New("project changed while the request was being approved; retry the update") +// errProjectUpdateTargetMissing declines the write when Update's id matches +// no project. It never leaves Update: the caller sees found == false. +var errProjectUpdateTargetMissing = errors.New("project to update not found") + // errProjectTokenUnrecorded means the rotation committed and the audit // write then failed: the new plaintext is withheld (ยง7.6's rotate_token // withhold, unchanged) but the old token is already dead either way, so a @@ -313,19 +317,24 @@ func (o *ProjectOps) Update(ctx context.Context, id string, f project.UpdateFiel } } updated, found, updateErr = project.ApplyUpdate(s, id, f, queued) + if updateErr != nil { + return updateErr + } + if !found { + return errProjectUpdateTargetMissing + } return nil }); err != nil { - if errors.Is(err, errProjectChangedDuringApproval) { + switch { + case updateErr != nil: + return updateErr + case errors.Is(err, errProjectUpdateTargetMissing): + return nil + case errors.Is(err, errProjectChangedDuringApproval): return err } return fmt.Errorf("%w: %w", errProjectSaveFailed, err) } - if updateErr != nil { - return updateErr - } - if !found { - return nil - } if touchesGrant { if auditErr := recordConfigChange(o.Issuance, auditCredentialProjectGrant, id, widened, via, credID, presenceID); auditErr != nil {