PR #156 round 3: a readRawNote rewrite to os.ReadFile or Root.ReadFile
passed the lint and brought back the escape (19k leaks in 4s) and the
FIFO hang. Listing forbidden APIs is whack-a-mole, so the control now
rests on behaviour:
- TestRawReadersLeakNothingUnderASymlinkSwapRace and
TestRawReadersDoNotHangUnderAFIFOSwapRace swap a regular note, a
symlink to an outside file, or a FIFO into raw/r.md while ListPending
and ReadIngestFile loop for 1.5s; any leaked byte or a hang fails.
- TestProcessDir_DoesNotHangUnderAFIFOSwapRace does the same for the
unattended watcher (read and archive copy).
Flipped: os.ReadFile -> leak + hang tests red; Root.ReadFile -> hang
test red; copyFromRoot back to Root.Open -> watcher test red.
The lint stays as a second layer: it also forbids ReadFile, matches the
selector (so a stored method value is caught), refuses a dot-import of
extract, and says it only covers the three files it parses.
Refs #153
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PR #156 round 2: the FIFO fix lives in OpenRegular, and reverting any of
its three callers to a plain Root.Open stayed green; the extract.Text
guard was a text search that an aliased import or a func value passed.
- TestRawReadersOpenOnlyThroughOpenRegular parses rawroot.go,
extract.go and watcher.go and fails on any .Open/.OpenFile call
outside OpenRegular, except a named os.OpenFile log append.
- TestNoPackageReferencesExtractTextOutsideExtract resolves the extract
import under any alias and fails on any reference to its Text,
called or taken as a value.
Each was flipped: the three caller reverts, an os.Open revert, an
alias and a func value all go red.
Refs #153
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Reverting the ingest call sites to extract.Text(path) stayed green;
a test now fails if anything outside internal/extract calls
extract.Text( (only TextIn, through an os.Root, may be used).
- The watcher symlink test passed for a watcher that ingested nothing,
or stopped at the first symlink (SkipAll: one synced link halting all
ingestion). It now plants a real note after the symlink and asserts
exactly one ingest, and the symlinked-raw refusal has its own test.
- A FIFO swapped in between Lstat and Open hung open(2) forever.
extract.OpenRegular opens non-blocking and checks the type on the
handle; readRawNote, TextIn and the watcher's archive copy use it.
- The symlinked-root test covers pending, promote, reject, resolve and
ReadIngestFile, and is named for exactly those.
- The hard-link residual is documented where it is accepted.
Out of scope, filed: #157 (symlinks elsewhere in the Syncthing-shared
brain dir reach search and other readers), #158 (writes under raw/
follow symlinks).
Refs #153
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
raw/ is writable via Syncthing (a sendreceive folder shared with
flamingo, symlinks synced) and by the brain CronJobs, and several
readers followed symlinks out of it:
- /pending and brain_pending showed a symlink target's contents;
brain_promote would copy it into wiki/ (then brain-sync to Gitea);
brain_reject would copy it into raw/rejected/.
- The watcher, which needs no API call at all, ingested symlinked files
(WalkDir yields them; extract.Text followed them). Not in the issue;
found while fixing it.
- Ingest checked a path then reopened it by name (TOCTOU).
OpenRawRoot opens brain/raw as an os.Root and refuses a raw/ that is
itself a symlink. ListPending, PromoteNote and RejectNote read through
it and refuse anything not a regular file. extract.TextIn reads through
a root (temp copy for pdf/docmark); /ingest-path, brain_ingest and the
watcher use it, and the watcher archives via the root too, so a file
swapped for a symlink after a check still cannot lead out.
Refs #153
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Found via a real live-cluster end-to-end test (POST /ingest-path with a real
.docx against the deployed ingestion + docmark): 'decode docmark response:
invalid character e looking for beginning of value'. docmark's Streamable-HTTP
transport frames every response as SSE (event: message\r\ndata: {...}\r\n\r\n,
Content-Type: text/event-stream) -- stateless_http=True removes the need for
an initialize handshake/session ID, it does NOT change the wire framing to
bare JSON. My original client + its own test mock both assumed bare JSON,
so the unit tests passed while the real call failed.
sseDataPayload() extracts the data: line before JSON-unmarshaling; falls back
to the raw body if no SSE framing is present (forward-compatible). Test mock
(docmark_test.go) now emits the SAME framing the live server actually sends,
confirmed by directly probing docmark's live response before writing the fix.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wires the sovereign docmark MCP server (git.d-ma.be/mathias/docmark) as the
converter for formats ingestion previously couldn't handle at all -- DOCX,
XLSX, PPTX, PNG/JPG. A single self-contained tools/call POST (docmark runs
stateless_http, no session handshake needed) does the conversion.
- internal/extract/docmark.go: extractViaDocmark(path) -- reads DOCMARK_URL +
DOCMARK_BEARER_TOKEN from env, base64-encodes the file, calls docmark's
convert_to_markdown tool, surfaces tool/HTTP errors with context.
- internal/extract/extract.go: routes .docx/.xlsx/.pptx/.png/.jpg/.jpeg to it.
- internal/api/handler.go: isSupportedExtension() replaces the static
supportedExtensions map -- the new extensions are gated on DOCMARK_URL being
set, checked per-request. When DOCMARK_URL is unset (e.g. this repo's
current deployed state), behavior is byte-for-byte unchanged from today:
same 400/silent-skip as before. Zero risk until the env var is actually
wired in infra.
TDD throughout: docmark.go tested via httptest mock (success, not-configured,
tool-error, HTTP-error, all 6 new extensions route correctly); handler-level
test proves the DOCMARK_URL gate (docx 400 when unset, 200 when a working
mock is configured). Full task check green (fmt/vet/govulncheck/all tests).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>