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] 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) <noreply@anthropic.com>
---
 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
-}