VH-Lab / VH-Lab/DID-python

Optional ingest-time gating for untrusted document sources

Open
#63 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
0
Forks
1
Avg merge
2h 15m
Merged PRs (30d)
40

Description

Context

Follow-up to #58 and #60. After #60 (PR #62), _ingest_location refuses only unsafe uid values via _is_safe_uid — the ingest source location is trusted verbatim (_resolve_local, no containment check), on the reasoning that the caller who says "add this file to my DB" chose the source. This unblocks legitimate ingest workflows (NDI-python stages sources under /tmp/ndi-vhsb-*/ and adds them to a DB whose .ndi lives elsewhere) and keeps the destination fully constrained under <FileDir>/<uid>.

The read-side _is_safe_local_location filter (_locations_from_files_table) 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. shutil.copyfile resolves the traversal, reads the target file, and writes its contents into <FileDir>/<uid>.
  • On a later open_doc, the read-side filter refuses orig_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-python 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 #60 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 (a relative path whose _resolve_local result escapes db_dir) — this refuses the ../../etc/passwd shape without refusing an absolute path that legitimately lives outside db_dir. It's stricter than "no traversal", weaker than "must be inside db_dir".
  • Or make the guard opt-in via add_docs(..., trust_sources=True|False), defaulting to True (today's behavior) and set to False by callers who pull documents from a cloud store. Downstream (NDI cloud pull) then sets trust_sources=False and gets the strict containment check.

Either would need matching changes on the MATLAB side and a bridge sync note. See the DID-matlab companion issue for the parity version.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

No implementation is selected yet. Read _ingest_location, _resolve_local, _locations_from_files_table, and add_docs to understand the current trust behavior, then review the DID-matlab companion issue and bridge sync requirements. Done would require an agreed optional gating design, matching Python and MATLAB changes, and tests for trusted and attacker-authored sources.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
database, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.