fix(vault): surface close errors + clarify slog label + drop test dup

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>
This commit is contained in:
Aaron VuandClaude Opus 4.7 committed 2026-05-28 20:55:23 +07:00
1 parent 1fad4e0b57
commit 9edcc00dbc
2 files changed
+13 -16

No files matched your search

+9 -4
View File
@@ -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
}
+4 -12
View File
@@ -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
}