fix: don't double the prefix when importing prefixed relationships (#460)

internal/cmd/import-test/prefixed-relations-validation-file.yaml

@@ -0,0 +1,3 @@
+relationships: |-
+  prefixtest/resource:doc1#viewer@prefixtest/user:alice
+  prefixtest/resource:doc2#viewer@prefixtest/user:bob[prefixtest/somecaveat]

internal/cmd/import-test/prefixed-schema.zed

@@ -0,0 +1,10 @@
+definition prefixtest/user {}
+
+caveat prefixtest/somecaveat(allowed bool) {
+	allowed == true
+}
+
+definition prefixtest/resource {
+	relation viewer: prefixtest/user | prefixtest/user with prefixtest/somecaveat
+	permission view = viewer
+}

internal/cmd/import.go

@@ -118,6 +118,15 @@ func importSchema(ctx context.Context, client v1.SchemaServiceClient, schema str
 	return nil
 }
 
+// addPrefixIfAbsent prepends prefix to name unless name already carries a
+// definition prefix (i.e. already contains a "/").
+func addPrefixIfAbsent(name, prefix string) string {
+	if strings.Contains(name, "/") {
+		return name
+	}
+	return fmt.Sprintf("%s/%s", prefix, name)
+}
+
 func importRelationships(ctx context.Context, client v1.PermissionsServiceClient, relationships string, definitionPrefix string, batchSize int, workers int) error {
 	relationshipUpdates := make([]*v1.RelationshipUpdate, 0)
 	scanner := bufio.NewScanner(strings.NewReader(relationships))
@@ -135,12 +144,15 @@ func importRelationships(ctx context.Context, client v1.PermissionsServiceClient
 		}
 		log.Trace().Str("line", line).Send()
 
-		// Rewrite the prefix on the references, if any.
+		// Rewrite the prefix on the references, if any. Only prefix names that
+		// are not already prefixed, so importing relationships that already
+		// carry the prefix (e.g. from a prefixed schemaFile) does not double it
+		// (`studio/x` becoming `studio/studio/x`). See issue #460.
 		if len(definitionPrefix) > 0 {
-			rel.Resource.ObjectType = fmt.Sprintf("%s/%s", definitionPrefix, rel.Resource.ObjectType)
-			rel.Subject.Object.ObjectType = fmt.Sprintf("%s/%s", definitionPrefix, rel.Subject.Object.ObjectType)
+			rel.Resource.ObjectType = addPrefixIfAbsent(rel.Resource.ObjectType, definitionPrefix)
+			rel.Subject.Object.ObjectType = addPrefixIfAbsent(rel.Subject.Object.ObjectType, definitionPrefix)
 			if rel.OptionalCaveat != nil {
-				rel.OptionalCaveat.CaveatName = fmt.Sprintf("%s/%s", definitionPrefix, rel.OptionalCaveat.CaveatName)
+				rel.OptionalCaveat.CaveatName = addPrefixIfAbsent(rel.OptionalCaveat.CaveatName, definitionPrefix)
 			}
 		}
 

internal/cmd/import_test.go

@@ -17,6 +17,17 @@ import (
 
 var fullyConsistent = &v1.Consistency{Requirement: &v1.Consistency_FullyConsistent{FullyConsistent: true}}
 
+func TestAddPrefixIfAbsent(t *testing.T) {
+	// Unprefixed names get the prefix.
+	require.Equal(t, "studio/permissions", addPrefixIfAbsent("permissions", "studio"))
+	require.Equal(t, "studio/user", addPrefixIfAbsent("user", "studio"))
+	// Already-prefixed names are left alone (regression for #460: importing
+	// relationships that already carry the prefix must not double it).
+	require.Equal(t, "studio/permissions", addPrefixIfAbsent("studio/permissions", "studio"))
+	// A different existing prefix is also preserved, not stacked.
+	require.Equal(t, "other/permissions", addPrefixIfAbsent("other/permissions", "studio"))
+}
+
 func TestImportCmd(t *testing.T) {
 	testcases := map[string]struct {
 		importSchema bool
@@ -181,3 +192,75 @@ func TestImportCmdRelationsOnly(t *testing.T) {
 		require.Contains(t, schemaResp.SchemaText, `permission view = user`)
 	})
 }
+
+// TestImportCmdRelationsOnlyAlreadyPrefixed is a regression test for #460:
+// importing relationships that are already prefixed (e.g. from a prefixed
+// schemaFile) with --schema=false must not double the auto-detected prefix
+// (`prefixtest/resource` becoming `prefixtest/prefixtest/resource`).
+func TestImportCmdRelationsOnlyAlreadyPrefixed(t *testing.T) {
+	cmd := zedtesting.CreateTestCobraCommandWithFlagValue(
+		t,
+		zedtesting.StringFlag{FlagName: "schema-definition-prefix"},
+		zedtesting.BoolFlag{FlagName: "schema", FlagValue: false},
+		zedtesting.BoolFlag{FlagName: "relationships", FlagValue: true},
+		zedtesting.IntFlag{FlagName: "batch-size", FlagValue: 100},
+		zedtesting.IntFlag{FlagName: "workers", FlagValue: 1},
+	)
+
+	ctx := t.Context()
+	srv := zedtesting.NewTestServer(ctx, t)
+	go func() {
+		assert.NoError(t, srv.Run(ctx))
+	}()
+	conn, err := srv.NewClient()
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		conn.Close()
+	})
+
+	c, err := zedtesting.ClientFromConn(conn)(cmd)
+	require.NoError(t, err)
+
+	// Write a prefixed schema so prefix auto-detection returns "prefixtest".
+	schemaBytes, err := os.ReadFile(filepath.Join("import-test", "prefixed-schema.zed"))
+	require.NoError(t, err)
+	_, err = c.WriteSchema(ctx, &v1.WriteSchemaRequest{Schema: string(schemaBytes)})
+	require.NoError(t, err)
+
+	// Mirror the import command: auto-detect the prefix from the running
+	// server, then import already-prefixed relationships with --schema=false.
+	prefix, err := determinePrefixForSchema(ctx, "", c, nil)
+	require.NoError(t, err)
+	require.Equal(t, "prefixtest", prefix)
+
+	f := filepath.Join("import-test", "prefixed-relations-validation-file.yaml")
+	require.NoError(t, importCmdFunc(cmd, c, c, prefix, f))
+
+	// The relationship must land at prefixtest/resource, not the doubled
+	// prefixtest/prefixtest/resource (which would fail with FailedPrecondition).
+	resp, err := c.CheckPermission(ctx, &v1.CheckPermissionRequest{
+		Consistency: fullyConsistent,
+		Subject:     &v1.SubjectReference{Object: &v1.ObjectReference{ObjectType: "prefixtest/user", ObjectId: "alice"}},
+		Permission:  "view",
+		Resource:    &v1.ObjectReference{ObjectType: "prefixtest/resource", ObjectId: "doc1"},
+	})
+	require.NoError(t, err)
+	require.Equal(t, v1.CheckPermissionResponse_PERMISSIONSHIP_HAS_PERMISSION, resp.Permissionship)
+
+	// The already-prefixed caveat name must also be preserved (not doubled to
+	// prefixtest/prefixtest/somecaveat). With the caveat context satisfied, the
+	// caveated relationship grants the permission.
+	caveatedResp, err := c.CheckPermission(ctx, &v1.CheckPermissionRequest{
+		Consistency: fullyConsistent,
+		Subject:     &v1.SubjectReference{Object: &v1.ObjectReference{ObjectType: "prefixtest/user", ObjectId: "bob"}},
+		Permission:  "view",
+		Resource:    &v1.ObjectReference{ObjectType: "prefixtest/resource", ObjectId: "doc2"},
+		Context: func() *structpb.Struct {
+			s, serr := structpb.NewStruct(map[string]any{"allowed": true})
+			require.NoError(t, serr)
+			return s
+		}(),
+	})
+	require.NoError(t, err)
+	require.Equal(t, v1.CheckPermissionResponse_PERMISSIONSHIP_HAS_PERMISSION, caveatedResp.Permissionship)
+}