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