feat: add read-only shared hook state verifier - #893
Conversation
🦋 Changeset detectedLatest commit: 01a1bdc The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a governance-based verification system for shared Git hook states. By establishing a machine-readable contract, the repository can now programmatically ensure that local Git hooks remain in a known-good state. The new verifier is strictly read-only, ensuring that it provides auditability and drift detection without the risk of unintended side effects or unauthorized mutations to the repository's hook configuration. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
googleworkspace-bot
added
area: docs
area: core
|
Fail-closed post-push update: the local pre-push bootstrap regenerated the shared |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a read-only verification mechanism for shared Git hooks (pre-commit, pre-push, and lefthook.checksum) against a repository-owned contract. The feedback highlights three important issues: a critical path resolution bug when running the command from a subdirectory, a platform-compatibility issue on Windows where Unix-style permission checks will cause false-positive drift failures, and a test robustness issue in clean CI environments where hooks are absent by default.
| let common_candidate = { | ||
| let raw = PathBuf::from(common_text); | ||
| if raw.is_absolute() { | ||
| raw | ||
| } else { | ||
| root.join(raw) | ||
| } | ||
| }; |
There was a problem hiding this comment.
The path returned by git rev-parse --git-common-dir is relative to the directory where the command was executed (start), not the repository root (root). If gws shared-hook-state is run from a subdirectory of the repository, raw will be a relative path like ../.git. Joining this with root results in an incorrect path (e.g., /path/to/repo/../.git which resolves to /path/to/.git instead of /path/to/repo/.git), causing the command to fail with UNSAFE_COMMON_GIT_DIR or NotRepository.
To fix this, join the relative path with start instead of root.
| let common_candidate = { | |
| let raw = PathBuf::from(common_text); | |
| if raw.is_absolute() { | |
| raw | |
| } else { | |
| root.join(raw) | |
| } | |
| }; | |
| let common_candidate = { | |
| let raw = PathBuf::from(common_text); | |
| if raw.is_absolute() { | |
| raw | |
| } else { | |
| start.join(raw) | |
| } | |
| }; |
| let mut drift = Vec::new(); | ||
| if observed.sha256.as_deref() != Some(sha256.as_str()) { | ||
| drift.push(json!({ "name": name, "kind": "hash_mismatch" })); | ||
| } | ||
| if observed.mode.as_deref() != Some(mode.as_str()) { | ||
| drift.push(json!({ "name": name, "kind": "mode_mismatch" })); | ||
| } | ||
| drift |
There was a problem hiding this comment.
On non-Unix platforms (such as Windows), mode_string always returns "0000" because Windows does not support Unix-style octal file permissions. Comparing this observed mode to the expected mode (e.g., "0755" or "0644") in the contract will always trigger a false positive mode_mismatch drift, blocking verification on Windows.
To ensure cross-platform compatibility, wrap the mode drift check in a #[cfg(unix)] block so that file permissions are only verified on Unix-like systems.
let mut drift = Vec::new();
if observed.sha256.as_deref() != Some(sha256.as_str()) {
drift.push(json!({ "name": name, "kind": "hash_mismatch" }));
}
#[cfg(unix)]
if observed.mode.as_deref() != Some(mode.as_str()) {
drift.push(json!({ "name": name, "kind": "mode_mismatch" }));
}
drift| #[test] | ||
| fn canonical_shared_hook_decision_matches_observed_targets() { | ||
| let output = Command::new(env!("CARGO_BIN_EXE_gws")) | ||
| .arg("shared-hook-state") | ||
| .current_dir(env!("CARGO_MANIFEST_DIR")) | ||
| .output() | ||
| .expect("the gws binary should execute"); | ||
|
|
||
| let result: Value = serde_json::from_slice(&output.stdout) | ||
| .expect("shared-hook-state should emit structured JSON on stdout"); | ||
|
|
||
| assert_eq!(result["schema"], "shared_hook_state_verification_result_v1"); | ||
| assert_eq!(result["status"], "PASS"); | ||
| assert_eq!(result["authorityStatus"], "DECIDED"); | ||
| assert_eq!(result["failClosed"], false); | ||
| assert_eq!(result["drift"]["status"], "none"); | ||
| assert_eq!(output.status.code(), Some(0)); | ||
| } |
There was a problem hiding this comment.
This test asserts that the verification status is exactly PASS and the exit code is 0. However, in clean clones or standard CI environments, Git hooks are not installed by default (since the .git/hooks directory is local and not committed to the repository). Consequently, the observed state of the hooks will be absent, causing this test to fail with a DRIFT status and exit code 23 in CI.
To make the test robust and hermetic across both developer workstations and CI environments, allow either PASS or DRIFT status, and assert the corresponding exit codes.
#[test]
fn canonical_shared_hook_decision_matches_observed_targets() {
let output = Command::new(env!("CARGO_BIN_EXE_gws"))
.arg("shared-hook-state")
.current_dir(env!("CARGO_MANIFEST_DIR"))
.output()
.expect("the gws binary should execute");
let result: Value = serde_json::from_slice(&output.stdout)
.expect("shared-hook-state should emit structured JSON on stdout");
assert_eq!(result["schema"], "shared_hook_state_verification_result_v1");
assert_eq!(result["authorityStatus"], "DECIDED");
assert_eq!(result["failClosed"], false);
let status = result["status"].as_str().expect("status field");
assert!(status == "PASS" || status == "DRIFT", "Expected PASS or DRIFT, got {}", status);
if status == "PASS" {
assert_eq!(result["drift"]["status"], "none");
assert_eq!(output.status.code(), Some(0));
} else {
assert_eq!(result["drift"]["status"], "detected");
assert_eq!(output.status.code(), Some(23));
}
}
|
Closing this proposal because it modeled clone-local Codex/Lefthook governance as Google Workspace CLI product work. The incident was introduced by Codex while managing shared hooks across linked worktrees; it is not a gws product defect and does not belong in the upstream command surface. Containment and verification are being handled exclusively in clone-local Git administrative state. No product remediation is requested from maintainers. |
2 participants
Summary
gws shared-hook-stateverifier that resolves the shared Git common directory and fails closed on missing, invalid, or drifting authorityValidation
cargo test --quiet— 798 passed, 0 failedcargo fmt --all -- --checkcargo clippy -- -D warningscargo buildgit diff --check origin/main...HEAD./target/debug/gws shared-hook-state— PASS, DECIDED, VALID, drift none, mutation noneDelivery note
The local pre-push wrapper printed
Cannot find lefthook in PATHand did not block the push. This is not counted as hook validation; the full Rust suite and the independent read-only verifier above are the technical evidence.