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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ file alike. It asks once, and remembers the answer until the commands change:
wt trust # approve every hook that applies here
wt trust --list # show every approval on this machine
wt untrust # revoke this repository's approval
wt untrust --path /removed/repo # revoke stale approvals for a removed repository
wt untrust --global # revoke your config file's approval
```

Expand Down
4 changes: 2 additions & 2 deletions cmd/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -612,9 +612,9 @@ func migrateTrustGain(from, to string) (string, error) {
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' there 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), nil
to, to), nil
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}
return "", nil
Expand Down
9 changes: 9 additions & 0 deletions cmd/migrate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,15 @@ 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) {
t.Errorf("migrate did not provide an actionable stale-approval command %q:\n%s", want, out)
}

runWtIn(t, tmpDir, primaryPath, homeDir, worktreeRoot, "untrust", "--path", oldPath)
out = runMigrate(t, tmpDir, primaryPath, homeDir, worktreeRoot)
if _, err := os.Stat(oldPath); err != nil {
t.Fatalf("migration did not proceed after the reported cleanup command (stat err = %v)\nOutput: %s", err, out)
}
}

// TestMigrateWillNotMoveARepositoryOntoAPathThatMayCarryAnApproval pins which
Expand Down
76 changes: 69 additions & 7 deletions cmd/trust.go
Original file line number Diff line number Diff line change
Expand Up @@ -867,15 +867,23 @@ func mayBeSamePath(a, b string) bool { return foldPath(a) == foldPath(b) }
// and prevents migrate's deliberately conservative stale-record check from
// refusing a future placement there.
func dropTrustRecordsAt(path string) (int, error) {
// The whole directory, not just its .git: everything at or under an empty
// path belongs to whatever used to be there, including the scopes of its
// submodules and of a bare repository kept at the path itself.
want, ok := canonicalExistingPath(path)
// The whole directory as well as its .git: an unverified repository is
// scoped to the former, while a verified repository and its submodules are
// scoped to the latter. Resolve them separately because .git may itself be a
// symlink to somewhere outside the checkout.
root, ok := canonicalExistingPath(path)
if !ok {
return 0, fmt.Errorf(
"%s is reached through symlinks wt cannot follow to an end, so it cannot tell which hook\n"+
"approvals already name it — and an approval left behind by a repository that used to be\n"+
"there would cover this one. Run 'wt untrust' there first, or pick another path",
"there would cover this one. Repair or remove the unresolved symlink, or pick another path",
path)
}
gitRoot, ok := canonicalExistingPath(filepath.Join(path, ".git"))
if !ok {
return 0, fmt.Errorf(
"%s is reached through symlinks wt cannot follow to an end, so it cannot tell which hook approvals name it. "+
"Repair or remove the unresolved .git symlink before retrying",
path)
}

Expand All @@ -895,7 +903,8 @@ func dropTrustRecordsAt(path string) (int, error) {
// A user-config approval is not pinned to a path, so no path can free
// 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, want) {
if got, ok := canonicalExistingPath(rec.Scope); ok &&
(mayBeScopedUnder(got, root) || mayBeScopedUnder(got, gitRoot)) {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
continue
}
}
Expand All @@ -916,6 +925,7 @@ func dropTrustRecordsAt(path string) (int, error) {
var (
trustList bool
untrustGlobal bool
untrustPath string
)

var trustCmd = &cobra.Command{
Expand All @@ -934,6 +944,7 @@ file in a repo you cloned this morning does not inherit the answer.
wt trust approve the hooks that apply here
wt trust --list show every approval on this machine
wt untrust revoke this repository's approvals
wt untrust --path P revoke stale approvals for a removed repository
wt untrust --global revoke the approvals for your own config file`,
Args: cobra.NoArgs,
RunE: func(cmd *cobra.Command, args []string) error {
Expand All @@ -947,7 +958,12 @@ file in a repo you cloned this morning does not inherit the answer.
var untrustCmd = &cobra.Command{
Use: "untrust",
Short: "Revoke hook approvals for this repository",
Args: cobra.NoArgs,
Long: `Revoke hook approvals for this repository.

Use --path to revoke stale approvals after a repository has been removed. The
path may be absent; wt resolves its existing parent directories before matching
the approvals that used to belong below it.`,
Args: cobra.NoArgs,
RunE: func(cmd *cobra.Command, args []string) error {
return runUntrust(cmd)
},
Expand All @@ -956,6 +972,8 @@ var untrustCmd = &cobra.Command{
func init() {
trustCmd.Flags().BoolVar(&trustList, "list", false, "List all approved hooks")
untrustCmd.Flags().BoolVar(&untrustGlobal, "global", false, "Revoke the approvals for your own config file instead")
untrustCmd.Flags().StringVar(&untrustPath, "path", "", "Revoke stale approvals for a removed repository at this path")
untrustCmd.MarkFlagsMutuallyExclusive("global", "path")
}

// runTrust approves every hook set that applies where the user is standing —
Expand Down Expand Up @@ -1035,6 +1053,50 @@ func runTrust(cmd *cobra.Command) error {
}

func runUntrust(cmd *cobra.Command) error {
pathSelected := untrustPath != ""
if cmd != nil && cmd.Flags().Changed("path") {
pathSelected = true
}
if pathSelected {
if untrustGlobal {
return errors.New("--global and --path cannot be used together")
}
if untrustPath == "" {
return errors.New("--path cannot be empty")
}
path, err := filepath.Abs(untrustPath)
if err != nil {
return fmt.Errorf("resolve --path %s: %w", untrustPath, err)
}
path = filepath.Clean(path)
state, err := detectTargetState(path)
if err != nil {
return err
}
if state != targetMissing && state != targetDirEmpty {
return fmt.Errorf(
"refusing to revoke approvals by path: %s is not absent or an empty directory; "+
"run 'wt untrust' from inside a live repository instead",
path)
}
removed, err := dropTrustRecordsAt(path)
if err != nil {
return err
}
if isJSONOutput() {
return emitJSONSuccess(cmd, map[string]any{
"path": path,
"removed": removed,
})
}
if removed == 0 {
fmt.Printf("Nothing to revoke at %s.\n", path)
} else {
fmt.Printf("Revoked %d approval(s) at %s\n", removed, path)
}
return nil
}

scope := trustScopeUser
whitelisted := false
if !untrustGlobal {
Expand Down
155 changes: 155 additions & 0 deletions cmd/trust_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cmd

import (
"bytes"
"encoding/json"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -574,6 +575,160 @@ func TestUntrustRevokes(t *testing.T) {
}
}

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

parent := t.TempDir()
removed := filepath.Join(parent, "removed")
other := filepath.Join(parent, "other")
store := trustStore{Trusted: []trustRecord{
{Scope: filepath.Join(removed, ".git"), RepositoryInstance: "old", SHA256: "root"},
{Scope: filepath.Join(removed, ".git", "modules", "sub"), RepositoryInstance: "old-sub", SHA256: "sub"},
{Scope: filepath.Join(other, ".git"), RepositoryInstance: "other", SHA256: "other"},
{Scope: trustScopeUser, SHA256: "user"},
}}
if err := saveTrustStore(store); err != nil {
t.Fatal(err)
}

savedPath, savedGlobal, savedFn := untrustPath, untrustGlobal, repoTrustKeyFn
untrustPath, untrustGlobal = removed, false
repoTrustKeyFn = func() (repoIdentity, error) {
return repoIdentity{}, errors.New("--path must not require a current repository")
}
t.Cleanup(func() {
untrustPath, untrustGlobal, repoTrustKeyFn = savedPath, savedGlobal, savedFn
})

var err error
out := captureStdout(t, func() { err = runUntrust(nil) })
if err != nil {
t.Fatal(err)
}
if !strings.Contains(out, "Revoked 2 approval(s)") || !strings.Contains(out, removed) {
t.Errorf("untrust did not report what it removed:\n%s", out)
}

after, err := loadTrustStore()
if err != nil {
t.Fatal(err)
}
if len(after.Trusted) != 2 {
t.Fatalf("untrust --path kept %d records, want the unrelated repository and user config records: %+v", len(after.Trusted), after.Trusted)
}
if !after.isTrusted(filepath.Join(other, ".git"), "other", "other") ||
!after.isTrusted(trustScopeUser, "", "user") {
t.Errorf("untrust --path removed an unrelated approval: %+v", after.Trusted)
}
}

func TestDropTrustRecordsAtFollowsTheGitDirectorySeparately(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("symlink creation needs a privilege we cannot assume on Windows")
}
withIsolatedTrustStore(t)

parent := t.TempDir()
repo := filepath.Join(parent, "removed")
gitDir := filepath.Join(parent, "gitdirs", "removed.git")
if err := os.MkdirAll(repo, 0o755); err != nil {
t.Fatal(err)
}
if err := os.MkdirAll(gitDir, 0o755); err != nil {
t.Fatal(err)
}
if err := os.Symlink(gitDir, filepath.Join(repo, ".git")); err != nil {
t.Fatal(err)
}
if err := saveTrustStore(trustStore{Trusted: []trustRecord{{
Scope: filepath.Join(gitDir, "modules", "sub"), RepositoryInstance: "old-sub", SHA256: "sub",
}}}); err != nil {
t.Fatal(err)
}

removed, err := dropTrustRecordsAt(repo)
if err != nil {
t.Fatal(err)
}
if removed != 1 {
t.Fatalf("dropTrustRecordsAt() removed %d records, want the record beneath the separately resolved .git", removed)
}
}

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

root := t.TempDir()
if err := os.WriteFile(filepath.Join(root, "still-here"), []byte("x"), 0o644); err != nil {
t.Fatal(err)
}
if err := saveTrustStore(trustStore{Trusted: []trustRecord{{
Scope: filepath.Join(root, "repo", ".git"), RepositoryInstance: "repo", SHA256: "hash",
}}}); err != nil {
t.Fatal(err)
}

savedPath, savedGlobal := untrustPath, untrustGlobal
untrustPath, untrustGlobal = root, false
t.Cleanup(func() { untrustPath, untrustGlobal = savedPath, savedGlobal })

if err := runUntrust(nil); err == nil || !strings.Contains(err.Error(), "not absent or an empty directory") {
t.Fatalf("untrust --path on a non-empty tree returned %v, want a refusal", err)
}
after, err := loadTrustStore()
if err != nil {
t.Fatal(err)
}
if len(after.Trusted) != 1 {
t.Fatalf("refused untrust changed the trust store: %+v", after.Trusted)
}
}

func TestUntrustRejectsGlobalWithPath(t *testing.T) {
savedPath, savedGlobal := untrustPath, untrustGlobal
untrustPath, untrustGlobal = filepath.Join(t.TempDir(), "removed"), true
t.Cleanup(func() { untrustPath, untrustGlobal = savedPath, savedGlobal })

if err := runUntrust(nil); err == nil || !strings.Contains(err.Error(), "cannot be used together") {
t.Fatalf("runUntrust() with --global and --path returned %v, want a mutual-exclusion error", err)
}
}

func TestUntrustPathJSONReportsTheTarget(t *testing.T) {
withIsolatedTrustStore(t)
removed := filepath.Join(t.TempDir(), "removed")
if err := saveTrustStore(trustStore{Trusted: []trustRecord{{
Scope: filepath.Join(removed, ".git"), RepositoryInstance: "old", SHA256: "hash",
}}}); err != nil {
t.Fatal(err)
}

savedPath, savedGlobal, savedFormat := untrustPath, untrustGlobal, outputFormat
untrustPath, untrustGlobal, outputFormat = removed, false, formatJSON
t.Cleanup(func() {
untrustPath, untrustGlobal, outputFormat = savedPath, savedGlobal, savedFormat
})

var runErr error
out := captureStdout(t, func() { runErr = runUntrust(untrustCmd) })
if runErr != nil {
t.Fatal(runErr)
}
var payload struct {
OK bool `json:"ok"`
Data struct {
Path string `json:"path"`
Removed int `json:"removed"`
} `json:"data"`
}
if err := json.Unmarshal([]byte(out), &payload); err != nil {
t.Fatalf("invalid JSON output: %v\n%s", err, out)
}
if !payload.OK || payload.Data.Path != removed || payload.Data.Removed != 1 {
t.Errorf("unexpected JSON result: %+v", payload)
}
}

// TestConfigFileHooksNeedApprovalToo is the inversion: wt used to run anything
// not labelled "repo config" unprompted, which made the permissive answer the
// one a source got by omission. Nothing runs unapproved now, whoever wrote it.
Expand Down
9 changes: 6 additions & 3 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -978,6 +978,7 @@ Approve ahead of time, or review what is approved:
wt trust # approve everything that applies here
wt trust --list # every approval on this machine
wt untrust # revoke this repository's approval
wt untrust --path P # revoke stale approvals after a repository at P was removed
wt untrust --global # revoke your config file's approval
```

Expand Down Expand Up @@ -1114,9 +1115,11 @@ The same conservative rule applies to an approval record still naming the
destination. Repository approvals now include a filesystem identity, so a
checkout replaced in place does not inherit one, even when plain git performed
the clone. `wt migrate` nevertheless declines to move onto a path carrying an
old record and says which path to run `wt untrust` in: before the move there is
no destination filesystem identity to compare, and moving first in order to ask
would be too late. Compared as command sets, not as "is anything approved here":
old record: before the move there is no destination filesystem identity to
compare, and moving first in order to ask would be too late. Run the reported
`wt untrust --path <destination>` command to remove those stale records, then
retry the migration. Compared as command sets, not as "is anything approved
here":
a repository you approved once holds a record for the commands it had then, and
asking only whether its current path has *an* approval would let that one wave
through a move onto a different set waiting at the destination.
Expand Down
1 change: 1 addition & 0 deletions plugins/wt/skills/wt/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ wt runs no hook until it has been approved — a repository's committed `.wt.tom
wt trust # approve every hook source that applies here
wt trust --list # show every approval on this machine
wt untrust # revoke this repository's approval
wt untrust --path P # revoke stale approvals after a repository at P was removed
wt untrust --global # revoke the config file's approval
```

Expand Down