Harden catalog, plugin, sync, config and review paths (SR-4a)

changelog.d/add-from-credentials-redacted.md

@@ -0,0 +1,2 @@
+- **Hardening: `chock add --from` never stores or prints a URL's credentials.** The `user:token@` part of a
+  catalog URL is dropped before the source is written to `chock.lock`, printed, or put in an error message.

changelog.d/add-locate-contained.md

@@ -0,0 +1,3 @@
+- **Hardening: `chock add` finds a pack only inside the catalog.** The fallback lookup (`<area>/<id>`) now
+  applies the same containment check as a `registry.yaml` entry, a pack directory that is itself a symlink
+  is refused, and the copy never follows a link.

changelog.d/catalog-url-never-read-as-path.md

@@ -0,0 +1,3 @@
+- **Hardening: a catalog URL is never read as a path in the working directory.** A source spelled as a URL
+  or `user@host:path` is always fetched from the remote, and its published-branch checks always run, even
+  when a directory of the same shape exists under the current directory. A local catalog is a path.

changelog.d/eval-export-validates-ids.md

@@ -0,0 +1,2 @@
+- **Hardening: `chock eval export` writes only under `--out`.** A policy whose manifest `id` is not a valid
+  policy id (`^[a-z][a-z0-9-]{2,63}$`) is reported as an error and exports nothing.

changelog.d/plugin-build-refuses-links.md

@@ -0,0 +1,2 @@
+- **Hardening: `chock plugin build` in place refuses a symlink under the policy tree.** The build no longer
+  writes through a link at any path it generates; it stops with an error naming the link and writes nothing.

changelog.d/review-checks-from-base.md

@@ -0,0 +1,5 @@
+- **Hardening: `chock review` defines checks from the base only.** With a base ref, the built-in checks and
+  the checks the base's `.chock/config.yaml` declares are the whole registry. A check a pull request adds
+  to its own config, or a built-in it redefines there, is neither run nor part of `command_set_hash`.
+  A check a pull request both adds and requires is not required of that pull request: `chock review require`
+  prints that it is required from the merge on. Without a base ref the working tree still defines.

changelog.d/sync-refuses-linked-agent-config.md

@@ -0,0 +1,4 @@
+- **Hardening: `chock sync` refuses an agent config that is a symlink.** Writing hooks into
+  `.claude/settings.json` and the other merged vendor configs now goes through the same no-follow writer as
+  the MCP registration: a link at the file or any directory above it stops the sync, and nothing is written.
+  The merged config file is now written with owner-only permissions.

changelog.d/yaml-bombs-bounded.md

@@ -0,0 +1,4 @@
+- **Hardening: repo YAML cannot hang or crash the guidance server or `chock check`.** Only string entries
+  of `policies.disabled` are read (an alias-built nested value is no longer stringified), and
+  the guidance config's nesting limit is measured on parser events, so compact block nesting is refused
+  like bracket and indent nesting.

docs/reviewer-evidence.md

@@ -90,8 +90,10 @@ chock:
     unattestable_paths: [tools/, .github/workflows/]
 ```
 
-Config is safe as a source here for the reason evidence is not: it is committed, and CODEOWNERS
-routes it for review. An unknown `check` fails verification rather than being skipped — a claim
+Only the base branch's config defines a check: a check a pull request adds to its own config is not
+run, and a built-in it redefines there keeps the built-in's command. A new check takes effect once it
+is merged. Config is safe as a source here for the reason evidence is not: it is committed, and
+CODEOWNERS routes it for review. An unknown `check` fails verification rather than being skipped — a claim
 nobody can re-derive must not pass quietly.
 
 ## `chock review require` -- is this PR merge-ready?
@@ -144,9 +146,12 @@ judgement that is silent when nothing is declared stays silent, exactly like `un
 
 A pull request edits `.chock/config.yaml` like any other file, so `emit`, `verify` and `require`
 judge by the **stricter of the base branch's policy and the head's**, per setting: the required
-set and `unattestable_paths` are the union, `attestation_floor` the higher, and a check both
-define runs as the base defines it. Tightening in a PR applies at once; loosening applies after
-it merges, in a PR a human approves for that reason (`review/policy.py`).
+set and `unattestable_paths` are the union, and `attestation_floor` the higher. Checks are the
+exception: only the base defines them, and nothing the head declares ever runs. A check the base
+defines and the head requires applies at once. A check the head both adds and requires is not
+required of that pull request; `require` says so (`<X> is added by this change and is required from
+the merge on`) and it is required from the merge on. Loosening applies after it merges, in a PR a
+human approves for that reason (`review/policy.py`).
 `applies_to` is not enforced by `require` itself; it is there for your own workflow's `if:` condition
 (§docs/adopting.md), because requiring evidence from everyone is friction that lands on volunteers.
 

src/chock/config.py

@@ -96,7 +96,7 @@ def _disabled_list(config: dict[str, Any]) -> set[str]:
     disabled = _policies(config).get("disabled")
     if isinstance(disabled, str):
         return {disabled}
-    return {str(p) for p in disabled} if isinstance(disabled, list) else set()
+    return {p for p in disabled if isinstance(p, str)} if isinstance(disabled, list) else set()
 
 
 def policy_status(

src/chock/eval/context_report.py

@@ -138,6 +138,9 @@ def _frontmatter(data: dict[str, Any], body: str) -> str:
     return "---\n" + yaml.safe_dump(data, sort_keys=False) + "---\n" + body.rstrip() + "\n"
 
 
+POLICY_ID = re.compile(r"[a-z][a-z0-9-]{2,63}")
+
+
 def _write(path: Path, text: str) -> None:
     path.parent.mkdir(parents=True, exist_ok=True)
     write_generated(path, text)
@@ -210,6 +213,9 @@ def export(repo_root: Path, out_dir: Path, policy_ids: list[str] | None = None)
     result = ExportResult()
     policies = _resolve(repo_root, policy_ids, result)
     for policy in policies:
+        if not POLICY_ID.fullmatch(policy.id):
+            result.errors.append(f"{policy.id!r}: not a valid policy id ({POLICY_ID.pattern}); nothing exported")
+            continue
         try:
             notice = _export_one(policy, repo_root, out_dir)
         except SuiteShapeError as exc:

src/chock/guidance/advise.py

@@ -61,7 +61,9 @@ def _too_deep(text: str) -> bool:
                 return True
         elif char in "]}":
             depth = max(depth - 1, 0)
-    return any(len(line) - len(line.lstrip(" ")) > 2 * _MAX_DEPTH for line in text.splitlines())
+    if any(len(line) - len(line.lstrip(" ")) > 2 * _MAX_DEPTH for line in text.splitlines()):
+        return True
+    return yamlio.nests_deeper_than(text, _MAX_DEPTH)
 
 
 def _parsed(text: str | None, cap: int) -> dict:

src/chock/hooks/in_agent_merged.py

@@ -15,10 +15,12 @@
 from typing import NamedTuple
 
 from chock import vendors
-from chock.emit import write_generated_json
 from chock.hooks import heartbeat, stop_check
 from chock.hooks.in_agent_generic import load_config as _load_config
 from chock.hooks.runtime_vendor import owned_markers, runtime_rel, vendor_runtime
+from chock.scaffold.mcp_write import _lstat_checked, write_file
+
+_CONFIG_MODE = 0o600
 
 
 class Wiring(NamedTuple):
@@ -186,7 +188,9 @@ def install_merged(repo_root: Path, vendor: str, *, uninstall: bool = False) ->
     repo_root = Path(repo_root)
     markers = owned_markers(vendor)
 
-    config_path = repo_root / vendors.config_path(vendor)
+    config_rel = Path(vendors.config_path(vendor))
+    config_path = repo_root / config_rel
+    _lstat_checked(repo_root, config_rel.as_posix())
     settings = _load_config(config_path)
     hooks = settings.setdefault("hooks", {}) if isinstance(settings.get("hooks", {}), dict) else {}
     settings["hooks"] = hooks
@@ -214,8 +218,7 @@ def install_merged(repo_root: Path, vendor: str, *, uninstall: bool = False) ->
         if not vendor_wiring.write_when_absent and not hooks and not config_path.exists():
             return []
 
-    config_path.parent.mkdir(parents=True, exist_ok=True)
-    write_generated_json(config_path, settings)
+    write_file(repo_root, config_rel.as_posix(), json.dumps(settings, indent=2).encode("utf-8"), _CONFIG_MODE)
     return reported
 
 

src/chock/install/trust.py

@@ -8,7 +8,7 @@
 from chock.install import package
 from chock.install.resolve import REFUSED
 from chock.scaffold.add import IntegrityError
-from chock.scaffold.pin import INERT, REMOTE_TIMEOUT, _run
+from chock.scaffold.pin import INERT, REMOTE_TIMEOUT, _run, local_source
 
 
 def check_source(source: str, allowed: list[str]) -> None:
@@ -25,8 +25,8 @@ def check_source(source: str, allowed: list[str]) -> None:
 def require_on_branch(source: str, sha: str, into: Path) -> None:
     """Refuse `sha` unless it is reachable from the source's published branch (a fork's commit is not)."""
     branch = package.settings()["published_branch"]
-    local = Path(source).expanduser()
-    remote = str(local.resolve()) if local.exists() else source
+    local = local_source(source)
+    remote = str(local.resolve()) if local is not None else source
     clone = ["git", *INERT, "clone", "--quiet", "--bare", "--filter=blob:none", "--single-branch", "--branch"]
     result = _run([*clone, branch, "--", remote, str(into)], REMOTE_TIMEOUT)
     if result.returncode != 0:

src/chock/lock.py

@@ -10,6 +10,7 @@
 from typing import Any
 
 from chock.policies import discover_policy_dirs
+from chock.scaffold.pin import is_remote_source
 from chock.vendored import vendored_differences
 
 LOCKFILE_NAME = "chock.lock"
@@ -114,6 +115,8 @@ def is_local_source(source: object, repo_root: Path) -> bool:
         return False
     if source in {LOCAL_SOURCE, "."}:
         return True
+    if is_remote_source(source):
+        return False
     try:
         return Path(source).expanduser().resolve() == repo_root.resolve()
     except (OSError, RuntimeError):

src/chock/plugin/build.py

@@ -38,7 +38,7 @@
 
 
 class PluginNameError(ValueError):
-    """A policy id that cannot be a conformant plugin name."""
+    """A policy id that is not a conformant plugin name, or a plugin path that is a link."""
 
 
 def plugin_name(policy_id: str) -> str:
@@ -229,13 +229,26 @@ def plugin_files(
     return files
 
 
+def _reject_links(target: Path, rel: Path) -> None:
+    """Refuse a destination that is, or sits under, a symlink inside the target."""
+    here = Path(target)
+    for part in rel.parts:
+        here = here / part
+        if here.is_symlink():
+            msg = f"{here} is a symlink; refusing to write through it. Remove the link and build again."
+            raise PluginNameError(msg)
+
+
 def build_plugin(
     policy_dir: Path, manifest: dict[str, Any], repo_root: Path, out_dir: Path | None = None
 ) -> list[Path]:
     """Write the Agent Plugins package for one policy. Defaults to in place."""
     target = Path(out_dir) if out_dir else Path(policy_dir)
     written: list[Path] = []
-    for rel, content in plugin_files(Path(policy_dir), manifest, Path(repo_root), packaged=out_dir is not None).items():
+    files = plugin_files(Path(policy_dir), manifest, Path(repo_root), packaged=out_dir is not None)
+    for rel in files:
+        _reject_links(target, rel)
+    for rel, content in files.items():
         dest = target / rel
         dest.parent.mkdir(parents=True, exist_ok=True)
         write_generated(dest, content)

src/chock/review/cli.py

@@ -19,6 +19,7 @@
     required_checks,
     verify,
 )
+from chock.review.policy import deferred_checks
 
 DEFAULT_BASE = "origin/main"
 
@@ -93,6 +94,8 @@ def _verify(args: argparse.Namespace) -> int:
 def _require(args: argparse.Namespace) -> int:
     root = Path(args.repo).resolve()
     failures = require(root, args.base)
+    for name in deferred_checks(root, args.base):
+        print(f"{name} is added by this change and is required from the merge on.", file=sys.stderr)
     if failures:
         print(f"PR is not merge-ready ({len(failures)} problem(s)):", file=sys.stderr)
         for failure in failures:

src/chock/review/policy.py

@@ -74,14 +74,35 @@ def unattestable_paths(root: Path, base_ref: str | None = None) -> list[str]:
     return sorted(paths)
 
 
+def _required_names(review: dict[str, Any] | None) -> set[str]:
+    configured = (review or {}).get("required_checks")
+    return {str(n) for n in configured} if isinstance(configured, list) else set()
+
+
+def _base_and_head_required(root: Path, base_ref: str | None) -> tuple[set[str], set[str]]:
+    """(names the base requires, names the head requires); without a base ref the working tree is the head."""
+    head = _required_names(_review_section(load_config(root)))
+    return (_required_names(_review_at(root, base_ref)) if base_ref else set()), head
+
+
 def required_checks(root: Path, base_ref: str | None = None) -> list[str]:
-    """The repository's own required set, unioned across base and head. Never from evidence."""
-    names: set[str] = set()
-    for review in review_policies(root, base_ref):
-        configured = review.get("required_checks")
-        if isinstance(configured, list):
-            names |= {str(n) for n in configured}
-    return sorted(names)
+    """The repository's own required set, unioned across base and head. Never from evidence.
+
+    With a base ref, a name only the head requires and the base does not define is not required yet: see
+    `deferred_checks`. A name the base requires stays required, defined or not (an undefined one fails as unknown).
+    """
+    base, head = _base_and_head_required(root, base_ref)
+    if base_ref:
+        head &= set(check_registry(root, base_ref)) | base
+    return sorted(base | head)
+
+
+def deferred_checks(root: Path, base_ref: str | None = None) -> list[str]:
+    """Checks only the head requires and the base does not define: a change that adds one is bound from the merge on."""
+    if not base_ref:
+        return []
+    base, head = _base_and_head_required(root, base_ref)
+    return sorted((head - base) - set(check_registry(root, base_ref)))
 
 
 def attestation_floor(root: Path, base_ref: str | None = None) -> int:
@@ -105,9 +126,19 @@ def command_set_hash(root: Path, base_ref: str | None = None) -> str:
 
 
 def check_registry(root: Path, base_ref: str | None = None) -> dict[str, list[str]]:
-    """Built-in checks plus any the repository declares; a check the base defines runs as the base defines it."""
+    """The built-in checks plus the ones the base declares; with a base, nothing the head declares is ever defined.
+
+    A pull request edits `.chock/config.yaml`, so a built-in redefined there, or a check added there, would
+    run the author's command inside the gate that is judging the author. Without a base ref there is no
+    other side, and the working tree's own config defines.
+    """
     registry = dict(BUILTIN_CHECKS)
-    for review in reversed(review_policies(root, base_ref)):
+    if not base_ref:
+        defining = [_review_section(load_config(root))]
+    else:
+        base = _review_at(root, base_ref)
+        defining = [] if base is None else [base]
+    for review in defining:
         checks = review.get("checks")
         for name, argv in checks.items() if isinstance(checks, dict) else ():
             if isinstance(argv, list) and all(isinstance(a, str) for a in argv):

src/chock/scaffold/add.py

@@ -16,7 +16,7 @@
 from chock.config import agents_from_config as _agents_from_config
 from chock.lock import LOCAL_SOURCE, compute_pack_hash, is_local_source, pack_bytecode, read_lock, write_lock
 from chock.output import error
-from chock.scaffold.pin import PinError, fetch_catalog
+from chock.scaffold.pin import PinError, fetch_catalog, redact_source
 from chock.scaffold.recompile import BookkeepingError, recompile
 
 
@@ -42,7 +42,7 @@ def _reject_unsafe_id(artifact_id: str) -> None:
 
 def _reject_symlinks(pack_dir: Path, artifact_id: str) -> None:
     """Refuse a pack carrying a symlink: copying it would read a file the catalog never held."""
-    links = []
+    links = [artifact_id] if pack_dir.is_symlink() else []
     for parent, dirs, files in os.walk(pack_dir):
         links += [
             Path(parent, n).relative_to(pack_dir).as_posix() for n in dirs + files if Path(parent, n).is_symlink()
@@ -94,6 +94,8 @@ def locate(catalog_root: Path, artifact_id: str) -> tuple[Path, Path]:
 
     for area, dest in _AREAS.items():
         candidate = catalog_root / area / artifact_id
+        if not candidate.resolve().is_relative_to(catalog_resolved):
+            continue
         if (candidate / "manifest.yaml").exists() or (candidate / "SKILL.md").exists():
             return candidate, dest
 
@@ -145,7 +147,7 @@ def add(
         if dest.exists():
             shutil.rmtree(dest)
         dest.parent.mkdir(parents=True, exist_ok=True)
-        shutil.copytree(src, dest)
+        shutil.copytree(src, dest, symlinks=True)
         return Added(path=dest, sha256=digest, commit=commit)
 
 
@@ -160,7 +162,7 @@ def record_provenance(repo_root: Path, artifact_id: str, source: str, ref: str |
             entry.pop("source_commit", None)
             entry.pop("source_ref", None)
         else:
-            entry["source"] = source
+            entry["source"] = redact_source(source)
             entry["source_commit"] = added.commit
             if ref:
                 entry["source_ref"] = ref
@@ -200,7 +202,7 @@ def main(argv: list[str] | None = None) -> int:
     print(f"Added {args.artifact_id} to {rel}")
 
     if added.commit:
-        print(f"  from {args.source} at {added.commit}")
+        print(f"  from {redact_source(args.source)} at {added.commit}")
     print(f"  sha256 {added.sha256}")
     if not args.ref and added.commit:
         pin = f"Pin with --ref {added.commit}, or --verify-sha {added.sha256}"