Recognize linked worktree .git files in QuarantineTools

[Adam Ratzman (adamint)]

Description

Resolve the repository root through bounded git rev-parse --show-toplevel instead of trusting an ancestor .git marker alone.

The command drains stdout and stderr concurrently, has a bounded timeout, and accepts the result only when its canonical path matches the nearest .git marker root. This supports normal checkouts and linked worktrees while refusing malformed or redirected metadata that points at another tree. Repository-root validation failures return exit code 4 rather than editing a different checkout.

Verification

• Real git repository and linked-worktree root resolution.
• Nested-directory invocation.
• Missing or failed git command handling.
• Wrong-tree refusal, including an outer-ancestor core.worktree redirection.
• Focused root suite: 9 passing.
• Full QuarantineTools suite: 50 passing.
• Formatting and diff checks pass.

Checklist

• Is this feature complete?
  • Yes. Ready to ship.
  • No. Follow-up changes expected.
• Are you including unit tests for the changes and scenario tests if relevant?
  • Yes
  • No
• Did you add public API?
  • Yes
  • No
• Does the change make any security assumptions or guarantees?
  • Yes. It fails closed rather than editing a repository root that does not match the caller's nearest Git marker.
  • No

[github-actions]

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19157

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19157"

[Copilot]

Pull request overview

Resolves QuarantineTools repository roots through Git, preventing edits to an outer checkout from nested worktrees.

Changes:

• Adds Git-based root discovery with timeout and fallback.
• Adds ancestor safety validation.
• Adds linked-worktree regression tests.

Show a summary per file

File	Description
tools/QuarantineTools/QuarantineTools.csproj	Exposes internals to tests.
tools/QuarantineTools/Quarantine.cs	Implements root discovery and validation.
tests/QuarantineTools.Tests/RepoRootTests.cs	Tests worktree and path scenarios.
tests/QuarantineTools.Tests/QuarantineTools.Tests.csproj	References the production tool.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (1)

tools/QuarantineTools/Quarantine.cs:517

• The linked-worktree tests always have git available, so they return from the primary probe and never exercise this fallback. Reverting only File.Exists(gitPath) would therefore leave the suite green and reintroduce wrong-tree writes whenever git is unavailable. Add a regression test that forces the git probe to fail and verifies the nested worktree's .git file is selected instead of the outer checkout.

            if (Directory.Exists(gitPath) || File.Exists(gitPath))

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (1)

tools/QuarantineTools/Quarantine.cs:690

• remainingDepth is decremented for every ordinary parent component, not only when following a link. A valid path with more than 40 components therefore exhausts the link budget before reaching a symlinked ancestor, leaves that ancestor unresolved, and makes the new wrong-tree guard reject a legitimate run. Keep the budget unchanged while walking parents; only decrement it when substituting a link target.

            ? Path.Combine(CanonicalizeCore(parent.FullName, remainingDepth - 1), info.Name)

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (2)

tools/QuarantineTools/Quarantine.cs:775

• This probe still accepts a wrong tree when a case-sensitive parent contains two checkouts named with inverse casing (for example, OUTER and outer). The flipped path exists because it is a distinct sibling, so this reports case-insensitive behavior; IsSameOrAncestorDirectory then approves OUTER as an ancestor of outer/tools and permits the cross-tree write. The new rejection test bypasses this probe by passing caseSensitive: true. Distinguish an alternate lookup of the same directory from a distinct sibling, or conservatively reject this ambiguous case.

        // A case-sensitive volume that happens to hold a real sibling differing only by case is read as
        // case-insensitive here. That is acceptable: it only restores the behavior this guard had
        // before the probe existed, and such a pair is not something a checkout layout produces.
        return !Directory.Exists(Path.Combine(parent, flipped));

tools/QuarantineTools/Quarantine.cs:691

• remainingDepth is intended to cap symlink hops, but this branch decrements it for every ordinary parent component. From a path deeper than 40 segments, canonicalization can stop before reaching a symlink near the root, leaving equivalent real/link spellings different and incorrectly returning exit code 4. Keep the counter unchanged while walking non-link parents; only decrement it when resolving a link.

        return info.Parent is { } parent
            ? Path.Combine(CanonicalizeCore(parent.FullName, remainingDepth - 1), info.Name)

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (1)

tools/QuarantineTools/Quarantine.cs:602

• Trim() removes valid whitespace from the repository path, not just git's line terminator. On Unix a checkout directory can end in a space, so this resolves a different path and the new guard incorrectly refuses a valid run. Remove only CR/LF terminators.

            var trimmed = standardOutputTask.Result.Trim();

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (2)

tools/QuarantineTools/Quarantine.cs:693

• remainingDepth is decremented for every ordinary parent component, not only when a link is followed. A path deeper than 40 components therefore exhausts the link budget before reaching a symlink near the root, leaving the two spellings uncanonicalized and incorrectly refusing a valid run. Preserve the budget while walking ordinary parents; only decrement it when resolving a link.

        return info.Parent is { } parent
            ? Path.Combine(CanonicalizeCore(parent.FullName, remainingDepth - 1), info.Name)
            : Path.TrimEndingDirectorySeparator(info.FullName);

tools/QuarantineTools/Quarantine.cs:821

• This new Windows-native case-sensitivity path is not exercised by CI: QuarantineTools.Tests.csproj explicitly disables both Windows and macOS runs, while the delegate-based test only verifies dispatch and the filesystem test runs on Linux. Enable platform runs for this project or move focused tests to platform-enabled projects so the P/Invoke behavior and macOS fallback/symlink behavior are covered.

        using var handle = CreateFile(

• Files reviewed: 4/4 changed files
• Comments generated: 2
• Review effort level: Balanced

[github-actions]

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

[github-actions]

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

[github-actions]

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

[Copilot]

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (1)

tests/QuarantineTools.Tests/RepoRootTests.cs:440

• This summary claims the guard handles subst, but Program.Canonicalize explicitly does not resolve DOS device mappings (Quarantine.cs:663-669), and this test only creates POSIX symlinks. Limit the summary to the link aliases this regression actually covers so it does not imply nonexistent subst coverage.

    /// A junction or `subst` drive on Windows lets the caller's directory and git's answer name the same
    /// tree with different spellings. Refusing that would block a legitimate run, so the guard resolves
    /// links before it gives up. Symlink creation needs elevation on Windows, so this is POSIX-only.

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[github-actions]

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

[github-actions]

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

[github-actions]

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

[Copilot]

Review details

Suppressed comments (2)

tests/QuarantineTools.Tests/RepoRootTests.cs:441

• This summary still implies that the canonicalization path handles subst, but Canonicalize only resolves symlinks/junctions and the production comment at tools/QuarantineTools/Quarantine.cs:663-669 explicitly documents subst as unsupported. This test is POSIX-only and exercises symlinks, so remove the subst claim to avoid documenting behavior that is neither implemented nor tested.

    /// A junction or `subst` drive on Windows lets the caller's directory and git's answer name the same
    /// tree with different spellings. Refusing that would block a legitimate run, so the guard resolves
    /// links before it gives up. Symlink creation needs elevation on Windows, so this is POSIX-only.

tests/QuarantineTools.Tests/RepoRootTests.cs:532

• This expectation contradicts the PR's verification claim that stale GIT_DIR/GIT_WORK_TREE exercises the wrong-tree guard, exits with code 4, and names both paths. The implementation now strips those variables and this test correctly observes the ordinary missing-tests exit code 2 instead. Update the PR description to describe the current behavior; if an end-to-end exit-4 regression is still required, it needs a different setup.

        Assert.Equal(2, exitCode);
        Assert.Contains(Path.Combine(here, "tests"), standardError, StringComparison.Ordinal);
        Assert.Contains(here, standardError, StringComparison.Ordinal);

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (1)

tools/QuarantineTools/Quarantine.cs:484

• The implementation in this hunk does not match the PR's stated fix: it still resolves the root solely by walking for a .git marker. There is no git rev-parse --show-toplevel probe, timeout/concurrent stream draining, ancestor safety check, or distinct exit code 4 anywhere in Quarantine.cs. As a result, the behavior and verification described in the PR—including the real git-backed worktree scenarios and wrong-tree refusal—are not present in the submitted changes. Please either restore the described implementation and tests or update the PR scope and description to reflect this narrower marker-file fix.

            var gitPath = Path.Combine(dir.FullName, ".git");
            if (Directory.Exists(gitPath) || File.Exists(gitPath))

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (2)

tools/QuarantineTools/Quarantine.cs:635

• None of the new end-to-end tests sets GIT_DIR or GIT_WORK_TREE; the core.worktree tests exercise a different redirect path. Without a regression test, removing either environment cleanup reintroduces the outer-checkout write this guard prevents. Add a nested-worktree test that points both variables at the outer checkout and verifies only the nested file is changed.

        foreach (var name in s_gitRepositoryLocationEnvironmentVariables)
        {
            startInfo.Environment.Remove(name);

tools/QuarantineTools/Quarantine.cs:816

• RepoRootTests only calls this method with identically-cased path segments, so this casing-dependent safety branch is never exercised. A regression here can treat case-distinct checkouts as the same directory and permit edits in the wrong tree. Add focused tests through the injected casing delegate for sensitive and insensitive parents, including the case-only-sibling and unknown-probe outcomes.

            if (!string.Equals(ancestorSegments[i], currentSegments[i], StringComparison.Ordinal)
                && (isCaseSensitiveDirectory(parent)
                    || !string.Equals(ancestorSegments[i], currentSegments[i], StringComparison.OrdinalIgnoreCase)))

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (2)

tools/QuarantineTools/Quarantine.cs:816

• The case-sensitive wrong-tree guard has no regression coverage in RepoRootTests: the current ancestry theory uses only lowercase paths, and no test calls the injected casing overloads or child-probe seam. A regression that treats a case-sensitive parent or distinct case-flipped siblings as insensitive would therefore let a wrong checkout through without failing the suite. Restore focused tests for per-parent casing rules, the Windows metadata/fail-closed branches, and the child probe's distinct-sibling and unknown outcomes.

            if (!string.Equals(ancestorSegments[i], currentSegments[i], StringComparison.Ordinal)
                && (isCaseSensitiveDirectory(parent)
                    || !string.Equals(ancestorSegments[i], currentSegments[i], StringComparison.OrdinalIgnoreCase)))

tools/QuarantineTools/Quarantine.cs:579

• The caller-cancellation test uses an already-canceled token, so it never exercises this internal timeout or the process-termination path. Removing CancelAfter, failing to kill the child, or incorrectly skipping marker fallback would still leave the suite green despite the PR's bounded-probe guarantee. Add a fake git process that blocks, use an injectable short timeout, and assert that the probe terminates it and falls back within the bound.

This issue also appears on line 814 of the same file.

            using var timeoutSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
            timeoutSource.CancelAfter(s_gitProbeTimeout);

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[github-actions]

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.