From 3bed2c216bf0404c4d9bab55c19e4f39bebf9754 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Wed, 29 Jul 2026 21:07:34 +0900 Subject: [PATCH 1/3] Fix truthful hook dry-run planning (#4883) --- TESTING_GUIDE.md | 4 +- USER_GUIDE.md | 12 +- changelog.d/unreleased/4883.fixed.md | 21 + src/CodeIndex/Cli/CliFlagSchema.cs | 4 +- src/CodeIndex/Cli/HookCommandRunner.cs | 635 ++++++++++++++---- src/CodeIndex/Cli/JsonOutputContracts.cs | 2 + tests/CodeIndex.Tests/CliFlagSchemaTests.cs | 2 +- .../CodeIndex.Tests/HookCommandRunnerTests.cs | 243 ++++++- 8 files changed, 768 insertions(+), 155 deletions(-) create mode 100644 changelog.d/unreleased/4883.fixed.md diff --git a/TESTING_GUIDE.md b/TESTING_GUIDE.md index 6cde624bb..b8abf5ac9 100644 --- a/TESTING_GUIDE.md +++ b/TESTING_GUIDE.md @@ -591,7 +591,7 @@ Use `docs/test-doc-maintenance-plan.md` before moving oversized suites or adding - `CliNonDatabaseErrorContractTests.cs` Table-driven empty, not-found, invalid-argument, invalid-config, and hook-platform failures. Keep JSON assertions focused on the stable versioned envelope and sanitized optional context, pair every scenario with its coded human `Error` / `Hint` / `Usage` form, and restore current-directory and hook filesystem seams after each case. - `GitHelperTests.cs`, `GitProcessRunnerTests.cs`, `HookCommandRunnerTests.cs` - Git-specific behavior, including worktrees, commit-based updates, direct git process runner diagnostics, cancellation of git subprocesses, portable absolute executable overrides, and fail-closed metadata-write boundaries. Hook install coverage keeps `installed`, `updated`, and `already_installed` distinct, proves an exact executable UTF-8/no-BOM rerun preserves the hook write time, repairs non-executable and differently encoded managed hooks, and exercises dry-run create, managed replacement, custom-hook chain, blocked, and no-op plans without creating the missing hooks directory or changing either hook file. Keep executable-override fixtures owner-only-writable, owned by the current test user, under trusted ancestors (including an explicit root-owned sticky `/tmp` case), and able to return `git version` from `--version` unless the test is asserting a rejection reason. Symlinked `.git` directory/file, intermediate metadata components, hard-linked metadata files, worktree target, `commondir`, `info`, `exclude`, `hooks`, and hook-file coverage must prove resolution is rejected before metadata is written outside the project boundary. Tests that create real repositories or launch real/fake git subprocesses use `ExternalProcessFactAttribute` / `ExternalProcessTheoryAttribute` and run only on the `net8.0` test target; keep pure `.git` metadata parsing, trust diagnostics, and trusted-candidate enumeration cross-target. Timeout and cancellation wall-clock assertions should stay below the fake git scripts' natural completion while leaving room for macOS CI scheduling and process-cleanup overhead. Fake git scripts that run after commit-ref validation should echo the verified commit argument for `rev-parse --verify ^{commit}` so timeout tests reach the intended git command. + Git-specific behavior, including worktrees, commit-based updates, direct git process runner diagnostics, cancellation of git subprocesses, portable absolute executable overrides, and fail-closed metadata-write boundaries. Hook install coverage keeps `installed`, `updated`, and `already_installed` distinct, proves an exact executable UTF-8/no-BOM rerun preserves the hook write time, and repairs non-executable or differently encoded managed hooks. Hook dry-run coverage keeps successful preview status at `dry_run`, asserts `filesystem_mutation: false`, and validates state plus hash/provenance-bearing `planned_changes` for install create/replacement/chain/no-op and uninstall managed-block removal/chained restoration/force/blocked/idempotent plans without changing content, mode, or write time. Keep unmanaged/conflicted marker and symlink cases fail-closed. Keep executable-override fixtures owner-only-writable, owned by the current test user, under trusted ancestors (including an explicit root-owned sticky `/tmp` case), and able to return `git version` from `--version` unless the test is asserting a rejection reason. Symlinked `.git` directory/file, intermediate metadata components, hard-linked metadata files, worktree target, `commondir`, `info`, `exclude`, `hooks`, and hook-file coverage must prove resolution is rejected before metadata is written outside the project boundary. Tests that create real repositories or launch real/fake git subprocesses use `ExternalProcessFactAttribute` / `ExternalProcessTheoryAttribute` and run only on the `net8.0` test target; keep pure `.git` metadata parsing, trust diagnostics, and trusted-candidate enumeration cross-target. Timeout and cancellation wall-clock assertions should stay below the fake git scripts' natural completion while leaving room for macOS CI scheduling and process-cleanup overhead. Fake git scripts that run after commit-ref validation should echo the verified commit argument for `rev-parse --verify ^{commit}` so timeout tests reach the intended git command. - `WorkspaceMetadataEnricherTests.cs` Workspace freshness and git metadata enrichment behavior. - `DiagnosticRedactorTests.cs`, `SuggestionStoreTests.cs` @@ -1513,7 +1513,7 @@ dotnet test --filter "FullyQualifiedName~GitHelperTests" - `CliNonDatabaseErrorContractTests.cs` empty、not-found、不正な引数、不正な設定、hook platform failure を表駆動で検証します。JSON assertion は安定したバージョン付き envelope と sanitization 済みの任意 context に集中させ、各 scenario で code 付き human `Error` / `Hint` / `Usage` も対にして検証し、current directory と hook filesystem seam を各 case の後に復元してください。 - `GitHelperTests.cs`、`GitProcessRunnerTests.cs`、`HookCommandRunnerTests.cs` - worktree、commit ベース更新、direct git process runner diagnostics、git subprocess cancellation、portable な absolute executable override、fail-closed な metadata-write boundary を含む Git まわりのテスト。hook install coverage では `installed`、`updated`、`already_installed` を区別し、同一かつ実行可能な UTF-8/no-BOM の内容での再実行が hook の write time を維持すること、実行不可または異なる encoding の managed hook を修復すること、および dry-run の create、managed replacement、custom-hook chain、blocked、no-op plan が存在しない hooks directory を作成せず、どちらの hook file も変更しないことを検証します。executable override fixture は rejection reason 自体を検証する場合を除き owner-only-writable、test user 所有、trusted ancestor 配下（root 所有の sticky `/tmp` case を明示的に含む）とし、`--version` で `git version` を返せるようにします。symlinked `.git` directory / file、中間 metadata component、hard-linked metadata file、worktree target、`commondir`、`info`、`exclude`、`hooks`、hook file の coverage は、project boundary 外へ metadata を書く前に解決が拒否されることを証明してください。実 repo を作る、または real/fake git subprocess を起動するテストは `ExternalProcessFactAttribute` / `ExternalProcessTheoryAttribute` を使い、`net8.0` test target だけで実行します。純粋な `.git` metadata parsing、trust diagnostics、trusted-candidate enumeration は cross-target のままにしてください。Timeout と cancellation の wall-clock assertion は fake git script の自然完了より短く保ちつつ、macOS CI の scheduling や process cleanup の遅れを許容する余裕を持たせます。commit-ref validation 後に使う fake git script は `rev-parse --verify ^{commit}` の検証対象 commit 引数を返し、timeout テストが意図した git command まで到達するようにします。 + worktree、commit ベース更新、direct git process runner diagnostics、git subprocess cancellation、portable な absolute executable override、fail-closed な metadata-write boundary を含む Git まわりのテスト。hook install coverage では `installed`、`updated`、`already_installed` を区別し、同一かつ実行可能な UTF-8/no-BOM の内容での再実行が hook の write time を維持すること、および実行不可または異なる encoding の managed hook を修復することを検証します。hook の dry-run coverage では、成功したプレビューの status を `dry_run` に保ち、`filesystem_mutation: false` を検証します。また、install の create / replacement / chain / no-op と uninstall の managed block 除去 / chained 復元 / force / blocked / idempotent plan について、state と hash / provenance 付き `planned_changes` を検証し、content、mode、write time が変化しないことを確認します。unmanaged / conflicted marker と symlink の case は fail-closed に保ってください。executable override fixture は rejection reason 自体を検証する場合を除き owner-only-writable、test user 所有、trusted ancestor 配下（root 所有の sticky `/tmp` case を明示的に含む）とし、`--version` で `git version` を返せるようにします。symlinked `.git` directory / file、中間 metadata component、hard-linked metadata file、worktree target、`commondir`、`info`、`exclude`、`hooks`、hook file の coverage は、project boundary 外へ metadata を書く前に解決が拒否されることを証明してください。実 repo を作る、または real/fake git subprocess を起動するテストは `ExternalProcessFactAttribute` / `ExternalProcessTheoryAttribute` を使い、`net8.0` test target だけで実行します。純粋な `.git` metadata parsing、trust diagnostics、trusted-candidate enumeration は cross-target のままにしてください。Timeout と cancellation の wall-clock assertion は fake git script の自然完了より短く保ちつつ、macOS CI の scheduling や process cleanup の遅れを許容する余裕を持たせます。commit-ref validation 後に使う fake git script は `rev-parse --verify ^{commit}` の検証対象 commit 引数を返し、timeout テストが意図した git command まで到達するようにします。 - `WorkspaceMetadataEnricherTests.cs` ワークスペース鮮度と git メタデータ付与のテスト。 - `DiagnosticRedactorTests.cs`、`SuggestionStoreTests.cs` diff --git a/USER_GUIDE.md b/USER_GUIDE.md index a6c32c373..db882be16 100644 --- a/USER_GUIDE.md +++ b/USER_GUIDE.md @@ -216,13 +216,15 @@ local index automatically: |---|---| | `cdidx hooks install [--dry-run]` | Install the optional git pre-commit hook, or preview it without writing. | | `cdidx hooks status` | Show whether the hook is installed. | -| `cdidx hooks uninstall` | Remove the hook. | +| `cdidx hooks uninstall [--dry-run]` | Remove the cdidx hook, or preview managed-block removal/chained-hook restoration without writing. | | Hook behavior | Detail | |---|---| | Refresh command | The installed hook runs `cdidx index --quiet` before the commit completes. When `--project` is omitted, the selected path is the current directory at install time. | | Quiet mode | `--quiet` suppresses normal progress and success output for hook contexts while still printing indexing errors to stderr and returning a non-zero exit code. | -| Install preview | `cdidx hooks install --dry-run` does not create or change hook files. Human output prints the planned managed hook, while JSON adds `dry_run: true`, `planned_action` (`create`, `replace_managed`, `chain_existing`, `none`, or `blocked`), and `managed_hook_preview`. | +| Preview contract | `cdidx hooks install --dry-run` and `cdidx hooks uninstall --dry-run` return `status: dry_run` for a successful preview, `dry_run: true`, `filesystem_mutation: false`, `planned_action`, `hook_state`, `chained_hook_state`, and `planned_changes`. Each planned change carries its action/path/source, content hashes, executable-mode transition, and provenance. A blocked preview keeps `status: error` while retaining the same non-mutating plan fields. | +| Install preview | Install plans use `create`, `replace_managed`, `chain_existing`, `none`, or `blocked`; `managed_hook_preview` contains the generated managed hook. Preview never creates the missing hooks directory or rewrites an existing file. | +| Uninstall preview | Uninstall plans use `delete_managed`, `remove_managed_block`, `restore_chained`, `force_delete_unmanaged`, `force_restore_chained`, `none`, or `blocked`. A well-formed managed block is removed while preserving surrounding custom hook content; a chained backup is reported as the restoration source. Unmanaged or conflicted marker content is blocked unless `--force` is explicit. | | Install result | A real install returns `status: installed` when it creates the hook, `status: updated` when it replaces managed content, repairs a non-executable or non-UTF-8/no-BOM managed hook, or chains an existing custom hook, and `status: already_installed` without rewriting when the exact executable UTF-8/no-BOM managed hook is already present. | | Status JSON diagnostics | `cdidx hooks status --json` keeps `project_path`, `hook_path`, and `chained_hook_path` for compatibility, and also emits `diagnostic_project_path`, `diagnostic_hook_path`, and `diagnostic_chained_hook_path` with path-sanitized values for logs and support bundles. | | Existing hooks | If `.git/hooks/pre-commit` already exists, `cdidx hooks install` moves it to `.git/hooks/pre-commit.cdidx-chain` and calls it after the cdidx refresh, preserving tools such as Husky, pre-commit, and lefthook. | @@ -3481,13 +3483,15 @@ pre-commit hook をインストールします: |---|---| | `cdidx hooks install [--dry-run]` | 任意の git pre-commit hook をインストール、または書き込まずにプレビュー。 | | `cdidx hooks status` | hook のインストール状態を表示。 | -| `cdidx hooks uninstall` | hook を削除。 | +| `cdidx hooks uninstall [--dry-run]` | cdidx hook を削除、または managed block の除去／chained hook の復元を書き込まずにプレビュー。 | | hook の動作 | 詳細 | |---|---| | 更新コマンド | インストールされた hook はコミット完了前に `cdidx index --quiet` を実行します。`--project` を省略した場合、選択パスはインストール時のカレントディレクトリです。 | | quiet mode | `--quiet` は hook 環境向けに通常の進捗・成功出力を抑制しつつ、indexing エラーは引き続き stderr に出力し、非ゼロの終了コードを返します。 | -| インストールのプレビュー | `cdidx hooks install --dry-run` は hook ファイルを作成・変更しません。human 出力では予定される managed hook を表示し、JSON には `dry_run: true`、`planned_action`（`create`、`replace_managed`、`chain_existing`、`none`、`blocked` のいずれか）、`managed_hook_preview` を追加します。 | +| プレビュー契約 | `cdidx hooks install --dry-run` と `cdidx hooks uninstall --dry-run` は、成功時に `status: dry_run`、`dry_run: true`、`filesystem_mutation: false`、`planned_action`、`hook_state`、`chained_hook_state`、`planned_changes` を返します。各変更予定には、操作、対象／変更元のパス、内容のハッシュ、実行モードの遷移、由来が含まれます。実行できないプレビューは `status: error` を維持しつつ、同じ非変更の計画フィールドを返します。 | +| インストールのプレビュー | インストール計画は `create`、`replace_managed`、`chain_existing`、`none`、`blocked` のいずれかで、`managed_hook_preview` に生成予定の管理対象 hook を含めます。プレビューは存在しない hooks ディレクトリを作成せず、既存ファイルも再書き込みしません。 | +| アンインストールのプレビュー | アンインストール計画は `delete_managed`、`remove_managed_block`、`restore_chained`、`force_delete_unmanaged`、`force_restore_chained`、`none`、`blocked` のいずれかです。正しい管理対象 block は周囲の独自 hook 内容を保持して除去し、連結されたバックアップがある場合は復元元として報告します。管理対象外の内容や marker が競合した内容は、明示的な `--force` がない限り実行できません。 | | インストール結果 | 実際のインストールでは、hook を新規作成した場合は `status: installed`、managed content の置換、実行不可または UTF-8/no-BOM 以外になった managed hook の修復、あるいは既存 custom hook の chain 化を行った場合は `status: updated`、同一かつ実行可能な UTF-8/no-BOM の managed hook がすでに存在して再書き込みを行わなかった場合は `status: already_installed` を返します。 | | status JSON 診断 | `cdidx hooks status --json` は互換性のため `project_path`、`hook_path`、`chained_hook_path` を維持しつつ、ログやサポートバンドル向けに path をサニタイズした `diagnostic_project_path`、`diagnostic_hook_path`、`diagnostic_chained_hook_path` も出力します。 | | 既存 hook の扱い | リポジトリに `.git/hooks/pre-commit` がある場合、`cdidx hooks install` はそれを `.git/hooks/pre-commit.cdidx-chain` に移動し、cdidx の更新後に呼び出すため、Husky、pre-commit、lefthook などのツールも維持されます。 | diff --git a/changelog.d/unreleased/4883.fixed.md b/changelog.d/unreleased/4883.fixed.md new file mode 100644 index 000000000..255dded25 --- /dev/null +++ b/changelog.d/unreleased/4883.fixed.md @@ -0,0 +1,21 @@ +--- +category: fixed +issues: + - 4883 +affected: + - src/CodeIndex/Cli/HookCommandRunner.cs + - src/CodeIndex/Cli/CliFlagSchema.cs + - src/CodeIndex/Cli/JsonOutputContracts.cs + - tests/CodeIndex.Tests/HookCommandRunnerTests.cs + - tests/CodeIndex.Tests/CliFlagSchemaTests.cs + - USER_GUIDE.md + - TESTING_GUIDE.md +--- + +## English + +- **Hook dry-runs now report truthful install and uninstall plans (#4883)** — successful previews use `status: dry_run` with `filesystem_mutation: false` and hash/provenance-bearing planned changes, while uninstall can preview managed-block removal, chained-hook restoration, forced unmanaged removal, blocked conflicts, and no-op states without changing hook content or metadata. + +## 日本語 + +- **hook の dry-run が install と uninstall の正確な計画を報告するようになりました (#4883)** — 成功したプレビューは `status: dry_run`、`filesystem_mutation: false`、hash / provenance 付きの変更予定を返します。また uninstall では、hook の内容や metadata を変更せずに managed block の除去、chained hook の復元、unmanaged hook の強制削除、競合による停止、no-op 状態をプレビューできます。 diff --git a/src/CodeIndex/Cli/CliFlagSchema.cs b/src/CodeIndex/Cli/CliFlagSchema.cs index fed30c072..34338995c 100644 --- a/src/CodeIndex/Cli/CliFlagSchema.cs +++ b/src/CodeIndex/Cli/CliFlagSchema.cs @@ -613,11 +613,11 @@ private static IReadOnlyList BuildAll() new() { Name = "--dry-run", - Description = "Preview without writing; hooks supports it only for install", + Description = "Preview without writing; hooks supports install and uninstall", PrimaryCommands = Set("index", "hooks", "backfill-fold", "optimize", "vacuum"), CompletionSubcommands = new Dictionary>(StringComparer.Ordinal) { - ["hooks"] = Set("install"), + ["hooks"] = Set("install", "uninstall"), }, Safety = CliOptionSafety.Preview, }, diff --git a/src/CodeIndex/Cli/HookCommandRunner.cs b/src/CodeIndex/Cli/HookCommandRunner.cs index de6692eee..7e6a63385 100644 --- a/src/CodeIndex/Cli/HookCommandRunner.cs +++ b/src/CodeIndex/Cli/HookCommandRunner.cs @@ -1,3 +1,4 @@ +using System.Security.Cryptography; using System.Text; using System.Text.Json; using System.Text.Json.Nodes; @@ -168,8 +169,12 @@ internal static HookCommandOptions ParseArgs(string[] args) } } - if (dryRun && !string.Equals(command, "install", StringComparison.Ordinal)) - parseError ??= "--dry-run is supported only for hooks install"; + if (dryRun + && !string.Equals(command, "install", StringComparison.Ordinal) + && !string.Equals(command, "uninstall", StringComparison.Ordinal)) + { + parseError ??= "--dry-run is supported only for hooks install or uninstall"; + } return new HookCommandOptions(command, projectPath, json, force, dryRun, showHelp, parseError); } @@ -178,153 +183,178 @@ private static int Install(HookCommandOptions options, JsonSerializerOptions jso { var warnings = new List(); var hookScript = BuildHookScript(chainedHookPath, projectPath); + var plan = BuildInstallPlan(options, hookPath, chainedHookPath, hookScript); if (options.DryRun) + return WriteDryRunResult(options, jsonOptions, projectPath, hookPath, chainedHookPath, plan, hookScript); + + if (plan.Blocked) + return WriteBlockedPlanResult(options, jsonOptions, projectPath, hookPath, chainedHookPath, plan); + + Directory.CreateDirectory(LongPath.EnsureWindowsPrefix(hooksDir)); + if (!TryResolveHookWritePaths(gitDir, out hooksDir, out hookPath, out chainedHookPath)) + return WriteResult(options.Json, jsonOptions, "error", "unsafe Git hook file path", projectPath, null, null, CommandExitCodes.InstallError); + + hookScript = BuildHookScript(chainedHookPath, projectPath); + plan = BuildInstallPlan(options, hookPath, chainedHookPath, hookScript); + if (plan.Blocked) + return WriteBlockedPlanResult(options, jsonOptions, projectPath, hookPath, chainedHookPath, plan); + + if (plan.PlannedAction == "none") { - return PreviewInstall( - options, + return WriteResult( + options.Json, jsonOptions, + "already_installed", + "cdidx pre-commit hook is already installed", projectPath, hookPath, - chainedHookPath, - hookScript); + plan.ChainedHookState == "present" ? chainedHookPath : null, + CommandExitCodes.Success); } - Directory.CreateDirectory(LongPath.EnsureWindowsPrefix(hooksDir)); - if (!TryResolveHookWritePaths(gitDir, out hooksDir, out hookPath, out chainedHookPath)) - return WriteResult(options.Json, jsonOptions, "error", "unsafe Git hook file path", projectPath, null, null, CommandExitCodes.InstallError); - - var ioHookPath = LongPath.EnsureWindowsPrefix(hookPath); - var ioChainedHookPath = LongPath.EnsureWindowsPrefix(chainedHookPath); - if (File.Exists(ioHookPath)) + if (plan.PlannedAction == "chain_existing") { - var existingHook = ReadHookFileWithinLimit(ioHookPath); - if (existingHook is not null && IsManagedHook(existingHook)) + try { - if (IsExactHookScriptFile(ioHookPath, hookScript) - && IsExecutableHook(ioHookPath)) - { - return WriteResult( - options.Json, - jsonOptions, - "already_installed", - "cdidx pre-commit hook is already installed", - projectPath, - hookPath, - File.Exists(ioChainedHookPath) ? chainedHookPath : null, - CommandExitCodes.Success); - } + if (!TryResolveHookWritePaths(gitDir, out hooksDir, out hookPath, out chainedHookPath)) + return WriteResult(options.Json, jsonOptions, "error", "Git hook file path became unsafe before write", projectPath, null, null, CommandExitCodes.InstallError); + ReplaceCustomHookWithManagedHook(hooksDir, hookPath, chainedHookPath, projectPath, warnings); } - else + catch (Exception ex) when (IsHookFileOperationException(ex)) { - if (File.Exists(ioChainedHookPath) && !options.Force) - return WriteResult(options.Json, jsonOptions, "error", $"chained hook already exists: {chainedHookPath}", projectPath, hookPath, chainedHookPath, CommandExitCodes.UsageError); - - try - { - if (!TryResolveHookWritePaths(gitDir, out hooksDir, out hookPath, out chainedHookPath)) - return WriteResult(options.Json, jsonOptions, "error", "Git hook file path became unsafe before write", projectPath, null, null, CommandExitCodes.InstallError); - ReplaceCustomHookWithManagedHook(hooksDir, hookPath, chainedHookPath, projectPath, warnings); - } - catch (Exception ex) when (IsHookFileOperationException(ex)) - { - RecordHookWarning(warnings, "chained_hook_backup", chainedHookPath, "failed to back up existing hook", ex); - var message = $"failed to install cdidx pre-commit hook ({CommandErrorWriter.FormatSanitizedException(ex)})"; - return WriteResult(options.Json, jsonOptions, "error", message, projectPath, hookPath, chainedHookPath, CommandExitCodes.InstallError, warnings); - } - - return WriteResult(options.Json, jsonOptions, "updated", "cdidx pre-commit hook updated", projectPath, hookPath, chainedHookPath, CommandExitCodes.Success, warnings); + RecordHookWarning(warnings, "chained_hook_backup", chainedHookPath, "failed to back up existing hook", ex); + var message = $"failed to install cdidx pre-commit hook ({CommandErrorWriter.FormatSanitizedException(ex)})"; + return WriteResult(options.Json, jsonOptions, "error", message, projectPath, hookPath, chainedHookPath, CommandExitCodes.InstallError, warnings); } + + return WriteResult(options.Json, jsonOptions, "updated", "cdidx pre-commit hook updated", projectPath, hookPath, chainedHookPath, CommandExitCodes.Success, warnings); } if (!TryResolveHookWritePaths(gitDir, out hooksDir, out hookPath, out chainedHookPath)) return WriteResult(options.Json, jsonOptions, "error", "Git hook file path became unsafe before write", projectPath, null, null, CommandExitCodes.InstallError); - ioHookPath = LongPath.EnsureWindowsPrefix(hookPath); - ioChainedHookPath = LongPath.EnsureWindowsPrefix(chainedHookPath); hookScript = BuildHookScript(chainedHookPath, projectPath); - var status = File.Exists(ioHookPath) ? "updated" : "installed"; + var status = plan.PlannedAction == "create" ? "installed" : "updated"; var resultMessage = status == "updated" ? "cdidx pre-commit hook updated" : "cdidx pre-commit hook installed"; AtomicFileWriter.WriteText(hookPath, hookScript, new UTF8Encoding(encoderShouldEmitUTF8Identifier: false), MakeExecutable); - return WriteResult(options.Json, jsonOptions, status, resultMessage, projectPath, hookPath, File.Exists(ioChainedHookPath) ? chainedHookPath : null, CommandExitCodes.Success, warnings); + return WriteResult( + options.Json, + jsonOptions, + status, + resultMessage, + projectPath, + hookPath, + File.Exists(LongPath.EnsureWindowsPrefix(chainedHookPath)) ? chainedHookPath : null, + CommandExitCodes.Success, + warnings); } - private static int PreviewInstall( + private static HookOperationPlan BuildInstallPlan( HookCommandOptions options, - JsonSerializerOptions jsonOptions, - string projectPath, string hookPath, string chainedHookPath, string hookScript) { var ioHookPath = LongPath.EnsureWindowsPrefix(hookPath); var ioChainedHookPath = LongPath.EnsureWindowsPrefix(chainedHookPath); + var hookExists = File.Exists(ioHookPath); var chainedHookExists = File.Exists(ioChainedHookPath); - var status = "installed"; - var message = "cdidx pre-commit hook would be installed"; - var plannedAction = "create"; - string? reportedChainedHookPath = chainedHookExists ? chainedHookPath : null; + var chainedHookState = chainedHookExists ? "present" : "absent"; + var generatedHash = ComputeContentSha256(hookScript); + if (!hookExists) + { + return new HookOperationPlan( + "create", + "cdidx pre-commit hook would be installed", + "absent", + chainedHookState, + [ + new HookCommandFileChangeJsonResult( + "create", + hookPath, + "generated_managed_hook", + null, + null, + generatedHash, + null, + true), + ]); + } - if (File.Exists(ioHookPath)) + var existingHook = ReadHookFileWithinLimit(ioHookPath); + var analysis = AnalyzeManagedHook(existingHook); + var existingHash = ComputeFileSha256(ioHookPath); + var executable = IsExecutableHook(ioHookPath); + if (analysis.State == "managed") { - var existingHook = ReadHookFileWithinLimit(ioHookPath); - if (existingHook is not null && IsManagedHook(existingHook)) + if (IsExactHookScriptFile(ioHookPath, hookScript) && executable) { - if (IsExactHookScriptFile(ioHookPath, hookScript) - && IsExecutableHook(ioHookPath)) - { - status = "already_installed"; - message = "cdidx pre-commit hook is already installed"; - plannedAction = "none"; - } - else - { - status = "updated"; - message = "cdidx pre-commit hook would be updated"; - plannedAction = "replace_managed"; - } - - reportedChainedHookPath = chainedHookExists ? chainedHookPath : null; + return new HookOperationPlan( + "none", + "cdidx pre-commit hook is already installed; no filesystem change is planned", + analysis.State, + chainedHookState, + []); } - else - { - if (chainedHookExists && !options.Force) - { - return WriteResult( - options.Json, - jsonOptions, - "error", - $"chained hook already exists: {chainedHookPath}", - projectPath, + + return new HookOperationPlan( + "replace_managed", + "cdidx pre-commit hook would be updated", + analysis.State, + chainedHookState, + [ + new HookCommandFileChangeJsonResult( + "replace", hookPath, - chainedHookPath, - CommandExitCodes.UsageError, - dryRun: true, - plannedAction: "blocked", - managedHookPreview: hookScript); - } - - status = "updated"; - message = "cdidx pre-commit hook would be updated"; - plannedAction = "chain_existing"; - reportedChainedHookPath = chainedHookPath; - } + "generated_managed_hook", + null, + existingHash, + generatedHash, + executable, + true), + ]); } - return WriteResult( - options.Json, - jsonOptions, - status, - message, - projectPath, - hookPath, - reportedChainedHookPath, - CommandExitCodes.Success, - dryRun: true, - plannedAction: plannedAction, - managedHookPreview: hookScript); + if (chainedHookExists && !options.Force) + { + return new HookOperationPlan( + "blocked", + $"chained hook already exists: {chainedHookPath}", + analysis.State, + chainedHookState, + [], + Blocked: true, + BlockExitCode: CommandExitCodes.UsageError); + } + + return new HookOperationPlan( + "chain_existing", + "cdidx pre-commit hook would be updated and the existing hook would be chained", + analysis.State, + chainedHookState, + [ + new HookCommandFileChangeJsonResult( + chainedHookExists ? "replace" : "create", + chainedHookPath, + "existing_pre_commit_hook", + hookPath, + chainedHookExists ? ComputeFileSha256(ioChainedHookPath) : null, + existingHash, + chainedHookExists ? IsExecutableHook(ioChainedHookPath) : null, + executable), + new HookCommandFileChangeJsonResult( + "replace", + hookPath, + "generated_managed_hook", + null, + existingHash, + generatedHash, + executable, + true), + ]); } private static bool TryResolveHookWritePaths( @@ -360,6 +390,22 @@ private static int Uninstall(HookCommandOptions options, JsonSerializerOptions j { var warnings = new List(); if (!Directory.Exists(LongPath.EnsureWindowsPrefix(hooksDir))) + { + var absentPlan = HookOperationPlan.Absent; + return options.DryRun + ? WriteDryRunResult(options, jsonOptions, projectPath, hookPath, chainedHookPath, absentPlan) + : WriteResult(options.Json, jsonOptions, "absent", "cdidx pre-commit hook is not installed", projectPath, hookPath, null, CommandExitCodes.Success); + } + + if (!TryResolveHookWritePaths(gitDir, out _, out hookPath, out chainedHookPath)) + return WriteResult(options.Json, jsonOptions, "error", "unsafe Git hook file path", projectPath, null, null, CommandExitCodes.InstallError); + + var plan = BuildUninstallPlan(options, hookPath, chainedHookPath); + if (options.DryRun) + return WriteDryRunResult(options, jsonOptions, projectPath, hookPath, chainedHookPath, plan); + if (plan.Blocked) + return WriteBlockedPlanResult(options, jsonOptions, projectPath, hookPath, chainedHookPath, plan); + if (plan.PlannedAction == "none") { return WriteResult( options.Json, @@ -368,29 +414,19 @@ private static int Uninstall(HookCommandOptions options, JsonSerializerOptions j "cdidx pre-commit hook is not installed", projectPath, hookPath, - null, + plan.ChainedHookState == "present" ? chainedHookPath : null, CommandExitCodes.Success); } if (!TryResolveHookWritePaths(gitDir, out _, out hookPath, out chainedHookPath)) - return WriteResult(options.Json, jsonOptions, "error", "unsafe Git hook file path", projectPath, null, null, CommandExitCodes.InstallError); - - var ioHookPath = LongPath.EnsureWindowsPrefix(hookPath); - var ioChainedHookPath = LongPath.EnsureWindowsPrefix(chainedHookPath); - if (!File.Exists(ioHookPath)) - return WriteResult(options.Json, jsonOptions, "absent", "cdidx pre-commit hook is not installed", projectPath, hookPath, File.Exists(ioChainedHookPath) ? chainedHookPath : null, CommandExitCodes.Success); - - if (!IsManagedHookFile(ioHookPath) && !options.Force) - return WriteResult(options.Json, jsonOptions, "error", "pre-commit hook is not managed by cdidx; pass --force to remove it", projectPath, hookPath, null, CommandExitCodes.UsageError); + return WriteResult(options.Json, jsonOptions, "error", "Git hook file path became unsafe before write", projectPath, null, null, CommandExitCodes.InstallError); - if (File.Exists(ioChainedHookPath)) + if (plan.PlannedAction is "restore_chained" or "force_restore_chained") { try { - if (!TryResolveHookWritePaths(gitDir, out _, out hookPath, out chainedHookPath)) - return WriteResult(options.Json, jsonOptions, "error", "Git hook file path became unsafe before write", projectPath, null, null, CommandExitCodes.InstallError); - ioHookPath = LongPath.EnsureWindowsPrefix(hookPath); - ioChainedHookPath = LongPath.EnsureWindowsPrefix(chainedHookPath); + var ioHookPath = LongPath.EnsureWindowsPrefix(hookPath); + var ioChainedHookPath = LongPath.EnsureWindowsPrefix(chainedHookPath); ReplaceFile(ioChainedHookPath, ioHookPath, destinationBackupFileName: null); MakeExecutable(ioHookPath); } @@ -400,11 +436,17 @@ private static int Uninstall(HookCommandOptions options, JsonSerializerOptions j return WriteResult(options.Json, jsonOptions, "error", "failed to restore chained pre-commit hook", projectPath, hookPath, chainedHookPath, CommandExitCodes.InstallError, warnings); } } + else if (plan.PlannedAction == "remove_managed_block") + { + AtomicFileWriter.WriteText( + hookPath, + plan.ResultingHookContent!, + new UTF8Encoding(encoderShouldEmitUTF8Identifier: false), + MakeExecutable); + } else { - if (!TryResolveHookWritePaths(gitDir, out _, out hookPath, out chainedHookPath)) - return WriteResult(options.Json, jsonOptions, "error", "Git hook file path became unsafe before write", projectPath, null, null, CommandExitCodes.InstallError); - ioHookPath = LongPath.EnsureWindowsPrefix(hookPath); + var ioHookPath = LongPath.EnsureWindowsPrefix(hookPath); if (!TryDeleteFile(ioHookPath, hookPath, "managed_hook", warnings)) return WriteResult(options.Json, jsonOptions, "error", "failed to delete managed pre-commit hook", projectPath, hookPath, null, CommandExitCodes.InstallError, warnings); } @@ -412,6 +454,111 @@ private static int Uninstall(HookCommandOptions options, JsonSerializerOptions j return WriteResult(options.Json, jsonOptions, "uninstalled", "cdidx pre-commit hook uninstalled", projectPath, hookPath, null, CommandExitCodes.Success, warnings); } + private static HookOperationPlan BuildUninstallPlan( + HookCommandOptions options, + string hookPath, + string chainedHookPath) + { + var ioHookPath = LongPath.EnsureWindowsPrefix(hookPath); + var ioChainedHookPath = LongPath.EnsureWindowsPrefix(chainedHookPath); + var chainedHookExists = File.Exists(ioChainedHookPath); + var chainedHookState = chainedHookExists ? "present" : "absent"; + if (!File.Exists(ioHookPath)) + return HookOperationPlan.Absent with { ChainedHookState = chainedHookState }; + + var hookContent = ReadHookFileWithinLimit(ioHookPath); + var analysis = AnalyzeManagedHook(hookContent); + var hookHash = ComputeFileSha256(ioHookPath); + var hookExecutable = IsExecutableHook(ioHookPath); + if (analysis.State != "managed" && !options.Force) + { + var message = analysis.State == "conflicted" + ? "pre-commit hook has conflicted cdidx managed markers; pass --force to remove it" + : "pre-commit hook is not managed by cdidx; pass --force to remove it"; + return new HookOperationPlan( + "blocked", + message, + analysis.State, + chainedHookState, + [], + Blocked: true, + BlockExitCode: CommandExitCodes.UsageError); + } + + if (chainedHookExists) + { + var chainedHash = ComputeFileSha256(ioChainedHookPath); + return new HookOperationPlan( + options.Force && analysis.State != "managed" ? "force_restore_chained" : "restore_chained", + "the chained pre-commit hook would be restored", + analysis.State, + chainedHookState, + [ + new HookCommandFileChangeJsonResult( + "restore", + hookPath, + "chained_hook_backup", + chainedHookPath, + hookHash, + chainedHash, + hookExecutable, + true), + new HookCommandFileChangeJsonResult( + "consume", + chainedHookPath, + "chained_hook_backup", + chainedHookPath, + chainedHash, + null, + IsExecutableHook(ioChainedHookPath), + null), + ]); + } + + if (analysis.State == "managed" + && analysis.ContentWithoutManagedBlock is { } remainingContent + && !IsOnlyManagedHookPreamble(remainingContent)) + { + return new HookOperationPlan( + "remove_managed_block", + "the cdidx managed block would be removed while preserving the surrounding hook content", + analysis.State, + chainedHookState, + [ + new HookCommandFileChangeJsonResult( + "replace", + hookPath, + "existing_hook_without_cdidx_managed_block", + hookPath, + hookHash, + ComputeContentSha256(remainingContent), + hookExecutable, + true), + ], + ResultingHookContent: remainingContent); + } + + var plannedAction = options.Force && analysis.State != "managed" + ? "force_delete_unmanaged" + : "delete_managed"; + return new HookOperationPlan( + plannedAction, + "the pre-commit hook would be deleted", + analysis.State, + chainedHookState, + [ + new HookCommandFileChangeJsonResult( + "delete", + hookPath, + analysis.State == "managed" ? "cdidx_managed_hook" : "force_selected_hook", + null, + hookHash, + null, + hookExecutable, + null), + ]); + } + private static int Status(HookCommandOptions options, JsonSerializerOptions jsonOptions, string projectPath, string hookPath, string chainedHookPath) { var ioHookPath = LongPath.EnsureWindowsPrefix(hookPath); @@ -430,7 +577,7 @@ private static int UnknownCommand(HookCommandOptions options, JsonSerializerOpti } private static bool IsManagedHook(string content) - => content.Contains(BeginMarker, StringComparison.Ordinal) && content.Contains(EndMarker, StringComparison.Ordinal); + => AnalyzeManagedHook(content).State == "managed"; private static bool IsManagedHookFile(string ioHookPath) { @@ -441,6 +588,77 @@ private static bool IsManagedHookFile(string ioHookPath) private static string? ReadHookFileWithinLimit(string ioHookPath) => DataDirectorySecurity.ReadTextWithinLimit(ioHookPath, MaxHookMarkerBytes, FileShare.ReadWrite); + private static ManagedHookAnalysis AnalyzeManagedHook(string? content) + { + if (content is null) + return new ManagedHookAnalysis("unmanaged", null); + + var beginCount = CountOccurrences(content, BeginMarker); + var endCount = CountOccurrences(content, EndMarker); + if (beginCount == 0 && endCount == 0) + return new ManagedHookAnalysis("unmanaged", null); + if (beginCount != 1 || endCount != 1) + return new ManagedHookAnalysis("conflicted", null); + + var beginIndex = content.IndexOf(BeginMarker, StringComparison.Ordinal); + var endIndex = content.IndexOf(EndMarker, StringComparison.Ordinal); + if (endIndex < beginIndex + || !IsMarkerOnlyLine(content, beginIndex, BeginMarker) + || !IsMarkerOnlyLine(content, endIndex, EndMarker)) + { + return new ManagedHookAnalysis("conflicted", null); + } + + var blockStart = content.LastIndexOf('\n', beginIndex); + blockStart = blockStart < 0 ? 0 : blockStart + 1; + var blockEnd = content.IndexOf('\n', endIndex + EndMarker.Length); + blockEnd = blockEnd < 0 ? content.Length : blockEnd + 1; + return new ManagedHookAnalysis( + "managed", + content.Remove(blockStart, blockEnd - blockStart)); + } + + private static int CountOccurrences(string value, string marker) + { + var count = 0; + var offset = 0; + while ((offset = value.IndexOf(marker, offset, StringComparison.Ordinal)) >= 0) + { + count++; + offset += marker.Length; + } + + return count; + } + + private static bool IsMarkerOnlyLine(string content, int markerIndex, string marker) + { + var lineStart = content.LastIndexOf('\n', markerIndex); + lineStart = lineStart < 0 ? 0 : lineStart + 1; + var lineEnd = content.IndexOf('\n', markerIndex + marker.Length); + lineEnd = lineEnd < 0 ? content.Length : lineEnd; + return content[lineStart..lineEnd].Trim().Equals(marker, StringComparison.Ordinal); + } + + private static bool IsOnlyManagedHookPreamble(string content) + => string.IsNullOrWhiteSpace(content) + || content.Trim().Equals("#!/bin/sh", StringComparison.Ordinal); + + private static string ComputeContentSha256(string content) + => Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(content))).ToLowerInvariant(); + + private static string ComputeFileSha256(string ioPath) + { + using var stream = new FileStream( + ioPath, + FileMode.Open, + FileAccess.Read, + FileShare.ReadWrite, + bufferSize: 4096, + FileOptions.SequentialScan); + return Convert.ToHexString(SHA256.HashData(stream)).ToLowerInvariant(); + } + private static bool IsExactHookScriptFile(string ioHookPath, string hookScript) { var bytes = DataDirectorySecurity.ReadBytesWithinLimit( @@ -590,6 +808,54 @@ private static bool IsExecutableHook(string path) => OperatingSystem.IsWindows() || (File.GetUnixFileMode(path) & UnixFileMode.UserExecute) != 0; + private static int WriteDryRunResult( + HookCommandOptions options, + JsonSerializerOptions jsonOptions, + string projectPath, + string hookPath, + string chainedHookPath, + HookOperationPlan plan, + string? managedHookPreview = null) + { + var reportedChainedHookPath = plan.ChainedHookState == "present" + || plan.PlannedAction == "chain_existing" + ? chainedHookPath + : null; + return WriteResult( + options.Json, + jsonOptions, + plan.Blocked ? "error" : "dry_run", + plan.Message, + projectPath, + hookPath, + reportedChainedHookPath, + plan.Blocked ? plan.BlockExitCode : CommandExitCodes.Success, + dryRun: true, + plannedAction: plan.PlannedAction, + managedHookPreview: managedHookPreview, + filesystemMutation: false, + hookState: plan.HookState, + chainedHookState: plan.ChainedHookState, + plannedChanges: plan.PlannedChanges); + } + + private static int WriteBlockedPlanResult( + HookCommandOptions options, + JsonSerializerOptions jsonOptions, + string projectPath, + string hookPath, + string chainedHookPath, + HookOperationPlan plan) + => WriteResult( + options.Json, + jsonOptions, + "error", + plan.Message, + projectPath, + hookPath, + plan.ChainedHookState == "present" ? chainedHookPath : null, + plan.BlockExitCode); + private static int WriteResult( bool json, JsonSerializerOptions jsonOptions, @@ -602,7 +868,11 @@ private static int WriteResult( IReadOnlyList? warnings = null, bool? dryRun = null, string? plannedAction = null, - string? managedHookPreview = null) + string? managedHookPreview = null, + bool? filesystemMutation = null, + string? hookState = null, + string? chainedHookState = null, + IReadOnlyList? plannedChanges = null) { var hasWarnings = warnings is { Count: > 0 }; if (exitCode != CommandExitCodes.Success) @@ -628,6 +898,15 @@ private static int WriteResult( DiagnosticSanitizer.ForMessage(warning.Message))) .ToArray() : null; + var safePlannedChanges = plannedChanges? + .Select(static change => change with + { + Path = DiagnosticSanitizer.ForPath(change.Path), + SourcePath = change.SourcePath == null + ? null + : DiagnosticSanitizer.ForPath(change.SourcePath), + }) + .ToArray(); additionalJsonProperties = JsonSerializer.SerializeToNode( new HookCommandJsonResult( status, @@ -643,7 +922,11 @@ private static int WriteResult( plannedAction, managedHookPreview == null ? null - : DiagnosticRedactor.RedactSensitiveText(managedHookPreview, "[redacted]", redactPaths: true)), + : DiagnosticRedactor.RedactSensitiveText(managedHookPreview, "[redacted]", redactPaths: true), + filesystemMutation, + hookState, + chainedHookState, + safePlannedChanges), CliJsonSerializerContextFactory.Create(jsonOptions).HookCommandJsonResult)!.AsObject(); } @@ -667,6 +950,16 @@ CommandExitCodes.UsageError or CommandExitCodes.InvalidArgument "hooks", DiagnosticSanitizer.ForPath(projectPath), additionalJsonProperties); + if (!json && dryRun == true) + { + WriteDryRunDetails( + plannedAction, + hookState, + chainedHookState, + plannedChanges, + managedHookPreview); + } + return result; } @@ -685,7 +978,11 @@ CommandExitCodes.UsageError or CommandExitCodes.InvalidArgument hasWarnings ? warnings : null, dryRun, plannedAction, - managedHookPreview), + managedHookPreview, + filesystemMutation, + hookState, + chainedHookState, + plannedChanges), CliJsonSerializerContextFactory.Create(jsonOptions).HookCommandJsonResult)); } else @@ -711,20 +1008,76 @@ CommandExitCodes.UsageError or CommandExitCodes.InvalidArgument if (dryRun == true) { - CommandErrorWriter.WriteStdout($"Planned action: {plannedAction}"); - CommandErrorWriter.WriteStdout("Managed hook preview:"); - CommandErrorWriter.WriteStdout(managedHookPreview ?? string.Empty); + WriteDryRunDetails( + plannedAction, + hookState, + chainedHookState, + plannedChanges, + managedHookPreview); } } return exitCode; } + private static void WriteDryRunDetails( + string? plannedAction, + string? hookState, + string? chainedHookState, + IReadOnlyList? plannedChanges, + string? managedHookPreview) + { + CommandErrorWriter.WriteStdout($"Planned action: {plannedAction}"); + CommandErrorWriter.WriteStdout("Filesystem mutation: false"); + CommandErrorWriter.WriteStdout($"Hook state: {hookState}"); + CommandErrorWriter.WriteStdout($"Chained hook state: {chainedHookState}"); + if (plannedChanges is { Count: > 0 }) + { + CommandErrorWriter.WriteStdout("Planned changes:"); + foreach (var change in plannedChanges) + { + var source = change.SourcePath == null + ? string.Empty + : $" from {change.SourcePath}"; + CommandErrorWriter.WriteStdout( + $"- {change.Action} {change.Path}{source} ({change.Provenance})"); + } + } + + if (managedHookPreview != null) + { + CommandErrorWriter.WriteStdout("Managed hook preview:"); + CommandErrorWriter.WriteStdout(managedHookPreview); + } + } + private static string GetUsage() => "cdidx hooks [--project ] [--force] [--dry-run] [--json]"; private static void PrintUsage() => CommandErrorWriter.WriteStderr($"Usage: {GetUsage()}"); + + private sealed record ManagedHookAnalysis( + string State, + string? ContentWithoutManagedBlock); + + private sealed record HookOperationPlan( + string PlannedAction, + string Message, + string HookState, + string ChainedHookState, + IReadOnlyList PlannedChanges, + bool Blocked = false, + int BlockExitCode = CommandExitCodes.Success, + string? ResultingHookContent = null) + { + public static HookOperationPlan Absent { get; } = new( + "none", + "cdidx pre-commit hook is not installed; no filesystem change is planned", + "absent", + "absent", + []); + } } public sealed record HookCommandOptions(string? Command, string? ProjectPath, bool Json, bool Force, bool DryRun, bool ShowHelp, string? ParseError); @@ -735,6 +1088,16 @@ public sealed record HookCommandWarningJsonResult( [property: JsonPropertyName("diagnostic_path")] string DiagnosticPath, [property: JsonPropertyName("message")] string Message); +public sealed record HookCommandFileChangeJsonResult( + [property: JsonPropertyName("action")] string Action, + [property: JsonPropertyName("path")] string Path, + [property: JsonPropertyName("provenance")] string Provenance, + [property: JsonPropertyName("source_path")] string? SourcePath, + [property: JsonPropertyName("before_sha256")] string? BeforeSha256, + [property: JsonPropertyName("after_sha256")] string? AfterSha256, + [property: JsonPropertyName("executable_before")] bool? ExecutableBefore, + [property: JsonPropertyName("executable_after")] bool? ExecutableAfter); + public sealed record HookCommandJsonResult( string Status, string Message, @@ -748,4 +1111,8 @@ public sealed record HookCommandJsonResult( bool? DryRun = null, string? PlannedAction = null, string? ManagedHookPreview = null, + bool? FilesystemMutation = null, + string? HookState = null, + string? ChainedHookState = null, + IReadOnlyList? PlannedChanges = null, [property: JsonPropertyName("api_version")] string ApiVersion = JsonOutputContract.ApiVersion) : IVersionedJsonResult; diff --git a/src/CodeIndex/Cli/JsonOutputContracts.cs b/src/CodeIndex/Cli/JsonOutputContracts.cs index 4de33d48c..7271b9bb1 100644 --- a/src/CodeIndex/Cli/JsonOutputContracts.cs +++ b/src/CodeIndex/Cli/JsonOutputContracts.cs @@ -1178,6 +1178,8 @@ internal sealed record ValidateConfigJsonResult( [JsonSerializable(typeof(IndexWatchRecoveryCommandJsonResult))] [JsonSerializable(typeof(ExportImportCommandRunner.ImportResult))] [JsonSerializable(typeof(HookCommandJsonResult))] +[JsonSerializable(typeof(HookCommandFileChangeJsonResult))] +[JsonSerializable(typeof(List))] [JsonSerializable(typeof(HookCommandWarningJsonResult))] [JsonSerializable(typeof(List))] [JsonSerializable(typeof(JsonStreamDoneResult))] diff --git a/tests/CodeIndex.Tests/CliFlagSchemaTests.cs b/tests/CodeIndex.Tests/CliFlagSchemaTests.cs index ca2f78f02..a6b2a1926 100644 --- a/tests/CodeIndex.Tests/CliFlagSchemaTests.cs +++ b/tests/CodeIndex.Tests/CliFlagSchemaTests.cs @@ -453,7 +453,7 @@ public void RegistrySurfacesSafetyAndAcceptedOptionsWithoutAdvertisingRejectedGo Assert.Contains("--dry-run", hookFlags); Assert.DoesNotContain(CliFlagSchema.GetCompletionFlagsForCommand("hooks"), flag => flag.Name == "--dry-run"); Assert.Contains(CliFlagSchema.GetCompletionFlagsForCommand("hooks", "install"), flag => flag.Name == "--dry-run"); - Assert.DoesNotContain(CliFlagSchema.GetCompletionFlagsForCommand("hooks", "uninstall"), flag => flag.Name == "--dry-run"); + Assert.Contains(CliFlagSchema.GetCompletionFlagsForCommand("hooks", "uninstall"), flag => flag.Name == "--dry-run"); Assert.DoesNotContain(CliFlagSchema.GetCompletionFlagsForCommand("hooks", "status"), flag => flag.Name == "--dry-run"); Assert.Contains(CliFlagSchema.GetCompletionFlagsForCommand("hooks", "install"), flag => flag.Name == "--force"); Assert.Contains(CliFlagSchema.GetCompletionFlagsForCommand("hooks", "uninstall"), flag => flag.Name == "--force"); diff --git a/tests/CodeIndex.Tests/HookCommandRunnerTests.cs b/tests/CodeIndex.Tests/HookCommandRunnerTests.cs index 487589108..3e70ca5d8 100644 --- a/tests/CodeIndex.Tests/HookCommandRunnerTests.cs +++ b/tests/CodeIndex.Tests/HookCommandRunnerTests.cs @@ -113,7 +113,7 @@ public void Hooks_Install_RepairsUnusableManagedHook_Issue4716() Assert.Equal(CommandExitCodes.Success, preview.ExitCode); using (var document = JsonDocument.Parse(preview.StdOut)) { - Assert.Equal("updated", document.RootElement.GetProperty("status").GetString()); + Assert.Equal("dry_run", document.RootElement.GetProperty("status").GetString()); Assert.Equal("replace_managed", document.RootElement.GetProperty("planned_action").GetString()); } Assert.Equal( @@ -140,7 +140,7 @@ public void Hooks_Install_RepairsUnusableManagedHook_Issue4716() Assert.Equal(CommandExitCodes.Success, encodingPreview.ExitCode); using (var document = JsonDocument.Parse(encodingPreview.StdOut)) { - Assert.Equal("updated", document.RootElement.GetProperty("status").GetString()); + Assert.Equal("dry_run", document.RootElement.GetProperty("status").GetString()); Assert.Equal("replace_managed", document.RootElement.GetProperty("planned_action").GetString()); } Assert.Equal([0xff, 0xfe], File.ReadAllBytes(hookPath)[..2]); @@ -160,7 +160,7 @@ public void Hooks_Install_RepairsUnusableManagedHook_Issue4716() } [Fact] - public void Hooks_InstallDryRun_PreviewsCreateChainReplaceAndNoOpWithoutWriting_Issue4716() + public void Hooks_InstallDryRun_PreviewsCreateChainReplaceAndNoOpWithoutWriting_Issues4716And4883() { var projectRoot = TestProjectHelper.CreateTempProject("hook_install_dry_run"); try @@ -189,9 +189,16 @@ public void Hooks_InstallDryRun_PreviewsCreateChainReplaceAndNoOpWithoutWriting_ string managedHookPreview; using (var document = JsonDocument.Parse(createPreview.StdOut)) { - Assert.Equal("installed", document.RootElement.GetProperty("status").GetString()); + Assert.Equal("dry_run", document.RootElement.GetProperty("status").GetString()); Assert.True(document.RootElement.GetProperty("dry_run").GetBoolean()); + Assert.False(document.RootElement.GetProperty("filesystem_mutation").GetBoolean()); + Assert.Equal("absent", document.RootElement.GetProperty("hook_state").GetString()); Assert.Equal("create", document.RootElement.GetProperty("planned_action").GetString()); + var change = Assert.Single(document.RootElement.GetProperty("planned_changes").EnumerateArray()); + Assert.Equal("create", change.GetProperty("action").GetString()); + Assert.Equal("generated_managed_hook", change.GetProperty("provenance").GetString()); + Assert.Equal(JsonValueKind.Null, change.GetProperty("before_sha256").ValueKind); + Assert.Equal(64, change.GetProperty("after_sha256").GetString()!.Length); managedHookPreview = document.RootElement.GetProperty("managed_hook_preview").GetString()!; Assert.Contains("BEGIN CDIDX MANAGED PRE-COMMIT", managedHookPreview, StringComparison.Ordinal); } @@ -207,7 +214,8 @@ public void Hooks_InstallDryRun_PreviewsCreateChainReplaceAndNoOpWithoutWriting_ Assert.Equal(CommandExitCodes.Success, orphanChainPreview.ExitCode); using (var document = JsonDocument.Parse(orphanChainPreview.StdOut)) { - Assert.Equal("installed", document.RootElement.GetProperty("status").GetString()); + Assert.Equal("dry_run", document.RootElement.GetProperty("status").GetString()); + Assert.False(document.RootElement.GetProperty("filesystem_mutation").GetBoolean()); Assert.Equal("create", document.RootElement.GetProperty("planned_action").GetString()); Assert.Equal(chainedHookPath, document.RootElement.GetProperty("chained_hook_path").GetString()); } @@ -224,9 +232,13 @@ public void Hooks_InstallDryRun_PreviewsCreateChainReplaceAndNoOpWithoutWriting_ Assert.Equal(CommandExitCodes.Success, chainPreview.ExitCode); using (var document = JsonDocument.Parse(chainPreview.StdOut)) { - Assert.Equal("updated", document.RootElement.GetProperty("status").GetString()); + Assert.Equal("dry_run", document.RootElement.GetProperty("status").GetString()); Assert.Equal("chain_existing", document.RootElement.GetProperty("planned_action").GetString()); Assert.Equal(chainedHookPath, document.RootElement.GetProperty("chained_hook_path").GetString()); + var changes = document.RootElement.GetProperty("planned_changes").EnumerateArray().ToArray(); + Assert.Equal(2, changes.Length); + Assert.Equal("existing_pre_commit_hook", changes[0].GetProperty("provenance").GetString()); + Assert.Equal(hookPath, changes[0].GetProperty("source_path").GetString()); } Assert.Equal(customHook, File.ReadAllText(hookPath)); Assert.False(File.Exists(chainedHookPath)); @@ -243,6 +255,7 @@ public void Hooks_InstallDryRun_PreviewsCreateChainReplaceAndNoOpWithoutWriting_ { Assert.Equal("error", document.RootElement.GetProperty("status").GetString()); Assert.True(document.RootElement.GetProperty("dry_run").GetBoolean()); + Assert.False(document.RootElement.GetProperty("filesystem_mutation").GetBoolean()); Assert.Equal("blocked", document.RootElement.GetProperty("planned_action").GetString()); Assert.Contains( "BEGIN CDIDX MANAGED PRE-COMMIT", @@ -256,7 +269,9 @@ public void Hooks_InstallDryRun_PreviewsCreateChainReplaceAndNoOpWithoutWriting_ ["install", "--project", projectRoot, "--dry-run"]); Assert.Equal(CommandExitCodes.UsageError, blockedHumanPreview.ExitCode); - Assert.Equal(string.Empty, blockedHumanPreview.StdOut); + Assert.Contains("Planned action: blocked", blockedHumanPreview.StdOut, StringComparison.Ordinal); + Assert.Contains("Filesystem mutation: false", blockedHumanPreview.StdOut, StringComparison.Ordinal); + Assert.Contains("Managed hook preview:", blockedHumanPreview.StdOut, StringComparison.Ordinal); Assert.Contains("chained hook already exists", blockedHumanPreview.StdErr, StringComparison.Ordinal); Assert.Contains("Hint:", blockedHumanPreview.StdErr, StringComparison.Ordinal); Assert.Contains("Usage:", blockedHumanPreview.StdErr, StringComparison.Ordinal); @@ -273,8 +288,12 @@ public void Hooks_InstallDryRun_PreviewsCreateChainReplaceAndNoOpWithoutWriting_ Assert.Equal(CommandExitCodes.Success, replacePreview.ExitCode); using (var document = JsonDocument.Parse(replacePreview.StdOut)) { - Assert.Equal("updated", document.RootElement.GetProperty("status").GetString()); + Assert.Equal("dry_run", document.RootElement.GetProperty("status").GetString()); Assert.Equal("replace_managed", document.RootElement.GetProperty("planned_action").GetString()); + var change = Assert.Single(document.RootElement.GetProperty("planned_changes").EnumerateArray()); + Assert.NotEqual( + change.GetProperty("before_sha256").GetString(), + change.GetProperty("after_sha256").GetString()); } Assert.Equal(staleManagedHook, File.ReadAllText(hookPath)); @@ -294,8 +313,9 @@ public void Hooks_InstallDryRun_PreviewsCreateChainReplaceAndNoOpWithoutWriting_ Assert.Equal(CommandExitCodes.Success, noOpPreview.ExitCode); using (var document = JsonDocument.Parse(noOpPreview.StdOut)) { - Assert.Equal("already_installed", document.RootElement.GetProperty("status").GetString()); + Assert.Equal("dry_run", document.RootElement.GetProperty("status").GetString()); Assert.Equal("none", document.RootElement.GetProperty("planned_action").GetString()); + Assert.Empty(document.RootElement.GetProperty("planned_changes").EnumerateArray()); } Assert.Equal(managedHookPreview, File.ReadAllText(hookPath)); Assert.False(File.Exists(chainedHookPath)); @@ -307,7 +327,7 @@ public void Hooks_InstallDryRun_PreviewsCreateChainReplaceAndNoOpWithoutWriting_ } [Fact] - public void Hooks_DryRun_RejectsNonInstallSubcommand_Issue4716() + public void Hooks_DryRun_RejectsStatusSubcommand_Issues4716And4883() { var (exitCode, stdout, stderr) = RunHooksAndCaptureStreams(["status", "--dry-run", "--json"]); @@ -316,10 +336,205 @@ public void Hooks_DryRun_RejectsNonInstallSubcommand_Issue4716() using var document = JsonDocument.Parse(stdout); Assert.Equal("error", document.RootElement.GetProperty("status").GetString()); Assert.Equal( - "--dry-run is supported only for hooks install", + "--dry-run is supported only for hooks install or uninstall", document.RootElement.GetProperty("message").GetString()); } + [Fact] + public void Hooks_UninstallDryRun_PreviewsBlockRemovalRestoreForceAndIdempotenceWithoutWriting_Issue4883() + { + var projectRoot = TestProjectHelper.CreateTempProject("hook_uninstall_dry_run"); + try + { + TestProjectHelper.InitializeGitRepo(projectRoot); + var hooksDir = Path.Combine(projectRoot, ".git", "hooks"); + var hookPath = Path.Combine(hooksDir, "pre-commit"); + var chainedHookPath = Path.Combine(hooksDir, "pre-commit.cdidx-chain"); + TestProjectHelper.DeleteDirectory(hooksDir); + + var absentPreview = RunHooksAndCaptureStreams( + ["uninstall", "--project", projectRoot, "--dry-run", "--json"]); + + Assert.Equal(CommandExitCodes.Success, absentPreview.ExitCode); + using (var document = JsonDocument.Parse(absentPreview.StdOut)) + { + Assert.Equal("dry_run", document.RootElement.GetProperty("status").GetString()); + Assert.True(document.RootElement.GetProperty("dry_run").GetBoolean()); + Assert.False(document.RootElement.GetProperty("filesystem_mutation").GetBoolean()); + Assert.Equal("none", document.RootElement.GetProperty("planned_action").GetString()); + Assert.Equal("absent", document.RootElement.GetProperty("hook_state").GetString()); + Assert.Empty(document.RootElement.GetProperty("planned_changes").EnumerateArray()); + } + var repeatedAbsentPreview = RunHooksAndCaptureStreams( + ["uninstall", "--project", projectRoot, "--dry-run", "--json"]); + Assert.Equal(absentPreview.StdOut, repeatedAbsentPreview.StdOut); + Assert.False(Directory.Exists(hooksDir)); + + Assert.Equal( + CommandExitCodes.Success, + RunHooksAndCaptureStreams(["install", "--project", projectRoot]).ExitCode); + var managedHook = File.ReadAllText(hookPath); + var managedBlockWithSurroundingContent = managedHook + .Replace("#!/bin/sh\n", "#!/bin/sh\necho before\n", StringComparison.Ordinal) + + "\necho after\n"; + File.WriteAllText(hookPath, managedBlockWithSurroundingContent); + var expectedRemainingContent = "#!/bin/sh\necho before\necho after\n"; + var managedWriteTime = File.GetLastWriteTimeUtc(hookPath); + UnixFileMode? managedMode = null; + if (!OperatingSystem.IsWindows()) + managedMode = File.GetUnixFileMode(hookPath); + + var blockPreview = RunHooksAndCaptureStreams( + ["uninstall", "--project", projectRoot, "--dry-run", "--json"]); + + Assert.Equal(CommandExitCodes.Success, blockPreview.ExitCode); + using (var document = JsonDocument.Parse(blockPreview.StdOut)) + { + Assert.Equal("dry_run", document.RootElement.GetProperty("status").GetString()); + Assert.Equal("remove_managed_block", document.RootElement.GetProperty("planned_action").GetString()); + Assert.Equal("managed", document.RootElement.GetProperty("hook_state").GetString()); + var change = Assert.Single(document.RootElement.GetProperty("planned_changes").EnumerateArray()); + Assert.Equal("replace", change.GetProperty("action").GetString()); + Assert.Equal( + "existing_hook_without_cdidx_managed_block", + change.GetProperty("provenance").GetString()); + Assert.NotEqual( + change.GetProperty("before_sha256").GetString(), + change.GetProperty("after_sha256").GetString()); + } + Assert.Equal(managedBlockWithSurroundingContent, File.ReadAllText(hookPath)); + Assert.Equal(managedWriteTime, File.GetLastWriteTimeUtc(hookPath)); + if (!OperatingSystem.IsWindows()) + Assert.Equal(managedMode!.Value, File.GetUnixFileMode(hookPath)); + + Assert.Equal( + CommandExitCodes.Success, + RunHooksAndCaptureStreams(["uninstall", "--project", projectRoot, "--json"]).ExitCode); + Assert.Equal(expectedRemainingContent, File.ReadAllText(hookPath)); + + const string unmanagedHook = "#!/bin/sh\necho unmanaged\n"; + File.WriteAllText(hookPath, unmanagedHook); + var unmanagedWriteTime = File.GetLastWriteTimeUtc(hookPath); + + var blockedPreview = RunHooksAndCaptureStreams( + ["uninstall", "--project", projectRoot, "--dry-run", "--json"]); + + Assert.Equal(CommandExitCodes.UsageError, blockedPreview.ExitCode); + using (var document = JsonDocument.Parse(blockedPreview.StdOut)) + { + Assert.Equal("error", document.RootElement.GetProperty("status").GetString()); + Assert.False(document.RootElement.GetProperty("filesystem_mutation").GetBoolean()); + Assert.Equal("blocked", document.RootElement.GetProperty("planned_action").GetString()); + Assert.Equal("unmanaged", document.RootElement.GetProperty("hook_state").GetString()); + } + Assert.Equal(unmanagedHook, File.ReadAllText(hookPath)); + Assert.Equal(unmanagedWriteTime, File.GetLastWriteTimeUtc(hookPath)); + + HookCommandRunner.DeleteFileForTesting = _ => throw new UnauthorizedAccessException("delete denied"); + var forcePreview = RunHooksAndCaptureStreams( + ["uninstall", "--project", projectRoot, "--force", "--dry-run", "--json"]); + HookCommandRunner.DeleteFileForTesting = null; + + Assert.Equal(CommandExitCodes.Success, forcePreview.ExitCode); + using (var document = JsonDocument.Parse(forcePreview.StdOut)) + { + Assert.Equal("dry_run", document.RootElement.GetProperty("status").GetString()); + Assert.Equal("force_delete_unmanaged", document.RootElement.GetProperty("planned_action").GetString()); + var change = Assert.Single(document.RootElement.GetProperty("planned_changes").EnumerateArray()); + Assert.Equal("delete", change.GetProperty("action").GetString()); + Assert.Equal("force_selected_hook", change.GetProperty("provenance").GetString()); + } + Assert.Equal(unmanagedHook, File.ReadAllText(hookPath)); + Assert.Equal(unmanagedWriteTime, File.GetLastWriteTimeUtc(hookPath)); + + const string conflictedHook = "#!/bin/sh\n# BEGIN CDIDX MANAGED PRE-COMMIT\necho conflicted\n"; + File.WriteAllText(hookPath, conflictedHook); + var conflictedPreview = RunHooksAndCaptureStreams( + ["uninstall", "--project", projectRoot, "--dry-run", "--json"]); + + Assert.Equal(CommandExitCodes.UsageError, conflictedPreview.ExitCode); + using (var document = JsonDocument.Parse(conflictedPreview.StdOut)) + { + Assert.Equal("conflicted", document.RootElement.GetProperty("hook_state").GetString()); + Assert.Equal("blocked", document.RootElement.GetProperty("planned_action").GetString()); + } + Assert.Equal(conflictedHook, File.ReadAllText(hookPath)); + + const string originalHook = "#!/bin/sh\necho restore me\n"; + File.WriteAllText(hookPath, originalHook); + Assert.Equal( + CommandExitCodes.Success, + RunHooksAndCaptureStreams(["install", "--project", projectRoot]).ExitCode); + var installedContent = File.ReadAllText(hookPath); + var chainedContent = File.ReadAllText(chainedHookPath); + var installedWriteTime = File.GetLastWriteTimeUtc(hookPath); + var chainedWriteTime = File.GetLastWriteTimeUtc(chainedHookPath); + UnixFileMode? installedMode = null; + UnixFileMode? chainedMode = null; + if (!OperatingSystem.IsWindows()) + { + installedMode = File.GetUnixFileMode(hookPath); + chainedMode = File.GetUnixFileMode(chainedHookPath); + } + + var humanRestorePreview = RunHooksAndCaptureStreams( + ["uninstall", "--project", projectRoot, "--dry-run"]); + var restorePreview = RunHooksAndCaptureStreams( + ["uninstall", "--project", projectRoot, "--dry-run", "--json"]); + var repeatedRestorePreview = RunHooksAndCaptureStreams( + ["uninstall", "--project", projectRoot, "--dry-run", "--json"]); + + Assert.Equal(CommandExitCodes.Success, humanRestorePreview.ExitCode); + Assert.Contains("Planned action: restore_chained", humanRestorePreview.StdOut, StringComparison.Ordinal); + Assert.Contains("Filesystem mutation: false", humanRestorePreview.StdOut, StringComparison.Ordinal); + Assert.DoesNotContain("Managed hook preview:", humanRestorePreview.StdOut, StringComparison.Ordinal); + Assert.Equal(CommandExitCodes.Success, restorePreview.ExitCode); + using (var document = JsonDocument.Parse(restorePreview.StdOut)) + { + Assert.Equal("dry_run", document.RootElement.GetProperty("status").GetString()); + Assert.Equal("restore_chained", document.RootElement.GetProperty("planned_action").GetString()); + Assert.Equal("present", document.RootElement.GetProperty("chained_hook_state").GetString()); + var changes = document.RootElement.GetProperty("planned_changes").EnumerateArray().ToArray(); + Assert.Equal(2, changes.Length); + Assert.Equal("restore", changes[0].GetProperty("action").GetString()); + Assert.Equal("chained_hook_backup", changes[0].GetProperty("provenance").GetString()); + Assert.Equal(chainedHookPath, changes[0].GetProperty("source_path").GetString()); + } + Assert.Equal(installedContent, File.ReadAllText(hookPath)); + Assert.Equal(chainedContent, File.ReadAllText(chainedHookPath)); + Assert.Equal(installedWriteTime, File.GetLastWriteTimeUtc(hookPath)); + Assert.Equal(chainedWriteTime, File.GetLastWriteTimeUtc(chainedHookPath)); + Assert.Equal(restorePreview.StdOut, repeatedRestorePreview.StdOut); + if (!OperatingSystem.IsWindows()) + { + Assert.Equal(installedMode!.Value, File.GetUnixFileMode(hookPath)); + Assert.Equal(chainedMode!.Value, File.GetUnixFileMode(chainedHookPath)); + } + + Assert.Equal( + CommandExitCodes.Success, + RunHooksAndCaptureStreams(["uninstall", "--project", projectRoot]).ExitCode); + Assert.Equal(originalHook, File.ReadAllText(hookPath)); + Assert.False(File.Exists(chainedHookPath)); + + var idempotentPreview = RunHooksAndCaptureStreams( + ["uninstall", "--project", projectRoot, "--dry-run", "--json"]); + Assert.Equal(CommandExitCodes.UsageError, idempotentPreview.ExitCode); + using (var document = JsonDocument.Parse(idempotentPreview.StdOut)) + { + Assert.Equal("blocked", document.RootElement.GetProperty("planned_action").GetString()); + Assert.Equal("unmanaged", document.RootElement.GetProperty("hook_state").GetString()); + } + Assert.Equal(originalHook, File.ReadAllText(hookPath)); + } + finally + { + HookCommandRunner.DeleteFileForTesting = null; + HookCommandRunner.ReplaceFileForTesting = null; + TestProjectHelper.DeleteDirectory(projectRoot); + } + } + [Fact] public void Hooks_Install_QuotesSelectedProjectPathInGeneratedHook() { @@ -401,7 +616,7 @@ public void Hooks_Install_RejectsSymlinkedHooksDescendantBeforeExternalWrite_Iss } [Fact] - public void Hooks_Install_RejectsSymlinkedHookFileBeforeExternalWrite_Issue4599() + public void Hooks_InstallAndUninstallDryRun_RejectSymlinkedHookFileBeforeExternalAccess_Issues4599And4883() { if (OperatingSystem.IsWindows()) return; @@ -418,10 +633,14 @@ public void Hooks_Install_RejectsSymlinkedHookFileBeforeExternalWrite_Issue4599( File.CreateSymbolicLink(hookLink, externalHook); var result = RunHooksAndCaptureStreams(["install", "--project", projectRoot]); + var uninstallPreview = RunHooksAndCaptureStreams( + ["uninstall", "--project", projectRoot, "--dry-run"]); Assert.Equal(CommandExitCodes.InstallError, result.ExitCode); + Assert.Equal(CommandExitCodes.InstallError, uninstallPreview.ExitCode); Assert.Equal("external sentinel\n", File.ReadAllText(externalHook)); Assert.Contains("unsafe Git hook file path", result.StdErr, StringComparison.Ordinal); + Assert.Contains("unsafe Git hook file path", uninstallPreview.StdErr, StringComparison.Ordinal); } finally { From b76b459f089f7ac5fe3e298a8e514ba1439dfa83 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Wed, 29 Jul 2026 21:39:45 +0900 Subject: [PATCH 2/3] Preserve hook bytes during uninstall (#4883) --- src/CodeIndex/Cli/HookCommandRunner.cs | 354 +++++++++++++++--- .../CodeIndex.Tests/HookCommandRunnerTests.cs | 83 +++- 2 files changed, 383 insertions(+), 54 deletions(-) diff --git a/src/CodeIndex/Cli/HookCommandRunner.cs b/src/CodeIndex/Cli/HookCommandRunner.cs index 7e6a63385..07901d710 100644 --- a/src/CodeIndex/Cli/HookCommandRunner.cs +++ b/src/CodeIndex/Cli/HookCommandRunner.cs @@ -15,6 +15,9 @@ public static class HookCommandRunner private const string ChainedHookName = "pre-commit.cdidx-chain"; private const string BeginMarker = "# BEGIN CDIDX MANAGED PRE-COMMIT"; private const string EndMarker = "# END CDIDX MANAGED PRE-COMMIT"; + private static readonly byte[] BeginMarkerBytes = Encoding.ASCII.GetBytes(BeginMarker); + private static readonly byte[] EndMarkerBytes = Encoding.ASCII.GetBytes(EndMarker); + private static readonly byte[] HookPreambleBytes = Encoding.ASCII.GetBytes("#!/bin/sh"); internal const int MaxHookMarkerBytes = 64 * 1024; internal static Action? DeleteFileForTesting { get; set; } internal static Action? ReplaceFileForTesting { get; set; } @@ -284,7 +287,7 @@ private static HookOperationPlan BuildInstallPlan( ]); } - var existingHook = ReadHookFileWithinLimit(ioHookPath); + var existingHook = ReadHookBytesWithinLimit(ioHookPath); var analysis = AnalyzeManagedHook(existingHook); var existingHash = ComputeFileSha256(ioHookPath); var executable = IsExecutableHook(ioHookPath); @@ -438,11 +441,13 @@ private static int Uninstall(HookCommandOptions options, JsonSerializerOptions j } else if (plan.PlannedAction == "remove_managed_block") { - AtomicFileWriter.WriteText( + Action? applyFileMode = null; + if (plan.ResultingHookMode is { } resultingHookMode) + applyFileMode = path => ApplyUnixFileMode(path, resultingHookMode); + AtomicFileWriter.Write( hookPath, - plan.ResultingHookContent!, - new UTF8Encoding(encoderShouldEmitUTF8Identifier: false), - MakeExecutable); + stream => stream.Write(plan.ResultingHookBytes!), + applyFileMode); } else { @@ -466,10 +471,13 @@ private static HookOperationPlan BuildUninstallPlan( if (!File.Exists(ioHookPath)) return HookOperationPlan.Absent with { ChainedHookState = chainedHookState }; - var hookContent = ReadHookFileWithinLimit(ioHookPath); + var hookContent = ReadHookBytesWithinLimit(ioHookPath); var analysis = AnalyzeManagedHook(hookContent); var hookHash = ComputeFileSha256(ioHookPath); var hookExecutable = IsExecutableHook(ioHookPath); + UnixFileMode? hookMode = null; + if (!OperatingSystem.IsWindows()) + hookMode = File.GetUnixFileMode(ioHookPath); if (analysis.State != "managed" && !options.Force) { var message = analysis.State == "conflicted" @@ -516,8 +524,8 @@ private static HookOperationPlan BuildUninstallPlan( } if (analysis.State == "managed" - && analysis.ContentWithoutManagedBlock is { } remainingContent - && !IsOnlyManagedHookPreamble(remainingContent)) + && analysis.BytesWithoutManagedBlock is { } remainingBytes + && !analysis.BytesWithoutManagedBlockArePreamble) { return new HookOperationPlan( "remove_managed_block", @@ -531,11 +539,12 @@ private static HookOperationPlan BuildUninstallPlan( "existing_hook_without_cdidx_managed_block", hookPath, hookHash, - ComputeContentSha256(remainingContent), + ComputeBytesSha256(remainingBytes), hookExecutable, - true), + hookExecutable), ], - ResultingHookContent: remainingContent); + ResultingHookBytes: remainingBytes, + ResultingHookMode: hookMode); } var plannedAction = options.Force && analysis.State != "managed" @@ -576,76 +585,319 @@ private static int UnknownCommand(HookCommandOptions options, JsonSerializerOpti return WriteResult(options.Json, jsonOptions, "error", $"unknown hooks command: {ConsoleUi.FormatBoundedValue(options.Command)}", projectPath, null, null, CommandExitCodes.UsageError); } - private static bool IsManagedHook(string content) - => AnalyzeManagedHook(content).State == "managed"; - private static bool IsManagedHookFile(string ioHookPath) { - var content = ReadHookFileWithinLimit(ioHookPath); - return content is not null && IsManagedHook(content); + var content = ReadHookBytesWithinLimit(ioHookPath); + return AnalyzeManagedHook(content).State == "managed"; } - private static string? ReadHookFileWithinLimit(string ioHookPath) - => DataDirectorySecurity.ReadTextWithinLimit(ioHookPath, MaxHookMarkerBytes, FileShare.ReadWrite); + private static byte[]? ReadHookBytesWithinLimit(string ioHookPath) + => DataDirectorySecurity.ReadBytesWithinLimit(ioHookPath, MaxHookMarkerBytes, FileShare.ReadWrite); - private static ManagedHookAnalysis AnalyzeManagedHook(string? content) + private static ManagedHookAnalysis AnalyzeManagedHook(byte[]? content) { if (content is null) - return new ManagedHookAnalysis("unmanaged", null); + return new ManagedHookAnalysis("unmanaged", null, false); - var beginCount = CountOccurrences(content, BeginMarker); - var endCount = CountOccurrences(content, EndMarker); + var rawAnalysis = AnalyzeRawManagedHook(content); + if (rawAnalysis.State != "unmanaged" + || !TryGetBomEncoding(content, out var encoding, out var bomLength)) + { + return rawAnalysis; + } + + return AnalyzeBomEncodedManagedHook(content, encoding, bomLength); + } + + private static ManagedHookAnalysis AnalyzeRawManagedHook(byte[] content) + { + var contentSpan = content.AsSpan(); + var beginCount = CountOccurrences(contentSpan, BeginMarkerBytes); + var endCount = CountOccurrences(contentSpan, EndMarkerBytes); if (beginCount == 0 && endCount == 0) - return new ManagedHookAnalysis("unmanaged", null); + return new ManagedHookAnalysis("unmanaged", null, false); if (beginCount != 1 || endCount != 1) - return new ManagedHookAnalysis("conflicted", null); + return new ManagedHookAnalysis("conflicted", null, false); - var beginIndex = content.IndexOf(BeginMarker, StringComparison.Ordinal); - var endIndex = content.IndexOf(EndMarker, StringComparison.Ordinal); + var beginIndex = contentSpan.IndexOf(BeginMarkerBytes); + var endIndex = contentSpan.IndexOf(EndMarkerBytes); if (endIndex < beginIndex - || !IsMarkerOnlyLine(content, beginIndex, BeginMarker) - || !IsMarkerOnlyLine(content, endIndex, EndMarker)) + || !IsMarkerOnlyLine(contentSpan, beginIndex, BeginMarkerBytes) + || !IsMarkerOnlyLine(contentSpan, endIndex, EndMarkerBytes)) { - return new ManagedHookAnalysis("conflicted", null); + return new ManagedHookAnalysis("conflicted", null, false); } - var blockStart = content.LastIndexOf('\n', beginIndex); - blockStart = blockStart < 0 ? 0 : blockStart + 1; - var blockEnd = content.IndexOf('\n', endIndex + EndMarker.Length); - blockEnd = blockEnd < 0 ? content.Length : blockEnd + 1; + var blockStart = FindLineStart(contentSpan, beginIndex); + var blockEnd = FindLineEndIncludingTerminator( + contentSpan, + endIndex + EndMarkerBytes.Length); + var remainingBytes = new byte[content.Length - (blockEnd - blockStart)]; + contentSpan[..blockStart].CopyTo(remainingBytes); + contentSpan[blockEnd..].CopyTo(remainingBytes.AsSpan(blockStart)); return new ManagedHookAnalysis( "managed", - content.Remove(blockStart, blockEnd - blockStart)); + remainingBytes, + IsOnlyManagedHookPreamble(remainingBytes)); + } + + private static ManagedHookAnalysis AnalyzeBomEncodedManagedHook( + byte[] content, + Encoding encoding, + int bomLength) + { + string text; + try + { + text = encoding.GetString(content, bomLength, content.Length - bomLength); + } + catch (DecoderFallbackException) + { + return new ManagedHookAnalysis("unmanaged", null, false); + } + + var beginCount = CountOccurrences(text, BeginMarker); + var endCount = CountOccurrences(text, EndMarker); + if (beginCount == 0 && endCount == 0) + return new ManagedHookAnalysis("unmanaged", null, false); + if (beginCount != 1 || endCount != 1) + return new ManagedHookAnalysis("conflicted", null, false); + + var beginIndex = text.IndexOf(BeginMarker, StringComparison.Ordinal); + var endIndex = text.IndexOf(EndMarker, StringComparison.Ordinal); + if (endIndex < beginIndex + || !IsMarkerOnlyLine(text, beginIndex, BeginMarker) + || !IsMarkerOnlyLine(text, endIndex, EndMarker)) + { + return new ManagedHookAnalysis("conflicted", null, false); + } + + var blockStart = FindLineStart(text, beginIndex); + var blockEnd = FindLineEndIncludingTerminator( + text, + endIndex + EndMarker.Length); + var byteBlockStart = bomLength + encoding.GetByteCount(text.AsSpan(0, blockStart)); + var byteBlockEnd = bomLength + encoding.GetByteCount(text.AsSpan(0, blockEnd)); + var remainingBytes = new byte[content.Length - (byteBlockEnd - byteBlockStart)]; + content.AsSpan(0, byteBlockStart).CopyTo(remainingBytes); + content.AsSpan(byteBlockEnd).CopyTo(remainingBytes.AsSpan(byteBlockStart)); + var remainingText = text.Remove(blockStart, blockEnd - blockStart); + return new ManagedHookAnalysis( + "managed", + remainingBytes, + IsOnlyManagedHookPreamble(remainingText)); + } + + private static bool TryGetBomEncoding( + ReadOnlySpan content, + out Encoding encoding, + out int bomLength) + { + if (content.Length >= 4 + && content[0] == 0xFF + && content[1] == 0xFE + && content[2] == 0x00 + && content[3] == 0x00) + { + encoding = new UTF32Encoding( + bigEndian: false, + byteOrderMark: false, + throwOnInvalidCharacters: true); + bomLength = 4; + return true; + } + + if (content.Length >= 4 + && content[0] == 0x00 + && content[1] == 0x00 + && content[2] == 0xFE + && content[3] == 0xFF) + { + encoding = new UTF32Encoding( + bigEndian: true, + byteOrderMark: false, + throwOnInvalidCharacters: true); + bomLength = 4; + return true; + } + + if (content.Length >= 2 + && content[0] == 0xFF + && content[1] == 0xFE) + { + encoding = new UnicodeEncoding( + bigEndian: false, + byteOrderMark: false, + throwOnInvalidBytes: true); + bomLength = 2; + return true; + } + + if (content.Length >= 2 + && content[0] == 0xFE + && content[1] == 0xFF) + { + encoding = new UnicodeEncoding( + bigEndian: true, + byteOrderMark: false, + throwOnInvalidBytes: true); + bomLength = 2; + return true; + } + + encoding = Encoding.UTF8; + bomLength = 0; + return false; } private static int CountOccurrences(string value, string marker) { var count = 0; var offset = 0; - while ((offset = value.IndexOf(marker, offset, StringComparison.Ordinal)) >= 0) + while (offset <= value.Length - marker.Length) { + var index = value.IndexOf(marker, offset, StringComparison.Ordinal); + if (index < 0) + break; count++; - offset += marker.Length; + offset = index + marker.Length; } return count; } - private static bool IsMarkerOnlyLine(string content, int markerIndex, string marker) + private static bool IsMarkerOnlyLine( + string content, + int markerIndex, + string marker) + { + var lineStart = FindLineStart(content, markerIndex); + var lineEnd = FindLineEnd(content, markerIndex + marker.Length); + return content[lineStart..lineEnd].Trim().Equals( + marker, + StringComparison.Ordinal); + } + + private static int FindLineStart(string content, int offset) { - var lineStart = content.LastIndexOf('\n', markerIndex); - lineStart = lineStart < 0 ? 0 : lineStart + 1; - var lineEnd = content.IndexOf('\n', markerIndex + marker.Length); - lineEnd = lineEnd < 0 ? content.Length : lineEnd; - return content[lineStart..lineEnd].Trim().Equals(marker, StringComparison.Ordinal); + while (offset > 0 && !IsLineTerminator(content[offset - 1])) + offset--; + return offset; + } + + private static int FindLineEnd(string content, int offset) + { + while (offset < content.Length && !IsLineTerminator(content[offset])) + offset++; + return offset; + } + + private static int FindLineEndIncludingTerminator(string content, int offset) + { + var lineEnd = FindLineEnd(content, offset); + if (lineEnd == content.Length) + return lineEnd; + if (content[lineEnd] == '\r' + && lineEnd + 1 < content.Length + && content[lineEnd + 1] == '\n') + { + return lineEnd + 2; + } + + return lineEnd + 1; + } + + private static bool IsLineTerminator(char value) + => value is '\r' or '\n'; + + private static int CountOccurrences(ReadOnlySpan value, ReadOnlySpan marker) + { + var count = 0; + var offset = 0; + while (offset <= value.Length - marker.Length) + { + var relativeIndex = value[offset..].IndexOf(marker); + if (relativeIndex < 0) + break; + count++; + offset += relativeIndex + marker.Length; + } + + return count; + } + + private static bool IsMarkerOnlyLine( + ReadOnlySpan content, + int markerIndex, + ReadOnlySpan marker) + { + var lineStart = FindLineStart(content, markerIndex); + var lineEnd = FindLineEnd(content, markerIndex + marker.Length); + var line = TrimAsciiWhitespace(content[lineStart..lineEnd]); + return line.SequenceEqual(marker); + } + + private static int FindLineStart(ReadOnlySpan content, int offset) + { + while (offset > 0 && !IsLineTerminator(content[offset - 1])) + offset--; + return offset; + } + + private static int FindLineEnd(ReadOnlySpan content, int offset) + { + while (offset < content.Length && !IsLineTerminator(content[offset])) + offset++; + return offset; + } + + private static int FindLineEndIncludingTerminator(ReadOnlySpan content, int offset) + { + var lineEnd = FindLineEnd(content, offset); + if (lineEnd == content.Length) + return lineEnd; + if (content[lineEnd] == (byte)'\r' + && lineEnd + 1 < content.Length + && content[lineEnd + 1] == (byte)'\n') + { + return lineEnd + 2; + } + + return lineEnd + 1; + } + + private static bool IsLineTerminator(byte value) + => value is (byte)'\r' or (byte)'\n'; + + private static ReadOnlySpan TrimAsciiWhitespace(ReadOnlySpan value) + { + var start = 0; + while (start < value.Length && value[start] <= 0x20) + start++; + var end = value.Length; + while (end > start && value[end - 1] <= 0x20) + end--; + return value[start..end]; + } + + private static bool IsOnlyManagedHookPreamble(ReadOnlySpan content) + { + var trimmed = TrimAsciiWhitespace(content); + return trimmed.IsEmpty || trimmed.SequenceEqual(HookPreambleBytes); } private static bool IsOnlyManagedHookPreamble(string content) - => string.IsNullOrWhiteSpace(content) - || content.Trim().Equals("#!/bin/sh", StringComparison.Ordinal); + { + var trimmed = content.Trim(); + return trimmed.Length == 0 + || trimmed.Equals("#!/bin/sh", StringComparison.Ordinal); + } private static string ComputeContentSha256(string content) - => Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(content))).ToLowerInvariant(); + => ComputeBytesSha256(Encoding.UTF8.GetBytes(content)); + + private static string ComputeBytesSha256(ReadOnlySpan content) + => Convert.ToHexString(SHA256.HashData(content)).ToLowerInvariant(); private static string ComputeFileSha256(string ioPath) { @@ -804,6 +1056,12 @@ private static void MakeExecutable(string path) UnixFileMode.OtherRead | UnixFileMode.OtherExecute); } + private static void ApplyUnixFileMode(string path, UnixFileMode mode) + { + if (!OperatingSystem.IsWindows()) + File.SetUnixFileMode(path, mode); + } + private static bool IsExecutableHook(string path) => OperatingSystem.IsWindows() || (File.GetUnixFileMode(path) & UnixFileMode.UserExecute) != 0; @@ -1059,7 +1317,8 @@ private static void PrintUsage() private sealed record ManagedHookAnalysis( string State, - string? ContentWithoutManagedBlock); + byte[]? BytesWithoutManagedBlock, + bool BytesWithoutManagedBlockArePreamble); private sealed record HookOperationPlan( string PlannedAction, @@ -1069,7 +1328,8 @@ private sealed record HookOperationPlan( IReadOnlyList PlannedChanges, bool Blocked = false, int BlockExitCode = CommandExitCodes.Success, - string? ResultingHookContent = null) + byte[]? ResultingHookBytes = null, + UnixFileMode? ResultingHookMode = null) { public static HookOperationPlan Absent { get; } = new( "none", diff --git a/tests/CodeIndex.Tests/HookCommandRunnerTests.cs b/tests/CodeIndex.Tests/HookCommandRunnerTests.cs index 3e70ca5d8..9c1dfa600 100644 --- a/tests/CodeIndex.Tests/HookCommandRunnerTests.cs +++ b/tests/CodeIndex.Tests/HookCommandRunnerTests.cs @@ -145,6 +145,19 @@ public void Hooks_Install_RepairsUnusableManagedHook_Issue4716() } Assert.Equal([0xff, 0xfe], File.ReadAllBytes(hookPath)[..2]); + var encodedHookBytes = File.ReadAllBytes(hookPath); + var encodingUninstallPreview = RunHooksAndCaptureStreams( + ["uninstall", "--project", projectRoot, "--dry-run", "--json"]); + + Assert.Equal(CommandExitCodes.Success, encodingUninstallPreview.ExitCode); + using (var document = JsonDocument.Parse(encodingUninstallPreview.StdOut)) + { + Assert.Equal("dry_run", document.RootElement.GetProperty("status").GetString()); + Assert.Equal("managed", document.RootElement.GetProperty("hook_state").GetString()); + Assert.Equal("delete_managed", document.RootElement.GetProperty("planned_action").GetString()); + } + Assert.Equal(encodedHookBytes, File.ReadAllBytes(hookPath)); + var encodingRepair = RunHooksAndCaptureStreams( ["install", "--project", projectRoot, "--json"]); @@ -374,11 +387,32 @@ public void Hooks_UninstallDryRun_PreviewsBlockRemovalRestoreForceAndIdempotence CommandExitCodes.Success, RunHooksAndCaptureStreams(["install", "--project", projectRoot]).ExitCode); var managedHook = File.ReadAllText(hookPath); - var managedBlockWithSurroundingContent = managedHook - .Replace("#!/bin/sh\n", "#!/bin/sh\necho before\n", StringComparison.Ordinal) - + "\necho after\n"; - File.WriteAllText(hookPath, managedBlockWithSurroundingContent); - var expectedRemainingContent = "#!/bin/sh\necho before\necho after\n"; + var managedBody = managedHook["#!/bin/sh\n".Length..]; + byte[] customPrefix = + [ + .. Encoding.ASCII.GetBytes("#!/bin/sh\necho caf"), + 0xe9, + (byte)'\n', + ]; + byte[] managedBlockWithSurroundingContent = + [ + .. customPrefix, + .. Encoding.UTF8.GetBytes(managedBody), + (byte)'\n', + .. Encoding.ASCII.GetBytes("echo after\n"), + ]; + byte[] expectedRemainingContent = + [ + .. customPrefix, + .. Encoding.ASCII.GetBytes("echo after\n"), + ]; + File.WriteAllBytes(hookPath, managedBlockWithSurroundingContent); + if (!OperatingSystem.IsWindows()) + { + File.SetUnixFileMode( + hookPath, + UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + } var managedWriteTime = File.GetLastWriteTimeUtc(hookPath); UnixFileMode? managedMode = null; if (!OperatingSystem.IsWindows()) @@ -402,7 +436,7 @@ public void Hooks_UninstallDryRun_PreviewsBlockRemovalRestoreForceAndIdempotence change.GetProperty("before_sha256").GetString(), change.GetProperty("after_sha256").GetString()); } - Assert.Equal(managedBlockWithSurroundingContent, File.ReadAllText(hookPath)); + Assert.Equal(managedBlockWithSurroundingContent, File.ReadAllBytes(hookPath)); Assert.Equal(managedWriteTime, File.GetLastWriteTimeUtc(hookPath)); if (!OperatingSystem.IsWindows()) Assert.Equal(managedMode!.Value, File.GetUnixFileMode(hookPath)); @@ -410,7 +444,42 @@ public void Hooks_UninstallDryRun_PreviewsBlockRemovalRestoreForceAndIdempotence Assert.Equal( CommandExitCodes.Success, RunHooksAndCaptureStreams(["uninstall", "--project", projectRoot, "--json"]).ExitCode); - Assert.Equal(expectedRemainingContent, File.ReadAllText(hookPath)); + Assert.Equal(expectedRemainingContent, File.ReadAllBytes(hookPath)); + if (!OperatingSystem.IsWindows()) + Assert.Equal(managedMode!.Value, File.GetUnixFileMode(hookPath)); + + var carriageReturnManagedHook = Encoding.UTF8.GetBytes( + managedHook.Replace("\n", "\r", StringComparison.Ordinal)); + File.WriteAllBytes(hookPath, carriageReturnManagedHook); + if (!OperatingSystem.IsWindows()) + { + File.SetUnixFileMode( + hookPath, + UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + } + + var carriageReturnInstallPreview = RunHooksAndCaptureStreams( + ["install", "--project", projectRoot, "--dry-run", "--json"]); + var carriageReturnUninstallPreview = RunHooksAndCaptureStreams( + ["uninstall", "--project", projectRoot, "--dry-run", "--json"]); + + Assert.Equal(CommandExitCodes.Success, carriageReturnInstallPreview.ExitCode); + using (var document = JsonDocument.Parse(carriageReturnInstallPreview.StdOut)) + { + Assert.Equal("managed", document.RootElement.GetProperty("hook_state").GetString()); + Assert.Equal("replace_managed", document.RootElement.GetProperty("planned_action").GetString()); + } + Assert.Equal(CommandExitCodes.Success, carriageReturnUninstallPreview.ExitCode); + using (var document = JsonDocument.Parse(carriageReturnUninstallPreview.StdOut)) + { + Assert.Equal("managed", document.RootElement.GetProperty("hook_state").GetString()); + Assert.Equal("delete_managed", document.RootElement.GetProperty("planned_action").GetString()); + } + Assert.Equal(carriageReturnManagedHook, File.ReadAllBytes(hookPath)); + Assert.Equal( + CommandExitCodes.Success, + RunHooksAndCaptureStreams(["uninstall", "--project", projectRoot]).ExitCode); + Assert.False(File.Exists(hookPath)); const string unmanagedHook = "#!/bin/sh\necho unmanaged\n"; File.WriteAllText(hookPath, unmanagedHook); From c31fce29bf9bff53fcf347245641bce7b19d6767 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Wed, 29 Jul 2026 22:01:26 +0900 Subject: [PATCH 3/3] Bound hook preview hashing (#4883) --- src/CodeIndex/Cli/HookCommandRunner.cs | 44 ++++++++----------- .../CodeIndex.Tests/HookCommandRunnerTests.cs | 34 ++++++++++++++ 2 files changed, 53 insertions(+), 25 deletions(-) diff --git a/src/CodeIndex/Cli/HookCommandRunner.cs b/src/CodeIndex/Cli/HookCommandRunner.cs index 07901d710..260bd3813 100644 --- a/src/CodeIndex/Cli/HookCommandRunner.cs +++ b/src/CodeIndex/Cli/HookCommandRunner.cs @@ -18,6 +18,7 @@ public static class HookCommandRunner private static readonly byte[] BeginMarkerBytes = Encoding.ASCII.GetBytes(BeginMarker); private static readonly byte[] EndMarkerBytes = Encoding.ASCII.GetBytes(EndMarker); private static readonly byte[] HookPreambleBytes = Encoding.ASCII.GetBytes("#!/bin/sh"); + private static readonly byte[] Utf8BomBytes = [0xEF, 0xBB, 0xBF]; internal const int MaxHookMarkerBytes = 64 * 1024; internal static Action? DeleteFileForTesting { get; set; } internal static Action? ReplaceFileForTesting { get; set; } @@ -266,7 +267,8 @@ private static HookOperationPlan BuildInstallPlan( var hookExists = File.Exists(ioHookPath); var chainedHookExists = File.Exists(ioChainedHookPath); var chainedHookState = chainedHookExists ? "present" : "absent"; - var generatedHash = ComputeContentSha256(hookScript); + var generatedBytes = Encoding.UTF8.GetBytes(hookScript); + var generatedHash = ComputeBytesSha256(generatedBytes); if (!hookExists) { return new HookOperationPlan( @@ -289,11 +291,15 @@ private static HookOperationPlan BuildInstallPlan( var existingHook = ReadHookBytesWithinLimit(ioHookPath); var analysis = AnalyzeManagedHook(existingHook); - var existingHash = ComputeFileSha256(ioHookPath); + var existingHash = existingHook is null + ? null + : ComputeBytesSha256(existingHook); var executable = IsExecutableHook(ioHookPath); if (analysis.State == "managed") { - if (IsExactHookScriptFile(ioHookPath, hookScript) && executable) + if (existingHook is not null + && existingHook.AsSpan().SequenceEqual(generatedBytes) + && executable) { return new HookOperationPlan( "none", @@ -344,7 +350,7 @@ private static HookOperationPlan BuildInstallPlan( chainedHookPath, "existing_pre_commit_hook", hookPath, - chainedHookExists ? ComputeFileSha256(ioChainedHookPath) : null, + chainedHookExists ? ComputeFileSha256WithinLimit(ioChainedHookPath) : null, existingHash, chainedHookExists ? IsExecutableHook(ioChainedHookPath) : null, executable), @@ -473,7 +479,9 @@ private static HookOperationPlan BuildUninstallPlan( var hookContent = ReadHookBytesWithinLimit(ioHookPath); var analysis = AnalyzeManagedHook(hookContent); - var hookHash = ComputeFileSha256(ioHookPath); + var hookHash = hookContent is null + ? null + : ComputeBytesSha256(hookContent); var hookExecutable = IsExecutableHook(ioHookPath); UnixFileMode? hookMode = null; if (!OperatingSystem.IsWindows()) @@ -495,7 +503,7 @@ private static HookOperationPlan BuildUninstallPlan( if (chainedHookExists) { - var chainedHash = ComputeFileSha256(ioChainedHookPath); + var chainedHash = ComputeFileSha256WithinLimit(ioChainedHookPath); return new HookOperationPlan( options.Force && analysis.State != "managed" ? "force_restore_chained" : "restore_chained", "the chained pre-commit hook would be restored", @@ -883,6 +891,8 @@ private static ReadOnlySpan TrimAsciiWhitespace(ReadOnlySpan value) private static bool IsOnlyManagedHookPreamble(ReadOnlySpan content) { var trimmed = TrimAsciiWhitespace(content); + if (trimmed.StartsWith(Utf8BomBytes)) + trimmed = TrimAsciiWhitespace(trimmed[Utf8BomBytes.Length..]); return trimmed.IsEmpty || trimmed.SequenceEqual(HookPreambleBytes); } @@ -893,32 +903,16 @@ private static bool IsOnlyManagedHookPreamble(string content) || trimmed.Equals("#!/bin/sh", StringComparison.Ordinal); } - private static string ComputeContentSha256(string content) - => ComputeBytesSha256(Encoding.UTF8.GetBytes(content)); - private static string ComputeBytesSha256(ReadOnlySpan content) => Convert.ToHexString(SHA256.HashData(content)).ToLowerInvariant(); - private static string ComputeFileSha256(string ioPath) - { - using var stream = new FileStream( - ioPath, - FileMode.Open, - FileAccess.Read, - FileShare.ReadWrite, - bufferSize: 4096, - FileOptions.SequentialScan); - return Convert.ToHexString(SHA256.HashData(stream)).ToLowerInvariant(); - } - - private static bool IsExactHookScriptFile(string ioHookPath, string hookScript) + private static string? ComputeFileSha256WithinLimit(string ioPath) { var bytes = DataDirectorySecurity.ReadBytesWithinLimit( - ioHookPath, + ioPath, MaxHookMarkerBytes, FileShare.ReadWrite); - return bytes is not null - && bytes.AsSpan().SequenceEqual(Encoding.UTF8.GetBytes(hookScript)); + return bytes is null ? null : ComputeBytesSha256(bytes); } private static void ReplaceCustomHookWithManagedHook( diff --git a/tests/CodeIndex.Tests/HookCommandRunnerTests.cs b/tests/CodeIndex.Tests/HookCommandRunnerTests.cs index 9c1dfa600..f5d8d31df 100644 --- a/tests/CodeIndex.Tests/HookCommandRunnerTests.cs +++ b/tests/CodeIndex.Tests/HookCommandRunnerTests.cs @@ -165,6 +165,30 @@ public void Hooks_Install_RepairsUnusableManagedHook_Issue4716() using (var document = JsonDocument.Parse(encodingRepair.StdOut)) Assert.Equal("updated", document.RootElement.GetProperty("status").GetString()); Assert.Equal(Encoding.UTF8.GetBytes(managedHook), File.ReadAllBytes(hookPath)); + + byte[] utf8BomManagedHook = + [ + 0xef, + 0xbb, + 0xbf, + .. Encoding.UTF8.GetBytes(managedHook), + ]; + File.WriteAllBytes(hookPath, utf8BomManagedHook); + + var utf8BomUninstallPreview = RunHooksAndCaptureStreams( + ["uninstall", "--project", projectRoot, "--dry-run", "--json"]); + + Assert.Equal(CommandExitCodes.Success, utf8BomUninstallPreview.ExitCode); + using (var document = JsonDocument.Parse(utf8BomUninstallPreview.StdOut)) + { + Assert.Equal("managed", document.RootElement.GetProperty("hook_state").GetString()); + Assert.Equal("delete_managed", document.RootElement.GetProperty("planned_action").GetString()); + } + Assert.Equal(utf8BomManagedHook, File.ReadAllBytes(hookPath)); + Assert.Equal( + CommandExitCodes.Success, + RunHooksAndCaptureStreams(["uninstall", "--project", projectRoot]).ExitCode); + Assert.False(File.Exists(hookPath)); } finally { @@ -1051,12 +1075,22 @@ public void Hooks_TreatsOversizedPreCommitHookAsCustom() File.WriteAllText(hookPath, new string('x', HookCommandRunner.MaxHookMarkerBytes + 1)); var (statusExit, statusStdout, _) = RunHooksAndCaptureStreams(["status", "--project", projectRoot, "--json"]); + var installPreview = RunHooksAndCaptureStreams( + ["install", "--project", projectRoot, "--dry-run", "--json"]); var uninstallExit = RunHooksAndCaptureStreams(["uninstall", "--project", projectRoot]).ExitCode; var installExit = RunHooksAndCaptureStreams(["install", "--project", projectRoot]).ExitCode; Assert.Equal(CommandExitCodes.Success, statusExit); using (var document = JsonDocument.Parse(statusStdout)) Assert.Equal("custom", document.RootElement.GetProperty("status").GetString()); + Assert.Equal(CommandExitCodes.Success, installPreview.ExitCode); + using (var document = JsonDocument.Parse(installPreview.StdOut)) + { + Assert.Equal("chain_existing", document.RootElement.GetProperty("planned_action").GetString()); + var changes = document.RootElement.GetProperty("planned_changes").EnumerateArray().ToArray(); + Assert.Equal(JsonValueKind.Null, changes[0].GetProperty("after_sha256").ValueKind); + Assert.Equal(JsonValueKind.Null, changes[1].GetProperty("before_sha256").ValueKind); + } Assert.Equal(CommandExitCodes.UsageError, uninstallExit); Assert.Equal(CommandExitCodes.Success, installExit); Assert.True(File.Exists(chainedHookPath));