fix(consume): every read path into a bundle is contained, not just okf_fetch

`okf_fetch` resolved a concept through `connectors.safe_resolve` from the day
the server was written. The other three ways into the same bytes did not.
`okf consume` and `okf_ask` reach `consume.build_payload`, `okf_describe`
reaches `mcp_server.card`, and both built the concept path by joining the
index's own name onto the bundle root. `consume._join` refuses a `..` segment
and an absolute target, but it is a STRING rule over the index text, and a
symlink is a fact about the filesystem that reading that text cannot see: the
index could name `lekkasje.md`, that name could be a link to a file outside the
bundle, and the file came back in the answer.

Measured before the fix, on a bundle carrying one honest concept and one
escaping link: 8 of 11 new rows red, the 3 green ones being `okf_fetch` on the
same two links and the known-positive that the clean bundle still answers. So
the suite was not red for an unrelated reason, and the fix is not "refuse every
bundle holding a link".

One place, not three copies: `consume.resolve_in_bundle` makes the check and
`consume.read_path_in_bundle` adds the file's presence. Every reader here goes
through them -- the index walk, the ref, the document prior, the payload, the
card, `okf_fetch`, and the three outside `consume` (`skill`, `quality`,
`project`) that joined the same way.

Two more failure modes in the same check, because they are the same question:

* A NAMED PIPE is not a regular file. `read_text` on one blocks for as long as
  nobody writes to it, which on a server is the whole process; the red row for
  it ran 60 s to a subprocess deadline and now returns in under a second.
* A DEAD INDEX LINK raised `FileNotFoundError`, and the broad handler in
  `handle` wrote `{error}` into the refusal -- the SERVER's absolute path,
  handed to whoever asked, over one index entry naming a file nobody wrote.
  It is `concept_unreadable` now, naming the concept and not the machine.

The returned path is the JOINED one, never the resolved one: `read_concept`
derives a concept id by taking the read path relative to the bundle root, and
once containment holds the two are the same bytes.

2334 passed, 2 skipped (was 2323 + 2). `mypy --strict src/` clean over 25 files.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-09-20 15:20:38 +02:00
commit bf697bfcad
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
6 changed files with 310 additions and 15 deletions

View file

@ -54,7 +54,9 @@ from dataclasses import dataclass, replace
from pathlib import Path
from typing import Literal
from .connectors import safe_resolve
from .corpus import LOG_NAME
from .errors import SourceError
from .inbox import (
ADJUDICATION_ADJUDICATED,
ADJUDICATION_PROPOSED,
@ -106,7 +108,7 @@ def _walk_index_tree(
if relative in indexes:
continue
indexes.add(relative)
index_file = bundle_root / relative
index_file = resolve_in_bundle(bundle_root, relative)
if not index_file.is_file():
continue
parent = _parent_dir(relative)
@ -201,6 +203,60 @@ def _join(parent: str, target: str) -> str | None:
return "/".join(parts) if parts else None
def resolve_in_bundle(bundle_root: Path, relative: str) -> Path:
"""The ONE containment check every read path into a bundle goes through.
`_join` above refuses a `..` segment and an absolute target, but it is a
STRING rule over the index text, and a symlink is a fact about the
filesystem that no amount of reading that text can see. So this resolves
the canonical path (`connectors.safe_resolve`) and refuses anything that
lands outside the root -- the check `okf_fetch` has always made, now made
in the one place every reader here passes through instead of in the one
tool that happened to have it.
The same check also refuses a target that exists and is NOT a regular
file, because that is the other way a name reaches something the bundle
does not hold: a named pipe is opened without complaint and blocks the
reader for as long as nobody writes to it, which on a server is the whole
process.
Returns the path as JOINED, never as resolved: the resolved form is the
symlink's target, and `read_concept` derives a concept id by taking the
read path relative to the bundle root. Once containment holds, reading
through the link and reading the file are the same bytes.
"""
try:
safe_resolve(bundle_root, relative)
except SourceError as error:
raise ConsumeError(
f"`{relative}` resolves outside the bundle that names it; refused rather than read",
code="path_escape",
) from error
joined = bundle_root / relative
if joined.exists() and not joined.is_file():
raise ConsumeError(
f"`{relative}` names something that is not a regular file; refused rather than opened",
code="path_escape",
)
return joined
def read_path_in_bundle(bundle_root: Path, relative: str) -> Path:
"""`resolve_in_bundle`, and the file has to be there.
Refuses by NAME. A `FileNotFoundError` escaping from here carried the
reader's own absolute path into an answer, over one index entry naming a
file nobody wrote -- a fact about the machine, handed to whoever asked.
"""
path = resolve_in_bundle(bundle_root, relative)
if not path.is_file():
raise ConsumeError(
f"the index names `{relative}`, and there is no readable file there",
code="concept_unreadable",
)
return path
def bundle_ref(bundle_root: Path, *, profile: BundleProfile = DEFAULT_PROFILE) -> str:
"""A content identity for the bundle: `sha256-tree:<hex>`.
@ -223,7 +279,7 @@ def bundle_ref(bundle_root: Path, *, profile: BundleProfile = DEFAULT_PROFILE) -
suffix = profile.paths.concept_suffix
lines: list[bytes] = []
for relative in (*indexes, *(f"{concept}{suffix}" for concept in concepts)):
path = bundle_root / relative
path = resolve_in_bundle(bundle_root, relative)
if not path.is_file():
continue
digest = hashlib.sha256(path.read_bytes()).hexdigest()
@ -1206,7 +1262,10 @@ def document_scores(
document = relative.split("/", 1)[0]
if document == profile.index.name:
continue
for line in (bundle_root / relative).read_text(encoding="utf-8").splitlines():
index_file = resolve_in_bundle(bundle_root, relative)
if not index_file.is_file():
continue
for line in index_file.read_text(encoding="utf-8").splitlines():
entry = profile.index.parse_entry(line)
if entry is None:
continue
@ -2073,7 +2132,7 @@ def root_bundle_id_of(bundle_root: Path, *, profile: BundleProfile = DEFAULT_PRO
"""The `bundle_id` the root index declares, or a refusal naming which half
of SS 3.1's identity tuple is missing. Shared with the skill generator, so
the two agree on what makes a directory a readable bundle."""
root_index = bundle_root / profile.index.name
root_index = resolve_in_bundle(bundle_root, profile.index.name)
if not root_index.is_file():
raise ConsumeError(
f"{bundle_root} carries no {profile.index.name}, so there is no index "
@ -2210,7 +2269,7 @@ def build_payload(
concepts = link_parents(
[
read_concept(
bundle_root / f"{concept_id}{profile.paths.concept_suffix}",
read_path_in_bundle(bundle_root, f"{concept_id}{profile.paths.concept_suffix}"),
bundle_root=bundle_root,
root_bundle_id=root_bundle_id,
)

View file

@ -39,6 +39,13 @@ if the bundle's own index names it (`consume.enumerate_concepts`, which
refuses a target climbing above the root) AND its resolved path is inside the
bundle (`connectors.safe_resolve`, on canonical paths). Either alone would be
defensible; the pair is what makes a defect in one of them survivable.
AND IT IS EVERY READ PATH, not the one tool that happened to have it. Until
`consume.resolve_in_bundle` existed, the second check was made by `okf_fetch`
alone: `okf_ask` and `okf_describe` joined the index's own name onto the root
and opened whatever was there, so a link out of the bundle was read and
delivered. The index rule is a STRING rule -- it cannot see a symlink -- which
is exactly why one of the two checks is not enough.
"""
from __future__ import annotations
@ -53,7 +60,6 @@ from typing import Any, TextIO
from . import consume as okf_consume
from . import materialize
from .connectors import safe_resolve
from .errors import SourceError
from .profiles import BundleProfile
@ -276,7 +282,9 @@ def card(bundle_root: Path, *, profile: BundleProfile, concept_sample: int = 50)
concepts = okf_consume.link_parents(
[
okf_consume.read_concept(
bundle_root / f"{concept_id}{profile.paths.concept_suffix}",
okf_consume.read_path_in_bundle(
bundle_root, f"{concept_id}{profile.paths.concept_suffix}"
),
bundle_root=bundle_root,
root_bundle_id=bundle_id,
)
@ -489,10 +497,11 @@ def call_fetch(surface: Surface, arguments: Mapping[str, Any]) -> dict[str, Any]
code="concept_unknown",
)
suffix = surface.profile.paths.concept_suffix
try:
path = safe_resolve(served.root, f"{concept_id}{suffix}")
except SourceError as error:
raise ToolError(str(error), code="path_escape") from error
# The SECOND of the two independent checks, and since the read paths were
# unified it is the same one `okf_ask` and `okf_describe` make. Left as its
# own call rather than folded into the index check above: a defect in one
# of the two is survivable only while the other is still asked.
path = okf_consume.read_path_in_bundle(served.root, f"{concept_id}{suffix}")
size = path.stat().st_size
if size > MAX_CONCEPT_BYTES:
raise ToolError(

View file

@ -98,7 +98,9 @@ def inventory(folder: Path, bundle: Path) -> tuple[tuple[str, ...], tuple[str, .
whole: set[str] = set()
root_bundle_id = consume.root_bundle_id_of(bundle, profile=SEGMENTED_OKF_V0_2)
for concept_id in consume.enumerate_concepts(bundle, profile=SEGMENTED_OKF_V0_2):
path = bundle / f"{concept_id}{SEGMENTED_OKF_V0_2.paths.concept_suffix}"
path = consume.read_path_in_bundle(
bundle, f"{concept_id}{SEGMENTED_OKF_V0_2.paths.concept_suffix}"
)
concept = consume.read_concept(path, bundle_root=bundle, root_bundle_id=root_bundle_id)
represented.add(concept.source_file)
# A concept id with no `/` sits at the bundle root rather than under a

View file

@ -41,7 +41,13 @@ from collections import Counter
from dataclasses import dataclass
from pathlib import Path
from .consume import ConsumeError, enumerate_concepts, read_concept, root_bundle_id_of
from .consume import (
ConsumeError,
enumerate_concepts,
read_concept,
read_path_in_bundle,
root_bundle_id_of,
)
from .corpus import LOG_NAME
from .profiles import SEGMENTED_OKF_V0_2, BundleProfile
@ -487,7 +493,7 @@ def measure_bundle(
pairs: set[tuple[str, str]] = set()
for concept_id in enumerate_concepts(bundle_root, profile=profile):
concept = read_concept(
bundle_root / f"{concept_id}{profile.paths.concept_suffix}",
read_path_in_bundle(bundle_root, f"{concept_id}{profile.paths.concept_suffix}"),
bundle_root=bundle_root,
root_bundle_id=root_bundle_id,
)

View file

@ -347,7 +347,9 @@ def render(
concepts = okf_consume.link_parents(
[
okf_consume.read_concept(
bundle_root / f"{concept_id}{profile.paths.concept_suffix}",
okf_consume.read_path_in_bundle(
bundle_root, f"{concept_id}{profile.paths.concept_suffix}"
),
bundle_root=bundle_root,
root_bundle_id=bundle_id,
)