Replace the PATH_ALLOW_BROAD environment variable with a recorded grant,
the shape agreed in review: `codebase-memory-mcp allow-root
--approve-broad <path>` records an explicit human approval for one root
refused as too broad (CBM_WS_DENY_TOO_SHALLOW), mirroring
--approve-sensitive end to end.
- cbm_workspace_grant_add() takes approve_broad next to
approve_sensitive. Each flag lifts only its own verdict, absolute
denials stay unliftable, and a refusal names the flag that would
lift it.
- The approval is stored with a '^' marker, listed by allow-root --list
as "(approved broad)", and honoured by cbm_workspace_root_allowed()
only for CBM_WS_DENY_TOO_SHALLOW and only on an exact path match,
keeping the exact-match rule of the original change.
- cbm_workspace_root_allowed() returns to its previous signature. Every
caller (MCP index_repository and index-job start, auto-index, daemon
sessions, the HTTP UI) reads the same grant store, so stdio and
daemon sessions decide identically and nothing depends on the
environment a daemon was started with.
- The too-broad refusal names `allow-root --approve-broad <path>`, a
command a person runs.
- cbm_workspace_classify_root() and cbm_workspace_verdict_is_overridable()
are unchanged.
The regression test keeps its name and its exact-match, sibling,
absolute and sensitive checks, now against the grant store, and adds
that a broad marker never lifts a sensitive verdict.
Co-authored-by: Mark Murawski <github@kobaz.net>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Resolve the conflict in src/mcp/mcp.c: main moved the index_repository
boundary-check cleanup into index_args_free(); keep that, and keep this
branch's extra cbm_workspace_root_allowed() argument. Pass the same
argument at the index-job call site main added, so the merged tree
builds. The next commit replaces the argument with a recorded grant.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The Windows half of ws_approve_sensitive_matches_across_separator_styles
asserted the sensitive-root hint for <cache>/.ssh/cfg/sub without creating
that directory. Containment canonicalizes the candidate, which on Windows
opens it (CreateFileW, OPEN_EXISTING), so a missing descendant was reported
as outside the allowed root and the assertion failed on the Windows shard.
Create it the way its parent is created.
The grant file is now written through cbm_fopen, the wrapper every
repository or user path goes through, in binary mode.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
main landed the same separator-equivalent comparison in 2e6dc087
(ws_prefix_equal), so the conflict in ws_is_ancestor_or_equal resolves to
main's helper; the #1718 comment above the function is kept. The two
test-file conflicts were both sides adding tests at the same place, and
keep both.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
A project's .cbmpathwhitelist lists extra roots it would like indexed.
When such a manifest was approved, and again each time it was consulted,
every entry was classified exactly as written in the file, while the
directory that would actually be indexed is the one the entry resolves
to. An entry that reached the home directory, a credential directory or
the volume root through a link or through `..` segments was judged by
its spelling and passed as an ordinary path; only the containment check
resolved it. An entry naming a directory that did not exist was approved
as well.
Both cbm_workspace_manifest_approve and cbm_workspace_manifest_allows now
resolve each entry with cbm_canonical_path, the resolver every other
caller of the policy applies, and classify that form. Approval refuses an
entry that is relative, does not exist or is not a directory, with a
message that names the entry and, when the resolved directory differs
from what was written, shows both. Approval stays bound to the manifest
text; where an entry leads is read again when it is used, so a link that
is retargeted after approval is judged by its new target. At use time an
entry that no longer resolves matches nothing, and the other entries of
the manifest are consulted as before.
An ordinary, existing, absolute entry keeps its result. A manifest that
names a directory which does not exist yet is now refused at approval
and has to be approved once the directory is there.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
cbm_workspace_home_dir() returned the raw HOME (or USERPROFILE) string,
while cbm_workspace_classify_root() compares it against a canonical,
link-resolved path. Where the home directory is reached through a link
(/home kept on another volume, a macOS firmlink, a HOME that is itself a
link) the two spellings differ and the home rule never fired, so the home
directory was not classified as the home directory.
The helper now resolves the value through cbm_canonical_path, the same
resolver every caller applies to the path it classifies, and falls back
to the raw value when the directory cannot be resolved. The result lives
in a per-thread buffer recomputed on each call, as cbm_resolve_cache_dir
does, so a changed environment is seen at once. The environment is read
with cbm_safe_getenv so a non-ASCII USERPROFILE arrives as UTF-8.
The ancestor comparison now treats "/" and "\" as the same separator: on
Windows the resolver returns backslashes while some callers normalize to
forward slashes, and both spell one directory.
Two tests cover the helper: one with a plain temp directory (already a
firmlink on macOS), one with a home reached through a symbolic link
(POSIX; creating a link needs a privilege on Windows). The existing
explicit-approval test in the mcp suite now presents its session root in
resolved form, as the daemon does; with the raw spelling it would have
passed as an ordinary root and stopped exercising the approval path.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
CBM_WS_DENY_TOO_SHALLOW ("path is too broad to index as one root") has
no override today: cbm_workspace_verdict_is_overridable() only lifts
CBM_WS_DENY_SENSITIVE, and `allow-root --list` already documents
shallow/absolute refusals as "always-refused" rather than configurable.
That default is right -- a bare top-level tree like "/etc" or "/home"
is almost always a mistake -- but it leaves no way to say "I mean this
one specific broad root, on purpose." That's a real case: a person who
deliberately wants one combined project spanning several sibling repos
under a shared parent has no path forward today short of physically
restructuring their checkout.
PATH_ALLOW_BROAD names one exact canonical path that lifts a
CBM_WS_DENY_TOO_SHALLOW verdict, and only that verdict:
- exact match only, not a prefix -- naming one broad root must not
quietly approve every root below it too, the same "/Users"-style
breadth this depth rule exists to catch in the first place
- process environment, not a recorded grant: it never appears in
cbm_workspace_grant_list, and cbm_workspace_verdict_is_overridable()
is untouched, so CBM_WS_DENY_ABSOLUTE and CBM_WS_DENY_SENSITIVE stay
exactly as unliftable as before
- read once per call site with getenv(), mirroring how CBM_ALLOWED_ROOT
is already threaded into cbm_workspace_root_allowed() from each of
its four callers, rather than read inside
cbm_workspace_classify_root() itself -- that function is documented
as a pure function of its arguments precisely so it stays
host-independent and directly testable
Also updates the refusal message to name the fix, the same way the
sensitive-root refusal already names `allow-root --approve-sensitive`.
Adds ws_allow_broad_root_lifts_too_shallow_for_an_exact_match_only
alongside the existing too-shallow coverage, checking the exact-match
requirement, the untouched sensitive/absolute paths, and that nothing
is written to the grant store.
Signed-off-by: Mark Murawski <github@kobaz.net>
The Windows sensitive-root escape hatch broke on separator spelling: the
grant is recorded from cbm_canonical_path (backslashes) while the indexer
hands the candidate normalized to '/'. ws_is_ancestor_or_equal compared
bytes, so an approved 'Program Files' root never matched the candidate and
--approve-sensitive could not lift the refusal.
Compare component boundaries with separator equivalence, consistent with
ws_is_sep used everywhere else in this module. The containment rule (a
separator byte is never part of a component name) is unchanged.
Fixes#1718
Signed-off-by: Ulises Millan Guerrero <ulises.millanguerrero@gmail.com>
A repository may ship .cbmpathwhitelist listing outside roots it would like
indexed alongside it. The file REQUESTS; it never GRANTS. Both attackers in this
project's threat model can write a file inside a repository — a malicious indexed
repo, and an agent with write access to the project — so a repo-local file cannot
be authoritative without handing them the boundary.
Approval is recorded user-level and keyed to a SHA-256 of the manifest bytes, so
editing the file lapses approval and it must be granted again. Without the hash a
repo approved once could widen itself forever on a later pull. This is direnv's
model, and it is the only shape that keeps a checked-in file's ergonomics without
making the file a permission slip. The hash covers the raw bytes rather than the
parsed entries: a reordering or comment change is still a change the approver has
not seen.
`allow-root --approve-manifest <project>` is the human action that grants.
Approval refuses outright if any requested entry would not stand on its own as an
indexing root, so approving cannot become a route around the breadth policy, and
entries are re-classified at use time as well so a stored approval cannot outrank
a credential list that has grown since.
Entries containing control characters are rejected — the same shape as the
newline splitting that let a crafted indexed path inject an extra entry into the
scoped file list.
Deliberately NOT wired into the root boundary. A project's manifest authorizes
outside roots FOR THAT PROJECT, so the question is "may project P pull in tree T",
not "may T be indexed standalone", and the boundary check only ever sees one path
with no project context to ask that with. A first draft did wire it there by
passing the candidate as its own project root, which read a manifest that by
definition was not the one requesting it and so authorized nothing — caught by
exercising it end to end. The consuming half belongs where project context exists,
in discovery walking a project's approved extra roots;
cbm_workspace_manifest_allows is the query it will use.
Five tests, including the one the design rests on: approval lapses when the
manifest content changes.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
"Users" was matched the way credential directory names are — against every
path component. On Windows every user's files live under C:\Users\<name>, so
that refused every ordinary project path, including a CI runner's own
workspace. The Windows smoke job on the pull request caught it; the unit
tests could not, because they only ever run compiled on macOS here.
System trees (Windows, ProgramData, Program Files) now match only as the
first component below the drive, which still refuses anything inside them at
any depth. "Users" is handled separately as the tree root only, mirroring
POSIX where "/Users" is refused but "/Users/dev/projects" is not.
Tests cover the regression directly: C:/Users/dev/projects/app and
C:/Users/runneradmin/work/repo are allowed, C:/Users is not, C:/Windows/System32
is refused at depth, and C:/dev/Windows-app — merely named after a system
tree — is allowed.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Routes the MCP index_repository handler and the graph UI's POST /api/index
through a single decision function. The UI route previously checked only that
root_path was a directory, so an operator's configured boundary held on one
entry point and not the other; it now canonicalizes first and applies the same
policy, answering 403 with the reason.
The decision is two-tier, because a bare default-deny would refuse every
first run and a bare opt-in leaves the default open:
- Breadth is always enforced, with nothing configured. Filesystem, drive and
share roots, top-level system trees, the home directory itself and
credential directories are refused as indexing roots out of the box.
- Containment in a declared root applies once CBM_ALLOWED_ROOT is set or a
grant exists, and is evaluated first so a path outside a configured root is
reported as exactly that.
Three things the tests caught, each a real defect rather than a test fix:
- On macOS /etc, /tmp and /var are firmlinked under /private, so
canonicalizing "/etc" yields "/private/etc" and counted two deep — sailing
past a minimum of two, missing the very path being guarded. Depth now
discounts a leading "private" component.
- An earlier draft refused any root containing the cache directory. That was
over-claimed: the indexer only parses recognised source files and a graph
database is binary SQLite it would never extract. Refusing a whole root is
also the wrong remedy where the concern does hold — not walking the cache
is. Removed, with the reasoning recorded at the site.
- Rewording the refusal to "outside every allowed root" broke an assertion
matching "outside the allowed root", and that test's early return skipped
its CBM_ALLOWED_ROOT cleanup, leaking the variable into every later test in
the suite. The original wording is kept and guidance appended instead.
Worth remembering: these contracts match strings, not properties.
Docs updated in the same change, since both env-var tables said "unset
imposes no restriction" and that is no longer true: CONFIGURATION.md and
README.md describe the two tiers, and CONFIGURATION.md lists the always-
refused roots along with the two limits that matter — this constrains scope
rather than sensitivity, and the credential list is a denylist that raises the
cost of a mistake rather than closing the class. SECURITY.md's supported-
versions table was still on 0.8.x and now reads 0.9.x.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Adds cbm_workspace_classify_root, the breadth half of the workspace
boundary. It answers "is this path obviously too broad or too sensitive to
index as one root", not "is this caller allowed to index it" — that second
question needs the user-level grant store, since it is the only input a
caller cannot write. The header says so, because a reader who mistakes one
for the other would draw the wrong conclusion about what is enforced.
Three rules, each doing what the others cannot:
- Depth below the volume. Two components on POSIX refuses every top-level
tree in one rule with no list to maintain. Windows and UNC count
drive/share-relative and require one, because there the first component is
already user space ("D:/repos") — the system trees that sit at the same
depth ("C:/Windows") are covered by name instead, which is tractable
because that set is small and stable.
- The home directory itself, which is two deep on macOS and Linux and so is
invisible to depth.
- Credential directory names, matched against every component so both
"…/.ssh" and "…/.ssh/keys" are refused. This list is deliberately additive
so extending it needs no design authority.
A path that is or contains the cache directory is refused outright: indexing
such a tree would pull every other project's graph database into this
project's index.
Classification order is load-bearing and commented as such. The home
directory normally contains the cache directory, so testing the cache first
reported every home as "holds the cache" and made it non-overridable, which
contradicts the intent that a person may override it. Depth precedes the
cache test for the same reason: "/Users" satisfies both and "too broad" is
the reason that helps the reader.
Takes home and cache as parameters rather than reading the environment, so
the policy is a pure function and the tests need no filesystem. Twelve table
tests; three of them failed on the first implementation and drove the
ordering fix above.
Not yet wired to any caller — deliberately, so the policy can be reviewed on
its own.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Martin Vogel
Ulises Millan Guerrero