diff --git a/packages/client/src/effect/api/api.ts b/packages/client/src/effect/api/api.ts index 00a16898092a..df7962cfef88 100644 --- a/packages/client/src/effect/api/api.ts +++ b/packages/client/src/effect/api/api.ts @@ -1726,6 +1726,16 @@ export type PermissionSavedListOperation = ( input?: PermissionSavedListInput, ) => Effect.Effect +export type PermissionSavedUpdateInput = { + readonly id: PermissionSaved.ID + readonly action?: string | undefined + readonly resource?: string | undefined +} +export type PermissionSavedUpdateOutput = PermissionSaved.Info +export type PermissionSavedUpdateOperation = ( + input: PermissionSavedUpdateInput, +) => Effect.Effect + export type PermissionSavedRemoveInput = { readonly id: PermissionSaved.ID } export type PermissionSavedRemoveOutput = void export type PermissionSavedRemoveOperation = ( @@ -1768,7 +1778,11 @@ export type PermissionReplyOperation = ( export interface PermissionApi { readonly request: { readonly list: PermissionRequestListOperation } - readonly saved: { readonly list: PermissionSavedListOperation; readonly remove: PermissionSavedRemoveOperation } + readonly saved: { + readonly list: PermissionSavedListOperation + readonly update: PermissionSavedUpdateOperation + readonly remove: PermissionSavedRemoveOperation + } readonly create: PermissionCreateOperation readonly list: PermissionListOperation readonly get: PermissionGetOperation diff --git a/packages/client/src/effect/generated/client.ts b/packages/client/src/effect/generated/client.ts index 057240d30d2c..8fa86e433c7f 100644 --- a/packages/client/src/effect/generated/client.ts +++ b/packages/client/src/effect/generated/client.ts @@ -176,6 +176,8 @@ import type { PermissionRequestListOutput, PermissionSavedListInput, PermissionSavedListOutput, + PermissionSavedUpdateInput, + PermissionSavedUpdateOutput, PermissionSavedRemoveInput, PermissionSavedRemoveOutput, PermissionCreateInput, @@ -1155,6 +1157,17 @@ const EndpointPermissionSavedList = (raw: RawClient["server.permission"]) => (in ), ) +const EndpointPermissionSavedUpdate = (raw: RawClient["server.permission"]) => (input: PermissionSavedUpdateInput) => + preserveEffect()( + raw["permission.saved.update"]({ + params: { id: input["id"] }, + payload: { action: input["action"], resource: input["resource"] }, + }).pipe( + Effect.mapError(mapClientError), + Effect.map((value) => value.data), + ), + ) + const EndpointPermissionSavedRemove = (raw: RawClient["server.permission"]) => (input: PermissionSavedRemoveInput) => preserveEffect()( raw["permission.saved.remove"]({ params: { id: input["id"] } }).pipe(Effect.mapError(mapClientError)), @@ -1205,7 +1218,11 @@ const EndpointPermissionReply = (raw: RawClient["server.permission"]) => (input: const adaptGroupPermission = (raw: RawClient["server.permission"]) => ({ request: { list: EndpointPermissionRequestList(raw) }, - saved: { list: EndpointPermissionSavedList(raw), remove: EndpointPermissionSavedRemove(raw) }, + saved: { + list: EndpointPermissionSavedList(raw), + update: EndpointPermissionSavedUpdate(raw), + remove: EndpointPermissionSavedRemove(raw), + }, create: EndpointPermissionCreate(raw), list: EndpointPermissionList(raw), get: EndpointPermissionGet(raw), diff --git a/packages/client/src/promise/generated/client.ts b/packages/client/src/promise/generated/client.ts index 444118120a65..9503bd30e760 100644 --- a/packages/client/src/promise/generated/client.ts +++ b/packages/client/src/promise/generated/client.ts @@ -170,6 +170,8 @@ import type { PermissionRequestListOutput, PermissionSavedListInput, PermissionSavedListOutput, + PermissionSavedUpdateInput, + PermissionSavedUpdateOutput, PermissionSavedRemoveInput, PermissionSavedRemoveOutput, PermissionCreateInput, @@ -1562,6 +1564,18 @@ export function make(options: ClientOptions) { }, requestOptions, ).then((value) => value.data), + update: (input: PermissionSavedUpdateInput, requestOptions?: RequestOptions) => + request<{ readonly data: PermissionSavedUpdateOutput }>( + { + method: "PATCH", + path: `/api/permission/saved/${encodeURIComponent(input.id)}`, + body: { action: input["action"], resource: input["resource"] }, + successStatus: 200, + declaredStatuses: [400, 401, 404, 409], + empty: false, + }, + requestOptions, + ).then((value) => value.data), remove: (input: PermissionSavedRemoveInput, requestOptions?: RequestOptions) => request( { diff --git a/packages/client/src/promise/generated/types.ts b/packages/client/src/promise/generated/types.ts index bd28b8256167..7e601e0dcec5 100644 --- a/packages/client/src/promise/generated/types.ts +++ b/packages/client/src/promise/generated/types.ts @@ -2847,6 +2847,14 @@ export type ProjectNotFoundError = { export const isProjectNotFoundError = (value: unknown): value is ProjectNotFoundError => typeof value === "object" && value !== null && "_tag" in value && value["_tag"] === "ProjectNotFoundError" +export type PermissionSavedNotFoundError = { + readonly _tag: "PermissionSavedNotFoundError" + readonly id: string + readonly message: string +} +export const isPermissionSavedNotFoundError = (value: unknown): value is PermissionSavedNotFoundError => + typeof value === "object" && value !== null && "_tag" in value && value["_tag"] === "PermissionSavedNotFoundError" + export type PermissionNotFoundError = { readonly _tag: "PermissionNotFoundError" readonly requestID: string @@ -6251,6 +6259,14 @@ export type PermissionSavedListInput = { readonly projectID?: { readonly project export type PermissionSavedListOutput = { data: Array }["data"] +export type PermissionSavedUpdateInput = { + readonly id: { readonly id: string }["id"] + readonly action?: { readonly action?: string | undefined; readonly resource?: string | undefined }["action"] + readonly resource?: { readonly action?: string | undefined; readonly resource?: string | undefined }["resource"] +} + +export type PermissionSavedUpdateOutput = { data: PermissionSavedInfo }["data"] + export type PermissionSavedRemoveInput = { readonly id: { readonly id: string }["id"] } export type PermissionSavedRemoveOutput = void diff --git a/packages/core/src/permission/saved.ts b/packages/core/src/permission/saved.ts index 8ba50b16beb1..78aa4124c848 100644 --- a/packages/core/src/permission/saved.ts +++ b/packages/core/src/permission/saved.ts @@ -1,7 +1,9 @@ export * as PermissionSaved from "./saved.js" import { eq } from "drizzle-orm" -import { Context, DateTime, Effect, Layer, Schema } from "effect" +import { Cause, Context, DateTime, Effect, Layer, Schema } from "effect" +import { isSqlError } from "effect/sql/SqlError" +import type { EffectDrizzleQueryError } from "drizzle-orm/effect-core/errors" import { Project } from "@opencode/schema/project" import { Database } from "../database/database.js" import { makeGlobalNode } from "@opencode/util/effect/app-node" @@ -26,9 +28,27 @@ export const AddInput = Schema.Struct({ }).annotate({ identifier: "PermissionSaved.AddInput" }) export type AddInput = typeof AddInput.Type +export const UpdateInput = Schema.Struct({ + id: ID, + action: Schema.String.pipe(Schema.optional), + resource: Schema.String.pipe(Schema.optional), +}).annotate({ identifier: "PermissionSaved.UpdateInput" }) +export type UpdateInput = typeof UpdateInput.Type + +export class NotFoundError extends Schema.TaggedError()("PermissionSaved.NotFoundError", { + id: ID, +}) {} + +export class ConflictError extends Schema.TaggedError()("PermissionSaved.ConflictError", { + id: ID, + action: Schema.String, + resource: Schema.String, +}) {} + export interface Interface { readonly list: (input?: ListInput) => Effect.Effect> readonly add: (input: AddInput) => Effect.Effect + readonly update: (input: UpdateInput) => Effect.Effect readonly remove: (id: ID) => Effect.Effect } @@ -46,18 +66,7 @@ const layer = Layer.effect( .where(input?.projectID ? eq(PermissionTable.project_id, input.projectID) : undefined) .all() .pipe(Effect.orDie) - return rows.map( - (row): Info => ({ - id: row.id, - projectID: row.project_id, - action: row.action, - resource: row.resource, - time: { - created: DateTime.makeUnsafe(row.time_created), - updated: DateTime.makeUnsafe(row.time_updated), - }, - }), - ) + return rows.map(info) }) const add = Effect.fn("PermissionSaved.add")(function* (input: AddInput) { @@ -77,12 +86,58 @@ const layer = Layer.effect( .pipe(Effect.orDie) }) + const update = Effect.fn("PermissionSaved.update")(function* (input: UpdateInput) { + const current = yield* db + .select() + .from(PermissionTable) + .where(eq(PermissionTable.id, input.id)) + .get() + .pipe(Effect.orDie) + if (!current) return yield* new NotFoundError({ id: input.id }) + const action = input.action ?? current.action + const resource = input.resource ?? current.resource + // The unique index rejects an edit that duplicates another saved permission in the project. + const updated = yield* db + .update(PermissionTable) + .set({ action, resource }) + .where(eq(PermissionTable.id, input.id)) + .returning() + .get() + .pipe( + Effect.catchIf(isUniqueViolation, () => new ConflictError({ id: input.id, action, resource })), + Effect.catchTag("EffectDrizzleQueryError", (error) => Effect.die(error)), + ) + if (!updated) return yield* new NotFoundError({ id: input.id }) + return info(updated) + }) + const remove = Effect.fn("PermissionSaved.remove")(function* (id: ID) { yield* db.delete(PermissionTable).where(eq(PermissionTable.id, id)).run().pipe(Effect.orDie) }) - return Service.of({ list, add, remove }) + return Service.of({ list, add, update, remove }) }), ) +// Durable Object SQLite throws a plain Error without SQLite codes, so its unique violation is only in the message. +const isUniqueViolation = (error: EffectDrizzleQueryError) => { + const sql = Cause.isCause(error.cause) ? Cause.squash(error.cause) : error.cause + if (!isSqlError(sql)) return false + return ( + sql.reason._tag === "UniqueViolation" || + (sql.reason.cause instanceof Error && sql.reason.cause.message.includes("SQLITE_CONSTRAINT_UNIQUE")) + ) +} + +const info = (row: typeof PermissionTable.$inferSelect): Info => ({ + id: row.id, + projectID: row.project_id, + action: row.action, + resource: row.resource, + time: { + created: DateTime.makeUnsafe(row.time_created), + updated: DateTime.makeUnsafe(row.time_updated), + }, +}) + export const node = makeGlobalNode({ service: Service, layer, deps: [Database.node] }) diff --git a/packages/core/test/permission.test.ts b/packages/core/test/permission.test.ts index d906c937a690..e755fa57543d 100644 --- a/packages/core/test/permission.test.ts +++ b/packages/core/test/permission.test.ts @@ -346,6 +346,38 @@ describe("Permission", () => { }), ) + it.effect("edits saved approvals and enforces the edited rule", () => + Effect.gen(function* () { + yield* setup() + const saved = yield* PermissionSaved.Service + const service = yield* Permission.Service + yield* saved.add({ projectID: Project.ID.global, action: "read", resources: ["src/*", "docs/*"] }) + const src = (yield* saved.list()).find((item) => item.resource === "src/*") + + expect(yield* saved.update({ id: src!.id, resource: "src/allowed.ts" })).toMatchObject({ + id: src!.id, + projectID: Project.ID.global, + action: "read", + resource: "src/allowed.ts", + }) + expect( + yield* service.ask(assertion({ id: Permission.ID.create("per_allowed"), resources: ["src/allowed.ts"] })), + ).toMatchObject({ effect: "allow" }) + expect( + yield* service.ask(assertion({ id: Permission.ID.create("per_other"), resources: ["src/other.ts"] })), + ).toMatchObject({ effect: "ask" }) + + expect(yield* saved.update({ id: src!.id, resource: "docs/*" }).pipe(Effect.flip)).toMatchObject({ + _tag: "PermissionSaved.ConflictError", + id: src!.id, + resource: "docs/*", + }) + expect( + yield* saved.update({ id: PermissionSaved.ID.make("psv_missing"), action: "edit" }).pipe(Effect.flip), + ).toMatchObject({ _tag: "PermissionSaved.NotFoundError" }) + }), + ) + for (const guard of ["configured deny", "missing Session"] as const) { it.effect(`skips pending auto-approval after always for ${guard}`, () => Effect.gen(function* () { diff --git a/packages/protocol/src/errors.ts b/packages/protocol/src/errors.ts index e793d8d8f5d7..41994c9e4148 100644 --- a/packages/protocol/src/errors.ts +++ b/packages/protocol/src/errors.ts @@ -215,6 +215,15 @@ export class PermissionNotFoundError extends Schema.TaggedError()( + "PermissionSavedNotFoundError", + { + id: Schema.String, + message: Schema.String, + }, + { httpApiStatus: 404 }, +) {} + export class FormNotFoundError extends Schema.TaggedError()( "FormNotFoundError", { diff --git a/packages/protocol/src/groups/permission.ts b/packages/protocol/src/groups/permission.ts index a5f93eb61bab..5cc0eb4b8502 100644 --- a/packages/protocol/src/groups/permission.ts +++ b/packages/protocol/src/groups/permission.ts @@ -6,7 +6,13 @@ import { Project } from "@opencode/schema/project" import { Session } from "@opencode/schema/session" import { Context, Schema } from "effect" import { HttpApiEndpoint, HttpApiGroup, HttpApiMiddleware, HttpApiSchema, OpenApi } from "effect/http-api" -import { LocationNotFoundError, PermissionNotFoundError, SessionNotFoundError } from "../errors.js" +import { + ConflictError, + LocationNotFoundError, + PermissionNotFoundError, + PermissionSavedNotFoundError, + SessionNotFoundError, +} from "../errors.js" import { LocationQuery, locationQueryOpenApi } from "./location.js" export const makePermissionGroup = < @@ -45,6 +51,24 @@ export const makePermissionGroup = < }), ), ) + .add( + HttpApiEndpoint.patch("permission.saved.update", "/api/permission/saved/:id", { + params: { id: PermissionSaved.ID }, + payload: Schema.Struct({ + action: Schema.String.pipe(Schema.optional), + resource: Schema.String.pipe(Schema.optional), + }), + success: Schema.Struct({ data: PermissionSaved.Info }), + error: [PermissionSavedNotFoundError, ConflictError], + }).annotateMerge( + OpenApi.annotations({ + identifier: "permission.saved.update", + summary: "Update saved permission", + description: + "Change the action or resource of a saved permission. Fails with a conflict when the project already has a saved permission with the same action and resource.", + }), + ), + ) .add( HttpApiEndpoint.delete("permission.saved.remove", "/api/permission/saved/:id", { params: { id: PermissionSaved.ID }, diff --git a/packages/server/src/handlers/permission.ts b/packages/server/src/handlers/permission.ts index 2832c502d649..68d0543a4c15 100644 --- a/packages/server/src/handlers/permission.ts +++ b/packages/server/src/handlers/permission.ts @@ -6,7 +6,7 @@ import { Session } from "@opencode/core/session" import { Effect } from "effect" import { HttpApiBuilder, HttpApiSchema } from "effect/http-api" import { Api } from "../api" -import { PermissionNotFoundError } from "@opencode/protocol/errors" +import { ConflictError, PermissionNotFoundError, PermissionSavedNotFoundError } from "@opencode/protocol/errors" import { locationErrors, response, sessionInfo } from "../location" import { missingSession } from "./session-error" @@ -95,6 +95,30 @@ export const PermissionHandler = HttpApiBuilder.group(Api, "server.permission", } }), ) + .handle( + "permission.saved.update", + Effect.fn(function* (ctx) { + const saved = yield* PermissionSaved.Service + return { + data: yield* saved + .update({ id: ctx.params.id, action: ctx.payload.action, resource: ctx.payload.resource }) + .pipe( + Effect.catchTags({ + "PermissionSaved.NotFoundError": (error) => + new PermissionSavedNotFoundError({ + id: error.id, + message: `Saved permission not found: ${error.id}`, + }), + "PermissionSaved.ConflictError": (error) => + new ConflictError({ + resource: error.id, + message: `A saved permission already allows ${error.action} for ${error.resource}`, + }), + }), + ), + } + }), + ) .handle( "permission.saved.remove", Effect.fn(function* (ctx) { diff --git a/packages/server/test/permission-saved.test.ts b/packages/server/test/permission-saved.test.ts new file mode 100644 index 000000000000..e9d8fa9d4597 --- /dev/null +++ b/packages/server/test/permission-saved.test.ts @@ -0,0 +1,73 @@ +import { expect } from "bun:test" +import { PermissionSaved } from "@opencode/schema/permission-saved" +import { Session } from "@opencode/schema/session" +import { Effect, Schema } from "effect" +import { it } from "../../core/test/lib/effect" +import { ServerFetch } from "../src/fetch" + +const SessionResponse = Schema.Struct({ data: Schema.toEncoded(Session.Info) }) +const AskResponse = Schema.Struct({ data: Schema.Struct({ id: Schema.String, effect: Schema.String }) }) +const SavedList = Schema.Struct({ data: Schema.Array(Schema.toEncoded(PermissionSaved.Info)) }) +const SavedResponse = Schema.Struct({ data: Schema.toEncoded(PermissionSaved.Info) }) + +it.live("edits saved permissions through PATCH", () => + Effect.gen(function* () { + const handler = yield* ServerFetch.make({ + app: { version: "test" }, + database: { path: ":memory:" }, + fs: { filewatcher: false }, + models: { fetch: false }, + }) + const request = (path: string, method: string, body?: unknown) => + Effect.promise(async () => { + const response = await handler( + new Request(`http://opencode.local${path}`, { + method, + headers: { "content-type": "application/json" }, + body: body === undefined ? undefined : JSON.stringify(body), + }), + ) + return { status: response.status, body: response.status === 204 ? undefined : await response.json() } + }) + + const session = Schema.decodeUnknownSync(SessionResponse)( + (yield* request("/api/session", "POST", { + permissions: [{ action: "deploy", resource: "*", effect: "ask" }], + })).body, + ) + // Saved permissions are only created by answering "always", so save two through the real request flow. + for (const resource of ["src/*", "docs/*"]) { + const asked = Schema.decodeUnknownSync(AskResponse)( + (yield* request(`/api/session/${session.data.id}/permission`, "POST", { + action: "deploy", + resources: [resource], + save: [resource], + })).body, + ) + expect(asked.data.effect).toBe("ask") + expect( + (yield* request(`/api/session/${session.data.id}/permission/${asked.data.id}/reply`, "POST", { + decision: "always", + })).status, + ).toBe(204) + } + const saved = Schema.decodeUnknownSync(SavedList)((yield* request("/api/permission/saved", "GET")).body) + const src = saved.data.find((item) => item.resource === "src/*")! + + const updated = yield* request(`/api/permission/saved/${src.id}`, "PATCH", { resource: "src/allowed.ts" }) + expect(updated.status).toBe(200) + expect(Schema.decodeUnknownSync(SavedResponse)(updated.body).data).toMatchObject({ + id: src.id, + action: "deploy", + resource: "src/allowed.ts", + }) + expect( + Schema.decodeUnknownSync(SavedList)((yield* request("/api/permission/saved", "GET")).body) + .data.map((item) => item.resource) + .toSorted(), + ).toEqual(["docs/*", "src/allowed.ts"]) + + expect((yield* request(`/api/permission/saved/${src.id}`, "PATCH", { resource: "docs/*" })).status).toBe(409) + expect((yield* request("/api/permission/saved/psv_missing", "PATCH", { resource: "x" })).status).toBe(404) + }).pipe(Effect.scoped), +)