Optional ingest-time gating for untrusted document sources
Nobody has claimed this yet.
- Dominant language
- MATLAB
- Stars
- 1
- Forks
- 2
- Avg merge
- 2h 18m
- Merged PRs (30d)
- 39
Description
Context
Follow-up to #167 and #169. After #169 (PR #171), validateIngestFileEntry refuses only unsafe uid values — the ingest source location is trusted verbatim, on the reasoning that the caller who says "add this file to my DB" chose the source. This unblocks legitimate ingest workflows (NDI-matlab stages sources under /tmp/nditemp/ and adds them to a DB whose .ndi lives elsewhere) and keeps the destination fully constrained under <FileDir>/<uid> via isSafeUid.
The read-side isSafeLocalLocation filter (do_open_doc line 767) still defends orig_location, so a crafted stored location cannot steer open_doc to read outside db_dir through the orig_location branch.
The gap
There is one code path that ingestion isn't gated against, and it only matters when the document JSON is attacker-controlled (a cloud pull, not a locally authored document):
- A document with
location='../../etc/passwd'(or any traversal / absolute-outside path pointing at a readable file) now ingests successfully on POSIX.copyfileresolves the traversal, reads the target file, and writes its contents into<FileDir>/<uid>. - On a later
open_doc, the read-side filter refusesorig_location, but by then the cache candidate at<FileDir>/<uid>— built from the safe uid — already exists and wins the earlier loop; the smuggled contents are served back.
There is no such issue when the source of the document is trusted (the local NDI-matlab call, say). It is specifically the cloud-pull path where the JSON is not the caller's.
What we're not doing
For now, no change. The trade-off from #169 stands: reintroducing a source-side containment check breaks the legitimate workflow, and this residual surface only matters for the "attacker-authored document" case — one downstream packages can also mitigate by not blindly ingesting cloud-pulled documents.
What an optional gate might look like
If we do decide to close this later:
- Narrow the guard to relative locations with a traversal segment (
containsTraversal(location) && !isAbsolutePath(location)after rebasing againstdb_dir) — this refuses the../../etc/passwdshape without refusing an absolute path that legitimately lives outsidedb_dir. It's stricter than "no traversal", weaker than "must be inside db_dir". - Or make the guard opt-in via an
add_docs(..., 'trustSources', true|false)name-value, defaulting totrue(today's behavior) and set tofalseby callers who pull documents from a cloud store. Downstream (NDI cloud pull) then setstrustSources=falseand gets the strict containment check.
Either would need matching changes on the Python side and a bridge sync note. See the DID-python companion issue for the parity version.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with validateIngestFileEntry, the do_open_doc filter near line 767, and the add_docs entry point; review the proposed containsTraversal/isAbsolutePath behavior and the Python companion issue. No implementation is currently selected—the issue says no change for now—so completion would require an agreed gate design, matching Python changes, and a bridge sync note.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- matlab, python
- Domain
- databases, security
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100