From fc105d31a16ba8119854c4e2a4f02d9df799f9cb Mon Sep 17 00:00:00 2001 From: Aaron Vu <54255926+aaron-tsar@users.noreply.github.com> Date: Mon, 25 May 2026 14:25:56 +0700 Subject: [PATCH 1/4] fix(vault): accept and persist content on POST/PUT vault/documents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously POST /v1/{agents/{id}/}vault/documents and the matching PUT endpoint declared a body struct without a `content` field, so callers passing JSON `{"path": ..., "title": ..., "content": "..."}` got back HTTP 201 with a doc id, but the bytes were silently discarded: - bindJSON ignored the `content` key (no matching field) - the document row was inserted with content_hash="" and no file on disk - the enrichment pipeline (summary, embeddings, links) was never triggered because no EventVaultDocUpserted was published The endpoint was effectively a metadata-only create — there was no client path that combined "register doc + supply bytes" via JSON. /v1/vault/upload exists, but it requires multipart, flattens paths to basename, and is inconvenient for programmatic clients that already have UTF-8 text in memory (CI jobs, agent skills, MCP tools). This change makes `content` an optional field on both handlers: - POST: when content is non-empty, validate the file extension against the same allowedUploadExts whitelist used by /v1/vault/upload, write bytes to /, compute SHA-256, store it as content_hash on the doc, and publish EventVaultDocUpserted so the enrichment worker picks it up — exactly the same downstream behaviour as /v1/vault/upload. - PUT: same logic; nil content = no change, non-nil content = rewrite the file on disk (and an empty string truncates it). - Omitting `content` preserves the historical metadata-only behaviour (e.g. for callers that materialise files via filesystem rescan). - Path validation reuses the existing rules: no `..`, no absolute paths; an extra abs-path check rejects symlink escapes from the workspace root. Two small helpers (writeDocumentContent, publishDocUpserted) factor out the disk write + event publish so both handlers share the same code, and so they can be unit-tested in isolation later. The fix is backwards-compatible: existing callers that don't send `content` continue to work unchanged. Existing docs created without content remain metadata-only stubs; a subsequent PUT with `content` will materialise them. Verified live on a development instance: POST → row inserted, file at /, SHA-256 matches, enrichment event published. PUT → file rewritten, content_hash updated, updated_at refreshed. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/http/vault_handler_documents.go | 164 +++++++++++++++++++++++ 1 file changed, 164 insertions(+) diff --git a/internal/http/vault_handler_documents.go b/internal/http/vault_handler_documents.go index 1a589f1e..fbe08d4f 100644 --- a/internal/http/vault_handler_documents.go +++ b/internal/http/vault_handler_documents.go @@ -3,11 +3,73 @@ package http import ( "log/slog" "net/http" + "os" + "path/filepath" "strings" + "time" + "github.com/google/uuid" + "github.com/nextlevelbuilder/goclaw/internal/eventbus" "github.com/nextlevelbuilder/goclaw/internal/store" + "github.com/nextlevelbuilder/goclaw/internal/vault" ) +// writeDocumentContent writes the given content bytes to a tenant-workspace- +// relative path, after validating the path stays inside the workspace and uses +// an allowed extension. Returns the SHA-256 content hash on success. +// +// This is the shared writer used by handleCreateDocument and handleUpdateDocument +// to materialise inline JSON `content` payloads on disk, so the enrichment +// pipeline (which reads files by path) can later compute summaries, embeddings +// and links. +func (h *VaultHandler) writeDocumentContent(workspace, relPath string, content []byte) (string, error) { + target := filepath.Join(workspace, filepath.Clean(relPath)) + // Symlink/escape protection: the cleaned target must stay inside workspace. + absTarget, err := filepath.Abs(target) + if err != nil { + return "", err + } + absWS, err := filepath.Abs(workspace) + if err != nil { + return "", err + } + if !strings.HasPrefix(absTarget, absWS+string(os.PathSeparator)) && absTarget != absWS { + return "", os.ErrInvalid + } + if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil { + return "", err + } + if err := os.WriteFile(target, content, 0o644); err != nil { + return "", err + } + return vault.ContentHash(content), nil +} + +// publishDocUpserted enqueues the standard EventVaultDocUpserted so the +// enrichment worker re-runs summary/embedding/link extraction for this doc. +// No-op when the event bus is not wired (some test setups). +func (h *VaultHandler) publishDocUpserted(tenantID, agentID, docID, relPath, hash, workspace string) { + if h.eventBus == nil { + return + } + h.eventBus.Publish(eventbus.DomainEvent{ + ID: uuid.Must(uuid.NewV7()).String(), + Type: eventbus.EventVaultDocUpserted, + SourceID: docID + ":" + hash, + TenantID: tenantID, + AgentID: agentID, + Timestamp: time.Now(), + Payload: eventbus.VaultDocUpsertedPayload{ + DocID: docID, + TenantID: tenantID, + AgentID: agentID, + Path: relPath, + ContentHash: hash, + Workspace: workspace, + }, + }) +} + // handleListAllDocuments lists vault documents across all agents in tenant. // Optional query param agent_id to filter by specific agent. func (h *VaultHandler) handleListAllDocuments(w http.ResponseWriter, r *http.Request) { @@ -99,6 +161,16 @@ func (h *VaultHandler) handleGetDocument(w http.ResponseWriter, r *http.Request) } // handleCreateDocument creates a new vault document. +// +// When `content` is supplied in the JSON body, the bytes are materialised on +// disk at / and the SHA-256 hash is stored on the +// document record. An EventVaultDocUpserted is then emitted so the enrichment +// worker computes summary/embeddings/links — same code path the multipart +// /v1/vault/upload endpoint and the filesystem rescan use. +// +// Omitting `content` preserves the historical behaviour of registering a +// metadata-only stub (typically followed by a rescan that materialises the +// file from disk). func (h *VaultHandler) handleCreateDocument(w http.ResponseWriter, r *http.Request) { locale := extractLocale(r) tenantID := store.TenantIDFromContext(r.Context()) @@ -107,6 +179,7 @@ func (h *VaultHandler) handleCreateDocument(w http.ResponseWriter, r *http.Reque var body struct { Path string `json:"path"` Title string `json:"title"` + Content string `json:"content"` // optional; when set, written to disk DocType string `json:"doc_type"` Scope string `json:"scope"` TeamID string `json:"team_id"` @@ -137,6 +210,16 @@ func (h *VaultHandler) handleCreateDocument(w http.ResponseWriter, r *http.Reque writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid scope"}) return } + // Validate extension upfront if caller wants to write content — matches the + // whitelist enforced by /v1/vault/upload so the two endpoints accept the + // same file types. + if body.Content != "" { + ext := strings.ToLower(filepath.Ext(body.Path)) + if !allowedUploadExts[ext] { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "unsupported file type: " + ext}) + return + } + } doc := &store.VaultDocument{ TenantID: tenantID.String(), @@ -160,11 +243,50 @@ func (h *VaultHandler) handleCreateDocument(w http.ResponseWriter, r *http.Reque doc.Scope = "team" } } + + // Persist file content if provided, before the DB upsert so the + // content_hash column is set in a single write. + var ( + wsPath string + writtenHash string + contentWasSet bool + ) + if body.Content != "" { + wsPath = h.resolveTenantWorkspace(r.Context()) + if wsPath == "" { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "workspace not available"}) + return + } + hash, werr := h.writeDocumentContent(wsPath, body.Path, []byte(body.Content)) + if werr != nil { + slog.Warn("vault.create: write content failed", "path", body.Path, "error", werr) + if werr == os.ErrInvalid { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "path escapes workspace"}) + return + } + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": "failed to write content"}) + return + } + doc.ContentHash = hash + writtenHash = hash + contentWasSet = true + } + if err := h.store.UpsertDocument(r.Context(), doc); err != nil { slog.Warn("vault.create failed", "error", err) writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) return } + + // Fire enrichment after the DB row exists so the worker can locate the doc. + if contentWasSet { + evAgent := "" + if doc.AgentID != nil { + evAgent = *doc.AgentID + } + h.publishDocUpserted(tenantID.String(), evAgent, doc.ID, body.Path, writtenHash, wsPath) + } + // Re-fetch by ID (set via RETURNING) — unambiguous even when same path exists across teams. created, _ := h.store.GetDocumentByID(r.Context(), tenantID.String(), doc.ID) if created != nil { @@ -189,6 +311,7 @@ func (h *VaultHandler) handleUpdateDocument(w http.ResponseWriter, r *http.Reque var body struct { Title *string `json:"title"` + Content *string `json:"content"` // nil=no change; "" clears the file; non-empty rewrites it DocType *string `json:"doc_type"` Scope *string `json:"scope"` TeamID *string `json:"team_id"` // nil=no change, ""=clear, "uuid"=set @@ -225,11 +348,52 @@ func (h *VaultHandler) handleUpdateDocument(w http.ResponseWriter, r *http.Reque existing.Metadata = body.Metadata } + // Rewrite content on disk when caller supplied a `content` field. + var ( + wsPath string + writtenHash string + contentWasSet bool + ) + if body.Content != nil { + ext := strings.ToLower(filepath.Ext(existing.Path)) + if !allowedUploadExts[ext] { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "unsupported file type: " + ext}) + return + } + wsPath = h.resolveTenantWorkspace(r.Context()) + if wsPath == "" { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "workspace not available"}) + return + } + hash, werr := h.writeDocumentContent(wsPath, existing.Path, []byte(*body.Content)) + if werr != nil { + slog.Warn("vault.update: write content failed", "path", existing.Path, "error", werr) + if werr == os.ErrInvalid { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "path escapes workspace"}) + return + } + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": "failed to write content"}) + return + } + existing.ContentHash = hash + writtenHash = hash + contentWasSet = true + } + if err := h.store.UpsertDocument(r.Context(), existing); err != nil { slog.Warn("vault.update failed", "error", err) writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) return } + + if contentWasSet { + evAgent := "" + if existing.AgentID != nil { + evAgent = *existing.AgentID + } + h.publishDocUpserted(tenantID.String(), evAgent, existing.ID, existing.Path, writtenHash, wsPath) + } + updated, _ := h.store.GetDocumentByID(r.Context(), tenantID.String(), docID) if updated != nil { writeJSON(w, http.StatusOK, updated) From 5d5be0a64fa2186aa99ac2387f52805669491c7b Mon Sep 17 00:00:00 2001 From: Aaron Vu <54255926+aaron-tsar@users.noreply.github.com> Date: Thu, 28 May 2026 00:54:19 +0700 Subject: [PATCH 2/4] fix(vault): harden symlink containment + mirror upload enrich-progress Addresses review on #1174: - writeDocumentContent now resolves symlinks before writing. A symlink living inside the workspace but pointing outside (e.g. workspace/link -> /etc) previously passed the lexical prefix check and os.WriteFile followed it out of bounds. We now EvalSymlinks the workspace root and the deepest existing ancestor of the target, re-check containment before any mkdir/write, and refuse a symlink planted at the final component via Lstat. - publishDocUpserted starts enrichment progress (enrichProgress.Start) before publishing EventVaultDocUpserted, matching handleUpload's ordering so the worker pool can't drain the event before the progress tracker registers it. - Add regression tests: symlink dir escape, final-component symlink, parent escape, empty content, disallowed extension, event-after-upsert ordering, metadata-only no-write, and upsert-failure orphan behaviour. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/http/vault_handler_documents.go | 63 +++- internal/http/vault_handler_documents_test.go | 341 ++++++++++++++++++ 2 files changed, 391 insertions(+), 13 deletions(-) create mode 100644 internal/http/vault_handler_documents_test.go diff --git a/internal/http/vault_handler_documents.go b/internal/http/vault_handler_documents.go index fbe08d4f..745a1251 100644 --- a/internal/http/vault_handler_documents.go +++ b/internal/http/vault_handler_documents.go @@ -23,22 +23,52 @@ import ( // pipeline (which reads files by path) can later compute summaries, embeddings // and links. func (h *VaultHandler) writeDocumentContent(workspace, relPath string, content []byte) (string, error) { - target := filepath.Join(workspace, filepath.Clean(relPath)) - // Symlink/escape protection: the cleaned target must stay inside workspace. - absTarget, err := filepath.Abs(target) + // Resolve the workspace root through any symlinks so all containment checks + // compare canonical paths. + realWS, err := filepath.EvalSymlinks(workspace) if err != nil { return "", err } - absWS, err := filepath.Abs(workspace) - if err != nil { - return "", err - } - if !strings.HasPrefix(absTarget, absWS+string(os.PathSeparator)) && absTarget != absWS { + target := filepath.Join(realWS, filepath.Clean(relPath)) + + // Lexical containment: blocks "../" and absolute-path escapes. + if target != realWS && !strings.HasPrefix(target, realWS+string(os.PathSeparator)) { return "", os.ErrInvalid } + + // Symlink containment: a lexical prefix check is not enough — a symlink that + // lives *inside* the workspace but points outside it (e.g. workspace/link -> + // /etc) would otherwise let os.WriteFile follow it and clobber a file beyond + // the workspace. Resolve the deepest already-existing ancestor of the target + // and confirm it still canonicalises to a path inside the workspace, before + // any directory is created or byte written. + for ancestor := filepath.Dir(target); ; { + resolved, rerr := filepath.EvalSymlinks(ancestor) + if rerr == nil { + if resolved != realWS && !strings.HasPrefix(resolved, realWS+string(os.PathSeparator)) { + return "", os.ErrInvalid + } + break + } + if !os.IsNotExist(rerr) { + return "", rerr + } + parent := filepath.Dir(ancestor) + if parent == ancestor { + break // walked to the filesystem root without an existing ancestor + } + ancestor = parent + } + if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil { return "", err } + + // Refuse to follow a symlink planted at the final path component itself. + if fi, lerr := os.Lstat(target); lerr == nil && fi.Mode()&os.ModeSymlink != 0 { + return "", os.ErrInvalid + } + if err := os.WriteFile(target, content, 0o644); err != nil { return "", err } @@ -48,20 +78,27 @@ func (h *VaultHandler) writeDocumentContent(workspace, relPath string, content [ // publishDocUpserted enqueues the standard EventVaultDocUpserted so the // enrichment worker re-runs summary/embedding/link extraction for this doc. // No-op when the event bus is not wired (some test setups). -func (h *VaultHandler) publishDocUpserted(tenantID, agentID, docID, relPath, hash, workspace string) { +func (h *VaultHandler) publishDocUpserted(tenantID uuid.UUID, agentID, docID, relPath, hash, workspace string) { if h.eventBus == nil { return } + tenantIDStr := tenantID.String() + // Start enrichment progress before publishing so the worker pool can't drain + // the event and finish before the progress tracker registers it — same + // ordering handleUpload relies on to avoid that race. + if h.enrichProgress != nil { + h.enrichProgress.Start(1, tenantID) + } h.eventBus.Publish(eventbus.DomainEvent{ ID: uuid.Must(uuid.NewV7()).String(), Type: eventbus.EventVaultDocUpserted, SourceID: docID + ":" + hash, - TenantID: tenantID, + TenantID: tenantIDStr, AgentID: agentID, Timestamp: time.Now(), Payload: eventbus.VaultDocUpsertedPayload{ DocID: docID, - TenantID: tenantID, + TenantID: tenantIDStr, AgentID: agentID, Path: relPath, ContentHash: hash, @@ -284,7 +321,7 @@ func (h *VaultHandler) handleCreateDocument(w http.ResponseWriter, r *http.Reque if doc.AgentID != nil { evAgent = *doc.AgentID } - h.publishDocUpserted(tenantID.String(), evAgent, doc.ID, body.Path, writtenHash, wsPath) + h.publishDocUpserted(tenantID, evAgent, doc.ID, body.Path, writtenHash, wsPath) } // Re-fetch by ID (set via RETURNING) — unambiguous even when same path exists across teams. @@ -391,7 +428,7 @@ func (h *VaultHandler) handleUpdateDocument(w http.ResponseWriter, r *http.Reque if existing.AgentID != nil { evAgent = *existing.AgentID } - h.publishDocUpserted(tenantID.String(), evAgent, existing.ID, existing.Path, writtenHash, wsPath) + h.publishDocUpserted(tenantID, evAgent, existing.ID, existing.Path, writtenHash, wsPath) } updated, _ := h.store.GetDocumentByID(r.Context(), tenantID.String(), docID) diff --git a/internal/http/vault_handler_documents_test.go b/internal/http/vault_handler_documents_test.go new file mode 100644 index 00000000..2840c905 --- /dev/null +++ b/internal/http/vault_handler_documents_test.go @@ -0,0 +1,341 @@ +package http + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + "time" + + "github.com/google/uuid" + + "github.com/nextlevelbuilder/goclaw/internal/eventbus" + "github.com/nextlevelbuilder/goclaw/internal/store" + "github.com/nextlevelbuilder/goclaw/internal/vault" +) + +// masterTenant resolves TenantWorkspace to h.workspace unchanged, keeping the +// content-write tests pointed at a single temp dir. +var masterTenant = uuid.MustParse("0193a5b0-7000-7000-8000-000000000001") + +func masterCtx() context.Context { + return store.WithTenantID(context.Background(), masterTenant) +} + +// ---- writeDocumentContent: path & symlink containment (the blocking finding) ---- + +func TestWriteDocumentContent_WritesInsideWorkspace(t *testing.T) { + h := &VaultHandler{} + ws := t.TempDir() + + hash, err := h.writeDocumentContent(ws, "notes/hello.md", []byte("hi")) + if err != nil { + t.Fatalf("writeDocumentContent: %v", err) + } + if want := vault.ContentHash([]byte("hi")); hash != want { + t.Errorf("hash = %q, want %q", hash, want) + } + got, err := os.ReadFile(filepath.Join(ws, "notes", "hello.md")) + if err != nil { + t.Fatalf("read written file: %v", err) + } + if string(got) != "hi" { + t.Errorf("content = %q, want %q", got, "hi") + } +} + +func TestWriteDocumentContent_EmptyContent(t *testing.T) { + h := &VaultHandler{} + ws := t.TempDir() + + hash, err := h.writeDocumentContent(ws, "empty.md", []byte("")) + if err != nil { + t.Fatalf("writeDocumentContent: %v", err) + } + if want := vault.ContentHash([]byte("")); hash != want { + t.Errorf("hash = %q, want %q", hash, want) + } + fi, err := os.Stat(filepath.Join(ws, "empty.md")) + if err != nil { + t.Fatalf("stat empty file: %v", err) + } + if fi.Size() != 0 { + t.Errorf("size = %d, want 0", fi.Size()) + } +} + +func TestWriteDocumentContent_RejectsParentEscape(t *testing.T) { + h := &VaultHandler{} + ws := t.TempDir() + + _, err := h.writeDocumentContent(ws, "../escape.md", []byte("x")) + if !errors.Is(err, os.ErrInvalid) { + t.Fatalf("err = %v, want os.ErrInvalid", err) + } + if _, statErr := os.Stat(filepath.Join(filepath.Dir(ws), "escape.md")); statErr == nil { + t.Error("file escaped the workspace via ../") + } +} + +// A symlink that lives inside the workspace but points outside it must not be +// followed — this is the exact escape the lexical prefix check missed. +func TestWriteDocumentContent_RejectsSymlinkDirEscape(t *testing.T) { + h := &VaultHandler{} + ws := t.TempDir() + outside := t.TempDir() + + if err := os.Symlink(outside, filepath.Join(ws, "link")); err != nil { + t.Fatalf("symlink: %v", err) + } + + _, err := h.writeDocumentContent(ws, "link/pwn.md", []byte("x")) + if !errors.Is(err, os.ErrInvalid) { + t.Fatalf("err = %v, want os.ErrInvalid", err) + } + if _, statErr := os.Stat(filepath.Join(outside, "pwn.md")); statErr == nil { + t.Error("file written outside workspace through symlinked directory") + } +} + +// A symlink planted at the final path component must be refused rather than +// followed, so an attacker can't redirect a write onto an existing outside file. +func TestWriteDocumentContent_RejectsFinalComponentSymlink(t *testing.T) { + h := &VaultHandler{} + ws := t.TempDir() + outside := t.TempDir() + + outsideFile := filepath.Join(outside, "target.md") + if err := os.WriteFile(outsideFile, []byte("original"), 0o644); err != nil { + t.Fatalf("seed outside file: %v", err) + } + if err := os.Symlink(outsideFile, filepath.Join(ws, "evil.md")); err != nil { + t.Fatalf("symlink: %v", err) + } + + _, err := h.writeDocumentContent(ws, "evil.md", []byte("pwned")) + if !errors.Is(err, os.ErrInvalid) { + t.Fatalf("err = %v, want os.ErrInvalid", err) + } + got, _ := os.ReadFile(outsideFile) + if string(got) != "original" { + t.Errorf("outside file clobbered through final-component symlink: %q", got) + } +} + +// ---- handler-level: extension whitelist, event ordering, empty PUT, orphan ---- + +// fakeVaultStore embeds the interface (unimplemented methods panic if hit) and +// overrides only the two methods the content path touches. +type fakeVaultStore struct { + store.VaultStore + docs map[string]*store.VaultDocument + upserted bool + upsertErr error + upsertCall int +} + +func newFakeVaultStore() *fakeVaultStore { + return &fakeVaultStore{docs: map[string]*store.VaultDocument{}} +} + +func (f *fakeVaultStore) UpsertDocument(_ context.Context, doc *store.VaultDocument) error { + f.upsertCall++ + if f.upsertErr != nil { + return f.upsertErr + } + if doc.ID == "" { + doc.ID = "doc-" + uuid.NewString() + } + f.upserted = true + f.docs[doc.ID] = doc + return nil +} + +func (f *fakeVaultStore) GetDocumentByID(_ context.Context, _ /*tenantID*/, id string) (*store.VaultDocument, error) { + return f.docs[id], nil +} + +// fakeEventBus captures published events and records whether the DB upsert had +// already happened at publish time, to assert ordering. +type fakeEventBus struct { + store *fakeVaultStore + published []eventbus.DomainEvent + upsertedAtFire bool +} + +func (b *fakeEventBus) Publish(e eventbus.DomainEvent) { + if b.store != nil { + b.upsertedAtFire = b.store.upserted + } + b.published = append(b.published, e) +} +func (b *fakeEventBus) Subscribe(eventbus.EventType, eventbus.DomainEventHandler) func() { + return func() {} +} +func (b *fakeEventBus) Start(context.Context) {} +func (b *fakeEventBus) Drain(time.Duration) error { return nil } + +func newJSONRequest(t *testing.T, method, target string, body any) *http.Request { + t.Helper() + buf, err := json.Marshal(body) + if err != nil { + t.Fatalf("marshal body: %v", err) + } + req := httptest.NewRequest(method, target, bytes.NewReader(buf)) + req.Header.Set("Content-Type", "application/json") + return req.WithContext(masterCtx()) +} + +func TestHandleCreateDocument_RejectsDisallowedExtension(t *testing.T) { + h := &VaultHandler{} // extension check fires before any store/workspace access + req := newJSONRequest(t, http.MethodPost, "/v1/vault/documents", map[string]any{ + "path": "payload.exe", + "title": "x", + "content": "data", + }) + rr := httptest.NewRecorder() + + h.handleCreateDocument(rr, req) + + assertBadRequest(t, rr, "unsupported file type") +} + +func TestHandleCreateDocument_WritesContentAndPublishesAfterUpsert(t *testing.T) { + st := newFakeVaultStore() + bus := &fakeEventBus{store: st} + ws := t.TempDir() + h := &VaultHandler{store: st, eventBus: bus, workspace: ws} + + req := newJSONRequest(t, http.MethodPost, "/v1/vault/documents", map[string]any{ + "path": "decisions/adr-001.md", + "title": "ADR 001", + "content": "# Decision\nuse postgres", + }) + rr := httptest.NewRecorder() + + h.handleCreateDocument(rr, req) + + if rr.Code != http.StatusCreated { + t.Fatalf("status = %d, want 201 (body: %s)", rr.Code, rr.Body.String()) + } + // File materialised on disk. + got, err := os.ReadFile(filepath.Join(ws, "decisions", "adr-001.md")) + if err != nil { + t.Fatalf("read written doc: %v", err) + } + if string(got) != "# Decision\nuse postgres" { + t.Errorf("content = %q", got) + } + // Exactly one enrichment event, fired AFTER the DB upsert. + if len(bus.published) != 1 { + t.Fatalf("published %d events, want 1", len(bus.published)) + } + if !bus.upsertedAtFire { + t.Error("event published before DB upsert — enrichment worker could race ahead of the row") + } + if bus.published[0].Type != eventbus.EventVaultDocUpserted { + t.Errorf("event type = %v, want %v", bus.published[0].Type, eventbus.EventVaultDocUpserted) + } +} + +func TestHandleCreateDocument_NoContentSkipsWriteAndEvent(t *testing.T) { + st := newFakeVaultStore() + bus := &fakeEventBus{store: st} + ws := t.TempDir() + h := &VaultHandler{store: st, eventBus: bus, workspace: ws} + + req := newJSONRequest(t, http.MethodPost, "/v1/vault/documents", map[string]any{ + "path": "meta-only.md", + "title": "Meta only", + }) + rr := httptest.NewRecorder() + + h.handleCreateDocument(rr, req) + + if rr.Code != http.StatusCreated { + t.Fatalf("status = %d, want 201 (body: %s)", rr.Code, rr.Body.String()) + } + if _, statErr := os.Stat(filepath.Join(ws, "meta-only.md")); statErr == nil { + t.Error("metadata-only create must not write a file") + } + if len(bus.published) != 0 { + t.Errorf("metadata-only create published %d events, want 0", len(bus.published)) + } +} + +// Documents the orphan-file behaviour: when the DB upsert fails after the bytes +// are already on disk, the handler returns 500 and the file is intentionally +// left in place (no rollback). If that ever changes, this test should change +// with it. +func TestHandleCreateDocument_UpsertFailureLeavesFile(t *testing.T) { + st := newFakeVaultStore() + st.upsertErr = errors.New("db down") + bus := &fakeEventBus{store: st} + ws := t.TempDir() + h := &VaultHandler{store: st, eventBus: bus, workspace: ws} + + req := newJSONRequest(t, http.MethodPost, "/v1/vault/documents", map[string]any{ + "path": "orphan.md", + "title": "Orphan", + "content": "written before upsert", + }) + rr := httptest.NewRecorder() + + h.handleCreateDocument(rr, req) + + if rr.Code != http.StatusInternalServerError { + t.Fatalf("status = %d, want 500 (body: %s)", rr.Code, rr.Body.String()) + } + if _, statErr := os.Stat(filepath.Join(ws, "orphan.md")); statErr != nil { + t.Errorf("file should remain after upsert failure (orphan accepted): %v", statErr) + } + if len(bus.published) != 0 { + t.Errorf("no event should fire when upsert fails, got %d", len(bus.published)) + } +} + +func TestHandleUpdateDocument_EmptyContentClearsFile(t *testing.T) { + st := newFakeVaultStore() + st.docs["existing-1"] = &store.VaultDocument{ + ID: "existing-1", + TenantID: masterTenant.String(), + Path: "notes/keep.md", + Title: "Keep", + DocType: "note", + Scope: "shared", + } + bus := &fakeEventBus{store: st} + ws := t.TempDir() + h := &VaultHandler{store: st, eventBus: bus, workspace: ws} + + empty := "" + req := newJSONRequest(t, http.MethodPut, "/v1/vault/documents/existing-1", map[string]any{ + "content": empty, + }) + req.SetPathValue("docID", "existing-1") + rr := httptest.NewRecorder() + + h.handleUpdateDocument(rr, req) + + if rr.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body: %s)", rr.Code, rr.Body.String()) + } + fi, err := os.Stat(filepath.Join(ws, "notes", "keep.md")) + if err != nil { + t.Fatalf("stat cleared file: %v", err) + } + if fi.Size() != 0 { + t.Errorf("file size = %d, want 0 (empty content clears file)", fi.Size()) + } + if st.docs["existing-1"].ContentHash != vault.ContentHash([]byte("")) { + t.Errorf("content_hash = %q, want empty-content hash", st.docs["existing-1"].ContentHash) + } + if len(bus.published) != 1 { + t.Errorf("published %d events, want 1", len(bus.published)) + } +} From 1fad4e0b57e1f69b78fddcca5100c551f1528e76 Mon Sep 17 00:00:00 2001 From: Aaron Vu <54255926+aaron-tsar@users.noreply.github.com> Date: Thu, 28 May 2026 20:10:19 +0700 Subject: [PATCH 3/4] fix(vault): non-master tenant 500, O_NOFOLLOW, slog, *string content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses second review on #1174 (thieung): **B1 (blocking)** — first JSON content write into a fresh non-master tenant returned 500 because workspace/tenants/{slug}/ doesn't exist yet and EvalSymlinks fails with ErrNotExist. writeDocumentContent now MkdirAlls the workspace before resolving (mirrors handleUpload). Added a regression test that drives a non-master tenant context end-to-end and asserts the file lands under workspace/tenants/{slug}/. **I1** — replaced the Lstat→WriteFile sequence (TOCTOU window) with os.OpenFile(O_WRONLY|O_CREATE|O_TRUNC|O_NOFOLLOW). The Lstat short-fail stays as defense-in-depth + cleaner error logging. Portability handled via build-tagged constants in vault_handler_documents_nofollow_{unix, windows}.go — O_NOFOLLOW is no-op on Windows (desktop edition is single-user with no untrusted tenants). **I3** — every symlink/path-escape rejection now emits slog.Warn("security.vault_symlink_escape", ...) with the rejection site, resolved path and workspace, matching the storage.go pattern so oncall can alert on attempted escapes. **I4** — POST `content` field changed from `string` to `*string` to match PUT. Nil pointer = "no write" (metadata-only stub); empty string = "write a 0-byte file + fire event"; non-empty = write bytes. Updated handler godoc to spell out the symmetry. Switched `werr == os.ErrInvalid` to `errors.Is` per repo conventions. **Tests added** — non-master tenant first content write (B1), PUT with nil content (locks "no change" contract), handler-level ".." path rejection, POST content:"" writes empty file. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/http/vault_handler_documents.go | 70 ++++++--- .../vault_handler_documents_nofollow_unix.go | 20 +++ ...ault_handler_documents_nofollow_windows.go | 11 ++ internal/http/vault_handler_documents_test.go | 137 ++++++++++++++++++ 4 files changed, 219 insertions(+), 19 deletions(-) create mode 100644 internal/http/vault_handler_documents_nofollow_unix.go create mode 100644 internal/http/vault_handler_documents_nofollow_windows.go diff --git a/internal/http/vault_handler_documents.go b/internal/http/vault_handler_documents.go index 745a1251..30741714 100644 --- a/internal/http/vault_handler_documents.go +++ b/internal/http/vault_handler_documents.go @@ -1,6 +1,7 @@ package http import ( + "errors" "log/slog" "net/http" "os" @@ -23,6 +24,15 @@ import ( // pipeline (which reads files by path) can later compute summaries, embeddings // and links. func (h *VaultHandler) writeDocumentContent(workspace, relPath string, content []byte) (string, error) { + // Ensure the tenant workspace exists before resolving symlinks. Non-master + // tenants live under workspace/tenants/{slug}/ which handleUpload creates on + // demand (see vault_handler_upload.go) — without this, the very first JSON + // content write into a fresh tenant would fail EvalSymlinks with ErrNotExist + // and 500 the request. + if err := os.MkdirAll(workspace, 0o755); err != nil { + return "", err + } + // Resolve the workspace root through any symlinks so all containment checks // compare canonical paths. realWS, err := filepath.EvalSymlinks(workspace) @@ -33,12 +43,14 @@ func (h *VaultHandler) writeDocumentContent(workspace, relPath string, content [ // Lexical containment: blocks "../" and absolute-path escapes. if target != realWS && !strings.HasPrefix(target, realWS+string(os.PathSeparator)) { + slog.Warn("security.vault_symlink_escape", + "site", "lexical", "attempted", target, "workspace", realWS) return "", os.ErrInvalid } // Symlink containment: a lexical prefix check is not enough — a symlink that // lives *inside* the workspace but points outside it (e.g. workspace/link -> - // /etc) would otherwise let os.WriteFile follow it and clobber a file beyond + // /etc) would otherwise let the write follow it and clobber a file beyond // the workspace. Resolve the deepest already-existing ancestor of the target // and confirm it still canonicalises to a path inside the workspace, before // any directory is created or byte written. @@ -46,6 +58,8 @@ func (h *VaultHandler) writeDocumentContent(workspace, relPath string, content [ resolved, rerr := filepath.EvalSymlinks(ancestor) if rerr == nil { if resolved != realWS && !strings.HasPrefix(resolved, realWS+string(os.PathSeparator)) { + slog.Warn("security.vault_symlink_escape", + "site", "ancestor", "resolved", resolved, "workspace", realWS) return "", os.ErrInvalid } break @@ -64,12 +78,30 @@ func (h *VaultHandler) writeDocumentContent(workspace, relPath string, content [ return "", err } - // Refuse to follow a symlink planted at the final path component itself. + // Fast-fail when the final component is already a symlink — gives a clean + // rejection + security log without parsing OS-specific error codes. The + // O_NOFOLLOW open below closes the TOCTOU window this Lstat leaves open + // (an attacker could swap the file for a symlink between calls). if fi, lerr := os.Lstat(target); lerr == nil && fi.Mode()&os.ModeSymlink != 0 { + slog.Warn("security.vault_symlink_escape", + "site", "final_lstat", "target", target, "workspace", realWS) return "", os.ErrInvalid } - if err := os.WriteFile(target, content, 0o644); err != nil { + // O_NOFOLLOW makes the kernel refuse to follow a symlink at the final + // component atomically with the open, removing the Lstat→Open race entirely + // on Unix. On Windows oNoFollow == 0 and the Lstat above is best-effort. + f, err := os.OpenFile(target, os.O_WRONLY|os.O_CREATE|os.O_TRUNC|oNoFollow, 0o644) + if err != nil { + if isSymlinkLoopErr(err) { + slog.Warn("security.vault_symlink_escape", + "site", "open_nofollow", "target", target, "workspace", realWS) + return "", os.ErrInvalid + } + return "", err + } + defer f.Close() + if _, err := f.Write(content); err != nil { return "", err } return vault.ContentHash(content), nil @@ -199,15 +231,15 @@ func (h *VaultHandler) handleGetDocument(w http.ResponseWriter, r *http.Request) // handleCreateDocument creates a new vault document. // -// When `content` is supplied in the JSON body, the bytes are materialised on -// disk at / and the SHA-256 hash is stored on the -// document record. An EventVaultDocUpserted is then emitted so the enrichment -// worker computes summary/embeddings/links — same code path the multipart -// /v1/vault/upload endpoint and the filesystem rescan use. -// -// Omitting `content` preserves the historical behaviour of registering a -// metadata-only stub (typically followed by a rescan that materialises the -// file from disk). +// `content` semantics — symmetric with handleUpdateDocument: +// - field omitted (nil pointer): metadata-only stub, no file write, no event +// (typically followed by a rescan that materialises the file from disk). +// - field present and non-empty: bytes materialised at /, +// SHA-256 hash stored on the row, EventVaultDocUpserted emitted so the +// enrichment worker computes summary/embeddings/links — same code path the +// multipart /v1/vault/upload endpoint and the filesystem rescan use. +// - field present and empty (""): a 0-byte file is written and the event still +// fires. Same behaviour as PUT — pick the right shape on the client side. func (h *VaultHandler) handleCreateDocument(w http.ResponseWriter, r *http.Request) { locale := extractLocale(r) tenantID := store.TenantIDFromContext(r.Context()) @@ -216,7 +248,7 @@ func (h *VaultHandler) handleCreateDocument(w http.ResponseWriter, r *http.Reque var body struct { Path string `json:"path"` Title string `json:"title"` - Content string `json:"content"` // optional; when set, written to disk + Content *string `json:"content"` // nil=no write; ""=write empty file; "data"=write bytes (mirrors PUT) DocType string `json:"doc_type"` Scope string `json:"scope"` TeamID string `json:"team_id"` @@ -249,8 +281,8 @@ func (h *VaultHandler) handleCreateDocument(w http.ResponseWriter, r *http.Reque } // Validate extension upfront if caller wants to write content — matches the // whitelist enforced by /v1/vault/upload so the two endpoints accept the - // same file types. - if body.Content != "" { + // same file types. nil pointer = "no content field", which skips the check. + if body.Content != nil { ext := strings.ToLower(filepath.Ext(body.Path)) if !allowedUploadExts[ext] { writeJSON(w, http.StatusBadRequest, map[string]string{"error": "unsupported file type: " + ext}) @@ -288,16 +320,16 @@ func (h *VaultHandler) handleCreateDocument(w http.ResponseWriter, r *http.Reque writtenHash string contentWasSet bool ) - if body.Content != "" { + if body.Content != nil { wsPath = h.resolveTenantWorkspace(r.Context()) if wsPath == "" { writeJSON(w, http.StatusBadRequest, map[string]string{"error": "workspace not available"}) return } - hash, werr := h.writeDocumentContent(wsPath, body.Path, []byte(body.Content)) + hash, werr := h.writeDocumentContent(wsPath, body.Path, []byte(*body.Content)) if werr != nil { slog.Warn("vault.create: write content failed", "path", body.Path, "error", werr) - if werr == os.ErrInvalid { + if errors.Is(werr, os.ErrInvalid) { writeJSON(w, http.StatusBadRequest, map[string]string{"error": "path escapes workspace"}) return } @@ -405,7 +437,7 @@ func (h *VaultHandler) handleUpdateDocument(w http.ResponseWriter, r *http.Reque hash, werr := h.writeDocumentContent(wsPath, existing.Path, []byte(*body.Content)) if werr != nil { slog.Warn("vault.update: write content failed", "path", existing.Path, "error", werr) - if werr == os.ErrInvalid { + if errors.Is(werr, os.ErrInvalid) { writeJSON(w, http.StatusBadRequest, map[string]string{"error": "path escapes workspace"}) return } diff --git a/internal/http/vault_handler_documents_nofollow_unix.go b/internal/http/vault_handler_documents_nofollow_unix.go new file mode 100644 index 00000000..62eb2b81 --- /dev/null +++ b/internal/http/vault_handler_documents_nofollow_unix.go @@ -0,0 +1,20 @@ +//go:build !windows + +package http + +import ( + "errors" + "syscall" +) + +// oNoFollow is OR'd into the open flags so the kernel refuses to follow a +// symlink at the final path component — closes the Lstat→Open TOCTOU window +// where a tenant could swap a regular file for a symlink between the two +// syscalls. On Windows this is a no-op (see the _windows.go counterpart). +const oNoFollow = syscall.O_NOFOLLOW + +// isSymlinkLoopErr reports whether err is the kernel's "refused to follow +// symlink" signal from an O_NOFOLLOW open (ELOOP on Linux/macOS). +func isSymlinkLoopErr(err error) bool { + return errors.Is(err, syscall.ELOOP) +} diff --git a/internal/http/vault_handler_documents_nofollow_windows.go b/internal/http/vault_handler_documents_nofollow_windows.go new file mode 100644 index 00000000..ed76a65a --- /dev/null +++ b/internal/http/vault_handler_documents_nofollow_windows.go @@ -0,0 +1,11 @@ +//go:build windows + +package http + +// Windows has no O_NOFOLLOW concept and follows reparse points transparently; +// the Lstat short-circuit in writeDocumentContent is the only protection +// available there. Desktop edition is single-user with no untrusted tenants, +// so this degradation is acceptable. +const oNoFollow = 0 + +func isSymlinkLoopErr(err error) bool { return false } diff --git a/internal/http/vault_handler_documents_test.go b/internal/http/vault_handler_documents_test.go index 2840c905..a67329b7 100644 --- a/internal/http/vault_handler_documents_test.go +++ b/internal/http/vault_handler_documents_test.go @@ -339,3 +339,140 @@ func TestHandleUpdateDocument_EmptyContentClearsFile(t *testing.T) { t.Errorf("published %d events, want 1", len(bus.published)) } } + +// Locks the "no change" contract on PUT: omitting `content` (nil pointer in +// the body struct) must not touch the disk or fire an enrichment event, even +// when other metadata fields are present. +func TestHandleUpdateDocument_NilContentSkipsWriteAndEvent(t *testing.T) { + st := newFakeVaultStore() + st.docs["existing-2"] = &store.VaultDocument{ + ID: "existing-2", + TenantID: masterTenant.String(), + Path: "notes/keep.md", + Title: "Old", + DocType: "note", + Scope: "shared", + ContentHash: "preexisting-hash", + } + bus := &fakeEventBus{store: st} + ws := t.TempDir() + h := &VaultHandler{store: st, eventBus: bus, workspace: ws} + + // content field intentionally omitted → *string is nil → no write, no event. + req := newJSONRequest(t, http.MethodPut, "/v1/vault/documents/existing-2", map[string]any{ + "title": "Renamed but content untouched", + }) + req.SetPathValue("docID", "existing-2") + rr := httptest.NewRecorder() + + h.handleUpdateDocument(rr, req) + + if rr.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body: %s)", rr.Code, rr.Body.String()) + } + if _, statErr := os.Stat(filepath.Join(ws, "notes", "keep.md")); statErr == nil { + t.Error("nil content must not materialise a file") + } + if st.docs["existing-2"].ContentHash != "preexisting-hash" { + t.Errorf("content_hash = %q, want unchanged 'preexisting-hash'", st.docs["existing-2"].ContentHash) + } + if len(bus.published) != 0 { + t.Errorf("nil-content PUT must not publish events, got %d", len(bus.published)) + } +} + +// Handler-level guard at vault_handler_documents.go path validation must reject +// "..", short-circuiting before writeDocumentContent even runs. +func TestHandleCreateDocument_RejectsParentTraversalPath(t *testing.T) { + h := &VaultHandler{} // path check runs before any store/workspace access + req := newJSONRequest(t, http.MethodPost, "/v1/vault/documents", map[string]any{ + "path": "../../etc/passwd.md", + "title": "x", + "content": "evil", + }) + rr := httptest.NewRecorder() + + h.handleCreateDocument(rr, req) + + assertBadRequest(t, rr, "invalid path") +} + +// Mirrors PUT semantics on POST: `content: ""` writes a 0-byte file and +// fires the enrichment event (the I4 fix that aligned POST/PUT). +func TestHandleCreateDocument_EmptyContentWritesEmptyFile(t *testing.T) { + st := newFakeVaultStore() + bus := &fakeEventBus{store: st} + ws := t.TempDir() + h := &VaultHandler{store: st, eventBus: bus, workspace: ws} + + req := newJSONRequest(t, http.MethodPost, "/v1/vault/documents", map[string]any{ + "path": "placeholder.md", + "title": "Placeholder", + "content": "", + }) + rr := httptest.NewRecorder() + + h.handleCreateDocument(rr, req) + + if rr.Code != http.StatusCreated { + t.Fatalf("status = %d, want 201 (body: %s)", rr.Code, rr.Body.String()) + } + fi, err := os.Stat(filepath.Join(ws, "placeholder.md")) + if err != nil { + t.Fatalf("stat empty file: %v", err) + } + if fi.Size() != 0 { + t.Errorf("size = %d, want 0", fi.Size()) + } + if len(bus.published) != 1 { + t.Errorf("published %d events, want 1", len(bus.published)) + } +} + +// B1 regression: non-master tenants resolve to workspace/tenants/{slug}/ which +// is created on demand. The very first JSON content write into a fresh tenant +// must NOT 500 because EvalSymlinks can't find that dir — writeDocumentContent +// is responsible for creating it (mirrors what handleUpload does). +func TestHandleCreateDocument_NonMasterTenantFirstContentWrite(t *testing.T) { + st := newFakeVaultStore() + bus := &fakeEventBus{store: st} + ws := t.TempDir() + h := &VaultHandler{store: st, eventBus: bus, workspace: ws} + + nonMaster := uuid.MustParse("01999999-1111-7000-8000-000000000abc") + ctx := store.WithTenantSlug(store.WithTenantID(context.Background(), nonMaster), "acme") + + req := httptest.NewRequest(http.MethodPost, "/v1/vault/documents", bytes.NewReader(mustJSON(t, map[string]any{ + "path": "notes/first.md", + "title": "First", + "content": "hello from acme", + }))).WithContext(ctx) + req.Header.Set("Content-Type", "application/json") + rr := httptest.NewRecorder() + + h.handleCreateDocument(rr, req) + + if rr.Code != http.StatusCreated { + t.Fatalf("status = %d, want 201 (body: %s)", rr.Code, rr.Body.String()) + } + tenantDir := filepath.Join(ws, "tenants", "acme") + got, err := os.ReadFile(filepath.Join(tenantDir, "notes", "first.md")) + if err != nil { + t.Fatalf("read non-master tenant doc: %v", err) + } + if string(got) != "hello from acme" { + t.Errorf("content = %q", got) + } + if len(bus.published) != 1 { + t.Errorf("published %d events, want 1", len(bus.published)) + } +} + +func mustJSON(t *testing.T, v any) []byte { + t.Helper() + b, err := json.Marshal(v) + if err != nil { + t.Fatalf("marshal: %v", err) + } + return b +} From 9edcc00dbc64746663ddf594eab5ace0ffc659d2 Mon Sep 17 00:00:00 2001 From: Aaron Vu <54255926+aaron-tsar@users.noreply.github.com> Date: Thu, 28 May 2026 20:54:54 +0700 Subject: [PATCH 4/4] fix(vault): surface close errors + clarify slog label + drop test dup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trivial nits from thieung's review of ba59e7b0: - writeDocumentContent now closes the file explicitly and propagates any Close() error so a delayed FS failure (e.g. disk-full at flush) doesn't return success. - Renamed slog `site=lexical` to `site=path_traversal` — the lexical prefix check catches "../" / absolute-path escapes, not symlinks. Umbrella key `security.vault_symlink_escape` stays. - Non-master-tenant test now reuses newJSONRequest + WithContext, dropping the duplicate mustJSON helper. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/http/vault_handler_documents.go | 13 +++++++++---- internal/http/vault_handler_documents_test.go | 16 ++++------------ 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/internal/http/vault_handler_documents.go b/internal/http/vault_handler_documents.go index 30741714..e6ef22e7 100644 --- a/internal/http/vault_handler_documents.go +++ b/internal/http/vault_handler_documents.go @@ -44,7 +44,7 @@ func (h *VaultHandler) writeDocumentContent(workspace, relPath string, content [ // Lexical containment: blocks "../" and absolute-path escapes. if target != realWS && !strings.HasPrefix(target, realWS+string(os.PathSeparator)) { slog.Warn("security.vault_symlink_escape", - "site", "lexical", "attempted", target, "workspace", realWS) + "site", "path_traversal", "attempted", target, "workspace", realWS) return "", os.ErrInvalid } @@ -100,9 +100,14 @@ func (h *VaultHandler) writeDocumentContent(workspace, relPath string, content [ } return "", err } - defer f.Close() - if _, err := f.Write(content); err != nil { - return "", err + if _, werr := f.Write(content); werr != nil { + _ = f.Close() // best-effort cleanup; the write error is the real one + return "", werr + } + // Explicit Close so a delayed FS error (e.g. disk-full at flush) surfaces + // instead of being silently swallowed by a deferred close. + if cerr := f.Close(); cerr != nil { + return "", cerr } return vault.ContentHash(content), nil } diff --git a/internal/http/vault_handler_documents_test.go b/internal/http/vault_handler_documents_test.go index a67329b7..fa593f2b 100644 --- a/internal/http/vault_handler_documents_test.go +++ b/internal/http/vault_handler_documents_test.go @@ -442,12 +442,13 @@ func TestHandleCreateDocument_NonMasterTenantFirstContentWrite(t *testing.T) { nonMaster := uuid.MustParse("01999999-1111-7000-8000-000000000abc") ctx := store.WithTenantSlug(store.WithTenantID(context.Background(), nonMaster), "acme") - req := httptest.NewRequest(http.MethodPost, "/v1/vault/documents", bytes.NewReader(mustJSON(t, map[string]any{ + // newJSONRequest builds against masterCtx by default; WithContext swaps in + // the non-master tenant context for this single test. + req := newJSONRequest(t, http.MethodPost, "/v1/vault/documents", map[string]any{ "path": "notes/first.md", "title": "First", "content": "hello from acme", - }))).WithContext(ctx) - req.Header.Set("Content-Type", "application/json") + }).WithContext(ctx) rr := httptest.NewRecorder() h.handleCreateDocument(rr, req) @@ -467,12 +468,3 @@ func TestHandleCreateDocument_NonMasterTenantFirstContentWrite(t *testing.T) { t.Errorf("published %d events, want 1", len(bus.published)) } } - -func mustJSON(t *testing.T, v any) []byte { - t.Helper() - b, err := json.Marshal(v) - if err != nil { - t.Fatalf("marshal: %v", err) - } - return b -}