security(llc): a work product's storage_path is confined to the LLC root but not to its own tenant, so a known UUID reads across companies #17416

What

#17302 (PR #17408) confined LLCWorkProduct.storage_path to the LLC storage root before opening it, which closes the arbitrary-host-file read. It did not scope that containment to the product's own tenant.

llc/kb/artifact_ingestor.py:_contained_storage_path validates against one global root:

root = llc_local_storage_root()
return str(validate_path(raw, allowed_roots=(str(root.resolve()),)))

Attachments for every company live under that same root, laid out as <root>/<company_id>/<work_item_id>/<attachment_id><ext> (llc/services/attachment_service.py:_storage_path). So a path pointing at another company's attachment is inside the root and passes.

An authenticated agent for company A that knows or guesses company B's company_id, work_item_id and attachment_id can set storage_path to B's attachment and have its contents ingested into A's knowledge index, where they become retrievable.

Why it is filed rather than fixed in #17408

#17302's acceptance criteria made the strong form explicit and optional:

Preferably storage_path stops being caller-supplied at all: the service derives it, the way #17300 made the attachment route derive its path from ctx.org_id

The mandatory criterion was containment, which is delivered and tested. Widening #17408 from "confine it" to "derive it server-side" changes the shape of the fix and needs its own tests, which is the split this repo asks for rather than a larger single review.

Exploitability, stated honestly

Bounded by UUID unguessability, not by a control. All three path segments are UUIDv4 and _storage_path parses each one, so the attacker needs three specific values rather than a traversal primitive. No enumeration or leak of those UUIDs was found in the reviewed diff — but "we could not find a way to learn the UUIDs" is a weaker guarantee than "knowing them does not help", and only the second is a containment property.

Work item and attachment ids are also not uniformly secret: they appear in API responses, logs and UI state for anyone with legitimate access to a work item, and a cross-tenant path only needs them to have leaked once.

Acceptance criteria

• Either storage_path is derived server-side from the product's own company_id (the security(llc): an authenticated user can write a file anywhere on disk via the attachment company_id query param #17300 pattern), or containment is scoped to <root>/<product.company_id>/ rather than to <root>/
• A test proves a storage_path pointing at another company's attachment is refused, with both companies' trees actually present on disk — a test with only one tenant's directory passes against the current code
• A test proves the product's own in-tenant path still ingests (the contrast)
• Proved by mutation: widening containment back to the global root must redden the cross-tenant test by name
• Existing rows are considered: a pre-existing storage_path that is in-root but not in-tenant should be refused rather than silently ingested, and the refusal logged so an operator can see whether any exist

Provenance

Found by the code-reviewer pass on PR #17408, reported as out of that PR's mandatory scope. Recorded here rather than dropped.

Related: #17302, #17300.