Skip to content

Skip unapproved project hooks via declaration-digest trust store - #282

Merged
Zongwei9888 merged 1 commit into
HKUDS:mainfrom
raymondginger2018-sudo:pr/hook-trust
Oct 29, 2026
Merged

Zongwei9888 merged 1 commit into
HKUDS:mainfrom
raymondginger2018-sudo:pr/hook-trust

Conversation

@raymondginger2018-sudo

Copy link
Copy Markdown
Contributor

Summary

Treats a project's hooks.json as workspace-supplied code: project hook declarations are gated behind an explicit, content-addressed approval (borrowed from ZCode's trust rule). Discovery defaults are unchanged — the gate activates only when a caller passes a trust_store.

Changes

  • New core/harness/hooks/trust.py:
    • declaration_digest(workspace, *, home=None, paths=None) — sha256 over canonical declaration fields only (event, matcher, command, timeout, priority, type): cosmetic edits keep approval, semantic changes rotate the digest. User-level config never feeds the digest.
    • HookTrustStore — atomic JSON store; a corrupt file degrades to empty (treated as "nothing approved"), never crashes.
  • core/harness/hooks/discovery.py: discover_hooks(..., *, trust_store=None) — when given and the workspace digest is not approved, project hook sources are skipped with a warning (user-level hooks unaffected).
  • core/harness/hooks/engine.py: HooksEngine.discover(..., trust_store=None) passthrough.
  • core/harness/hooks/__init__.py: export the new trust surface.
  • Tests: tests/test_hook_declaration_trust.py (10 tests: digest canonicalization, digest rotation on semantic change, store approve/revoke/corruption, engine opt-in default, and an end-to-end discover_hooks gate test).

Tests

  • tests/test_hook_declaration_trust.py — 10 passed
  • tests/test_hooks.py — failure set identical to upstream baseline (32 baseline reds; zero new failures vs pristine checkout)
  • CI-exact lint: pre-commit run --all-files (ruff 0.15.21) — passed
  • Local preflight verdict recorded against this revision (KNOWN-RED baseline documented)

Notes

  • Independent PR; no follow-up required for callers that don't pass a store (default = today's behavior exactly).

Borrowed from ZCode's rule: a project's hooks.json is workspace-supplied
code. declaration_digest(workspace) hashes only canonical declaration
fields (event, matcher, command, timeout, priority, type) so cosmetic
edits keep approval while semantic changes rotate the digest.

HookTrustStore is an atomic JSON store; a corrupt file degrades to empty.
discover_hooks/HooksEngine.discover accept trust_store= keyword — when
None (default) discovery is exactly the old behavior; when given, project
sources are skipped with a warning until their digest is approved.
@Zongwei9888

Copy link
Copy Markdown
Collaborator

Thank you, @raymondginger2018-sudo. The digest is carefully canonicalised and the tests read well. We can't merge it as it is, though:

  1. Nothing passes trust_store or calls approve(), so the gate never runs and there is no way for a user to approve. DeepCode already has a workspace trust boundary: Project.trust_state (core/domain/project.py:21), and a Turn refuses an untrusted project (core/application/turn_service.py:590). Hook approval should live on that Project record rather than in a separate hook_trust.json. (trust.py also says hooks have a boolean project_trusted gate; they don't, they're covered only because the session can't start.)
  2. With a store passed, a workspace with no project hooks still gets the "not approved ... skipped" warning (discovery.py:97-108), which would log on every session.
  3. The digest covers the command string, not the script it runs (sh .deepcode/check.sh keeps its approval after the script changes), so please describe the guarantee that narrowly. The digest uses the unresolved path while the store key resolves it, so opening the workspace through a symlink counts as unapproved.
  4. tests/test_hooks.py is 53/53 green on main here, so the "32 baseline reds" look Windows-specific.
  5. Please drop "ZCode", and update the discovery.py docstring that says trust-hash gating is deliberately left out.

A version we'd merge: the approved digest stored on the Project, enforced in build_agent_session (core/agent_setup.py:410), the digest and approval state shown in the hooks listing (core/application/extension_service.py:54) with an approve action, and an end-to-end test.

@Zongwei9888
Zongwei9888 merged commit f87e601 into HKUDS:main Oct 29, 2026
12 checks passed
@Zongwei9888

Copy link
Copy Markdown
Collaborator

Thank you, @raymondginger2018-sudo! The canonical declaration digest is a really nice piece of work, and it's still the heart of this feature. Since we hadn't heard back, we merged your PR in f87e601d and finished it in two follow-ups, 7505c5e0 and 0d58a617:

  • The approved digest now lives on the Project record instead of a separate hook_trust.json. Trusting a project approves the hooks it declares at that moment. Projects trusted before this change keep their current hooks approved, so no existing setup breaks.
  • The gate is enforced on every Turn (build_agent_session, sub-agents included). Project hooks run only while their digest matches the approved one. Otherwise they're skipped with a single notice, and only when the project actually declares hooks.
  • New deepcode hooks status and deepcode hooks approve commands let users review and approve a project's hooks. Approval is refused for untrusted projects. The same is available over the app server as hooks/list (now with digest and status) and hooks/approve.
  • The digest names files relative to the workspace, so symlinked paths and worktrees match. The docs say plainly that it covers the declarations, not the scripts a command runs.
  • We added an end-to-end test (unapproved hooks are skipped, approving makes them run, a change skips them again) and a short section in the user guide.

Thanks again for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants