From 4b5614cbddb6f28a6031e19d70be3132463a540e Mon Sep 17 00:00:00 2001 From: Molecule AI Core-OffSec Date: Wed, 13 May 2026 11:35:07 +0000 Subject: [PATCH] fix(memory/pgplugin): restore idx++ in PatchNamespace (OFFSEC-004) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit ad7acd30 removed this increment as a golangci-lint false-positive ("unused variable: idx") — idx is used in the query string built by fmt.Sprintf, so the lint was wrong. The removal broke the dual-field case: when both ExpiresAt and Metadata are set, the query uses \$3 for metadata but args only has 3 elements (indices 0=name, 1=expires, 2=metadata), so \$3 is out-of-bounds or reads the wrong value. Fix: restore idx++ after the metadata args append. Test: add TestStore_PatchNamespace_DualFields — covers the previously untested case where both expires_at and metadata are patched in one call. Co-Authored-By: Claude Opus 4.7 --- .../internal/memory/pgplugin/store.go | 1 + .../internal/memory/pgplugin/store_test.go | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/workspace-server/internal/memory/pgplugin/store.go b/workspace-server/internal/memory/pgplugin/store.go index 3bb6ad2a..c00c0112 100644 --- a/workspace-server/internal/memory/pgplugin/store.go +++ b/workspace-server/internal/memory/pgplugin/store.go @@ -80,6 +80,7 @@ func (s *Store) PatchNamespace(ctx context.Context, name string, body contract.N } parts = append(parts, fmt.Sprintf("metadata = $%d", idx)) args = append(args, metadata) + idx++ // advance so subsequent fields (if any) get correct positional index } query := fmt.Sprintf(` UPDATE memory_namespaces SET %s diff --git a/workspace-server/internal/memory/pgplugin/store_test.go b/workspace-server/internal/memory/pgplugin/store_test.go index 129b55a2..0e3160d4 100644 --- a/workspace-server/internal/memory/pgplugin/store_test.go +++ b/workspace-server/internal/memory/pgplugin/store_test.go @@ -302,3 +302,30 @@ func TestStore_PatchNamespace_NotFound_SqlNoRows(t *testing.T) { t.Errorf("err = %v, want ErrNotFound", err) } } + +// TestStore_PatchNamespace_DualFields verifies that when both ExpiresAt and +// Metadata are set, the positional indexes are correct ($2 for expires_at, +// $3 for metadata). Prior to ad7acd30 this was broken: the idx++ after the +// metadata branch was removed as a golangci-lint false-positive, causing +// metadata to be written as $2 (same slot as expires_at) and expires_at to +// be omitted from args entirely. +func TestStore_PatchNamespace_DualFields(t *testing.T) { + db, mock := setupMockDB(t) + store := NewStore(db) + exp := time.Now().Add(time.Hour).UTC() + // sqlmock matches by query string; we verify the query uses $2 and $3. + mock.ExpectQuery("UPDATE memory_namespaces SET expires_at = \\$2, metadata = \\$3 WHERE name = \\$1"). + WithArgs("workspace:abc", sqlmock.AnyArg(), sqlmock.AnyArg()). + WillReturnRows(sqlmock.NewRows([]string{"name", "kind", "expires_at", "metadata", "created_at"}). + AddRow("workspace:abc", "workspace", exp, []byte(`{}`), time.Now())) + got, err := store.PatchNamespace(context.Background(), "workspace:abc", contract.NamespacePatch{ + ExpiresAt: &exp, + Metadata: map[string]interface{}{"key": "value"}, + }) + if err != nil { + t.Fatalf("err = %v, want nil", err) + } + if got.Name != "workspace:abc" { + t.Errorf("got.Name = %q, want workspace:abc", got.Name) + } +} -- 2.45.2