chore(coderd/database): enforce workspace ACL JSON object constraints (#22019)

The constraints prevent faulty code from saving 'null' as JSON and breaking the `workspaces_expanded` view.

coderd/database/check_constraint.go

@@ -17,4 +17,6 @@ const (
 	CheckTelemetryLockEventTypeConstraint        CheckConstraint = "telemetry_lock_event_type_constraint"         // telemetry_locks
 	CheckValidationMonotonicOrder                CheckConstraint = "validation_monotonic_order"                   // template_version_parameters
 	CheckUsageEventTypeCheck                     CheckConstraint = "usage_event_type_check"                       // usage_events
+	CheckGroupAclIsObject                        CheckConstraint = "group_acl_is_object"                          // workspaces
+	CheckUserAclIsObject                         CheckConstraint = "user_acl_is_object"                           // workspaces
 )

coderd/database/dump.sql

@@ -2736,7 +2736,9 @@ CREATE TABLE workspaces (
     favorite boolean DEFAULT false NOT NULL,
     next_start_at timestamp with time zone,
     group_acl jsonb DEFAULT '{}'::jsonb NOT NULL,
-    user_acl jsonb DEFAULT '{}'::jsonb NOT NULL
+    user_acl jsonb DEFAULT '{}'::jsonb NOT NULL,
+    CONSTRAINT group_acl_is_object CHECK ((jsonb_typeof(group_acl) = 'object'::text)),
+    CONSTRAINT user_acl_is_object CHECK ((jsonb_typeof(user_acl) = 'object'::text))
 );
 
 COMMENT ON COLUMN workspaces.favorite IS 'Favorite is true if the workspace owner has favorited the workspace.';

coderd/database/migrations/000417_workspace_acl_object_constraint.down.sql

@@ -0,0 +1,3 @@
+ALTER TABLE workspaces
+    DROP CONSTRAINT IF EXISTS group_acl_is_object,
+    DROP CONSTRAINT IF EXISTS user_acl_is_object;

coderd/database/migrations/000417_workspace_acl_object_constraint.up.sql

@@ -0,0 +1,9 @@
+-- Add constraints that reject 'null'::jsonb for group and user ACLs
+-- because they would break the new workspace_expanded view.
+
+UPDATE workspaces SET group_acl = '{}'::jsonb WHERE group_acl = 'null'::jsonb;
+UPDATE workspaces SET user_acl = '{}'::jsonb WHERE user_acl = 'null'::jsonb;
+
+ALTER TABLE workspaces
+    ADD CONSTRAINT group_acl_is_object CHECK (jsonb_typeof(group_acl) = 'object'),
+    ADD CONSTRAINT user_acl_is_object CHECK (jsonb_typeof(user_acl) = 'object');

coderd/database/migrations/testdata/fixtures/000416_pre_workspace_acl_object_constraint.up.sql

@@ -0,0 +1,35 @@
+-- Fixture for migration 000417_workspace_acl_object_constraint.
+-- Inserts a workspace with 'null'::json ACLs to ensure the migration
+-- correctly normalizes such values.
+
+INSERT INTO workspaces (
+    id,
+    created_at,
+    updated_at,
+    owner_id,
+    organization_id,
+    template_id,
+    deleted,
+    name,
+    last_used_at,
+    automatic_updates,
+    favorite,
+    group_acl,
+    user_acl
+)
+VALUES (
+    '6f6fdbee-4c18-4a5c-8a8d-9b811c9f0a28',
+    '2024-02-10 00:00:00+00',
+    '2024-02-10 00:00:00+00',
+    '30095c71-380b-457a-8995-97b8ee6e5307',
+    'bb640d07-ca8a-4869-b6bc-ae61ebb2fda1',
+    '4cc1f466-f326-477e-8762-9d0c6781fc56',
+    false,
+    'acl-null-workspace',
+    '0001-01-01 00:00:00+00',
+    'never',
+    false,
+    'null'::jsonb,
+    'null'::jsonb
+)
+ON CONFLICT DO NOTHING;

coderd/database/querier_test.go

@@ -6765,6 +6765,65 @@ func TestWorkspaceBuildDeadlineConstraint(t *testing.T) {
 	}
 }
 
+func TestWorkspaceACLObjectConstraint(t *testing.T) {
+	t.Parallel()
+
+	db, _ := dbtestutil.NewDB(t)
+	org := dbgen.Organization(t, db, database.Organization{})
+	user := dbgen.User(t, db, database.User{})
+	template := dbgen.Template(t, db, database.Template{
+		CreatedBy:      user.ID,
+		OrganizationID: org.ID,
+	})
+	workspace := dbgen.Workspace(t, db, database.WorkspaceTable{
+		OwnerID:    user.ID,
+		TemplateID: template.ID,
+		Deleted:    false,
+	})
+
+	t.Run("GroupACLNull", func(t *testing.T) {
+		t.Parallel()
+
+		var nilACL database.WorkspaceACL
+
+		ctx := testutil.Context(t, testutil.WaitLong)
+		err := db.UpdateWorkspaceACLByID(ctx, database.UpdateWorkspaceACLByIDParams{
+			ID:       workspace.ID,
+			GroupACL: nilACL,
+			UserACL:  database.WorkspaceACL{},
+		})
+		require.Error(t, err)
+		require.True(t, database.IsCheckViolation(err, database.CheckGroupAclIsObject))
+	})
+
+	t.Run("UserACLNull", func(t *testing.T) {
+		t.Parallel()
+
+		var nilACL database.WorkspaceACL
+
+		ctx := testutil.Context(t, testutil.WaitLong)
+		err := db.UpdateWorkspaceACLByID(ctx, database.UpdateWorkspaceACLByIDParams{
+			ID:       workspace.ID,
+			GroupACL: database.WorkspaceACL{},
+			UserACL:  nilACL,
+		})
+		require.Error(t, err)
+		require.True(t, database.IsCheckViolation(err, database.CheckUserAclIsObject))
+	})
+
+	t.Run("ValidEmptyObjects", func(t *testing.T) {
+		t.Parallel()
+
+		ctx := testutil.Context(t, testutil.WaitLong)
+		err := db.UpdateWorkspaceACLByID(ctx, database.UpdateWorkspaceACLByIDParams{
+			ID:       workspace.ID,
+			GroupACL: database.WorkspaceACL{},
+			UserACL:  database.WorkspaceACL{},
+		})
+		require.NoError(t, err)
+	})
+}
+
 // TestGetLatestWorkspaceBuildsByWorkspaceIDs populates the database with
 // workspaces and builds. It then tests that
 // GetLatestWorkspaceBuildsByWorkspaceIDs returns the latest build for some