Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions cmd/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"os"
"os/exec"
"path/filepath"
"runtime"
"strings"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -609,17 +610,29 @@ func migrateTrustGain(from, to string) (string, error) {
}
for sha := range waiting {
if !held[sha] {
displayPath := displayText(to)
return fmt.Sprintf(
"%s still carries a hook approval from a repository that used to be there, so moving it "+
"there would run this repository's hooks unasked if the commands match — and that path "+
"comes from the origin URL, not from you. Run 'wt untrust --path %s' first, or move it "+
"comes from the origin URL, not from you. Run wt untrust --path %s first, or move it "+
"yourself if you meant to",
to, to), nil
displayPath, shellQuoteArgument(to)), nil
}
}
return "", nil
}

// shellQuoteArgument renders one copyable argument for the platform's native
// shell: POSIX-style shells on Unix and PowerShell on Windows. displayText keeps
// an untrusted repository path from emitting terminal control sequences.
func shellQuoteArgument(arg string) string {
arg = displayText(arg)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if runtime.GOOS == "windows" {
return "'" + strings.ReplaceAll(arg, "'", "''") + "'"
}
return "'" + strings.ReplaceAll(arg, "'", "'\"'\"'") + "'"
}

// cannotTellReason is the skip for a path wt could not settle. Not knowing what
// a path names is not the same as knowing nothing is approved at it.
func cannotTellReason(path string) string {
Expand Down
16 changes: 15 additions & 1 deletion cmd/migrate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ func TestMigrateWillNotMoveARepositoryOntoAStaleApproval(t *testing.T) {
if !strings.Contains(out, "still carries a hook approval") {
t.Errorf("migrate did not say why it declined:\n%s", out)
}
if want := "wt untrust --path " + oldPath; !strings.Contains(out, want) {
if want := "wt untrust --path " + shellQuoteArgument(oldPath); !strings.Contains(out, want) {
t.Errorf("migrate did not provide an actionable stale-approval command %q:\n%s", want, out)
}

Expand All @@ -467,6 +467,20 @@ func TestMigrateWillNotMoveARepositoryOntoAStaleApproval(t *testing.T) {
}
}

func TestMigrationUntrustCommandQuotesTheDestination(t *testing.T) {
path := filepath.Join("parent with spaces", "repo's \"checkout\"")
got := shellQuoteArgument(path)
var want string
if runtime.GOOS == "windows" {
want = `'parent with spaces\repo''s "checkout"'`
} else {
want = `'parent with spaces/repo'"'"'s "checkout"'`
}
if got != want {
t.Errorf("shellQuoteArgument(%q) = %q, want %q", path, got, want)
}
}

// TestMigrateWillNotMoveARepositoryOntoAPathThatMayCarryAnApproval pins which
// side of the comparison folds, from the outside. The destination is spelled
// out of the origin URL, so a repository picks it — and on a filesystem that
Expand Down
10 changes: 5 additions & 5 deletions cmd/trust.go
Original file line number Diff line number Diff line change
Expand Up @@ -844,10 +844,10 @@ func hasPathPrefix(path, prefix string) bool {
// mayBeScopedUnder folds case, and on Windows the trailing dots and spaces
// Win32 drops from every component — the folds a filesystem applies to a path
// that nothing has been created at yet, before it settles which directory the
// name reaches. Fit only where the looser answer refuses or revokes:
// over-matching there costs an approval the user can give again, while
// under-matching lets a name that compares equal to nothing land on a directory
// something already covers.
// name reaches. Fit only where the looser answer refuses: under-matching lets a
// name that compares equal to nothing land on a directory something already
// covers. Revocation uses scopedUnder instead, because over-matching there would
// delete an approval for a distinct path on a case-sensitive filesystem.
func scopedUnder(scope, root string) bool { return hasPathPrefix(scope, root) }

func mayBeScopedUnder(scope, root string) bool { return hasPathPrefixFold(scope, root) }
Expand Down Expand Up @@ -904,7 +904,7 @@ func dropTrustRecordsAt(path string) (int, error) {
// it — and a record wt cannot resolve is not evidence about this path.
if rec.Scope != "" && rec.Scope != trustScopeUser {
if got, ok := canonicalExistingPath(rec.Scope); ok &&
(mayBeScopedUnder(got, root) || mayBeScopedUnder(got, gitRoot)) {
(scopedUnder(got, root) || scopedUnder(got, gitRoot)) {
continue
}
}
Expand Down
21 changes: 21 additions & 0 deletions cmd/trust_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,27 @@ func TestDropTrustRecordsAtFollowsTheGitDirectorySeparately(t *testing.T) {
}
}

func TestDropTrustRecordsAtKeepsACaseVariant(t *testing.T) {
withIsolatedTrustStore(t)

parent := t.TempDir()
target := filepath.Join(parent, "Tool")
other := filepath.Join(parent, "tool", ".git")
if err := saveTrustStore(trustStore{Trusted: []trustRecord{{
Scope: other, RepositoryInstance: "other", SHA256: "hash",
}}}); err != nil {
t.Fatal(err)
}

removed, err := dropTrustRecordsAt(target)
if err != nil {
t.Fatal(err)
}
if removed != 0 {
t.Fatalf("dropTrustRecordsAt(%q) removed an approval for distinct path %q", target, other)
}
}

func TestUntrustPathRefusesANonEmptyTree(t *testing.T) {
withIsolatedTrustStore(t)

Expand Down