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] 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)