diff --git a/go-api/internal/httpserver/memories.go b/go-api/internal/httpserver/memories.go new file mode 100644 index 0000000..8990c41 --- /dev/null +++ b/go-api/internal/httpserver/memories.go @@ -0,0 +1,191 @@ +package httpserver + +// Subject access and erasure for long-term memory. +// +// WHY THESE ROUTES EXIST AT ALL. memory.Held and memory.Forget were written +// the day the store was, and without a route the honest answer to "show me +// what you hold about this candidate" was "a developer runs a query". That is +// not a compliance posture, it is a promise with no mechanism: a subject +// access request has a statutory clock, and an erasure that depends on +// somebody being available is one that can be missed. +// +// WHAT AUTHORISES THEM. Memories about a candidate are read and erased by +// whoever may read and delete that candidate's application — the same policy +// row, not a new one. Inventing a `memories` permission would let the two +// drift: somebody barred from a candidate's record could still read what an +// agent inferred about them, which is the same disclosure by another route. +// +// WORKSPACE MEMORIES ARE NOT PERSONAL DATA and are listed to anyone who may +// read the organisation's own records. They are still erasable, because a +// wrong operational fact repeated into every answer is its own problem. + +import ( + "net/http" + "strings" + + "github.com/krow/krow-backend/go-api/internal/authctx" + "github.com/krow/krow-backend/go-api/internal/domain" + "github.com/krow/krow-backend/go-api/internal/memory" +) + +func (s *Server) routeMemories(mux *http.ServeMux) int { + if s.memories == nil { + return 0 + } + mux.HandleFunc("GET /api/v1/memories", s.handleMemoriesList) + mux.HandleFunc("DELETE /api/v1/memories", s.handleMemoriesForget) + return 2 +} + +// memoryResource maps a subject onto the record whose permission governs it, +// and the operation that permission must allow. +// +// THE OPERATION IS THE SECURITY DECISION, and the first version got it wrong. +// Gating a read of candidate memories on `list` of job-applications looked +// right and was not: a talent may list applications because every other read +// path scopes them to their OWN rows, and this one has no row scoping — so the +// check passed and the response would have carried the whole organisation's +// memories about everybody. Caught by a test before it shipped. +// +// So a personal memory requires `delete` on the record it concerns, for +// reading as much as for erasing. Deleting somebody's application is an +// administrative capability and nothing scopes it to self, which makes it the +// honest proxy for "may act on other people's records here". It needs no new +// permission and cannot drift from the record's own policy. +// +// A workspace fact has no personal subject and no disclosure risk, so it stays +// at `list` on the organisation's own postings — the least privileged thing +// that still means "works here". +func memoryResource(subject memory.Subject) (string, domain.Op, bool) { + switch subject { + case memory.SubjectCandidate: + return "job-applications", domain.OpDelete, true + case memory.SubjectUser: + return "users", domain.OpDelete, true + case memory.SubjectWorkspace: + return "job-postings", domain.OpList, true + default: + return "", 0, false + } +} + +// memoryRequest parses and authorises, or writes the error and returns false. +func (s *Server) memoryRequest(w http.ResponseWriter, r *http.Request) ( + authctx.Identity, memory.Subject, string, bool, +) { + ident, err := authctx.MustFrom(r.Context()) + if err != nil { + writeError(w, s.log, domain.Internal(err)) + return authctx.Identity{}, "", "", false + } + + subject := memory.Subject(strings.TrimSpace(r.URL.Query().Get("subject"))) + subjectID := strings.TrimSpace(r.URL.Query().Get("id")) + + path, op, known := memoryResource(subject) + if !known { + writeError(w, s.log, domain.Validation( + "subject must be workspace, candidate or user", map[string]string{ + "subject": "required", + })) + return authctx.Identity{}, "", "", false + } + if subject != memory.SubjectWorkspace && subjectID == "" { + writeError(w, s.log, domain.Validation( + "a candidate or user subject needs an id", map[string]string{"id": "required"})) + return authctx.Identity{}, "", "", false + } + + role, ok := domain.ParseRole(ident.Role) + if !ok { + s.log.Warn("memory request refused: unknown role", + "user_id", ident.UserID, "role", ident.Role) + writeError(w, s.log, domain.Forbidden()) + return authctx.Identity{}, "", "", false + } + svc, ok := s.api.Get(path) + if !ok { + writeError(w, s.log, domain.Internal(errUnregisteredResource(path))) + return authctx.Identity{}, "", "", false + } + if !svc.Resource().Policy.Allows(op, role) { + s.log.Warn("memory request refused", + "user_id", ident.UserID, "role", ident.Role, + "subject", string(subject), "required_resource", path) + writeError(w, s.log, domain.Forbidden()) + return authctx.Identity{}, "", "", false + } + return ident, subject, subjectID, true +} + +// memoryView is one memory as a subject access request should read it. +// +// Every field a person is entitled to know: what is held, who decided it, when +// it was written, when it goes. `author` is the one that matters most — "an +// agent inferred this" and "a recruiter wrote this" are different claims and a +// response that flattened them would be misleading. +type memoryView struct { + ID string `json:"id"` + Subject string `json:"subject"` + SubjectID string `json:"subjectId,omitempty"` + Text string `json:"text"` + Author string `json:"author"` + RunID string `json:"sourceRunId,omitempty"` + Written string `json:"written"` +} + +func (s *Server) handleMemoriesList(w http.ResponseWriter, r *http.Request) { + ident, subject, subjectID, ok := s.memoryRequest(w, r) + if !ok { + return + } + + records, err := s.memories.Held(r.Context(), ident, subject, subjectID) + if err != nil { + writeError(w, s.log, domain.Internal(err)) + return + } + + out := make([]memoryView, 0, len(records)) + for _, rec := range records { + out = append(out, memoryView{ + ID: rec.ID, Subject: string(rec.SubjectType), SubjectID: rec.SubjectID, + Text: rec.Text, Author: string(rec.Author), RunID: rec.SourceRunID, + Written: rec.CreatedDate.UTC().Format("2006-01-02T15:04:05Z"), + }) + } + writeJSON(w, http.StatusOK, envelope{Data: out}) +} + +func (s *Server) handleMemoriesForget(w http.ResponseWriter, r *http.Request) { + ident, subject, subjectID, ok := s.memoryRequest(w, r) + if !ok { + return + } + if subject == memory.SubjectWorkspace && subjectID == "" { + /* Refused rather than interpreted. "Erase every workspace memory" is + a plausible thing to want and a catastrophic thing to do by a + mistyped query string, so it is not reachable by omission. */ + writeError(w, s.log, domain.Validation( + "erasing workspace memories needs an explicit id", map[string]string{"id": "required"})) + return + } + + removed, err := s.memories.Forget(r.Context(), ident, subject, subjectID) + if err != nil { + writeError(w, s.log, domain.Internal(err)) + return + } + + /* Logged at Info, always. An erasure is the one memory operation somebody + may later need to prove happened, and the row itself is redacted — so + the log line is the durable record of who asked and when. */ + s.log.Info("memories erased", + "tenant_id", ident.OrgID, "user_id", ident.UserID, + "subject", string(subject), "subject_id", subjectID, "removed", removed) + + writeJSON(w, http.StatusOK, envelope{Data: map[string]any{ + "erased": removed, + "subject": string(subject), + }}) +} diff --git a/go-api/internal/httpserver/memories_test.go b/go-api/internal/httpserver/memories_test.go new file mode 100644 index 0000000..54287a4 --- /dev/null +++ b/go-api/internal/httpserver/memories_test.go @@ -0,0 +1,108 @@ +package httpserver_test + +import ( + "net/http" + "testing" +) + +/* Subject access and erasure for long-term memory. + The interesting assertions are the refusals: a route that lists what an + agent inferred about a named person is a disclosure route, and it has to be + gated on the same permission as the record itself. */ + +func TestMemoriesNeedASubject(t *testing.T) { + a := newAPI(t) + for _, path := range []string{ + "/api/v1/memories", + "/api/v1/memories?subject=everything", + } { + /* 422, which is this API's code for a well-formed request that cannot + be acted on — see domain.Validation. */ + if res := a.do(http.MethodGet, path, nil); res.code != http.StatusUnprocessableEntity { + t.Errorf("GET %s = %d, want 422", path, res.code) + } + } +} + +// A memory about a person that names no person cannot be produced for them, +// so asking for "all candidate memories" is a mistake rather than a query. +func TestAPersonalSubjectNeedsAnId(t *testing.T) { + a := newAPI(t) + res := a.do(http.MethodGet, "/api/v1/memories?subject=candidate", nil) + if res.code != http.StatusUnprocessableEntity { + t.Errorf("got %d, want 422", res.code) + } +} + +// Nothing held yet is an empty list, not an error: "we hold nothing about this +// person" is a valid and important answer to a subject access request. +func TestHoldingNothingIsAnEmptyList(t *testing.T) { + a := newAPI(t) + res := a.do(http.MethodGet, + "/api/v1/memories?subject=candidate&id=11111111-1111-1111-1111-111111111111", nil) + if res.code != http.StatusOK { + t.Fatalf("got %d, want 200: %v", res.code, res.body) + } + data, ok := res.body["data"].([]any) + if !ok && res.body["data"] != nil { + t.Fatalf("data is not a list: %#v", res.body["data"]) + } + if len(data) != 0 { + t.Errorf("got %d memories, want none", len(data)) + } +} + +// THE DISCLOSURE BOUNDARY. Somebody who may not read a candidate's +// application must not be able to read what an agent inferred about them — +// that is the same disclosure by another route. +func TestAReaderWithoutTheRecordCannotReadItsMemories(t *testing.T) { + a := newAPI(t) + talent := signInAs(t, a.handler, a.h.Pool, a.orgID, "talent", "talent-mem@example.test", "talent") + + res := a.as(talent, http.MethodGet, + "/api/v1/memories?subject=candidate&id=11111111-1111-1111-1111-111111111111", nil) + if res.code != http.StatusForbidden { + t.Errorf("got %d, want 403 — a talent read another person's inferred memories", res.code) + } +} + +func TestAReaderWithoutDeleteCannotErase(t *testing.T) { + a := newAPI(t) + talent := signInAs(t, a.handler, a.h.Pool, a.orgID, "talent", "talent-del@example.test", "talent") + + res := a.as(talent, http.MethodDelete, + "/api/v1/memories?subject=candidate&id=11111111-1111-1111-1111-111111111111", nil) + if res.code != http.StatusForbidden { + t.Errorf("got %d, want 403", res.code) + } +} + +// "Erase every workspace memory" is a plausible thing to want and a +// catastrophic thing to do by a mistyped query string. +func TestErasingWorkspaceMemoriesNeedsAnExplicitId(t *testing.T) { + a := newAPI(t) + res := a.do(http.MethodDelete, "/api/v1/memories?subject=workspace", nil) + if res.code != http.StatusUnprocessableEntity { + t.Errorf("got %d, want 422 — a bare delete reached the whole workspace", res.code) + } +} + +// An erasure against nothing is still a successful erasure: the caller asked +// for a state, and the state holds. +func TestErasingNothingSucceeds(t *testing.T) { + a := newAPI(t) + res := a.do(http.MethodDelete, + "/api/v1/memories?subject=candidate&id=11111111-1111-1111-1111-111111111111", nil) + if res.code != http.StatusOK { + t.Fatalf("got %d, want 200: %v", res.code, res.body) + } +} + +func TestMemoriesRefuseAnAnonymousCaller(t *testing.T) { + a := newAPI(t) + res := a.doAnon(http.MethodGet, + "/api/v1/memories?subject=candidate&id=11111111-1111-1111-1111-111111111111", nil) + if res.code != http.StatusUnauthorized { + t.Errorf("got %d, want 401", res.code) + } +} diff --git a/go-api/internal/httpserver/server.go b/go-api/internal/httpserver/server.go index 0e8e13e..1afd03f 100644 --- a/go-api/internal/httpserver/server.go +++ b/go-api/internal/httpserver/server.go @@ -314,6 +314,7 @@ func New(cfg *config.Config, database *db.DB, log *slog.Logger, opts ...Option) s.endpoints = s.routeAuth(mux) + s.routeResources(mux) + s.routeMe(mux) + s.routeDefinitions(mux) + s.routeWorkflows(mux) + s.routeOwliver(mux) + s.routeRuns(mux) + s.routeVersion(mux) + s.routeTools(mux) + + s.routeMemories(mux) + // The MCP surface and the OAuth server behind it. Both return 0 and // register nothing when OAUTH_ISSUER and MCP_RESOURCE are unset, which // is every deployment that has not asked for them.