From 8ade96e671bca7d84fd2d764ffca305e2fdc358d Mon Sep 17 00:00:00 2001 From: Jiayuan Zhang Date: Sun, 23 Aug 2026 22:47:37 +0800 Subject: [PATCH] MUL-6584: establish Public API v1 foundation (#7450) * feat(api): establish public v1 contract foundation Co-authored-by: multica-agent * fix(plugins): close public API contract gaps Co-authored-by: multica-agent * fix(issues): enforce revision guard during update Co-authored-by: multica-agent --------- Co-authored-by: multica-agent --- .env.example | 7 +- .../cmd/server/plugin_action_routes_test.go | 41 +- server/cmd/server/router.go | 34 +- server/internal/handler/plugin_action.go | 358 ++++++++++------ server/internal/handler/plugin_action_test.go | 209 ++++++++- .../internal/handler/plugin_callback_test.go | 18 +- server/internal/handler/plugin_hook.go | 13 +- server/internal/handler/plugin_hook_test.go | 47 ++ server/internal/middleware/plugin_auth.go | 4 +- .../internal/middleware/plugin_auth_test.go | 13 + .../internal/middleware/plugin_ratelimit.go | 40 ++ .../middleware/plugin_ratelimit_test.go | 65 +++ server/internal/service/issue_public.go | 55 +++ server/internal/service/plugin_hook.go | 3 + server/internal/service/plugin_token.go | 28 ++ server/pkg/db/generated/issue.sql.go | 41 +- server/pkg/db/queries/issue.sql | 5 + server/pkg/publicapi/v1/README.md | 55 +++ server/pkg/publicapi/v1/contract_test.go | 117 +++++ server/pkg/publicapi/v1/foundation.go | 79 ++++ server/pkg/publicapi/v1/openapi.yaml | 404 ++++++++++++++++++ server/pkg/publicapi/v1/problem.go | 111 +++++ server/pkg/publicapi/v1/problem_test.go | 36 ++ server/pkg/publicapi/v1/routes.go | 67 +++ server/pkg/publicapi/v1/spec.go | 14 + server/pkg/publicapi/v1/types.go | 103 +++++ 26 files changed, 1781 insertions(+), 186 deletions(-) create mode 100644 server/internal/middleware/plugin_ratelimit.go create mode 100644 server/internal/middleware/plugin_ratelimit_test.go create mode 100644 server/internal/service/issue_public.go create mode 100644 server/pkg/publicapi/v1/README.md create mode 100644 server/pkg/publicapi/v1/contract_test.go create mode 100644 server/pkg/publicapi/v1/foundation.go create mode 100644 server/pkg/publicapi/v1/openapi.yaml create mode 100644 server/pkg/publicapi/v1/problem.go create mode 100644 server/pkg/publicapi/v1/problem_test.go create mode 100644 server/pkg/publicapi/v1/routes.go create mode 100644 server/pkg/publicapi/v1/spec.go create mode 100644 server/pkg/publicapi/v1/types.go diff --git a/.env.example b/.env.example index c438890ae..f7cae0048 100644 --- a/.env.example +++ b/.env.example @@ -535,9 +535,10 @@ MULTICA_VCS_SECRET_KEY= # installation is bound to one immutable version. Hook endpoints and MCP servers # stay on the author's own infrastructure. Settings: # 1. MULTICA_PLUGIN_SECRET_KEY — a base64-encoded 32-byte key that encrypts -# `secret` config values at rest and seals short-lived surface launch URLs. -# Without it, saving a secret field and launching a surface both fail -# closed. Generate one with: +# `secret` config values at rest, seals short-lived surface launch URLs, +# and derives per-installation hook signing secrets. Without it, those +# secret/surface/hook features fail closed; `mpi_` Public API token issuance +# remains available and does not require hook signing. Generate one with: # openssl rand -base64 32 # 2. MULTICA_PLUGIN_SURFACE_ORIGIN — a dedicated browser origin routed to this # backend (for example https://plugins.example.com). It must differ from diff --git a/server/cmd/server/plugin_action_routes_test.go b/server/cmd/server/plugin_action_routes_test.go index 0336cdf77..0514faaa6 100644 --- a/server/cmd/server/plugin_action_routes_test.go +++ b/server/cmd/server/plugin_action_routes_test.go @@ -1,18 +1,23 @@ package main import ( + "encoding/json" "io" "net/http" "testing" + + publicapiv1 "github.com/multica-ai/multica/server/pkg/publicapi/v1" ) func TestPluginActionRouteTrustBoundaries(t *testing.T) { tests := []struct { name string + method string path string authorization string wantStatus int wantHandler bool + wantProblem bool }{ { name: "public API rejects a missing token before the handler", @@ -47,6 +52,22 @@ func TestPluginActionRouteTrustBoundaries(t *testing.T) { path: "/v1/hooks/summarize", authorization: "Bearer mpi_invalid", wantStatus: http.StatusNotFound, + wantProblem: true, + }, + { + name: "public API empty resource path uses the problem contract", + path: "/v1/issues/", + authorization: "Bearer mpi_invalid", + wantStatus: http.StatusNotFound, + wantProblem: true, + }, + { + name: "public API unsupported method uses the problem contract", + method: http.MethodPut, + path: "/v1/context", + authorization: "Bearer mpi_invalid", + wantStatus: http.StatusMethodNotAllowed, + wantProblem: true, }, { name: "surface bridge retains person-triggered hooks", @@ -58,7 +79,11 @@ func TestPluginActionRouteTrustBoundaries(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - req, err := http.NewRequest(http.MethodGet, testServer.URL+tt.path, nil) + method := tt.method + if method == "" { + method = http.MethodGet + } + req, err := http.NewRequest(method, testServer.URL+tt.path, nil) if err != nil { t.Fatalf("build request: %v", err) } @@ -70,7 +95,7 @@ func TestPluginActionRouteTrustBoundaries(t *testing.T) { t.Fatalf("request failed: %v", err) } defer response.Body.Close() - _, _ = io.Copy(io.Discard, response.Body) + body, _ := io.ReadAll(response.Body) if tt.wantHandler { if response.StatusCode == http.StatusUnauthorized || response.StatusCode == http.StatusNotFound { @@ -81,6 +106,18 @@ func TestPluginActionRouteTrustBoundaries(t *testing.T) { if response.StatusCode != tt.wantStatus { t.Fatalf("status = %d, want %d", response.StatusCode, tt.wantStatus) } + if tt.wantProblem { + if got := response.Header.Get("Content-Type"); got != publicapiv1.ProblemContentType { + t.Fatalf("Content-Type = %q, body=%s", got, body) + } + var problem publicapiv1.Problem + if err := json.Unmarshal(body, &problem); err != nil { + t.Fatalf("decode problem: %v; body=%s", err, body) + } + if problem.Status != tt.wantStatus || problem.RequestID == "" { + t.Fatalf("unexpected problem: %+v", problem) + } + } }) } } diff --git a/server/cmd/server/router.go b/server/cmd/server/router.go index 7eaaee87c..a2d66a6b4 100644 --- a/server/cmd/server/router.go +++ b/server/cmd/server/router.go @@ -45,6 +45,7 @@ import ( db "github.com/multica-ai/multica/server/pkg/db/generated" "github.com/multica-ai/multica/server/pkg/featureflag" "github.com/multica-ai/multica/server/pkg/llm" + publicapiv1 "github.com/multica-ai/multica/server/pkg/publicapi/v1" ) var defaultOrigins = []string{ @@ -53,10 +54,7 @@ var defaultOrigins = []string{ "http://localhost:5174", // electron-vite dev (fallback port) } -const ( - pluginActionPublicPrefix = "/v1" - pluginBridgePrefix = "/api/plugin-bridge/v1" -) +const pluginBridgePrefix = "/api/plugin-bridge/v1" // corsAllowedHeaders must list every header the browser clients send. A header // missing here fails the preflight, so the request never reaches the handler at @@ -69,6 +67,7 @@ var corsAllowedHeaders = []string{ "Authorization", "Content-Type", "Idempotency-Key", + "If-Match", "X-Workspace-ID", "X-Workspace-Slug", "X-Request-ID", @@ -92,20 +91,22 @@ var corsAllowedHeaders = []string{ // Referencing the handler constant rather than re-typing the string keeps a // rename from quietly switching the signal off (MUL-5492). var corsExposedHeaders = []string{ + "ETag", + "X-Request-ID", handler.HeaderCommentsTruncated, handler.HeaderTimelineTruncated, } func registerPluginActionRoutes(r chi.Router, h *handler.Handler) { - r.Get("/context", h.GetPluginContext) - r.Get("/issues/{id}", h.GetPluginIssue) - r.Patch("/issues/{id}", h.PatchPluginIssue) - r.Get("/issues/{id}/comments", h.ListPluginComments) - r.Post("/issues/{id}/comments", h.CreatePluginComment) - r.Get("/storage/{scope}", h.ListPluginStorage) - r.Get("/storage/{scope}/{key}", h.GetPluginStorage) - r.Put("/storage/{scope}/{key}", h.PutPluginStorage) - r.Delete("/storage/{scope}/{key}", h.DeletePluginStorage) + r.Get(publicapiv1.PathContext, h.GetPluginContext) + r.Get(publicapiv1.PathIssue, h.GetPluginIssue) + r.Patch(publicapiv1.PathIssue, h.PatchPluginIssue) + r.Get(publicapiv1.PathIssueComments, h.ListPluginComments) + r.Post(publicapiv1.PathIssueComments, h.CreatePluginComment) + r.Get(publicapiv1.PathStorageScope, h.ListPluginStorage) + r.Get(publicapiv1.PathStorageValue, h.GetPluginStorage) + r.Put(publicapiv1.PathStorageValue, h.PutPluginStorage) + r.Delete(publicapiv1.PathStorageValue, h.DeletePluginStorage) } func allowedOrigins() []string { @@ -154,7 +155,7 @@ func pluginActionBaseURL(publicURL string) string { if publicURL == "" { return "" } - return publicURL + pluginActionPublicPrefix + return publicURL + publicapiv1.BasePath } // parseTrustedProxies parses a comma-separated list of CIDR prefixes from the @@ -1403,8 +1404,11 @@ func NewRouterWithOptions(pool *pgxpool.Pool, hub *realtime.Hub, bus *events.Bus // boundary even when both hostnames route to the same Go service. r.Group(func(r chi.Router) { r.Use(middleware.PluginBearerOnly) - r.Route(pluginActionPublicPrefix, func(r chi.Router) { + r.Use(middleware.PluginRateLimit(rdb, envPositiveInt("RATE_LIMIT_PLUGIN_API", 120), time.Minute)) + r.Route(publicapiv1.BasePath, func(r chi.Router) { registerPluginActionRoutes(r, h) + r.NotFound(publicapiv1.NotFound) + r.MethodNotAllowed(publicapiv1.MethodNotAllowed) }) }) diff --git a/server/internal/handler/plugin_action.go b/server/internal/handler/plugin_action.go index 119b35d37..63798640a 100644 --- a/server/internal/handler/plugin_action.go +++ b/server/internal/handler/plugin_action.go @@ -2,8 +2,10 @@ package handler import ( "encoding/json" + "errors" + "fmt" "net/http" - + "strconv" "strings" "github.com/go-chi/chi/v5" @@ -15,6 +17,7 @@ import ( "github.com/multica-ai/multica/server/pkg/dbid" "github.com/multica-ai/multica/server/pkg/plugincontract" "github.com/multica-ai/multica/server/pkg/protocol" + publicapiv1 "github.com/multica-ai/multica/server/pkg/publicapi/v1" ) // The Action API is what a plugin surface reaches through the host bridge. @@ -62,17 +65,52 @@ type pluginActor struct { func (a pluginActor) isMember() bool { return a.Type == "member" } // requireMember refuses an endpoint that has no meaning without a person. -func (a pluginActor) requireMember(w http.ResponseWriter) bool { +func (a pluginActor) requireMember(w http.ResponseWriter, r *http.Request) bool { if a.isMember() { return true } - writeError(w, http.StatusForbidden, "this endpoint requires a user; the presented token acts as the Plugin itself") + publicapiv1.WriteProblem(w, r, http.StatusForbidden, "member_required", "this endpoint requires a user; the presented token acts as the Plugin itself") return false } +func (h *Handler) requirePluginActionV1(w http.ResponseWriter, r *http.Request) bool { + if h.pluginsV1Enabled(r.Context()) { + return true + } + publicapiv1.WriteProblem(w, r, http.StatusServiceUnavailable, "plugin_api_disabled", "Plugin management is not enabled") + return false +} + +// writePluginActionError maps service-layer failures into the one stable +// Public API problem contract. Management endpoints intentionally keep their +// existing App API errors; only the versioned Action/bridge surface uses this. +func writePluginActionError(w http.ResponseWriter, r *http.Request, err error, fallback string) { + var pluginErr *service.PluginError + if !errors.As(err, &pluginErr) { + publicapiv1.WriteProblem(w, r, http.StatusInternalServerError, "internal_error", fallback) + return + } + switch pluginErr.Kind { + case service.PluginErrorInvalid: + publicapiv1.WriteProblem(w, r, http.StatusBadRequest, "invalid_request", pluginErr.Message) + case service.PluginErrorNotFound: + publicapiv1.WriteProblem(w, r, http.StatusNotFound, "not_found", pluginErr.Message) + case service.PluginErrorConflict: + publicapiv1.WriteProblem(w, r, http.StatusConflict, "conflict", pluginErr.Message) + case service.PluginErrorForbidden: + publicapiv1.WriteProblem(w, r, http.StatusForbidden, "forbidden", pluginErr.Message) + case service.PluginErrorIncompatible: + publicapiv1.WriteProblem(w, r, http.StatusUnprocessableEntity, "incompatible", pluginErr.Message) + case service.PluginErrorQuota: + publicapiv1.WriteProblem(w, r, http.StatusInsufficientStorage, "quota_exceeded", pluginErr.Message) + default: + publicapiv1.WriteProblem(w, r, http.StatusBadGateway, "upstream_unavailable", pluginErr.Message) + } +} + // pluginCaller runs checks 1 and 2 and returns the authorized caller. func (h *Handler) pluginCaller(w http.ResponseWriter, r *http.Request, scope string) (service.PluginActionCaller, pluginActor, bool) { - if !h.requirePluginsV1(w, r) { + if !h.requirePluginActionV1(w, r) { return service.PluginActionCaller{}, pluginActor{}, false } if token := middleware.BearerToken(r); middleware.IsPluginBearerToken(token) { @@ -84,26 +122,29 @@ func (h *Handler) pluginCaller(w http.ResponseWriter, r *http.Request, scope str // pluginSessionCaller is the surface path, unchanged: a real signed-in user // whose own permissions bound everything the plugin can reach. func (h *Handler) pluginSessionCaller(w http.ResponseWriter, r *http.Request, scope string) (service.PluginActionCaller, pluginActor, bool) { - userID, ok := requireUserID(w, r) - if !ok { + userID := strings.TrimSpace(r.Header.Get("X-User-ID")) + if userID == "" { + publicapiv1.WriteProblem(w, r, http.StatusUnauthorized, "unauthorized", "missing authenticated user") return service.PluginActionCaller{}, pluginActor{}, false } - parsedUserID, ok := parseUUIDOrBadRequest(w, userID, "user_id") - if !ok { + parsedUserID, err := util.ParseUUID(userID) + if err != nil { + publicapiv1.WriteProblem(w, r, http.StatusBadRequest, "invalid_request", "user_id must be a valid UUID") return service.PluginActionCaller{}, pluginActor{}, false } caller, err := h.PluginService.AuthorizePluginAction(r.Context(), r.Header.Get(pluginInstallationHeader), parsedUserID, scope) if err != nil { - writePluginError(w, err, "failed to authorize the Plugin call") + writePluginActionError(w, r, err, "failed to authorize the Plugin call") return service.PluginActionCaller{}, pluginActor{}, false } // The workspace comes from the installation, never from a client header: // otherwise a caller could aim an installation at a workspace it was never // installed in. Membership is then checked against that workspace. - member, ok := h.requireWorkspaceMember(w, r, uuidToString(caller.WorkspaceID), "workspace not found") - if !ok { + member, err := h.getWorkspaceMember(r.Context(), userID, uuidToString(caller.WorkspaceID)) + if err != nil { + publicapiv1.WriteProblem(w, r, http.StatusNotFound, "not_found", "workspace not found") return service.PluginActionCaller{}, pluginActor{}, false } return caller, pluginActor{Type: "member", Member: member}, true @@ -124,12 +165,12 @@ func (h *Handler) pluginTokenCaller(w http.ResponseWriter, r *http.Request, toke switch { case strings.HasPrefix(token, "mpc_"): if h.PluginService.Callbacks == nil { - writeError(w, http.StatusForbidden, "callback tokens are not enabled") + publicapiv1.WriteProblem(w, r, http.StatusForbidden, "callback_tokens_disabled", "callback tokens are not enabled") return service.PluginActionCaller{}, pluginActor{}, false } grant, err := h.PluginService.Callbacks.Resolve(token) if err != nil { - writePluginError(w, err, "failed to authorize the Plugin call") + writePluginActionError(w, r, err, "failed to authorize the Plugin call") return service.PluginActionCaller{}, pluginActor{}, false } installationID = grant.InstallationID @@ -140,7 +181,7 @@ func (h *Handler) pluginTokenCaller(w http.ResponseWriter, r *http.Request, toke default: installation, err := h.PluginService.AuthenticateInstallToken(r.Context(), token) if err != nil { - writePluginError(w, err, "failed to authorize the Plugin call") + writePluginActionError(w, r, err, "failed to authorize the Plugin call") return service.PluginActionCaller{}, pluginActor{}, false } installationID = installation.ID @@ -148,7 +189,7 @@ func (h *Handler) pluginTokenCaller(w http.ResponseWriter, r *http.Request, toke caller, err := h.PluginService.AuthorizePluginAction(r.Context(), uuidToString(installationID), memberUserID, scope) if err != nil { - writePluginError(w, err, "failed to authorize the Plugin call") + writePluginActionError(w, r, err, "failed to authorize the Plugin call") return service.PluginActionCaller{}, pluginActor{}, false } @@ -162,7 +203,7 @@ func (h *Handler) pluginTokenCaller(w http.ResponseWriter, r *http.Request, toke if memberUserID.Valid { member, err := h.getWorkspaceMember(r.Context(), uuidToString(memberUserID), uuidToString(caller.WorkspaceID)) if err != nil { - writeError(w, http.StatusForbidden, "the user this callback acts for is no longer a member") + publicapiv1.WriteProblem(w, r, http.StatusForbidden, "actor_membership_revoked", "the user this callback acts for is no longer a member") return service.PluginActionCaller{}, pluginActor{}, false } actor = pluginActor{Type: "member", Member: member} @@ -178,7 +219,7 @@ func (h *Handler) pluginTokenCaller(w http.ResponseWriter, r *http.Request, toke // not be able to confirm that an id it cannot read exists. func (h *Handler) pluginIssueForUser(w http.ResponseWriter, r *http.Request, caller service.PluginActionCaller, issueID string) (db.Issue, bool) { if issueID == "" { - writeError(w, http.StatusNotFound, "issue not found") + publicapiv1.WriteProblem(w, r, http.StatusNotFound, "not_found", "issue not found") return db.Issue{}, false } // Resolve first, then compare ids: a caller may name an issue by identifier @@ -186,7 +227,7 @@ func (h *Handler) pluginIssueForUser(w http.ResponseWriter, r *http.Request, cal // pass under one spelling and fail under the other. issue, ok := h.resolvePluginIssue(r, caller, issueID) if !ok { - writeError(w, http.StatusNotFound, "issue not found") + publicapiv1.WriteProblem(w, r, http.StatusNotFound, "not_found", "issue not found") return db.Issue{}, false } // A callback grant issued about one issue reaches only that issue. Without @@ -196,7 +237,7 @@ func (h *Handler) pluginIssueForUser(w http.ResponseWriter, r *http.Request, cal // 404 rather than 403: the caller may well be able to see this issue by // other means, and "you are scoped elsewhere" would confirm the id exists. if caller.IssueScope.Valid && uuidToString(issue.ID) != uuidToString(caller.IssueScope) { - writeError(w, http.StatusNotFound, "issue not found") + publicapiv1.WriteProblem(w, r, http.StatusNotFound, "not_found", "issue not found") return db.Issue{}, false } if !h.authorizeIssueWindow(w, r, issue.ID, issue.WorkspaceID, "plugin") { @@ -236,7 +277,7 @@ func (h *Handler) GetPluginContext(w http.ResponseWriter, r *http.Request) { } workspace, err := h.Queries.GetWorkspace(r.Context(), caller.WorkspaceID) if err != nil { - writeError(w, http.StatusInternalServerError, "failed to load the workspace") + publicapiv1.WriteProblem(w, r, http.StatusInternalServerError, "internal_error", "failed to load the workspace") return } // A plugin-actor call has no user to describe, and the payload says so @@ -245,7 +286,7 @@ func (h *Handler) GetPluginContext(w http.ResponseWriter, r *http.Request) { if actor.isMember() { loaded, err := h.Queries.GetUser(r.Context(), actor.Member.UserID) if err != nil { - writeError(w, http.StatusInternalServerError, "failed to load the user") + publicapiv1.WriteProblem(w, r, http.StatusInternalServerError, "internal_error", "failed to load the user") return } user = &loaded @@ -260,28 +301,48 @@ func (h *Handler) GetPluginContext(w http.ResponseWriter, r *http.Request) { issue = &loaded } - writeJSON(w, http.StatusOK, h.PluginService.BuildPluginContext(caller, workspace, user, issue)) + writeJSON(w, http.StatusOK, publicPluginContext(h.PluginService.BuildPluginContext(caller, workspace, user, issue))) } -// GetPluginIssue — GET /v1/issues/{id} +func publicPluginContext(context service.PluginContext) publicapiv1.Context { + payload := publicapiv1.Context{ + Workspace: publicapiv1.ContextWorkspace{ + ID: context.Workspace.ID, + Name: context.Workspace.Name, + Slug: context.Workspace.Slug, + }, + Config: context.Config, + GrantedNetDomains: context.GrantedURLs, + Actor: context.Actor, + } + if context.User != nil { + payload.User = &publicapiv1.ContextUser{ID: context.User.ID, Name: context.User.Name} + } + if context.Issue != nil { + payload.Issue = &publicapiv1.ContextIssue{ + ID: context.Issue.ID, + Identifier: context.Issue.Identifier, + Title: context.Issue.Title, + } + } + return payload +} + +// GetPluginIssue — GET /v1/issues/{issue_ref} func (h *Handler) GetPluginIssue(w http.ResponseWriter, r *http.Request) { caller, _, ok := h.pluginCaller(w, r, plugincontract.ScopeIssuesRead) if !ok { return } - issue, ok := h.pluginIssueForUser(w, r, caller, chi.URLParam(r, "id")) + issue, ok := h.pluginIssueForUser(w, r, caller, chi.URLParam(r, "issue_ref")) if !ok { return } + setPublicIssueETag(w, issue.Revision) writeJSON(w, http.StatusOK, h.pluginIssuePayload(r, caller, issue)) } -type patchPluginIssueRequest struct { - Title *string `json:"title,omitempty"` - Description *string `json:"description,omitempty"` -} - -// PatchPluginIssue — PATCH /v1/issues/{id} +// PatchPluginIssue — PATCH /v1/issues/{issue_ref} // // Title and description only. Status, priority, assignee, parent, project and // stage each carry dispatch, catalog or hierarchy semantics — a status change @@ -294,71 +355,140 @@ func (h *Handler) PatchPluginIssue(w http.ResponseWriter, r *http.Request) { if !ok { return } - issue, ok := h.pluginIssueForUser(w, r, caller, chi.URLParam(r, "id")) + issue, ok := h.pluginIssueForUser(w, r, caller, chi.URLParam(r, "issue_ref")) if !ok { return } - var req patchPluginIssueRequest + var req publicapiv1.PatchIssueRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - writeError(w, http.StatusBadRequest, "invalid request body") + publicapiv1.WriteProblem(w, r, http.StatusBadRequest, "invalid_request", "invalid request body") return } if req.Title == nil && req.Description == nil { - writeError(w, http.StatusBadRequest, "title or description is required") + publicapiv1.WriteProblem(w, r, http.StatusBadRequest, "invalid_request", "title or description is required") return } - - // Every other field is passed back unchanged: UpdateIssue treats a null - // nargs as "clear", so omitting them here would silently unassign the issue - // or detach it from its parent. - params := db.UpdateIssueParams{ - ID: issue.ID, - AssigneeType: issue.AssigneeType, - AssigneeID: issue.AssigneeID, - StartDate: issue.StartDate, - DueDate: issue.DueDate, - ParentIssueID: issue.ParentIssueID, - ProjectID: issue.ProjectID, - Stage: issue.Stage, + expectedRevision, ok := publicIssueExpectedRevision(w, r, req.ExpectedRevision) + if !ok { + return } - if req.Title != nil { - title := sanitizeNullBytes(*req.Title) - if title == "" { - writeError(w, http.StatusBadRequest, "title must not be empty") + if expectedRevision != nil { + if issue.Revision != *expectedRevision { + writePublicIssueRevisionConflict(w, r) return } - params.Title = pgtype.Text{String: title, Valid: true} - } - if req.Description != nil { - params.Description = pgtype.Text{String: sanitizeNullBytes(*req.Description), Valid: true} } - updated, err := h.Queries.UpdateIssue(r.Context(), params) + var title *string + if req.Title != nil { + value := sanitizeNullBytes(*req.Title) + if value == "" { + publicapiv1.WriteProblem(w, r, http.StatusBadRequest, "invalid_request", "title must not be empty") + return + } + title = &value + } + var description *string + if req.Description != nil { + value := sanitizeNullBytes(*req.Description) + description = &value + } + + updated, err := h.IssueService.UpdateContent(r.Context(), issue, service.IssueContentPatch{ + Title: title, + Description: description, + ExpectedRevision: expectedRevision, + }) if err != nil { - writeError(w, http.StatusInternalServerError, "failed to update the issue") + if errors.Is(err, service.ErrIssueRevisionConflict) { + writePublicIssueRevisionConflict(w, r) + return + } + publicapiv1.WriteProblem(w, r, http.StatusInternalServerError, "internal_error", "failed to update the issue") return } + setPublicIssueETag(w, updated.Revision) writeJSON(w, http.StatusOK, h.pluginIssuePayload(r, caller, updated)) } -// pluginIssuePayload reuses the ordinary issue response so a surface sees the -// same shape the rest of the product does. -func (h *Handler) pluginIssuePayload(r *http.Request, caller service.PluginActionCaller, issue db.Issue) IssueResponse { +func setPublicIssueETag(w http.ResponseWriter, revision int64) { + w.Header().Set("ETag", fmt.Sprintf(`W/"%d"`, revision)) +} + +func publicIssueExpectedRevision(w http.ResponseWriter, r *http.Request, bodyRevision *int64) (*int64, bool) { + header := strings.TrimSpace(r.Header.Get("If-Match")) + if header == "" { + if bodyRevision != nil && *bodyRevision < 1 { + publicapiv1.WriteProblem(w, r, http.StatusBadRequest, "invalid_request", "expected_revision must be a positive integer") + return nil, false + } + return bodyRevision, true + } + if strings.HasPrefix(header, "W/") { + header = strings.TrimSpace(strings.TrimPrefix(header, "W/")) + } + header = strings.Trim(header, `"`) + parsed, err := strconv.ParseInt(header, 10, 64) + if err != nil || parsed < 1 { + publicapiv1.WriteProblem(w, r, http.StatusBadRequest, "invalid_if_match", "If-Match must contain a positive issue revision") + return nil, false + } + if bodyRevision != nil && *bodyRevision != parsed { + publicapiv1.WriteProblem(w, r, http.StatusBadRequest, "revision_mismatch", "If-Match and expected_revision must identify the same revision") + return nil, false + } + return &parsed, true +} + +func writePublicIssueRevisionConflict(w http.ResponseWriter, r *http.Request) { + publicapiv1.WriteProblem(w, r, http.StatusConflict, "revision_conflict", "resource changed since it was loaded") +} + +// pluginIssuePayload explicitly maps the App response into the stable Public +// DTO. New App-only fields cannot leak into the public contract by accident. +func (h *Handler) pluginIssuePayload(r *http.Request, caller service.PluginActionCaller, issue db.Issue) publicapiv1.Issue { prefix := "" if workspace, err := h.Queries.GetWorkspace(r.Context(), caller.WorkspaceID); err == nil { prefix = workspace.IssuePrefix } - return issueToResponse(issue, prefix) + app := issueToResponse(issue, prefix) + return publicapiv1.Issue{ + ID: app.ID, + WorkspaceID: app.WorkspaceID, + Number: app.Number, + Identifier: app.Identifier, + Title: app.Title, + Description: app.Description, + Status: app.Status, + StatusCategory: app.StatusCategory, + Priority: app.Priority, + AssigneeType: app.AssigneeType, + AssigneeID: app.AssigneeID, + CreatorType: app.CreatorType, + CreatorID: app.CreatorID, + ParentIssueID: app.ParentIssueID, + ProjectID: app.ProjectID, + Position: app.Position, + Stage: app.Stage, + StartDate: app.StartDate, + DueDate: app.DueDate, + CreatedAt: app.CreatedAt, + UpdatedAt: app.UpdatedAt, + Revision: app.Revision, + LastActivityAt: app.LastActivityAt, + Metadata: app.Metadata, + Properties: app.Properties, + } } -// ListPluginComments — GET /v1/issues/{id}/comments +// ListPluginComments — GET /v1/issues/{issue_ref}/comments func (h *Handler) ListPluginComments(w http.ResponseWriter, r *http.Request) { caller, _, ok := h.pluginCaller(w, r, plugincontract.ScopeCommentsRead) if !ok { return } - issue, ok := h.pluginIssueForUser(w, r, caller, chi.URLParam(r, "id")) + issue, ok := h.pluginIssueForUser(w, r, caller, chi.URLParam(r, "issue_ref")) if !ok { return } @@ -368,40 +498,29 @@ func (h *Handler) ListPluginComments(w http.ResponseWriter, r *http.Request) { Limit: maxPluginCommentsPerRead, }) if err != nil { - writeError(w, http.StatusInternalServerError, "failed to list comments") + publicapiv1.WriteProblem(w, r, http.StatusInternalServerError, "internal_error", "failed to list comments") return } - payload := make([]pluginCommentResponse, 0, len(comments)) + payload := make([]publicapiv1.Comment, 0, len(comments)) for _, comment := range comments { - payload = append(payload, pluginCommentResponse{ - ID: uuidToString(comment.ID), - AuthorType: comment.AuthorType, - AuthorID: uuidToString(comment.AuthorID), - Content: comment.Content, - Type: comment.Type, - ParentID: uuidToString(comment.ParentID), - CreatedAt: comment.CreatedAt.Time.UTC().Format(timeFormatRFC3339), - }) + payload = append(payload, publicPluginComment(comment)) } - writeJSON(w, http.StatusOK, map[string]any{"comments": payload}) + writeJSON(w, http.StatusOK, publicapiv1.CommentListResponse{Comments: payload}) } -type pluginCommentResponse struct { - ID string `json:"id"` - AuthorType string `json:"author_type"` - AuthorID string `json:"author_id"` - Content string `json:"content"` - Type string `json:"type"` - ParentID string `json:"parent_id,omitempty"` - CreatedAt string `json:"created_at"` +func publicPluginComment(comment db.Comment) publicapiv1.Comment { + return publicapiv1.Comment{ + ID: uuidToString(comment.ID), + AuthorType: comment.AuthorType, + AuthorID: uuidToString(comment.AuthorID), + Content: comment.Content, + Type: comment.Type, + ParentID: uuidToString(comment.ParentID), + CreatedAt: comment.CreatedAt.Time.UTC().Format(timeFormatRFC3339), + } } -type createPluginCommentRequest struct { - Content string `json:"content"` - ParentID *string `json:"parent_id,omitempty"` -} - -// CreatePluginComment — POST /v1/issues/{id}/comments +// CreatePluginComment — POST /v1/issues/{issue_ref}/comments // // The comment is authored BY THE USER and marked with the plugin that produced // it, so the timeline can render "Jiayuan (via Hello Panel)". @@ -416,36 +535,37 @@ func (h *Handler) CreatePluginComment(w http.ResponseWriter, r *http.Request) { if !ok { return } - issue, ok := h.pluginIssueForUser(w, r, caller, chi.URLParam(r, "id")) + issue, ok := h.pluginIssueForUser(w, r, caller, chi.URLParam(r, "issue_ref")) if !ok { return } - var req createPluginCommentRequest + var req publicapiv1.CreateCommentRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - writeError(w, http.StatusBadRequest, "invalid request body") + publicapiv1.WriteProblem(w, r, http.StatusBadRequest, "invalid_request", "invalid request body") return } content := sanitizeNullBytes(req.Content) if content == "" { - writeError(w, http.StatusBadRequest, "content is required") + publicapiv1.WriteProblem(w, r, http.StatusBadRequest, "invalid_request", "content is required") return } if len(content) > maxPluginCommentBytes { - writeError(w, http.StatusBadRequest, "content is too long") + publicapiv1.WriteProblem(w, r, http.StatusBadRequest, "invalid_request", "content is too long") return } var parentID pgtype.UUID var rootComment *db.Comment if req.ParentID != nil && *req.ParentID != "" { - parsed, ok := parseUUIDOrBadRequest(w, *req.ParentID, "parent_id") - if !ok { + parsed, err := util.ParseUUID(*req.ParentID) + if err != nil { + publicapiv1.WriteProblem(w, r, http.StatusBadRequest, "invalid_request", "parent_id must be a valid UUID") return } parent, err := h.Queries.GetComment(r.Context(), parsed) if err != nil || uuidToString(parent.IssueID) != uuidToString(issue.ID) { - writeError(w, http.StatusBadRequest, "invalid parent comment") + publicapiv1.WriteProblem(w, r, http.StatusBadRequest, "invalid_parent_comment", "invalid parent comment") return } parentID = parsed @@ -476,7 +596,7 @@ func (h *Handler) CreatePluginComment(w http.ResponseWriter, r *http.Request) { ViaPluginID: caller.Installation.ID, }) if err != nil { - writeError(w, http.StatusInternalServerError, "failed to create the comment") + publicapiv1.WriteProblem(w, r, http.StatusInternalServerError, "internal_error", "failed to create the comment") return } comment := createdComment.Comment() @@ -497,15 +617,7 @@ func (h *Handler) CreatePluginComment(w http.ResponseWriter, r *http.Request) { h.TaskService.AutoUnresolveThreadOnReply(r.Context(), rootComment, uuidToString(caller.WorkspaceID), authorType, uuidToString(authorID)) } - writeJSON(w, http.StatusCreated, pluginCommentResponse{ - ID: uuidToString(comment.ID), - AuthorType: comment.AuthorType, - AuthorID: uuidToString(comment.AuthorID), - Content: comment.Content, - Type: comment.Type, - ParentID: uuidToString(comment.ParentID), - CreatedAt: comment.CreatedAt.Time.UTC().Format(timeFormatRFC3339), - }) + writeJSON(w, http.StatusCreated, publicPluginComment(comment)) } // maxPluginCommentBytes keeps a surface from using comments as bulk storage. @@ -525,18 +637,18 @@ func (h *Handler) pluginStorageScope(w http.ResponseWriter, r *http.Request, cal required = plugincontract.ScopeStorageUser } if !hasGrantedScope(caller.Scopes, required) { - writeError(w, http.StatusForbidden, "this Plugin was not granted the "+required+" scope") + publicapiv1.WriteProblem(w, r, http.StatusForbidden, "missing_scope", "this Plugin was not granted the "+required+" scope") return "", pgtype.UUID{}, false } // storage:user is per-member state, so a caller with no member has no such // scope to resolve. Falling through would key every plugin-actor write to // the zero UUID — one shared bucket masquerading as somebody's private one. - if scopeType == service.PluginStorageUser && !actor.requireMember(w) { + if scopeType == service.PluginStorageUser && !actor.requireMember(w, r) { return "", pgtype.UUID{}, false } scopeID, err := service.ResolveStorageScope(scopeType, caller.WorkspaceID, actor.Member.UserID) if err != nil { - writePluginError(w, err, "invalid storage scope") + writePluginActionError(w, r, err, "invalid storage scope") return "", pgtype.UUID{}, false } return scopeType, scopeID, true @@ -563,10 +675,16 @@ func (h *Handler) ListPluginStorage(w http.ResponseWriter, r *http.Request) { } keys, err := h.PluginService.ListStorageKeys(r.Context(), caller.Installation.ID, scopeType, scopeID) if err != nil { - writePluginError(w, err, "failed to list storage") + writePluginActionError(w, r, err, "failed to list storage") return } - writeJSON(w, http.StatusOK, map[string]any{"keys": keys}) + payload := make([]publicapiv1.StorageKey, 0, len(keys)) + for _, key := range keys { + payload = append(payload, publicapiv1.StorageKey{ + Key: key.Key, SizeBytes: key.SizeBytes, UpdatedAt: key.UpdatedAt, + }) + } + writeJSON(w, http.StatusOK, publicapiv1.StorageKeyListResponse{Keys: payload}) } // GetPluginStorage — GET /v1/storage/{scope}/{key} @@ -581,14 +699,10 @@ func (h *Handler) GetPluginStorage(w http.ResponseWriter, r *http.Request) { } value, err := h.PluginService.GetStorageValue(r.Context(), caller.Installation.ID, scopeType, scopeID, chi.URLParam(r, "key")) if err != nil { - writePluginError(w, err, "failed to read storage") + writePluginActionError(w, r, err, "failed to read storage") return } - writeJSON(w, http.StatusOK, map[string]any{"value": value}) -} - -type putPluginStorageRequest struct { - Value string `json:"value"` + writeJSON(w, http.StatusOK, publicapiv1.StorageValueResponse{Value: value}) } // PutPluginStorage — PUT /v1/storage/{scope}/{key} @@ -601,13 +715,13 @@ func (h *Handler) PutPluginStorage(w http.ResponseWriter, r *http.Request) { if !ok { return } - var req putPluginStorageRequest + var req publicapiv1.PutStorageValueRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - writeError(w, http.StatusBadRequest, "invalid request body") + publicapiv1.WriteProblem(w, r, http.StatusBadRequest, "invalid_request", "invalid request body") return } if err := h.PluginService.SetStorageValue(r.Context(), caller.Installation.ID, scopeType, scopeID, chi.URLParam(r, "key"), req.Value); err != nil { - writePluginError(w, err, "failed to write storage") + writePluginActionError(w, r, err, "failed to write storage") return } w.WriteHeader(http.StatusNoContent) @@ -624,7 +738,7 @@ func (h *Handler) DeletePluginStorage(w http.ResponseWriter, r *http.Request) { return } if err := h.PluginService.DeleteStorageValue(r.Context(), caller.Installation.ID, scopeType, scopeID, chi.URLParam(r, "key")); err != nil { - writePluginError(w, err, "failed to delete storage") + writePluginActionError(w, r, err, "failed to delete storage") return } w.WriteHeader(http.StatusNoContent) diff --git a/server/internal/handler/plugin_action_test.go b/server/internal/handler/plugin_action_test.go index a87d15a5b..957baade0 100644 --- a/server/internal/handler/plugin_action_test.go +++ b/server/internal/handler/plugin_action_test.go @@ -4,12 +4,15 @@ import ( "bytes" "context" "encoding/json" + "fmt" "net/http" "net/http/httptest" "strings" + "sync" "testing" "github.com/go-chi/chi/v5" + publicapiv1 "github.com/multica-ai/multica/server/pkg/publicapi/v1" ) // These are the Action API's security properties, not its happy path. Each one @@ -69,6 +72,83 @@ func pluginActionRequest(method, path, installationID string, body any, params m return request.WithContext(context.WithValue(request.Context(), chi.RouteCtxKey, routeContext)) } +func pluginInstallTokenRequest(method, path, token string, body any, params map[string]string) *http.Request { + request := pluginActionRequest(method, path, "", body, params) + request.Header.Del("X-User-ID") + request.Header.Set("Authorization", "Bearer "+token) + return request +} + +func TestPluginInstallTokenRunsIssueCommentWorkflow(t *testing.T) { + installationID := installPluginForAction(t, []string{"issues:read", "issues:write", "comments:read", "comments:write"}) + token, err := testHandler.PluginService.IssueInstallToken(context.Background(), parseUUID(installationID)) + if err != nil { + t.Fatalf("issue install token: %v", err) + } + issueID := createTestIssue(t, "Install token workflow", "todo", "none") + + get := httptest.NewRecorder() + testHandler.GetPluginIssue(get, pluginInstallTokenRequest(http.MethodGet, "/v1/issues/"+issueID, token, nil, + map[string]string{"issue_ref": issueID})) + if get.Code != http.StatusOK { + t.Fatalf("install-token issue read status=%d body=%s", get.Code, get.Body.String()) + } + + patchRequest := pluginInstallTokenRequest(http.MethodPatch, "/v1/issues/"+issueID, token, + map[string]any{"title": "Updated by install token"}, map[string]string{"issue_ref": issueID}) + patchRequest.Header.Set("If-Match", get.Header().Get("ETag")) + patch := httptest.NewRecorder() + testHandler.PatchPluginIssue(patch, patchRequest) + if patch.Code != http.StatusOK { + t.Fatalf("install-token issue patch status=%d body=%s", patch.Code, patch.Body.String()) + } + + create := httptest.NewRecorder() + testHandler.CreatePluginComment(create, pluginInstallTokenRequest(http.MethodPost, "/v1/issues/"+issueID+"/comments", token, + map[string]any{"content": "created with a real mpi token"}, map[string]string{"issue_ref": issueID})) + if create.Code != http.StatusCreated { + t.Fatalf("install-token comment create status=%d body=%s", create.Code, create.Body.String()) + } + var comment publicapiv1.Comment + if err := json.Unmarshal(create.Body.Bytes(), &comment); err != nil { + t.Fatalf("decode install-token comment: %v", err) + } + if comment.AuthorType != "plugin" || comment.AuthorID != installationID { + t.Fatalf("install-token comment attribution = %+v", comment) + } + + list := httptest.NewRecorder() + testHandler.ListPluginComments(list, pluginInstallTokenRequest(http.MethodGet, "/v1/issues/"+issueID+"/comments", token, nil, + map[string]string{"issue_ref": issueID})) + if list.Code != http.StatusOK || !strings.Contains(list.Body.String(), comment.ID) { + t.Fatalf("install-token comment list status=%d body=%s", list.Code, list.Body.String()) + } +} + +func TestPluginInstallTokenEnforcesGrantedScope(t *testing.T) { + installationID := installPluginForAction(t, []string{"issues:read"}) + token, err := testHandler.PluginService.IssueInstallToken(context.Background(), parseUUID(installationID)) + if err != nil { + t.Fatalf("issue install token: %v", err) + } + issueID := createTestIssue(t, "Install token scope", "todo", "none") + + request := pluginInstallTokenRequest(http.MethodPatch, "/v1/issues/"+issueID, token, + map[string]any{"title": "must not update"}, map[string]string{"issue_ref": issueID}) + response := httptest.NewRecorder() + testHandler.PatchPluginIssue(response, request) + if response.Code != http.StatusForbidden { + t.Fatalf("ungranted install-token patch status=%d body=%s", response.Code, response.Body.String()) + } + var problem publicapiv1.Problem + if err := json.Unmarshal(response.Body.Bytes(), &problem); err != nil { + t.Fatalf("decode scope problem: %v", err) + } + if problem.Code != "forbidden" || !strings.Contains(problem.Detail, "issues:write") { + t.Fatalf("unexpected scope problem: %+v", problem) + } +} + func TestPluginActionRequiresAGrantedScope(t *testing.T) { // Installed with issues:read only. Everything else must be refused, and the // refusal must name the missing scope so an admin can act on it. @@ -77,20 +157,135 @@ func TestPluginActionRequiresAGrantedScope(t *testing.T) { recorder := httptest.NewRecorder() testHandler.GetPluginIssue(recorder, pluginActionRequest(http.MethodGet, "/issues", installationID, nil, - map[string]string{"id": issueID})) + map[string]string{"issue_ref": issueID})) if recorder.Code != http.StatusOK { t.Fatalf("granted scope was refused: status=%d body=%s", recorder.Code, recorder.Body.String()) } recorder = httptest.NewRecorder() testHandler.CreatePluginComment(recorder, pluginActionRequest(http.MethodPost, "/comments", installationID, - map[string]any{"content": "hello"}, map[string]string{"id": issueID})) + map[string]any{"content": "hello"}, map[string]string{"issue_ref": issueID})) if recorder.Code != http.StatusForbidden { t.Fatalf("ungranted scope status=%d body=%s", recorder.Code, recorder.Body.String()) } if !strings.Contains(recorder.Body.String(), "comments:write") { t.Fatalf("refusal does not name the missing scope: %s", recorder.Body.String()) } + var problem publicapiv1.Problem + if err := json.Unmarshal(recorder.Body.Bytes(), &problem); err != nil { + t.Fatalf("decode stable problem: %v", err) + } + if problem.Code != "forbidden" || problem.Error == "" || problem.RequestID == "" { + t.Fatalf("unexpected stable problem: %+v", problem) + } +} + +func TestPluginIssueUsesStableDTOAndRevisionETag(t *testing.T) { + installationID := installPluginForAction(t, []string{"issues:read", "issues:write"}) + issueID := createTestIssue(t, "Plugin public DTO test", "todo", "none") + + get := httptest.NewRecorder() + testHandler.GetPluginIssue(get, pluginActionRequest(http.MethodGet, "/v1/issues/"+issueID, installationID, nil, + map[string]string{"issue_ref": issueID})) + if get.Code != http.StatusOK { + t.Fatalf("get issue status=%d body=%s", get.Code, get.Body.String()) + } + var issue publicapiv1.Issue + if err := json.Unmarshal(get.Body.Bytes(), &issue); err != nil { + t.Fatalf("decode public issue: %v", err) + } + if issue.ID != issueID || issue.Identifier == "" || issue.Revision < 1 || issue.Metadata == nil || issue.Properties == nil { + t.Fatalf("unexpected public issue: %+v", issue) + } + wantETag := fmt.Sprintf(`W/"%d"`, issue.Revision) + if got := get.Header().Get("ETag"); got != wantETag { + t.Fatalf("ETag = %q, want %q", got, wantETag) + } + var raw map[string]any + if err := json.Unmarshal(get.Body.Bytes(), &raw); err != nil { + t.Fatalf("decode issue map: %v", err) + } + for _, appOnly := range []string{"labels", "attachments", "reactions"} { + if _, leaked := raw[appOnly]; leaked { + t.Fatalf("App-only field %q leaked into Public API: %s", appOnly, get.Body.String()) + } + } + + patchRequest := pluginActionRequest(http.MethodPatch, "/v1/issues/"+issueID, installationID, + map[string]any{"title": "Updated through Public API"}, map[string]string{"issue_ref": issueID}) + patchRequest.Header.Set("If-Match", wantETag) + patch := httptest.NewRecorder() + testHandler.PatchPluginIssue(patch, patchRequest) + if patch.Code != http.StatusOK { + t.Fatalf("patch issue status=%d body=%s", patch.Code, patch.Body.String()) + } + var updated publicapiv1.Issue + if err := json.Unmarshal(patch.Body.Bytes(), &updated); err != nil { + t.Fatalf("decode updated issue: %v", err) + } + if updated.Title != "Updated through Public API" || updated.Revision <= issue.Revision { + t.Fatalf("unexpected updated issue: %+v", updated) + } + if got := patch.Header().Get("ETag"); got != fmt.Sprintf(`W/"%d"`, updated.Revision) { + t.Fatalf("updated ETag = %q", got) + } + + staleRequest := pluginActionRequest(http.MethodPatch, "/v1/issues/"+issueID, installationID, + map[string]any{"description": "stale"}, map[string]string{"issue_ref": issueID}) + staleRequest.Header.Set("If-Match", wantETag) + stale := httptest.NewRecorder() + testHandler.PatchPluginIssue(stale, staleRequest) + if stale.Code != http.StatusConflict { + t.Fatalf("stale patch status=%d body=%s", stale.Code, stale.Body.String()) + } + var conflict publicapiv1.Problem + if err := json.Unmarshal(stale.Body.Bytes(), &conflict); err != nil { + t.Fatalf("decode revision conflict: %v", err) + } + if conflict.Code != "revision_conflict" { + t.Fatalf("conflict code = %q", conflict.Code) + } +} + +func TestPluginIssueConditionalPatchHasSingleWinner(t *testing.T) { + installationID := installPluginForAction(t, []string{"issues:read", "issues:write"}) + issueID := createTestIssue(t, "Plugin conditional write test", "todo", "none") + + get := httptest.NewRecorder() + testHandler.GetPluginIssue(get, pluginActionRequest(http.MethodGet, "/v1/issues/"+issueID, installationID, nil, + map[string]string{"issue_ref": issueID})) + if get.Code != http.StatusOK { + t.Fatalf("get issue status=%d body=%s", get.Code, get.Body.String()) + } + etag := get.Header().Get("ETag") + + start := make(chan struct{}) + results := make(chan int, 2) + var wg sync.WaitGroup + for _, title := range []string{"concurrent winner A", "concurrent winner B"} { + wg.Add(1) + go func(title string) { + defer wg.Done() + request := pluginActionRequest(http.MethodPatch, "/v1/issues/"+issueID, installationID, + map[string]any{"title": title}, map[string]string{"issue_ref": issueID}) + request.Header.Set("If-Match", etag) + <-start + response := httptest.NewRecorder() + testHandler.PatchPluginIssue(response, request) + results <- response.Code + }(title) + } + close(start) + wg.Wait() + close(results) + + counts := map[int]int{} + for status := range results { + counts[status]++ + } + if counts[http.StatusOK] != 1 || counts[http.StatusConflict] != 1 { + t.Fatalf("conditional write statuses = %v, want one 200 and one 409", counts) + } } func TestPluginActionRefusedWithoutAnInstallation(t *testing.T) { @@ -101,7 +296,7 @@ func TestPluginActionRefusedWithoutAnInstallation(t *testing.T) { // plugin" or to an unscoped call. recorder := httptest.NewRecorder() testHandler.GetPluginIssue(recorder, pluginActionRequest(http.MethodGet, "/issues", "", nil, - map[string]string{"id": issueID})) + map[string]string{"issue_ref": issueID})) if recorder.Code != http.StatusBadRequest { t.Fatalf("missing installation header status=%d body=%s", recorder.Code, recorder.Body.String()) } @@ -109,7 +304,7 @@ func TestPluginActionRefusedWithoutAnInstallation(t *testing.T) { // An installation id that does not exist is a 404, never a pass-through. recorder = httptest.NewRecorder() testHandler.GetPluginIssue(recorder, pluginActionRequest(http.MethodGet, "/issues", - "11111111-1111-1111-1111-111111111111", nil, map[string]string{"id": issueID})) + "11111111-1111-1111-1111-111111111111", nil, map[string]string{"issue_ref": issueID})) if recorder.Code != http.StatusNotFound { t.Fatalf("unknown installation status=%d body=%s", recorder.Code, recorder.Body.String()) } @@ -130,7 +325,7 @@ func TestPluginActionStopsWhenTheInstallationIsDisabled(t *testing.T) { // it off server-side or "disabled" only means "hidden". recorder = httptest.NewRecorder() testHandler.GetPluginIssue(recorder, pluginActionRequest(http.MethodGet, "/issues", installationID, nil, - map[string]string{"id": issueID})) + map[string]string{"issue_ref": issueID})) if recorder.Code != http.StatusForbidden { t.Fatalf("disabled installation status=%d body=%s", recorder.Code, recorder.Body.String()) } @@ -142,7 +337,7 @@ func TestPluginCommentIsAuthoredByTheUserAndMarkedWithThePlugin(t *testing.T) { recorder := httptest.NewRecorder() testHandler.CreatePluginComment(recorder, pluginActionRequest(http.MethodPost, "/comments", installationID, - map[string]any{"content": "posted through a plugin"}, map[string]string{"id": issueID})) + map[string]any{"content": "posted through a plugin"}, map[string]string{"issue_ref": issueID})) if recorder.Code != http.StatusCreated { t.Fatalf("create comment status=%d body=%s", recorder.Code, recorder.Body.String()) } @@ -178,7 +373,7 @@ func TestPluginActionCannotReachAnotherWorkspacesIssue(t *testing.T) { otherWorkspaceIssue := createIssueInForeignWorkspace(t) recorder := httptest.NewRecorder() testHandler.GetPluginIssue(recorder, pluginActionRequest(http.MethodGet, "/issues", installationID, nil, - map[string]string{"id": otherWorkspaceIssue})) + map[string]string{"issue_ref": otherWorkspaceIssue})) if recorder.Code != http.StatusNotFound { t.Fatalf("cross-workspace read status=%d body=%s", recorder.Code, recorder.Body.String()) } diff --git a/server/internal/handler/plugin_callback_test.go b/server/internal/handler/plugin_callback_test.go index 4e82a2200..a85905593 100644 --- a/server/internal/handler/plugin_callback_test.go +++ b/server/internal/handler/plugin_callback_test.go @@ -79,7 +79,7 @@ func TestCallbackFromAUserTriggeredHookWritesAsTheUser(t *testing.T) { testHandler.CreatePluginComment(recorder, callbackRequest(token, http.MethodPost, "/v1/issues/"+issueID+"/comments", map[string]any{"content": "posted by a manual hook"}, - map[string]string{"id": issueID})) + map[string]string{"issue_ref": issueID})) if recorder.Code != http.StatusCreated { t.Fatalf("status=%d body=%s", recorder.Code, recorder.Body.String()) } @@ -110,7 +110,7 @@ func TestCallbackFromAnEventHookWritesAsThePlugin(t *testing.T) { testHandler.CreatePluginComment(recorder, callbackRequest(token, http.MethodPost, "/v1/issues/"+issueID+"/comments", map[string]any{"content": "posted by an event hook"}, - map[string]string{"id": issueID})) + map[string]string{"issue_ref": issueID})) if recorder.Code != http.StatusCreated { t.Fatalf("status=%d body=%s", recorder.Code, recorder.Body.String()) } @@ -137,7 +137,7 @@ func TestCallbackTokenServesTheWholeInvocation(t *testing.T) { token := issueCallbackToken(t, installationID, service.HookActor{Type: "member", ID: parseUUID(testUserID)}) first := httptest.NewRecorder() testHandler.GetPluginIssue(first, callbackRequest(token, http.MethodGet, - "/v1/issues/"+issueID, nil, map[string]string{"id": issueID})) + "/v1/issues/"+issueID, nil, map[string]string{"issue_ref": issueID})) if first.Code != http.StatusOK { t.Fatalf("first use: status=%d body=%s", first.Code, first.Body.String()) } @@ -147,7 +147,7 @@ func TestCallbackTokenServesTheWholeInvocation(t *testing.T) { testHandler.CreatePluginComment(second, callbackRequest(token, http.MethodPost, "/v1/issues/"+issueID+"/comments", map[string]any{"content": "and now a comment about what I read"}, - map[string]string{"id": issueID})) + map[string]string{"issue_ref": issueID})) if second.Code != http.StatusCreated { t.Fatalf("a handler must be able to write after reading: status=%d body=%s", second.Code, second.Body.String()) } @@ -156,7 +156,7 @@ func TestCallbackTokenServesTheWholeInvocation(t *testing.T) { testHandler.PluginService.Callbacks.Revoke(token) third := httptest.NewRecorder() testHandler.GetPluginIssue(third, callbackRequest(token, http.MethodGet, - "/v1/issues/"+issueID, nil, map[string]string{"id": issueID})) + "/v1/issues/"+issueID, nil, map[string]string{"issue_ref": issueID})) if third.Code != http.StatusForbidden { t.Fatalf("a revoked grant must be refused: status=%d body=%s", third.Code, third.Body.String()) } @@ -259,7 +259,7 @@ func TestCallbackTokenCannotReachAnotherIssue(t *testing.T) { // The issue it was issued for: allowed. ok := httptest.NewRecorder() testHandler.GetPluginIssue(ok, callbackRequest(token, http.MethodGet, - "/v1/issues/"+granted, nil, map[string]string{"id": granted})) + "/v1/issues/"+granted, nil, map[string]string{"issue_ref": granted})) if ok.Code != http.StatusOK { t.Fatalf("the granted issue must be reachable: status=%d body=%s", ok.Code, ok.Body.String()) } @@ -267,7 +267,7 @@ func TestCallbackTokenCannotReachAnotherIssue(t *testing.T) { // Any other issue in the same workspace: refused, on both read and write. refusedRead := httptest.NewRecorder() testHandler.GetPluginIssue(refusedRead, callbackRequest(token, http.MethodGet, - "/v1/issues/"+other, nil, map[string]string{"id": other})) + "/v1/issues/"+other, nil, map[string]string{"issue_ref": other})) if refusedRead.Code != http.StatusNotFound { t.Fatalf("reading another issue must be refused: status=%d body=%s", refusedRead.Code, refusedRead.Body.String()) } @@ -276,7 +276,7 @@ func TestCallbackTokenCannotReachAnotherIssue(t *testing.T) { testHandler.CreatePluginComment(refusedWrite, callbackRequest(token, http.MethodPost, "/v1/issues/"+other+"/comments", map[string]any{"content": "should never land"}, - map[string]string{"id": other})) + map[string]string{"issue_ref": other})) if refusedWrite.Code != http.StatusNotFound { t.Fatalf("commenting on another issue must be refused: status=%d body=%s", refusedWrite.Code, refusedWrite.Body.String()) } @@ -295,7 +295,7 @@ func TestCallbackTokenWithoutAnIssueIsNotNarrowed(t *testing.T) { token := issueCallbackToken(t, installationID, service.HookActor{Type: "member", ID: parseUUID(testUserID)}) recorder := httptest.NewRecorder() testHandler.GetPluginIssue(recorder, callbackRequest(token, http.MethodGet, - "/v1/issues/"+issueID, nil, map[string]string{"id": issueID})) + "/v1/issues/"+issueID, nil, map[string]string{"issue_ref": issueID})) if recorder.Code != http.StatusOK { t.Fatalf("an unscoped grant must still reach the workspace: status=%d body=%s", recorder.Code, recorder.Body.String()) } diff --git a/server/internal/handler/plugin_hook.go b/server/internal/handler/plugin_hook.go index 9869acbdb..194dea2ee 100644 --- a/server/internal/handler/plugin_hook.go +++ b/server/internal/handler/plugin_hook.go @@ -40,7 +40,7 @@ func (h *Handler) InvokePluginHook(w http.ResponseWriter, r *http.Request) { // A hook invoked from the UI is invoked BY somebody. A plugin's own server // calling this would be asking us to call it back, which is a loop with no // person in it and no reason to exist. - if !actor.requireMember(w) { + if !actor.requireMember(w, r) { return } @@ -174,7 +174,7 @@ type pluginTokenResponse struct { // SigningSecret is what the author configures on their own server to verify // our signature. Derived from the deployment key rather than stored, so it // is stable for an installation and reproducible without a database copy. - SigningSecret string `json:"signing_secret"` + SigningSecret string `json:"signing_secret,omitempty"` } // RotatePluginToken — POST /api/workspaces/{id}/plugins/{installationId}/token @@ -183,17 +183,12 @@ func (h *Handler) RotatePluginToken(w http.ResponseWriter, r *http.Request) { if !ok { return } - token, err := h.PluginService.IssueInstallToken(r.Context(), installation.ID) + credentials, err := h.PluginService.RotateInstallCredentials(r.Context(), installation.ID) if err != nil { writePluginError(w, err, "failed to issue the Plugin token") return } - secret, err := h.PluginService.HookSigningSecret(installation.ID) - if err != nil { - writePluginError(w, err, "failed to derive the signing secret") - return - } - writeJSON(w, http.StatusOK, pluginTokenResponse{Token: token, SigningSecret: secret}) + writeJSON(w, http.StatusOK, pluginTokenResponse{Token: credentials.Token, SigningSecret: credentials.SigningSecret}) } // RevokePluginToken — DELETE /api/workspaces/{id}/plugins/{installationId}/token diff --git a/server/internal/handler/plugin_hook_test.go b/server/internal/handler/plugin_hook_test.go index 2a78100c7..c6c8ec28c 100644 --- a/server/internal/handler/plugin_hook_test.go +++ b/server/internal/handler/plugin_hook_test.go @@ -199,6 +199,53 @@ func TestRotatePluginTokenIssuesOnceAndInvalidatesThePrevious(t *testing.T) { } } +func TestRotatePluginTokenDoesNotRequireHookSigning(t *testing.T) { + withPluginsV1Flag(t, testHandler, true) + cleanupPluginInstallations(t) + installationID := installHookPlugin(t) + previousKey := testHandler.PluginService.DeploymentKey + testHandler.PluginService.DeploymentKey = nil + t.Cleanup(func() { testHandler.PluginService.DeploymentKey = previousKey }) + + issued := rotateToken(t, installationID) + if issued.Token == "" { + t.Fatal("rotation did not return a Public API token") + } + if issued.SigningSecret != "" { + t.Fatalf("signing_secret = %q without a deployment key", issued.SigningSecret) + } + installation, err := testHandler.PluginService.AuthenticateInstallToken(context.Background(), issued.Token) + if err != nil { + t.Fatalf("token issued without hook signing must authenticate: %v", err) + } + if uuidToString(installation.ID) != installationID { + t.Fatalf("token resolved to %s, want %s", uuidToString(installation.ID), installationID) + } +} + +func TestRotatePluginTokenPreservesPreviousTokenWhenPreparationFails(t *testing.T) { + withPluginsV1Flag(t, testHandler, true) + cleanupPluginInstallations(t) + installationID := installHookPlugin(t) + previousKey := testHandler.PluginService.DeploymentKey + t.Cleanup(func() { testHandler.PluginService.DeploymentKey = previousKey }) + + testHandler.PluginService.DeploymentKey = bytes.Repeat([]byte{9}, 32) + previous := rotateToken(t, installationID) + testHandler.PluginService.DeploymentKey = []byte("invalid") + + response := httptest.NewRecorder() + testHandler.RotatePluginToken(response, pluginHandlerRequest(http.MethodPost, "/token", nil, map[string]string{ + "id": testWorkspaceID, "installationId": installationID, + })) + if response.Code != http.StatusBadGateway { + t.Fatalf("failed preparation status=%d body=%s", response.Code, response.Body.String()) + } + if _, err := testHandler.PluginService.AuthenticateInstallToken(context.Background(), previous.Token); err != nil { + t.Fatalf("failed credential preparation invalidated the previous token: %v", err) + } +} + func TestRevokePluginTokenStopsAuthentication(t *testing.T) { withPluginsV1Flag(t, testHandler, true) cleanupPluginInstallations(t) diff --git a/server/internal/middleware/plugin_auth.go b/server/internal/middleware/plugin_auth.go index 9062d2630..dfa11c724 100644 --- a/server/internal/middleware/plugin_auth.go +++ b/server/internal/middleware/plugin_auth.go @@ -3,6 +3,8 @@ package middleware import ( "net/http" "strings" + + publicapiv1 "github.com/multica-ai/multica/server/pkg/publicapi/v1" ) // PluginBearerOnly keeps the public Action API on a machine-credential trust @@ -14,7 +16,7 @@ import ( func PluginBearerOnly(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if !IsPluginBearerToken(BearerToken(r)) { - writeError(w, http.StatusUnauthorized, "plugin bearer token required") + publicapiv1.WriteProblem(w, r, http.StatusUnauthorized, "plugin_bearer_required", "plugin bearer token required") return } next.ServeHTTP(w, r) diff --git a/server/internal/middleware/plugin_auth_test.go b/server/internal/middleware/plugin_auth_test.go index 306389eeb..e11ad51a7 100644 --- a/server/internal/middleware/plugin_auth_test.go +++ b/server/internal/middleware/plugin_auth_test.go @@ -1,9 +1,12 @@ package middleware import ( + "encoding/json" "net/http" "net/http/httptest" "testing" + + publicapiv1 "github.com/multica-ai/multica/server/pkg/publicapi/v1" ) func TestPluginBearerOnlyRejectsNonPluginCredentials(t *testing.T) { @@ -37,6 +40,16 @@ func TestPluginBearerOnlyRejectsNonPluginCredentials(t *testing.T) { if called { t.Fatal("non-plugin credential reached the Action API handler") } + if got := response.Header().Get("Content-Type"); got != publicapiv1.ProblemContentType { + t.Fatalf("Content-Type = %q", got) + } + var problem publicapiv1.Problem + if err := json.Unmarshal(response.Body.Bytes(), &problem); err != nil { + t.Fatalf("decode problem: %v", err) + } + if problem.Code != "plugin_bearer_required" || problem.Error == "" || problem.RequestID == "" { + t.Fatalf("unexpected problem: %+v", problem) + } }) } } diff --git a/server/internal/middleware/plugin_ratelimit.go b/server/internal/middleware/plugin_ratelimit.go new file mode 100644 index 000000000..c3a1b855e --- /dev/null +++ b/server/internal/middleware/plugin_ratelimit.go @@ -0,0 +1,40 @@ +package middleware + +import ( + "crypto/sha256" + "fmt" + "log/slog" + "net/http" + "time" + + publicapiv1 "github.com/multica-ai/multica/server/pkg/publicapi/v1" + "github.com/redis/go-redis/v9" +) + +// PluginRateLimit applies one fixed-window budget to each opaque Plugin +// credential. The token is hashed before it is used as a Redis key; secrets +// must never enter logs or operational data. A missing Redis client fails open, +// matching the existing public-edge limiter behavior. +func PluginRateLimit(rdb *redis.Client, limit int, window time.Duration) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + if rdb == nil { + return next + } + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + digest := sha256.Sum256([]byte(BearerToken(r))) + key := fmt.Sprintf("mul:ratelimit:plugin:%x", digest) + count, err := rateLimitScript.Run(r.Context(), rdb, []string{key}, int(window.Seconds())).Int64() + if err != nil { + slog.Warn("plugin ratelimit: redis error; allowing request", "error", err) + next.ServeHTTP(w, r) + return + } + if count > int64(limit) { + w.Header().Set("Retry-After", fmt.Sprintf("%d", int(window.Seconds()))) + publicapiv1.WriteProblem(w, r, http.StatusTooManyRequests, "rate_limited", "Plugin API rate limit exceeded") + return + } + next.ServeHTTP(w, r) + }) + } +} diff --git a/server/internal/middleware/plugin_ratelimit_test.go b/server/internal/middleware/plugin_ratelimit_test.go new file mode 100644 index 000000000..6f4a9bd28 --- /dev/null +++ b/server/internal/middleware/plugin_ratelimit_test.go @@ -0,0 +1,65 @@ +package middleware + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + publicapiv1 "github.com/multica-ai/multica/server/pkg/publicapi/v1" +) + +func TestPluginRateLimitIsPerCredentialAndUsesStableProblem(t *testing.T) { + rdb := newRedisTestClient(t) + handler := PluginRateLimit(rdb, 1, time.Minute)(okHandler) + + call := func(token string) *httptest.ResponseRecorder { + request := httptest.NewRequest(http.MethodGet, "/v1/issues/MUL-1", nil) + request.Header.Set("Authorization", "Bearer "+token) + response := httptest.NewRecorder() + handler.ServeHTTP(response, request) + return response + } + + if response := call("mpi_first-secret"); response.Code != http.StatusOK { + t.Fatalf("first credential call status = %d", response.Code) + } + limited := call("mpi_first-secret") + if limited.Code != http.StatusTooManyRequests || limited.Header().Get("Retry-After") != "60" { + t.Fatalf("limited response status=%d retry-after=%q body=%s", limited.Code, limited.Header().Get("Retry-After"), limited.Body.String()) + } + var problem publicapiv1.Problem + if err := json.Unmarshal(limited.Body.Bytes(), &problem); err != nil { + t.Fatalf("decode problem: %v", err) + } + if problem.Code != "rate_limited" || problem.RequestID == "" || problem.Error == "" { + t.Fatalf("unexpected problem: %+v", problem) + } + if response := call("mpi_second-secret"); response.Code != http.StatusOK { + t.Fatalf("different credential shared budget: status=%d", response.Code) + } + + keys, err := rdb.Keys(context.Background(), "mul:ratelimit:plugin:*").Result() + if err != nil { + t.Fatalf("list limiter keys: %v", err) + } + for _, key := range keys { + if strings.Contains(key, "first-secret") || strings.Contains(key, "second-secret") { + t.Fatalf("credential leaked into rate-limit key: %q", key) + } + } +} + +func TestPluginRateLimitWithoutRedisFailsOpen(t *testing.T) { + handler := PluginRateLimit(nil, 1, time.Minute)(okHandler) + request := httptest.NewRequest(http.MethodGet, "/v1/context", nil) + request.Header.Set("Authorization", "Bearer mpi_local") + response := httptest.NewRecorder() + handler.ServeHTTP(response, request) + if response.Code != http.StatusOK { + t.Fatalf("nil Redis status = %d", response.Code) + } +} diff --git a/server/internal/service/issue_public.go b/server/internal/service/issue_public.go new file mode 100644 index 000000000..5e7c42e5c --- /dev/null +++ b/server/internal/service/issue_public.go @@ -0,0 +1,55 @@ +package service + +import ( + "context" + "errors" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + db "github.com/multica-ai/multica/server/pkg/db/generated" +) + +// ErrIssueRevisionConflict means the caller based an update on a stale issue +// revision. Transports map it to their stable conflict response. +var ErrIssueRevisionConflict = errors.New("issue revision conflict") + +// IssueContentPatch is the first shared Public API write primitive. It is +// deliberately transport- and credential-agnostic: App, PAT, and Plugin +// entrypoints authorize independently, then call the same business operation. +type IssueContentPatch struct { + Title *string + Description *string + ExpectedRevision *int64 +} + +// UpdateContent updates only the low-risk issue content fields exposed in the +// first Public API slice. Assignment, status, project, and hierarchy changes +// remain separate operations because each has additional policy and side +// effects. +func (s *IssueService) UpdateContent(ctx context.Context, issue db.Issue, patch IssueContentPatch) (db.Issue, error) { + params := db.UpdateIssueParams{ + ID: issue.ID, + AssigneeType: issue.AssigneeType, + AssigneeID: issue.AssigneeID, + StartDate: issue.StartDate, + DueDate: issue.DueDate, + ParentIssueID: issue.ParentIssueID, + ProjectID: issue.ProjectID, + Stage: issue.Stage, + } + if patch.ExpectedRevision != nil { + params.ExpectedRevision = pgtype.Int8{Int64: *patch.ExpectedRevision, Valid: true} + } + if patch.Title != nil { + params.Title = pgtype.Text{String: *patch.Title, Valid: true} + } + if patch.Description != nil { + params.Description = pgtype.Text{String: *patch.Description, Valid: true} + } + + updated, err := s.Queries.UpdateIssue(ctx, params) + if patch.ExpectedRevision != nil && errors.Is(err, pgx.ErrNoRows) { + return db.Issue{}, ErrIssueRevisionConflict + } + return updated, err +} diff --git a/server/internal/service/plugin_hook.go b/server/internal/service/plugin_hook.go index 166ccfda2..334eb0abf 100644 --- a/server/internal/service/plugin_hook.go +++ b/server/internal/service/plugin_hook.go @@ -454,6 +454,9 @@ func (s *PluginService) hookSigningKey(installationID pgtype.UUID) ([]byte, erro if len(s.DeploymentKey) == 0 { return nil, pluginErrf(PluginErrorUnavailable, "hooks are disabled: MULTICA_PLUGIN_SECRET_KEY is not configured") } + if len(s.DeploymentKey) != 32 { + return nil, pluginErrf(PluginErrorUnavailable, "hooks are disabled: MULTICA_PLUGIN_SECRET_KEY must decode to 32 bytes") + } mac := hmac.New(sha256.New, s.DeploymentKey) mac.Write([]byte("multica-plugin-hook-signature:v1:")) mac.Write([]byte(uuidString(installationID))) diff --git a/server/internal/service/plugin_token.go b/server/internal/service/plugin_token.go index b2b1c77ac..2d4451c65 100644 --- a/server/internal/service/plugin_token.go +++ b/server/internal/service/plugin_token.go @@ -59,6 +59,34 @@ func (s *PluginService) IssueInstallToken(ctx context.Context, installationID pg return token, nil } +// InstallCredentials is returned exactly once when an admin rotates a Plugin's +// standing credential. SigningSecret is empty when hook signing is disabled; +// the install token remains usable for the Public API in that deployment. +type InstallCredentials struct { + Token string + SigningSecret string +} + +// RotateInstallCredentials prepares every response value before replacing the +// stored token hash. This keeps an optional hook-signing configuration failure +// from invalidating the previous token after the response has already become +// impossible to complete. +func (s *PluginService) RotateInstallCredentials(ctx context.Context, installationID pgtype.UUID) (InstallCredentials, error) { + var signingSecret string + if len(s.DeploymentKey) > 0 { + secret, err := s.HookSigningSecret(installationID) + if err != nil { + return InstallCredentials{}, err + } + signingSecret = secret + } + token, err := s.IssueInstallToken(ctx, installationID) + if err != nil { + return InstallCredentials{}, err + } + return InstallCredentials{Token: token, SigningSecret: signingSecret}, nil +} + // RevokeInstallToken drops the stored hash, so nothing presented afterwards // matches. Rotation is IssueInstallToken, which overwrites in place. func (s *PluginService) RevokeInstallToken(ctx context.Context, installationID pgtype.UUID) error { diff --git a/server/pkg/db/generated/issue.sql.go b/server/pkg/db/generated/issue.sql.go index 93c6bbbe3..26fcea222 100644 --- a/server/pkg/db/generated/issue.sql.go +++ b/server/pkg/db/generated/issue.sql.go @@ -1548,17 +1548,17 @@ const updateIssue = `-- name: UpdateIssue :one WITH candidate AS ( SELECT i.id, i.workspace_id, i.title, i.description, i.status, i.priority, i.assignee_type, i.assignee_id, i.creator_type, i.creator_id, i.parent_issue_id, i.acceptance_criteria, i.context_refs, i.position, i.due_date, i.created_at, i.updated_at, i.number, i.project_id, i.origin_type, i.origin_id, i.first_executed_at, i.start_date, i.metadata, i.stage, i.properties, i.revision, i.last_activity_at, - COALESCE($2::text, i.title) AS next_title, - COALESCE($3::text, i.description) AS next_description, - COALESCE($4::text, i.status) AS next_status, - COALESCE($5::text, i.priority) AS next_priority, - $6::text AS next_assignee_type, - $7::uuid AS next_assignee_id, + COALESCE($3::text, i.title) AS next_title, + COALESCE($4::text, i.description) AS next_description, + COALESCE($5::text, i.status) AS next_status, + COALESCE($6::text, i.priority) AS next_priority, + $7::text AS next_assignee_type, + $8::uuid AS next_assignee_id, CASE -- An explicit position wins. Cross-column drag-and-drop sends -- status and position together and means the slot it dropped on. - WHEN $8::double precision IS NOT NULL - THEN $8::double precision + WHEN $9::double precision IS NOT NULL + THEN $9::double precision -- position ranks an issue *within* its (workspace, status) -- column, so it stops meaning anything the moment the column -- changes: the value that put the issue on top of Todo lands it @@ -1573,23 +1573,23 @@ WITH candidate AS ( -- unstable across pages. Creation avoids the tie by computing its -- min under the workspace counter lock; a status change holds no -- such lock and is not worth taking one for. - WHEN i.status IS DISTINCT FROM COALESCE($4::text, i.status) + WHEN i.status IS DISTINCT FROM COALESCE($5::text, i.status) THEN ( SELECT COALESCE(MIN(target.position), 0) - 1 FROM issue AS target WHERE target.workspace_id = i.workspace_id - AND target.status = $4::text + AND target.status = $5::text ) ELSE i.position END AS next_position, - $9::date AS next_start_date, - $10::date AS next_due_date, - $11::uuid AS next_parent_issue_id, - $12::uuid AS next_project_id, - $13::integer AS next_stage + $10::date AS next_start_date, + $11::date AS next_due_date, + $12::uuid AS next_parent_issue_id, + $13::uuid AS next_project_id, + $14::integer AS next_stage FROM issue AS i WHERE i.id = $1 - AND ($14::bigint IS NULL OR i.revision = $14::bigint) + AND ($2::bigint IS NULL OR i.revision = $2::bigint) ), changed AS ( SELECT candidate.id, candidate.workspace_id, candidate.title, candidate.description, candidate.status, candidate.priority, candidate.assignee_type, candidate.assignee_id, candidate.creator_type, candidate.creator_id, candidate.parent_issue_id, candidate.acceptance_criteria, candidate.context_refs, candidate.position, candidate.due_date, candidate.created_at, candidate.updated_at, candidate.number, candidate.project_id, candidate.origin_type, candidate.origin_id, candidate.first_executed_at, candidate.start_date, candidate.metadata, candidate.stage, candidate.properties, candidate.revision, candidate.last_activity_at, candidate.next_title, candidate.next_description, candidate.next_status, candidate.next_priority, candidate.next_assignee_type, candidate.next_assignee_id, candidate.next_position, candidate.next_start_date, candidate.next_due_date, candidate.next_parent_issue_id, candidate.next_project_id, candidate.next_stage, @@ -1632,11 +1632,17 @@ UPDATE issue AS i SET updated_at = CASE WHEN changed.did_change THEN now() ELSE i.updated_at END FROM changed WHERE i.id = changed.id + -- Re-check the precondition on the row version that UPDATE actually locks. + -- Under READ COMMITTED, concurrent statements may both populate candidate + -- from the same snapshot; EvalPlanQual re-evaluates this target-row predicate + -- after waiting for the first writer, leaving the stale writer with 0 rows. + AND ($2::bigint IS NULL OR i.revision = $2::bigint) RETURNING i.id, i.workspace_id, i.title, i.description, i.status, i.priority, i.assignee_type, i.assignee_id, i.creator_type, i.creator_id, i.parent_issue_id, i.acceptance_criteria, i.context_refs, i.position, i.due_date, i.created_at, i.updated_at, i.number, i.project_id, i.origin_type, i.origin_id, i.first_executed_at, i.start_date, i.metadata, i.stage, i.properties, i.revision, i.last_activity_at ` type UpdateIssueParams struct { ID pgtype.UUID `json:"id"` + ExpectedRevision pgtype.Int8 `json:"expected_revision"` Title pgtype.Text `json:"title"` Description pgtype.Text `json:"description"` Status pgtype.Text `json:"status"` @@ -1649,12 +1655,12 @@ type UpdateIssueParams struct { ParentIssueID pgtype.UUID `json:"parent_issue_id"` ProjectID pgtype.UUID `json:"project_id"` Stage pgtype.Int4 `json:"stage"` - ExpectedRevision pgtype.Int8 `json:"expected_revision"` } func (q *Queries) UpdateIssue(ctx context.Context, arg UpdateIssueParams) (Issue, error) { row := q.db.QueryRow(ctx, updateIssue, arg.ID, + arg.ExpectedRevision, arg.Title, arg.Description, arg.Status, @@ -1667,7 +1673,6 @@ func (q *Queries) UpdateIssue(ctx context.Context, arg UpdateIssueParams) (Issue arg.ParentIssueID, arg.ProjectID, arg.Stage, - arg.ExpectedRevision, ) var i Issue err := row.Scan( diff --git a/server/pkg/db/queries/issue.sql b/server/pkg/db/queries/issue.sql index 1fb954095..fd7e6e159 100644 --- a/server/pkg/db/queries/issue.sql +++ b/server/pkg/db/queries/issue.sql @@ -232,6 +232,11 @@ UPDATE issue AS i SET updated_at = CASE WHEN changed.did_change THEN now() ELSE i.updated_at END FROM changed WHERE i.id = changed.id + -- Re-check the precondition on the row version that UPDATE actually locks. + -- Under READ COMMITTED, concurrent statements may both populate candidate + -- from the same snapshot; EvalPlanQual re-evaluates this target-row predicate + -- after waiting for the first writer, leaving the stale writer with 0 rows. + AND (sqlc.narg('expected_revision')::bigint IS NULL OR i.revision = sqlc.narg('expected_revision')::bigint) RETURNING i.*; -- name: UpdateIssueStatus :one diff --git a/server/pkg/publicapi/v1/README.md b/server/pkg/publicapi/v1/README.md new file mode 100644 index 000000000..03843cedc --- /dev/null +++ b/server/pkg/publicapi/v1/README.md @@ -0,0 +1,55 @@ +# Multica Public API v1 + +This package is the source of truth for the first versioned Public API slice: + +- `openapi.yaml` defines the wire contract. +- `routes.go` is the capability ledger used by router and contract tests. +- `types.go` contains DTOs that are deliberately independent from App API + responses and database models. +- `problem.go` defines the stable error envelope used before and after routing. +- `foundation.go` defines actor, credential, pagination, idempotency, revision, + risk, audit, and rate-limit vocabulary shared by every future resource slice. + +The Plugin API is the first deployed surface. Its installation/callback tokens, +manifest scopes, rate limits, and audit policy narrow the shared resource +contract. Plugin context and storage are marked `plugin_extension` and are not +implicitly part of a future user/PAT surface. + +Both surfaces should call the same Go service and handler layer. The Plugin API +must not make an additional HTTP request to the user/PAT API. + +Additive fields and endpoints remain in v1. A breaking wire change requires a +new major version and a migration window. New write operations should adopt the +shared `Idempotency-Key` and revision/ETag components rather than inventing +endpoint-specific concurrency rules. + +## Foundation rules + +- User OAuth and PAT authenticate a member; Plugin installation and invocation + credentials authenticate either a member-bounded invocation or the + installation itself. Authentication happens at the surface, before a shared + resource service runs. +- Collection endpoints use opaque cursor pagination with `limit` (default 50, + maximum 200) and return `next_cursor`. The migrated comment endpoint keeps + its existing newest-200 compatibility window until its dedicated pagination + slice lands; it is not a precedent for new collections. +- Create, trigger, replay, and retry operations must durably deduplicate an + `Idempotency-Key` (maximum 255 bytes) before the operation is documented as + idempotent. Merely accepting the header is insufficient. +- Mutable resources expose a revision and ETag. Conditional writes accept + `If-Match`; stale writes return `revision_conflict`. +- The capability ledger states credential families, scope, risk, audit, and + rate-limit policy. Plugin requests use the stricter surface profile even when + the resource DTO and service are shared with a user/PAT request. +- Audit requirements use an explicit lifecycle (`planned` or `enforced`). The + migrated operations remain `planned` until a durable audit sink is wired; + the ledger does not imply that a boolean marker performs an audit write. + +## Rollout ledger + +The migrated slice is Issue read/content update and Comment read/create. Context +and Storage remain Plugin extensions; Hook and invocation delivery remain on +the bridge. Projects, Members, Issue search/create/transition, Agents, Squads, +Skills, Tasks/Runs, and Autopilots are subsequent vertical slices. Each slice +adds its Public contract first, then explicitly opts the safe Plugin subset into +the capability ledger. diff --git a/server/pkg/publicapi/v1/contract_test.go b/server/pkg/publicapi/v1/contract_test.go new file mode 100644 index 000000000..00a4095f2 --- /dev/null +++ b/server/pkg/publicapi/v1/contract_test.go @@ -0,0 +1,117 @@ +package publicapiv1 + +import ( + "net/http" + "strings" + "testing" + + "gopkg.in/yaml.v3" +) + +func TestOpenAPICoversCapabilityLedger(t *testing.T) { + var doc struct { + OpenAPI string `yaml:"openapi"` + Info struct { + Version string `yaml:"version"` + } `yaml:"info"` + Paths map[string]map[string]any `yaml:"paths"` + } + if err := yaml.Unmarshal(OpenAPI(), &doc); err != nil { + t.Fatalf("parse embedded OpenAPI: %v", err) + } + if doc.OpenAPI != "3.1.0" || doc.Info.Version != "v1" { + t.Fatalf("unexpected contract version: openapi=%q info.version=%q", doc.OpenAPI, doc.Info.Version) + } + + want := make(map[string]map[string]bool) + for _, operation := range Operations { + methods := want[operation.Path] + if methods == nil { + methods = make(map[string]bool) + want[operation.Path] = methods + } + methods[strings.ToLower(operation.Method)] = true + } + if len(doc.Paths) != len(want) { + t.Fatalf("OpenAPI path count = %d, ledger path count = %d", len(doc.Paths), len(want)) + } + for path, methods := range want { + pathItem, ok := doc.Paths[path] + if !ok { + t.Errorf("OpenAPI is missing %s", path) + continue + } + for method := range methods { + if _, ok := pathItem[method]; !ok { + t.Errorf("OpenAPI is missing %s %s", strings.ToUpper(method), path) + } + } + } + for _, operation := range Operations { + method := strings.ToLower(operation.Method) + raw, ok := doc.Paths[operation.Path][method].(map[string]any) + if !ok { + t.Errorf("OpenAPI operation %s %s is not an object", operation.Method, operation.Path) + continue + } + if got, _ := raw["x-multica-contract"].(string); got != string(operation.Contract) { + t.Errorf("%s %s x-multica-contract = %q, want %q", operation.Method, operation.Path, got, operation.Contract) + } + gotScope, _ := raw["x-multica-scope"].(string) + if gotScope != operation.Policy.Scope { + t.Errorf("%s %s x-multica-scope = %q, want %q", operation.Method, operation.Path, gotScope, operation.Policy.Scope) + } + } + if _, exposed := doc.Paths["/hooks/{hook_key}"]; exposed { + t.Fatal("Plugin-only person hook leaked into the public contract") + } +} + +func TestOperationLedgerPinsSharedScopes(t *testing.T) { + want := map[string]string{ + http.MethodGet + " " + PathIssue: "issues:read", + http.MethodPatch + " " + PathIssue: "issues:write", + http.MethodGet + " " + PathIssueComments: "comments:read", + http.MethodPost + " " + PathIssueComments: "comments:write", + } + for _, operation := range Operations { + if operation.Contract != ContractSharedResource { + continue + } + key := operation.Method + " " + operation.Path + if operation.Policy.Scope != want[key] { + t.Errorf("%s scope = %q, want %q", key, operation.Policy.Scope, want[key]) + } + if len(operation.Policy.Credentials) != 4 { + t.Errorf("%s does not declare both user and Plugin credential families: %v", key, operation.Policy.Credentials) + } + if operation.Policy.Audit != AuditPlanned { + t.Errorf("%s audit status = %q, want %q until an audit sink is implemented", key, operation.Policy.Audit, AuditPlanned) + } + delete(want, key) + } + for key := range want { + t.Errorf("shared operation missing from ledger: %s", key) + } +} + +func TestPluginExtensionsRejectUserCredentialsByContract(t *testing.T) { + for _, operation := range Operations { + if operation.Contract != ContractPluginExtension { + continue + } + for _, credential := range operation.Policy.Credentials { + if credential == CredentialUserOAuth || credential == CredentialPersonalAccess { + t.Errorf("%s %s exposes Plugin extension to %s", operation.Method, operation.Path, credential) + } + } + } +} + +func TestOperationLedgerDeclaresAuditLifecycle(t *testing.T) { + for _, operation := range Operations { + if operation.Policy.Audit == "" { + t.Errorf("%s %s has no explicit audit lifecycle", operation.Method, operation.Path) + } + } +} diff --git a/server/pkg/publicapi/v1/foundation.go b/server/pkg/publicapi/v1/foundation.go new file mode 100644 index 000000000..c581442bc --- /dev/null +++ b/server/pkg/publicapi/v1/foundation.go @@ -0,0 +1,79 @@ +package publicapiv1 + +const ( + HeaderIdempotencyKey = "Idempotency-Key" + HeaderIfMatch = "If-Match" + MaxIdempotencyBytes = 255 + DefaultPageSize = 50 + MaxPageSize = 200 +) + +// CredentialKind identifies the trust boundary that authenticated a request. +// Resource services receive an authorized actor; they do not parse tokens. +type CredentialKind string + +const ( + CredentialUserOAuth CredentialKind = "user_oauth" + CredentialPersonalAccess CredentialKind = "personal_access_token" + CredentialPluginInstall CredentialKind = "plugin_installation" + CredentialPluginInvocation CredentialKind = "plugin_invocation" +) + +type ActorKind string + +const ( + ActorMember ActorKind = "member" + ActorPlugin ActorKind = "plugin" +) + +// Actor is the transport-neutral identity supplied to shared authorization, +// audit, and service layers after a surface authenticates its credential. +type Actor struct { + Kind ActorKind + SubjectID string + WorkspaceID string + Credential CredentialKind +} + +type RiskLevel string + +const ( + RiskRead RiskLevel = "read" + RiskContentWrite RiskLevel = "content_write" + RiskHigh RiskLevel = "high" +) + +type RateLimitProfile string + +const ( + RateLimitUserDefault RateLimitProfile = "user_default" + RateLimitPluginStrict RateLimitProfile = "plugin_strict" +) + +// AuditStatus distinguishes a declared requirement from an implemented audit +// sink. A capability must not claim enforcement merely because it is listed in +// the contract ledger. +type AuditStatus string + +const ( + AuditNotRequired AuditStatus = "not_required" + AuditPlanned AuditStatus = "planned" + AuditEnforced AuditStatus = "enforced" +) + +// OperationPolicy records authorization and observability requirements next +// to the route contract. Surface-specific middleware enforces the credential +// kind and rate profile; resource authorization enforces workspace and scope. +type OperationPolicy struct { + Credentials []CredentialKind + Scope string + Risk RiskLevel + Audit AuditStatus + RateLimits []RateLimitProfile +} + +// PageInfo is the common response metadata for cursor-paginated collections. +// Cursors are opaque and scoped to the authenticated actor and query filters. +type PageInfo struct { + NextCursor string `json:"next_cursor,omitempty"` +} diff --git a/server/pkg/publicapi/v1/openapi.yaml b/server/pkg/publicapi/v1/openapi.yaml new file mode 100644 index 000000000..0ebcbceff --- /dev/null +++ b/server/pkg/publicapi/v1/openapi.yaml @@ -0,0 +1,404 @@ +openapi: 3.1.0 +info: + title: Multica Public API + version: v1 + description: >- + Canonical v1 resource contract shared by Multica API surfaces. The Plugin + API is the first deployed consumer and applies installation identity plus + manifest scopes. Plugin-specific context and storage remain extensions. +servers: + - url: https://plugin-api.multica.ai/v1 + description: Installed Plugin and invocation callback tokens +security: + - pluginBearer: [] +paths: + /context: + get: + operationId: getPluginContext + summary: Get the authorized Plugin invocation context + x-multica-contract: plugin_extension + responses: + "200": + description: Plugin context + content: + application/json: + schema: + $ref: "#/components/schemas/Context" + default: + $ref: "#/components/responses/Problem" + /issues/{issue_ref}: + parameters: + - $ref: "#/components/parameters/IssueRef" + get: + operationId: getIssue + summary: Get an issue + x-multica-contract: shared_resource + x-multica-scope: issues:read + responses: + "200": + description: Issue + headers: + ETag: + $ref: "#/components/headers/ETag" + content: + application/json: + schema: + $ref: "#/components/schemas/Issue" + default: + $ref: "#/components/responses/Problem" + patch: + operationId: patchIssue + summary: Update an issue title or description + x-multica-contract: shared_resource + x-multica-scope: issues:write + parameters: + - $ref: "#/components/parameters/IfMatch" + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/PatchIssueRequest" + responses: + "200": + description: Updated issue + headers: + ETag: + $ref: "#/components/headers/ETag" + content: + application/json: + schema: + $ref: "#/components/schemas/Issue" + default: + $ref: "#/components/responses/Problem" + /issues/{issue_ref}/comments: + parameters: + - $ref: "#/components/parameters/IssueRef" + get: + operationId: listIssueComments + summary: List the newest 200 comments in chronological order + x-multica-contract: shared_resource + x-multica-scope: comments:read + responses: + "200": + description: Comments + content: + application/json: + schema: + $ref: "#/components/schemas/CommentListResponse" + default: + $ref: "#/components/responses/Problem" + post: + operationId: createIssueComment + summary: Create an issue comment without mention dispatch + x-multica-contract: shared_resource + x-multica-scope: comments:write + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/CreateCommentRequest" + responses: + "201": + description: Created comment + content: + application/json: + schema: + $ref: "#/components/schemas/Comment" + default: + $ref: "#/components/responses/Problem" + /storage/{scope}: + get: + operationId: listPluginStorageKeys + summary: List Plugin storage keys + x-multica-contract: plugin_extension + parameters: + - $ref: "#/components/parameters/StorageScope" + responses: + "200": + description: Storage keys + content: + application/json: + schema: + $ref: "#/components/schemas/StorageKeyListResponse" + default: + $ref: "#/components/responses/Problem" + /storage/{scope}/{key}: + parameters: + - $ref: "#/components/parameters/StorageScope" + - $ref: "#/components/parameters/StorageKey" + get: + operationId: getPluginStorageValue + summary: Get a Plugin storage value + x-multica-contract: plugin_extension + responses: + "200": + description: Storage value + content: + application/json: + schema: + $ref: "#/components/schemas/StorageValueResponse" + default: + $ref: "#/components/responses/Problem" + put: + operationId: putPluginStorageValue + summary: Put a Plugin storage value + x-multica-contract: plugin_extension + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/PutStorageValueRequest" + responses: + "204": + description: Stored + default: + $ref: "#/components/responses/Problem" + delete: + operationId: deletePluginStorageValue + summary: Delete a Plugin storage value + x-multica-contract: plugin_extension + responses: + "204": + description: Deleted + default: + $ref: "#/components/responses/Problem" +components: + securitySchemes: + pluginBearer: + type: http + scheme: bearer + bearerFormat: mpi_ or mpc_ token + parameters: + IssueRef: + name: issue_ref + in: path + required: true + description: Issue UUID or workspace identifier such as MUL-123 + schema: + type: string + StorageScope: + name: scope + in: path + required: true + schema: + type: string + enum: [user, workspace] + StorageKey: + name: key + in: path + required: true + schema: + type: string + maxLength: 1024 + IfMatch: + name: If-Match + in: header + required: false + description: Weak or strong ETag containing the expected positive revision + schema: + type: string + IdempotencyKey: + name: Idempotency-Key + in: header + required: false + description: Reserved for durable retry deduplication as write endpoints adopt it + schema: + type: string + maxLength: 255 + headers: + ETag: + description: Weak ETag derived from the resource revision + schema: + type: string + responses: + Problem: + description: Stable API problem + headers: + X-Request-Id: + description: Request correlation identifier + schema: + type: string + content: + application/problem+json: + schema: + $ref: "#/components/schemas/Problem" + schemas: + Problem: + type: object + required: [type, title, status, code, detail, request_id, error] + properties: + type: { type: string, format: uri } + title: { type: string } + status: { type: integer } + code: { type: string } + detail: { type: string } + request_id: { type: string } + error: + type: string + deprecated: true + description: Compatibility alias for detail + errors: + type: array + items: + $ref: "#/components/schemas/FieldError" + FieldError: + type: object + required: [field, code, message] + properties: + field: { type: string } + code: { type: string } + message: { type: string } + Context: + type: object + required: [workspace, config, granted_net_domains, actor] + properties: + workspace: + $ref: "#/components/schemas/ContextWorkspace" + user: + $ref: "#/components/schemas/ContextUser" + issue: + $ref: "#/components/schemas/ContextIssue" + config: + type: object + additionalProperties: true + granted_net_domains: + type: array + items: { type: string } + actor: + type: string + enum: [member, plugin] + ContextWorkspace: + type: object + required: [id, name, slug] + properties: + id: { type: string } + name: { type: string } + slug: { type: string } + ContextUser: + type: object + required: [id, name] + properties: + id: { type: string } + name: { type: string } + ContextIssue: + type: object + required: [id, identifier, title] + properties: + id: { type: string } + identifier: { type: string } + title: { type: string } + Issue: + type: object + required: + - id + - workspace_id + - number + - identifier + - title + - description + - status + - priority + - assignee_type + - assignee_id + - creator_type + - creator_id + - parent_issue_id + - project_id + - position + - stage + - start_date + - due_date + - created_at + - updated_at + - revision + - last_activity_at + - metadata + - properties + properties: + id: { type: string } + workspace_id: { type: string } + number: { type: integer, format: int32 } + identifier: { type: string } + title: { type: string } + description: { type: [string, "null"] } + status: { type: string } + status_category: { type: string } + priority: { type: string } + assignee_type: { type: [string, "null"] } + assignee_id: { type: [string, "null"] } + creator_type: { type: string } + creator_id: { type: string } + parent_issue_id: { type: [string, "null"] } + project_id: { type: [string, "null"] } + position: { type: number } + stage: { type: [integer, "null"], format: int32 } + start_date: { type: [string, "null"], format: date } + due_date: { type: [string, "null"], format: date } + created_at: { type: string, format: date-time } + updated_at: { type: string, format: date-time } + revision: { type: integer, format: int64, minimum: 1 } + last_activity_at: { type: [string, "null"], format: date-time } + metadata: { type: object, additionalProperties: true } + properties: { type: object, additionalProperties: true } + PatchIssueRequest: + type: object + properties: + expected_revision: { type: integer, format: int64, minimum: 1 } + title: { type: string } + description: { type: string } + anyOf: + - required: [title] + - required: [description] + Comment: + type: object + required: [id, author_type, author_id, content, type, created_at] + properties: + id: { type: string } + author_type: { type: string, enum: [member, agent, plugin] } + author_id: { type: string } + content: { type: string } + type: { type: string } + parent_id: { type: string } + created_at: { type: string, format: date-time } + CommentListResponse: + type: object + required: [comments] + properties: + comments: + type: array + items: + $ref: "#/components/schemas/Comment" + CreateCommentRequest: + type: object + required: [content] + properties: + content: { type: string, maxLength: 65536 } + parent_id: { type: string } + StorageKey: + type: object + required: [key, size_bytes, updated_at] + properties: + key: { type: string } + size_bytes: { type: integer, format: int64 } + updated_at: { type: string, format: date-time } + StorageKeyListResponse: + type: object + required: [keys] + properties: + keys: + type: array + items: + $ref: "#/components/schemas/StorageKey" + StorageValueResponse: + type: object + required: [value] + properties: + value: { type: string } + PutStorageValueRequest: + type: object + required: [value] + properties: + value: { type: string, maxLength: 102400 } diff --git a/server/pkg/publicapi/v1/problem.go b/server/pkg/publicapi/v1/problem.go new file mode 100644 index 000000000..53be14b87 --- /dev/null +++ b/server/pkg/publicapi/v1/problem.go @@ -0,0 +1,111 @@ +package publicapiv1 + +import ( + "encoding/json" + "net/http" + "strings" + + chimw "github.com/go-chi/chi/v5/middleware" + "github.com/google/uuid" +) + +const ProblemContentType = "application/problem+json" + +// FieldError describes one invalid request field. It is optional so endpoints +// can adopt field-level validation without changing the top-level envelope. +type FieldError struct { + Field string `json:"field"` + Code string `json:"code"` + Message string `json:"message"` +} + +// Problem is the stable v1 error envelope. Error is the compatibility alias +// consumed by existing Plugin clients; new clients should branch on Code and +// render Detail. +type Problem struct { + Type string `json:"type"` + Title string `json:"title"` + Status int `json:"status"` + Code string `json:"code"` + Detail string `json:"detail"` + RequestID string `json:"request_id"` + Errors []FieldError `json:"errors,omitempty"` + Error string `json:"error"` +} + +func CodeForStatus(status int) string { + switch status { + case http.StatusBadRequest: + return "invalid_request" + case http.StatusUnauthorized: + return "unauthorized" + case http.StatusPaymentRequired: + return "payment_required" + case http.StatusForbidden: + return "forbidden" + case http.StatusNotFound: + return "not_found" + case http.StatusConflict: + return "conflict" + case http.StatusUnprocessableEntity: + return "incompatible" + case http.StatusTooManyRequests: + return "rate_limited" + case http.StatusInsufficientStorage: + return "quota_exceeded" + case http.StatusServiceUnavailable: + return "service_unavailable" + case http.StatusBadGateway: + return "upstream_unavailable" + default: + return "internal_error" + } +} + +func requestID(r *http.Request) string { + if r != nil { + if id := strings.TrimSpace(chimw.GetReqID(r.Context())); id != "" { + return id + } + if id := strings.TrimSpace(r.Header.Get(chimw.RequestIDHeader)); id != "" { + return id + } + } + return uuid.NewString() +} + +// WriteProblem writes an RFC 9457-style problem with Multica's stable code and +// the legacy `error` alias. It is shared by auth middleware and handlers so a +// failure before routing has the same contract as one inside a resource. +func WriteProblem(w http.ResponseWriter, r *http.Request, status int, code, detail string) { + if code == "" { + code = CodeForStatus(status) + } + title := http.StatusText(status) + if title == "" { + title = "Request failed" + } + id := requestID(r) + payload := Problem{ + Type: "urn:multica:problem:" + code, + Title: title, + Status: status, + Code: code, + Detail: detail, + RequestID: id, + Error: detail, + } + + w.Header().Set("Content-Type", ProblemContentType) + w.Header().Set(chimw.RequestIDHeader, id) + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(payload) +} + +func NotFound(w http.ResponseWriter, r *http.Request) { + WriteProblem(w, r, http.StatusNotFound, "not_found", "resource not found") +} + +func MethodNotAllowed(w http.ResponseWriter, r *http.Request) { + WriteProblem(w, r, http.StatusMethodNotAllowed, "method_not_allowed", "method not allowed") +} diff --git a/server/pkg/publicapi/v1/problem_test.go b/server/pkg/publicapi/v1/problem_test.go new file mode 100644 index 000000000..99866fef4 --- /dev/null +++ b/server/pkg/publicapi/v1/problem_test.go @@ -0,0 +1,36 @@ +package publicapiv1 + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" +) + +func TestWriteProblemKeepsLegacyErrorAndStableFields(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/v1/issues/MUL-1", nil) + req.Header.Set("X-Request-ID", "request-123") + response := httptest.NewRecorder() + + WriteProblem(response, req, http.StatusForbidden, "missing_scope", "issues:read is required") + + if response.Code != http.StatusForbidden { + t.Fatalf("status = %d", response.Code) + } + if got := response.Header().Get("Content-Type"); got != ProblemContentType { + t.Fatalf("Content-Type = %q", got) + } + if got := response.Header().Get("X-Request-Id"); got != "request-123" { + t.Fatalf("X-Request-Id = %q", got) + } + var problem Problem + if err := json.Unmarshal(response.Body.Bytes(), &problem); err != nil { + t.Fatalf("decode problem: %v", err) + } + if problem.Code != "missing_scope" || problem.Detail != "issues:read is required" || problem.Error != problem.Detail { + t.Fatalf("unexpected problem: %+v", problem) + } + if problem.RequestID != "request-123" || problem.Type != "urn:multica:problem:missing_scope" { + t.Fatalf("unexpected identity fields: %+v", problem) + } +} diff --git a/server/pkg/publicapi/v1/routes.go b/server/pkg/publicapi/v1/routes.go new file mode 100644 index 000000000..f0f39d94d --- /dev/null +++ b/server/pkg/publicapi/v1/routes.go @@ -0,0 +1,67 @@ +// Package publicapiv1 owns the stable Multica Public API v1 contract. +// +// The same resource contract is exposed through different trust surfaces. The +// Plugin API is the first consumer: it accepts installation/callback tokens and +// applies manifest scopes. Future user/PAT entrypoints should reuse these +// paths, DTOs, and operation semantics instead of creating parallel handlers. +package publicapiv1 + +import "net/http" + +const BasePath = "/v1" + +const ( + PathContext = "/context" + PathIssue = "/issues/{issue_ref}" + PathIssueComments = "/issues/{issue_ref}/comments" + PathStorageScope = "/storage/{scope}" + PathStorageValue = "/storage/{scope}/{key}" +) + +type ContractKind string + +const ( + // ContractSharedResource is a Multica resource contract that can be exposed + // to multiple credential types through surface-specific authorization. + ContractSharedResource ContractKind = "shared_resource" + // ContractPluginExtension belongs to Plugin installations rather than the + // general Multica resource API. + ContractPluginExtension ContractKind = "plugin_extension" +) + +// Operation is the capability ledger for the currently implemented v1 slice. +// OpenAPI tests pin this registry to openapi.yaml so routes, scopes, and docs +// cannot drift independently. +type Operation struct { + Method string + Path string + Contract ContractKind + Policy OperationPolicy +} + +var sharedCredentials = []CredentialKind{ + CredentialUserOAuth, + CredentialPersonalAccess, + CredentialPluginInstall, + CredentialPluginInvocation, +} + +var pluginCredentials = []CredentialKind{ + CredentialPluginInstall, + CredentialPluginInvocation, +} + +var sharedRateLimits = []RateLimitProfile{RateLimitUserDefault, RateLimitPluginStrict} +var pluginRateLimits = []RateLimitProfile{RateLimitPluginStrict} + +var Operations = []Operation{ + {Method: http.MethodGet, Path: PathContext, Contract: ContractPluginExtension, Policy: OperationPolicy{Credentials: pluginCredentials, Risk: RiskRead, Audit: AuditNotRequired, RateLimits: pluginRateLimits}}, + {Method: http.MethodGet, Path: PathIssue, Contract: ContractSharedResource, Policy: OperationPolicy{Credentials: sharedCredentials, Scope: "issues:read", Risk: RiskRead, Audit: AuditPlanned, RateLimits: sharedRateLimits}}, + {Method: http.MethodPatch, Path: PathIssue, Contract: ContractSharedResource, Policy: OperationPolicy{Credentials: sharedCredentials, Scope: "issues:write", Risk: RiskContentWrite, Audit: AuditPlanned, RateLimits: sharedRateLimits}}, + {Method: http.MethodGet, Path: PathIssueComments, Contract: ContractSharedResource, Policy: OperationPolicy{Credentials: sharedCredentials, Scope: "comments:read", Risk: RiskRead, Audit: AuditPlanned, RateLimits: sharedRateLimits}}, + {Method: http.MethodPost, Path: PathIssueComments, Contract: ContractSharedResource, Policy: OperationPolicy{Credentials: sharedCredentials, Scope: "comments:write", Risk: RiskContentWrite, Audit: AuditPlanned, RateLimits: sharedRateLimits}}, + {Method: http.MethodGet, Path: PathStorageScope, Contract: ContractPluginExtension, Policy: OperationPolicy{Credentials: pluginCredentials, Risk: RiskRead, Audit: AuditNotRequired, RateLimits: pluginRateLimits}}, + {Method: http.MethodGet, Path: PathStorageValue, Contract: ContractPluginExtension, Policy: OperationPolicy{Credentials: pluginCredentials, Risk: RiskRead, Audit: AuditNotRequired, RateLimits: pluginRateLimits}}, + {Method: http.MethodPut, Path: PathStorageValue, Contract: ContractPluginExtension, Policy: OperationPolicy{Credentials: pluginCredentials, Risk: RiskContentWrite, Audit: AuditPlanned, RateLimits: pluginRateLimits}}, + {Method: http.MethodDelete, Path: PathStorageValue, Contract: ContractPluginExtension, Policy: OperationPolicy{Credentials: pluginCredentials, Risk: RiskContentWrite, Audit: AuditPlanned, RateLimits: pluginRateLimits}}, +} diff --git a/server/pkg/publicapi/v1/spec.go b/server/pkg/publicapi/v1/spec.go new file mode 100644 index 000000000..6fbaf2365 --- /dev/null +++ b/server/pkg/publicapi/v1/spec.go @@ -0,0 +1,14 @@ +package publicapiv1 + +import ( + "bytes" + _ "embed" +) + +//go:embed openapi.yaml +var openAPISpec []byte + +// OpenAPI returns an isolated copy of the canonical v1 contract document. +func OpenAPI() []byte { + return bytes.Clone(openAPISpec) +} diff --git a/server/pkg/publicapi/v1/types.go b/server/pkg/publicapi/v1/types.go new file mode 100644 index 000000000..5894bfb1b --- /dev/null +++ b/server/pkg/publicapi/v1/types.go @@ -0,0 +1,103 @@ +package publicapiv1 + +// Context is Plugin-specific bootstrap data. It lives in the versioned +// contract package so service-layer fields cannot leak into the wire format. +type Context struct { + Workspace ContextWorkspace `json:"workspace"` + User *ContextUser `json:"user,omitempty"` + Issue *ContextIssue `json:"issue,omitempty"` + Config map[string]any `json:"config"` + GrantedNetDomains []string `json:"granted_net_domains"` + Actor string `json:"actor"` +} + +type ContextWorkspace struct { + ID string `json:"id"` + Name string `json:"name"` + Slug string `json:"slug"` +} + +type ContextUser struct { + ID string `json:"id"` + Name string `json:"name"` +} + +type ContextIssue struct { + ID string `json:"id"` + Identifier string `json:"identifier"` + Title string `json:"title"` +} + +// Issue intentionally mirrors the fields already exposed by Plugin API v1, +// but is independent from handler.IssueResponse. Adding an App API field can +// therefore no longer widen the public contract by accident. +type Issue struct { + ID string `json:"id"` + WorkspaceID string `json:"workspace_id"` + Number int32 `json:"number"` + Identifier string `json:"identifier"` + Title string `json:"title"` + Description *string `json:"description"` + Status string `json:"status"` + StatusCategory string `json:"status_category,omitempty"` + Priority string `json:"priority"` + AssigneeType *string `json:"assignee_type"` + AssigneeID *string `json:"assignee_id"` + CreatorType string `json:"creator_type"` + CreatorID string `json:"creator_id"` + ParentIssueID *string `json:"parent_issue_id"` + ProjectID *string `json:"project_id"` + Position float64 `json:"position"` + Stage *int32 `json:"stage"` + StartDate *string `json:"start_date"` + DueDate *string `json:"due_date"` + CreatedAt string `json:"created_at"` + UpdatedAt string `json:"updated_at"` + Revision int64 `json:"revision"` + LastActivityAt *string `json:"last_activity_at"` + Metadata map[string]any `json:"metadata"` + Properties map[string]any `json:"properties"` +} + +type PatchIssueRequest struct { + ExpectedRevision *int64 `json:"expected_revision,omitempty"` + Title *string `json:"title,omitempty"` + Description *string `json:"description,omitempty"` +} + +type Comment struct { + ID string `json:"id"` + AuthorType string `json:"author_type"` + AuthorID string `json:"author_id"` + Content string `json:"content"` + Type string `json:"type"` + ParentID string `json:"parent_id,omitempty"` + CreatedAt string `json:"created_at"` +} + +type CommentListResponse struct { + Comments []Comment `json:"comments"` +} + +type CreateCommentRequest struct { + Content string `json:"content"` + ParentID *string `json:"parent_id,omitempty"` +} + +type StorageKey struct { + Key string `json:"key"` + SizeBytes int64 `json:"size_bytes"` + UpdatedAt string `json:"updated_at"` +} + +type StorageKeyListResponse struct { + Keys []StorageKey `json:"keys"` +} + +type StorageValueResponse struct { + Value string `json:"value"` +} + +type PutStorageValueRequest struct { + Value string `json:"value"` +}