diff --git a/cmd/felis/tui_edge.go b/cmd/felis/tui_edge.go index b7b3bad..936e014 100644 --- a/cmd/felis/tui_edge.go +++ b/cmd/felis/tui_edge.go @@ -437,6 +437,10 @@ func (m *edgeModel) errorView() string { if m.lastErr != nil { b.WriteString(tuiHint.Render(m.lastErr.Error()) + "\n") } + // Nothing done above is rolled back, and nothing needs to be: every step finds what an + // earlier attempt created (the tunnel, its DNS route, the Access app and policy) and + // carries on from it. + b.WriteString("\n" + tuiHint.Render("Retrying is safe: it reuses the tunnel, DNS record and Access app created so far instead of making duplicates.") + "\n") b.WriteString("\n" + tuiAction("enter", "retry", "esc", "edit")) return b.String() } diff --git a/internal/cfsetup/credentials_test.go b/internal/cfsetup/credentials_test.go new file mode 100644 index 0000000..4ac7e7e --- /dev/null +++ b/internal/cfsetup/credentials_test.go @@ -0,0 +1,117 @@ +package cfsetup + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" +) + +const testTunnelID = "0b6f3c1e-2d4a-4b8c-9e7f-1a2b3c4d5e6f" + +// fakeCloudflared stands in for `cloudflared tunnel token --cred-file `: it +// counts its calls and writes what FAKE_CRED names into the file (valid credentials for +// the tunnel it was asked about by default). +func fakeCloudflared(t *testing.T) (bin, calls string) { + t.Helper() + dir := t.TempDir() + bin = filepath.Join(dir, "cloudflared") + calls = filepath.Join(dir, "calls") + script := `#!/bin/sh +echo "$*" >> "` + calls + `" +case "${FAKE_CRED:-good}" in + good) printf '{"AccountTag":"acc","TunnelSecret":"c2VjcmV0","TunnelID":"%s"}' "$5" > "$4" ;; + garbage) printf 'not json' > "$4" ;; +esac +` + if err := os.WriteFile(bin, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + return bin, calls +} + +func callCount(t *testing.T, calls string) int { + t.Helper() + raw, err := os.ReadFile(calls) + if os.IsNotExist(err) { + return 0 + } + if err != nil { + t.Fatal(err) + } + return strings.Count(string(raw), "\n") +} + +func TestEnsureCredentialsKeepsAValidFile(t *testing.T) { + bin, calls := fakeCloudflared(t) + cred := filepath.Join(t.TempDir(), testTunnelID+".json") + good := `{"AccountTag":"acc","TunnelSecret":"c2VjcmV0","TunnelID":"` + strings.ToUpper(testTunnelID) + `"}` + if err := os.WriteFile(cred, []byte(good), 0o600); err != nil { + t.Fatal(err) + } + r := &ExecRunner{Cloudflared: bin} + if err := r.ensureCredentials(context.Background(), testTunnelID, cred); err != nil { + t.Fatalf("ensureCredentials: %v", err) + } + if n := callCount(t, calls); n != 0 { + t.Fatalf("a valid credentials file was re-fetched (%d cloudflared calls)", n) + } +} + +func TestEnsureCredentialsReplacesAnUnusableFile(t *testing.T) { + for name, content := range map[string]string{ + "empty": "", + "truncated": `{"AccountTag":"acc","TunnelSec`, + "missing secret": `{"AccountTag":"acc","TunnelID":"` + testTunnelID + `"}`, + "another tunnel": `{"AccountTag":"acc","TunnelSecret":"c2VjcmV0","TunnelID":"11111111-2222-3333-4444-555555555555"}`, + } { + t.Run(name, func(t *testing.T) { + bin, calls := fakeCloudflared(t) + cred := filepath.Join(t.TempDir(), testTunnelID+".json") + if err := os.WriteFile(cred, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + r := &ExecRunner{Cloudflared: bin} + if err := r.ensureCredentials(context.Background(), testTunnelID, cred); err != nil { + t.Fatalf("ensureCredentials: %v", err) + } + if n := callCount(t, calls); n != 1 { + t.Fatalf("want one re-fetch, got %d cloudflared calls", n) + } + if p := credentialsProblem(cred, testTunnelID); p != "" { + t.Fatalf("the regenerated file %s", p) + } + kept, err := os.ReadFile(cred + ".invalid") + if err != nil || string(kept) != content { + t.Fatalf("the unusable file was not kept aside: %q, %v", kept, err) + } + if fi, err := os.Stat(cred); err != nil || fi.Mode().Perm() != 0o600 { + t.Fatalf("the regenerated file must be 0600: %v, %v", fi, err) + } + }) + } +} + +func TestEnsureCredentialsFetchesAMissingFile(t *testing.T) { + bin, calls := fakeCloudflared(t) + cred := filepath.Join(t.TempDir(), "sub", testTunnelID+".json") + r := &ExecRunner{Cloudflared: bin} + if err := r.ensureCredentials(context.Background(), testTunnelID, cred); err != nil { + t.Fatalf("ensureCredentials: %v", err) + } + if n := callCount(t, calls); n != 1 { + t.Fatalf("want one fetch, got %d cloudflared calls", n) + } +} + +func TestEnsureCredentialsRefusesAnUnusableRegeneration(t *testing.T) { + bin, _ := fakeCloudflared(t) + t.Setenv("FAKE_CRED", "garbage") + cred := filepath.Join(t.TempDir(), testTunnelID+".json") + r := &ExecRunner{Cloudflared: bin} + err := r.ensureCredentials(context.Background(), testTunnelID, cred) + if err == nil || !strings.Contains(err.Error(), "is not valid JSON") { + t.Fatalf("want the unusable regeneration reported, got %v", err) + } +} diff --git a/internal/cfsetup/runner.go b/internal/cfsetup/runner.go index e116f4f..a09bf5b 100644 --- a/internal/cfsetup/runner.go +++ b/internal/cfsetup/runner.go @@ -129,18 +129,30 @@ func (r *ExecRunner) CreateTunnel(ctx context.Context, name string) (string, str // doesn't exist". `cloudflared tunnel token --cred-file` re-fetches the token into // the file (authenticating with cert.pem, keeping the same id/DNS/Access), healing // the re-run. The secret is written to the file, not stdout. +// +// A file that is there but unusable — truncated, zero bytes, or another tunnel's — would +// crash-loop the connector just the same while setup reported success, so it is moved +// aside to .invalid and re-fetched like a missing one. func (r *ExecRunner) ensureCredentials(ctx context.Context, id, credPath string) error { - // Any existing file counts as healthy; re-fetch only on absence - // (the failure actually seen). A truncated/zero-byte file would still - // crash-loop — validate the JSON here if that ever shows up. + state := "missing" if _, err := os.Stat(credPath); err == nil { - return nil + problem := credentialsProblem(credPath, id) + if problem == "" { + return nil + } + if err := os.Rename(credPath, credPath+".invalid"); err != nil { + return fmt.Errorf("cfsetup: tunnel %s credentials file %s %s, and moving it aside failed: %w", id, credPath, problem, err) + } + state = problem + " (kept as " + credPath + ".invalid)" } if err := os.MkdirAll(filepath.Dir(credPath), 0o700); err != nil { return fmt.Errorf("cfsetup: preparing credentials dir for tunnel %s: %w", id, err) } if _, err := r.runCloudflared(ctx, "tunnel", "token", "--cred-file", credPath, id); err != nil { - return fmt.Errorf("cfsetup: tunnel %s credentials file %s is missing and could not be regenerated: %w", id, credPath, err) + return fmt.Errorf("cfsetup: tunnel %s credentials file %s is %s and could not be regenerated: %w", id, credPath, state, err) + } + if problem := credentialsProblem(credPath, id); problem != "" { + return fmt.Errorf("cfsetup: cloudflared regenerated tunnel %s credentials at %s, but the file %s", id, credPath, problem) } // The credentials file is a secret sitting next to cert.pem; don't rely on // cloudflared's umask to keep it owner-only. @@ -150,6 +162,31 @@ func (r *ExecRunner) ensureCredentials(ctx context.Context, id, credPath string) return nil } +// credentialsProblem describes what makes the credentials JSON at path unusable for tunnel +// id, or returns "" when cloudflared can run the tunnel from it: the fields +// `cloudflared tunnel run` reads are present, and it names this tunnel. +func credentialsProblem(path, id string) string { + raw, err := os.ReadFile(path) + if err != nil { + return "cannot be read (" + err.Error() + ")" + } + var cred struct { + AccountTag string `json:"AccountTag"` + TunnelSecret string `json:"TunnelSecret"` + TunnelID string `json:"TunnelID"` + } + if err := json.Unmarshal(raw, &cred); err != nil { + return "is not valid JSON" + } + switch { + case cred.AccountTag == "" || cred.TunnelSecret == "" || cred.TunnelID == "": + return "lacks AccountTag, TunnelSecret or TunnelID" + case !strings.EqualFold(cred.TunnelID, id): + return "belongs to tunnel " + cred.TunnelID + } + return "" +} + // lookupTunnel finds an existing tunnel's id by name via `tunnel list`. func (r *ExecRunner) lookupTunnel(ctx context.Context, name string) (string, error) { out, err := r.runCloudflared(ctx, "tunnel", "list", "--name", name, "--output", "json")