Skip to content

Commit 54ef7c5

Browse files
tae2089claude
andcommitted
Enforce namespace ownership on annotation create
UpsertAnnotation's lookup was namespace-scoped but its create path was not, so a caller could attach an annotation to another namespace's node or to a nonexistent node id. Verify the node belongs to the caller's namespace inside the create transaction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent a2782ef commit 54ef7c5

2 files changed

Lines changed: 38 additions & 1 deletion

File tree

internal/store/gormstore/gormstore.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package gormstore
44
import (
55
"context"
66
"errors"
7+
"fmt"
78
"path"
89
"strings"
910

@@ -516,6 +517,17 @@ func (s *Store) UpsertAnnotation(ctx context.Context, ann *model.Annotation) err
516517

517518
if errors.Is(result.Error, gorm.ErrRecordNotFound) {
518519
return s.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
520+
// The read path is namespace-scoped; the create path must be too, or a caller
521+
// could attach an annotation to another namespace's node (or a missing node).
522+
var owned int64
523+
if err := tx.Model(&model.Node{}).
524+
Where("id = ? AND namespace = ?", ann.NodeID, ns).
525+
Count(&owned).Error; err != nil {
526+
return trace.Wrap(err, "verify annotation node namespace")
527+
}
528+
if owned == 0 {
529+
return fmt.Errorf("annotation node %d not found in namespace %q", ann.NodeID, ns)
530+
}
519531
return tx.Create(ann).Error
520532
})
521533
}

internal/store/gormstore/gormstore_test.go

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99
"gorm.io/driver/sqlite"
1010
"gorm.io/gorm"
1111
"gorm.io/gorm/logger"
12-
gormschema "gorm.io/gorm/schema"
12+
gormschema "gorm.io/gorm/schema"
1313

1414
"github.com/tae2089/code-context-graph/internal/ctxns"
1515
"github.com/tae2089/code-context-graph/internal/model"
@@ -626,6 +626,31 @@ func TestUpsertAnnotation_Insert(t *testing.T) {
626626
}
627627
}
628628

629+
func TestUpsertAnnotation_RejectsNodeOutsideNamespace(t *testing.T) {
630+
s := setupTestDB(t)
631+
ctx := context.Background()
632+
633+
s.UpsertNodes(ctx, []model.Node{
634+
{QualifiedName: "pkg.F", Kind: model.NodeKindFunction, Name: "F", FilePath: "a.go", StartLine: 1, EndLine: 2, Language: "go"},
635+
})
636+
node, _ := s.GetNode(ctx, "pkg.F")
637+
638+
// Writing from another namespace must not create an annotation on a foreign node.
639+
foreignCtx := ctxns.WithNamespace(context.Background(), "other-team")
640+
err := s.UpsertAnnotation(foreignCtx, &model.Annotation{NodeID: node.ID, Summary: "cross-namespace write"})
641+
if err == nil {
642+
t.Fatal("expected cross-namespace annotation create to be rejected")
643+
}
644+
if got, _ := s.GetAnnotation(ctx, node.ID); got != nil {
645+
t.Fatalf("annotation must not exist after rejected write, got %+v", got)
646+
}
647+
648+
// Nonexistent node id must also be rejected instead of creating an orphan row.
649+
if err := s.UpsertAnnotation(ctx, &model.Annotation{NodeID: 99999, Summary: "orphan"}); err == nil {
650+
t.Fatal("expected annotation create for missing node to be rejected")
651+
}
652+
}
653+
629654
func TestUpsertAnnotation_Update(t *testing.T) {
630655
s := setupTestDB(t)
631656
ctx := context.Background()

0 commit comments

Comments
 (0)