Skip to content

git-hooks installer: surface foreign shared entries before hooks-path #7613

Open

Description

@kyle-sexton

Split (2026-10-17): finding 2 moved to #7898, which carries its own Agent Brief. This issue now tracks finding 1 only: the owner's decision on what the hooks-path opt-in must disclose before it activates entries the user did not install. On main the SHA-256 null-id part of finding 2 is already fixed (checks_push_ranges matches ^0+$), and guardrails reference-verify is now a pre-push entry, reading blobs through its own helper.

Two review findings on #7547 (shared git-hook installer, lib/git-hooks/) deferred to the rollout phases that use them.

1. Shared-scope hooks-path opt-in activates foreign entries the user never sees

install.mjs emits the hooks-path decision (git config core.hooksPath .githooks) without listing what is already committed under .githooks/. Once set, the dispatcher runs every .githooks/<event>.pre-checks and every marker in .githooks/<event>.d/ and .githooks/local/<event>.d/, all repository content. The user consents to one plugin's entry and activates the rest uninformed.

Proposed: before hooks-path, plan lists existing markers not named <this plugin>.*, any *.pre-checks, and committed .githooks/local/ content as a decision the user sees; consider not scanning $dir/local when $dir is inside the work tree.

Split to #8989: the owner question whether the dispatcher stops scanning an in-tree local/ (a security trade-off that changes what opted-in clones run).

2. Moved to #7898

check-lib.sh pre-push mode mixes worktree content with pushed-commit line ranges. Tracked in #7898.

typos-check is pre-commit only, so #7547 is unaffected; this lands with the shared-scope rollout to the other check plugins.

Done when (this issue: the disclosure, additive)

  • With scope === "shared" and core.hooksPath not yet .githooks, plainDecisions (lib/git-hooks/install.mjs:486-497) emits a shared-entries decision before hooks-path, listing each .githooks/<event>.d/ marker not named <this plugin>.*, every .githooks/*.pre-checks, and every tracked file under .githooks/local/ (git ls-files). Check: the PR's CI runs lib/git-hooks/install.test.sh and it passes.
  • With none present, the plan has no such decision, so its output is unchanged. Check: as above.
  • Tests cover a repo with a foreign marker, a .pre-checks file, a tracked local/ file, and an empty .githooks/. Check: as above.
  • The registered copies of lib/git-hooks/ are regenerated (scripts/sync-shared-copies.sh), with a fragment for every carrier plugin (scripts/new-changelog-fragment.sh --carriers-of lib/git-hooks/install.mjs patch). Check: the PR's CI runs the shared-copy sync check and scripts/check-changelog-fragments.sh --check, and both pass.

Activity

  1. added
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Oct 16, 2026
  2. github-actions commented on Oct 16, 2026

    @github-actions
    Contributor

    Thanks for the report. Automated triage handed this issue to a maintainer.

    Automated triage. A maintainer reviews it.

  3. added
    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.
    on Oct 16, 2026
  4. kyle-sexton commented on Oct 18, 2026

    @kyle-sexton
    MemberAuthor

    This was generated by AI during triage.

    Triage: human-gated (Task, priority: low)

    Verified: both findings still hold on main (e59e0be), by code read.

    1. lib/git-hooks/install.mjs writes a dispatcher that runs $dir/<event>.pre-checks (lines 98-99) and every marker under <event>.d/ and local/<event>.d/ (lines 79-85), and the hooks-path decision lists none of the existing entries first.
    2. lib/git-hooks/check-lib.sh pre-push mode takes ranges from the pushed commits (checks_push_ranges, line 33) while checks read files from the worktree; the null id is the 40-zero SHA-1 form only (see check-lib.test.sh:160), with no --show-object-format read.

    No open PR covers either. Both were deferred from #7547 (merged) to the rollout phases that add pre-push entries or the shared-scope rollout, and no such phase is open yet.

    Why a human: finding 1 is a consent-design decision: what the hooks-path opt-in must show before it activates entries the user did not install, and whether to stop scanning local/ when it sits in the work tree. Finding 2 is concrete but only matters once a pre-push entry exists, so its timing belongs to the rollout plan.

    RECOMMENDED: split this issue in two. Finding 2 (read pushed blobs, derive the null id from the object format) becomes scoped work an agent can pick up once any pre-push entry is planned. Finding 1 stays here for the owner to decide the disclosure rule. Basis: judgment (the two share no code path and no decision).

    Brief

    Type: Task
    Summary: Before the shared git-hook installer activates core.hooksPath, disclose foreign entries; make pre-push checks read the pushed blobs.

    Desired behavior:

    • The installer's plan, before the hooks-path decision, lists every existing marker not owned by the installing plugin, any *.pre-checks file, and any committed clone-local content, as a decision the user sees.
    • In pre-push mode, a check reads each file's content from the pushed commit, not the worktree, so line ranges and content agree; the null id is recognized for both SHA-1 and SHA-256 repositories.

    Acceptance criteria:

    • A repository with a foreign marker under the hooks directory gets a plan that names it before hooks-path.
    • A pre-push check on a commit whose file was edited again in the worktree reports findings at the pushed commit's lines.
    • A SHA-256 repository's new branch and branch deletion are classified correctly in pre-push mode.

    Out of scope:

  5. added
    priority: lowNice-to-have, cosmetic, or speculative; opportunistic.
    and removed
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Oct 18, 2026
  6. added theissue type on Oct 18, 2026
  7. changed the title [-]git-hooks installer: surface foreign shared entries before hooks-path, and make check-lib pre-push read pushed blobs[/-] [+]git-hooks installer: surface foreign shared entries before hooks-path[/+] on Oct 18, 2026
  8. added
    status: confirmedA maintainer confirmed the item is real and in scope; ready to be worked.
    work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.
    and removed
    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.
    on Oct 22, 2026
  9. kyle-sexton commented on Oct 24, 2026

    @kyle-sexton
    MemberAuthor

    Released for re-triage under the owner's 2026-10-24 rule: an issue stops for a person only for an intent or vision question, a security call, a host or attended run, or spend outside prepaid credit. The hold label came off and intake triage runs again now; it re-applies a hold, with a numbered question, when one of those applies.

    To hold this issue again, add needs-human (or status: needs-decision) and say why in a comment.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority: lowNice-to-have, cosmetic, or speculative; opportunistic.status: confirmedA maintainer confirmed the item is real and in scope; ready to be worked.work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.

    Type

    Fields

    No fields configured for Task.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions