diff --git a/README.md b/README.md index f6fca1d..6d1d80c 100644 --- a/README.md +++ b/README.md @@ -230,10 +230,6 @@ tng completion fish > ~/.config/fish/completions/tng.fish Config file is at `~/.config/tng/config.yml`: ```yaml -editor: "" # Override $EDITOR -pager: "" # Override $PAGER -browser: "" # Override $BROWSER -git_protocol: ssh # ssh or https default_knot: "" # Default knot for repo creation api_host: "" # Override API host ``` diff --git a/api/client.go b/api/client.go index 0d607d0..04a3d16 100644 --- a/api/client.go +++ b/api/client.go @@ -22,6 +22,9 @@ import ( // slow or unresponsive server can't hang the CLI indefinitely. const defaultHTTPTimeout = 30 * time.Second +// maxBlobBytes caps blob downloads so a hostile PDS can't exhaust memory. +const maxBlobBytes = 64 << 20 // 64 MiB + // Client wraps an authenticated session for making Tangled API calls. // // Tangled uses AT Protocol service auth: the CLI authenticates to the @@ -335,6 +338,38 @@ func (c *Client) doPlainRequest(req *http.Request, nsid string, out interface{}) return nil } +// doRawRequest executes an HTTP request like doPlainRequest but returns the +// raw response body (bounded by maxBlobBytes) instead of decoding JSON. +func (c *Client) doRawRequest(req *http.Request, nsid string) ([]byte, error) { + httpClient := c.http + if httpClient == nil { + httpClient = http.DefaultClient + } + resp, err := httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("request failed: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + apiErr := &APIError{StatusCode: resp.StatusCode, NSID: nsid} + json.NewDecoder(resp.Body).Decode(apiErr) + if apiErr.ErrorType == "" { + apiErr.ErrorType = http.StatusText(resp.StatusCode) + } + return nil, apiErr + } + + data, err := io.ReadAll(io.LimitReader(resp.Body, maxBlobBytes+1)) + if err != nil { + return nil, fmt.Errorf("failed to read response: %w", err) + } + if len(data) > maxBlobBytes { + return nil, fmt.Errorf("response exceeds %d bytes", maxBlobBytes) + } + return data, nil +} + // ResolvedRepo holds metadata about a Tangled repository resolved from AT Protocol records. type ResolvedRepo struct { URI string // at://did/sh.tangled.repo/rkey @@ -385,7 +420,8 @@ func (c *Client) listRecordsAll(ctx context.Context, owner, collection string, l } all = append(all, rl.Records...) - if rl.Cursor == "" || len(rl.Records) == 0 { + // A repeated cursor means a misbehaving server; stop rather than loop forever. + if rl.Cursor == "" || len(rl.Records) == 0 || rl.Cursor == cursor { break } cursor = rl.Cursor @@ -432,7 +468,9 @@ func (c *Client) ResolveRepo(ctx context.Context, owner, name string) (*Resolved return nil, fmt.Errorf("repo %s/%s not found in AT Protocol records", owner, name) } -// GetRecord fetches a single AT Protocol record from the PDS. +// GetRecord fetches a single AT Protocol record. It tries the authenticated +// user's PDS first and falls back to the record owner's PDS for cross-PDS +// reads — the user's PDS only serves repos it hosts. func (c *Client) GetRecord(ctx context.Context, did, collection, rkey string) (*RecordEntry, error) { nsid, err := syntax.ParseNSID("com.atproto.repo.getRecord") if err != nil { @@ -446,7 +484,29 @@ func (c *Client) GetRecord(ctx context.Context, did, collection, rkey string) (* } var out RecordEntry - if err := c.pdsClient.Get(ctx, nsid, params, &out); err != nil { + ownErr := c.pdsClient.Get(ctx, nsid, params, &out) + if ownErr == nil { + return &out, nil + } + + // The user's PDS may not host this DID — query the owner's PDS directly. + pdsURL, resolvedDID, err := c.resolvePDS(ctx, did) + if err != nil { + return nil, ownErr + } + u := pdsURL + "/xrpc/com.atproto.repo.getRecord?" + url.Values{ + "repo": {resolvedDID}, + "collection": {collection}, + "rkey": {rkey}, + }.Encode() + + req, err := http.NewRequestWithContext(ctx, "GET", u, nil) + if err != nil { + return nil, err + } + req.Header.Set("Accept", "application/json") + + if err := c.doPlainRequest(req, "com.atproto.repo.getRecord", &out); err != nil { return nil, err } return &out, nil @@ -497,7 +557,8 @@ func Paginate[T any](ctx context.Context, fn func(cursor string) ([]T, string, e return nil, err } all = append(all, items...) - if nextCursor == "" { + // A repeated cursor means a misbehaving server; stop rather than loop forever. + if nextCursor == "" || nextCursor == cursor { break } cursor = nextCursor diff --git a/api/issue.go b/api/issue.go index d927c78..eda2deb 100644 --- a/api/issue.go +++ b/api/issue.go @@ -177,20 +177,6 @@ func (c *Client) CloseIssue(ctx context.Context, issueURI string) error { return err } -// ReopenIssue writes an issue state record to reopen the issue. -func (c *Client) ReopenIssue(ctx context.Context, issueURI string) error { - rkey := newTID() - - record := &issueStateRecord{ - Type: IssueStateNSID, - Issue: issueURI, - State: IssueStateOpen, - } - - _, err := c.PutRecord(ctx, IssueStateNSID, rkey, record) - return err -} - // CreateIssueComment adds a comment to an issue. // issueURI is the AT URI of the issue. func (c *Client) CreateIssueComment(ctx context.Context, issueURI, body string) error { diff --git a/api/pull.go b/api/pull.go index 1e9ad28..a5cb7be 100644 --- a/api/pull.go +++ b/api/pull.go @@ -6,6 +6,8 @@ import ( "context" "encoding/json" "fmt" + "io" + "net/http" "net/url" "time" @@ -268,20 +270,6 @@ func (c *Client) ClosePull(ctx context.Context, pullURI string) error { return err } -// ReopenPull writes a PR status record to reopen the PR. -func (c *Client) ReopenPull(ctx context.Context, pullURI string) error { - rkey := newTID() - - record := &pullStatusRecord{ - Type: PullStatusNSID, - Pull: pullURI, - Status: PullStatusOpen, - } - - _, err := c.PutRecord(ctx, PullStatusNSID, rkey, record) - return err -} - // CreatePullComment adds a comment to a pull request. func (c *Client) CreatePullComment(ctx context.Context, pullURI, body string) error { rkey := newTID() @@ -338,22 +326,34 @@ func (c *Client) GetPullPatch(ctx context.Context, authorDID, rkey string) (stri return "", err } - // Decompress gzip + // Not gzipped (older records stored the patch uncompressed). + if len(blobData) < 2 || blobData[0] != 0x1f || blobData[1] != 0x8b { + return string(blobData), nil + } + gz, err := gzip.NewReader(bytes.NewReader(blobData)) if err != nil { - // Might not be gzipped (older records) - return string(blobData), nil + return "", fmt.Errorf("failed to decompress patch: %w", err) } defer gz.Close() - var buf bytes.Buffer - if _, err := buf.ReadFrom(gz); err != nil { + // Bound the decompressed size — the blob is untrusted (any author's PDS) + // and could otherwise be a decompression bomb. + data, err := io.ReadAll(io.LimitReader(gz, maxPatchBytes+1)) + if err != nil { return "", fmt.Errorf("failed to decompress patch: %w", err) } - return buf.String(), nil + if len(data) > maxPatchBytes { + return "", fmt.Errorf("decompressed patch exceeds %d bytes", maxPatchBytes) + } + return string(data), nil } -// GetBlob downloads a blob from the PDS. +// maxPatchBytes caps the decompressed size of a PR patch blob. +const maxPatchBytes = 256 << 20 // 256 MiB + +// GetBlob downloads a blob. It tries the authenticated user's PDS first and +// falls back to the blob owner's PDS for cross-PDS reads. func (c *Client) GetBlob(ctx context.Context, did, cid string) ([]byte, error) { nsid, err := syntax.ParseNSID("com.atproto.sync.getBlob") if err != nil { @@ -366,10 +366,31 @@ func (c *Client) GetBlob(ctx context.Context, did, cid string) ([]byte, error) { } var buf bytes.Buffer - if err := c.pdsClient.LexDo(ctx, "GET", "", nsid.String(), params, nil, &buf); err != nil { + ownErr := c.pdsClient.LexDo(ctx, "GET", "", nsid.String(), params, nil, &buf) + if ownErr == nil { + return buf.Bytes(), nil + } + + // The user's PDS may not host this DID — fetch from the owner's PDS. + pdsURL, resolvedDID, err := c.resolvePDS(ctx, did) + if err != nil { + return nil, fmt.Errorf("failed to download blob: %w", ownErr) + } + u := pdsURL + "/xrpc/com.atproto.sync.getBlob?" + url.Values{ + "did": {resolvedDID}, + "cid": {cid}, + }.Encode() + + req, err := http.NewRequestWithContext(ctx, "GET", u, nil) + if err != nil { + return nil, err + } + + data, err := c.doRawRequest(req, "com.atproto.sync.getBlob") + if err != nil { return nil, fmt.Errorf("failed to download blob: %w", err) } - return buf.Bytes(), nil + return data, nil } // FormatPullURL returns the web URL for a repo's pull requests on tangled.sh. diff --git a/api/repo.go b/api/repo.go index ee10ea9..462f9fb 100644 --- a/api/repo.go +++ b/api/repo.go @@ -183,12 +183,20 @@ func (c *Client) DeleteRepo(ctx context.Context, knotURL string, did, name, rkey if err := c.PostToKnot(ctx, knotURL, "sh.tangled.repo.delete", &input, nil); err != nil { // Roll back: restore the PDS record so the repo remains resolvable. + // If the restore also fails, say so — otherwise the record is silently + // gone while the bare repo still exists on the knot. + var restoreErr error if len(record) > 0 { var raw map[string]interface{} if jsonErr := json.Unmarshal(record, &raw); jsonErr == nil { - _, _ = c.PutRecord(ctx, RepoRecordNSID, rkey, raw) + _, restoreErr = c.PutRecord(ctx, RepoRecordNSID, rkey, raw) + } else { + restoreErr = jsonErr } } + if restoreErr != nil { + return fmt.Errorf("%w (restoring the PDS record also failed: %v; the repo record %s is gone but the repo still exists on the knot)", err, restoreErr, rkey) + } return err } diff --git a/context/remote.go b/context/remote.go index 7292c23..ce8766e 100644 --- a/context/remote.go +++ b/context/remote.go @@ -17,28 +17,6 @@ type TangledRemote struct { // Remotes is a sorted list of Tangled-aware remotes. type Remotes []*TangledRemote -// FindByName returns the first remote matching one of the given names. -func (r Remotes) FindByName(names ...string) (*TangledRemote, error) { - for _, name := range names { - for _, remote := range r { - if remote.Remote.Name == name { - return remote, nil - } - } - } - return nil, fmt.Errorf("no Tangled remote found with name(s): %v", names) -} - -// FindByRepo returns the remote matching an owner/name combo. -func (r Remotes) FindByRepo(owner, name string) (*TangledRemote, error) { - for _, remote := range r { - if remote.Repo.Owner == owner && remote.Repo.Name == name { - return remote, nil - } - } - return nil, fmt.Errorf("no remote found for %s/%s", owner, name) -} - // ResolveTangledRemotes extracts Tangled remotes from git, sorted by priority. func ResolveTangledRemotes(gc *gitpkg.Client) (Remotes, error) { gitRemotes, err := gc.Remotes() @@ -95,8 +73,8 @@ func remotePriority(name string) int { // ResolveBaseRepo determines the current Tangled repository from context. // Resolution order: // 1. TNG_REPO env var -// 2. Git remotes matching tangled.sh -// 3. git config tng.default-repo +// 2. git config tng.default-repo +// 3. Git remotes matching Tangled hosts func ResolveBaseRepo(gc *gitpkg.Client) (*gitpkg.TangledRepo, error) { // 1. Check TNG_REPO env var if envRepo := os.Getenv("TNG_REPO"); envRepo != "" { diff --git a/context/remote_test.go b/context/remote_test.go index 8da491c..5976c60 100644 --- a/context/remote_test.go +++ b/context/remote_test.go @@ -96,43 +96,3 @@ func TestTranslateRemotes_TangledNamedRemote(t *testing.T) { t.Errorf("first remote = %q, want 'tangled'", remotes[0].Remote.Name) } } - -func TestRemotes_FindByName(t *testing.T) { - remotes := Remotes{ - {Remote: &gitpkg.Remote{Name: "origin"}, Repo: gitpkg.TangledRepo{Owner: "alice", Name: "repo"}}, - {Remote: &gitpkg.Remote{Name: "upstream"}, Repo: gitpkg.TangledRepo{Owner: "bob", Name: "repo"}}, - } - - r, err := remotes.FindByName("upstream") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if r.Repo.Owner != "bob" { - t.Errorf("found owner = %q, want 'bob'", r.Repo.Owner) - } - - _, err = remotes.FindByName("nonexistent") - if err == nil { - t.Error("expected error for nonexistent remote") - } -} - -func TestRemotes_FindByRepo(t *testing.T) { - remotes := Remotes{ - {Remote: &gitpkg.Remote{Name: "origin"}, Repo: gitpkg.TangledRepo{Owner: "alice", Name: "repo1"}}, - {Remote: &gitpkg.Remote{Name: "upstream"}, Repo: gitpkg.TangledRepo{Owner: "bob", Name: "repo2"}}, - } - - r, err := remotes.FindByRepo("bob", "repo2") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if r.Remote.Name != "upstream" { - t.Errorf("found remote = %q, want 'upstream'", r.Remote.Name) - } - - _, err = remotes.FindByRepo("alice", "nonexistent") - if err == nil { - t.Error("expected error for nonexistent repo") - } -} diff --git a/docs/index.html b/docs/index.html index 1157d47..f540ce5 100644 --- a/docs/index.html +++ b/docs/index.html @@ -413,34 +413,32 @@ tng pr create -t "Fix bug" --base main --head fix-branch

tng pr list

tng pr list
-tng pr list --status closed
-tng pr list --status all -L 50
+tng pr list --repo alice.bsky.social/my-project +tng pr list --web - - - + +
FlagDescription
-s, --statusFilter: open, closed, merged, all (default: open)
-R, --repoRepository override
-L, --limitMaximum results (default: 30)
-R, --repoFilter by repository (owner/name)
-w, --webPrint the tangled.sh pull requests URL
-

tng pr view <number>

-

Display PR details including title, status, branches, round number, and body.

+

tng pr view <rkey>

+

Display PR details including title, branches, and body.

- + -
FlagDescription
-c, --commentsInclude comments in output
--authorDID of the PR author (defaults to you)
-w, --webOpen in browser
-R, --repoRepository override
-

tng pr diff <number>

+

tng pr diff <rkey>

Print the raw diff of a pull request to stdout.

-

tng pr merge <number>

+

tng pr merge <rkey>

Merge a pull request. The PR must be in open status.

-

tng pr close <number>

+

tng pr close <rkey>

Close a pull request without merging.

-

tng pr comment <number>

+

tng pr comment <rkey>

Add a comment to a pull request.

@@ -463,26 +461,25 @@ tng issue create -t "Feature request"

tng issue list

tng issue list
-tng issue list --status closed
-tng issue list --status all -L 50
+tng issue list --repo alice.bsky.social/my-project +tng issue list --web
FlagDescription
- - - + +
FlagDescription
-s, --statusFilter: open, closed, all (default: open)
-R, --repoRepository override
-L, --limitMaximum results (default: 30)
-R, --repoFilter by repository (owner/name)
-w, --webPrint the tangled.sh issues URL
-

tng issue view <number>

+

tng issue view <rkey>

- +
FlagDescription
-c, --commentsInclude comments
--authorDID of the issue author (defaults to you)
-w, --webOpen in browser
-

tng issue close <number>

+

tng issue close <rkey>

Close an issue.

-

tng issue comment <number>

+

tng issue comment <rkey>

@@ -513,22 +510,16 @@ tng browse -R alice.bsky.social/proj # Specify repo

Configuration

Config is stored at ~/.config/tng/config.yml (respects XDG_CONFIG_HOME and TNG_CONFIG_DIR).

-
editor: ""           # Override $EDITOR
-pager: ""            # Override $PAGER
-browser: ""          # Override $BROWSER
-git_protocol: ssh    # ssh or https
-default_knot: ""     # Default knot for repo creation
+    
default_knot: ""     # Default knot for repo creation
 api_host: ""         # Override API host (default: https://tangled.sh)

Environment Variables

FlagDescription
-b, --bodyComment text (required)
- - - +
VariableDescription
TNG_TOKENOverride auth token for CI/scripts
TNG_HOSTOverride API host (default: https://tangled.sh)
TNG_CONFIG_DIROverride config directory
TNG_REPOOverride repo context (owner/name)
TNG_PAGEROverride pager program
TNG_FORCE_TTYForce TTY output in non-TTY contexts
TNG_FORCE_TTYForce color output in non-TTY contexts
NO_COLORDisable all color output
diff --git a/git/client.go b/git/client.go index 26e8c7f..d8341ef 100644 --- a/git/client.go +++ b/git/client.go @@ -54,33 +54,18 @@ func (c *Client) Config(key string) (string, error) { return strings.TrimSpace(out), nil } -// SetConfig writes a git config value. -func (c *Client) SetConfig(key, value string) error { - _, err := c.run("config", key, value) - return err -} - -// Clone clones a repository. +// Clone clones a repository. Options go before the "--" separator so a URL or +// directory starting with "-" can't be parsed as a git flag. func (c *Client) Clone(url string, dir string, args ...string) error { - cmdArgs := []string{"clone", url} + cmdArgs := append([]string{"clone"}, args...) + cmdArgs = append(cmdArgs, "--", url) if dir != "" { cmdArgs = append(cmdArgs, dir) } - cmdArgs = append(cmdArgs, args...) _, err := c.runGlobal(cmdArgs...) return err } -// Fetch fetches from a remote. -func (c *Client) Fetch(remote string, refspec string) error { - args := []string{"fetch", remote} - if refspec != "" { - args = append(args, refspec) - } - _, err := c.run(args...) - return err -} - // IsInsideWorkTree returns true if the current directory is inside a git repo. func (c *Client) IsInsideWorkTree() bool { out, err := c.run("rev-parse", "--is-inside-work-tree") @@ -110,6 +95,10 @@ func (c *Client) run(args ...string) (string, error) { // FormatPatch generates a format-patch from the given base ref to HEAD. func (c *Client) FormatPatch(base string) ([]byte, error) { + // The base ref lands in git argv; reject flag-like values (argument injection). + if strings.HasPrefix(base, "-") { + return nil, fmt.Errorf("invalid base ref %q", base) + } cmd := exec.Command(c.GitPath, "format-patch", "--stdout", base+"..HEAD") cmd.Dir = c.RepoDir var stdout, stderr bytes.Buffer diff --git a/git/client_test.go b/git/client_test.go index 62a8390..a838a5b 100644 --- a/git/client_test.go +++ b/git/client_test.go @@ -162,8 +162,10 @@ func TestConfig(t *testing.T) { dir := setupTestRepo(t) c := &Client{GitPath: "git", RepoDir: dir} - if err := c.SetConfig("tng.test-key", "test-value"); err != nil { - t.Fatalf("SetConfig: %v", err) + cmd := exec.Command("git", "config", "tng.test-key", "test-value") + cmd.Dir = dir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("%v: %s", err, out) } val, err := c.Config("tng.test-key") diff --git a/git/remote.go b/git/remote.go index da49039..bc4b4e8 100644 --- a/git/remote.go +++ b/git/remote.go @@ -36,9 +36,10 @@ var knownTangledHosts = map[string]bool{ } // ParseTangledURL extracts owner and repo name from a Tangled remote URL. -// Recognizes tangled.sh directly and knot hosts via git config markers -// set during `tng repo clone`. -// Returns nil if the URL is not a recognized Tangled remote. +// Recognizes tangled.sh and hosts following the "knot." naming convention +// (see isTangledHost). Returns nil if the URL is not a recognized Tangled +// remote; self-hosted knots on other hostnames need `git config +// tng.default-repo` or TNG_REPO instead. func ParseTangledURL(rawURL string) *TangledRepo { if m := sshPattern.FindStringSubmatch(rawURL); m != nil { host, owner, name := m[1], m[2], m[3] @@ -68,6 +69,16 @@ func isTangledHost(host string) bool { return false } +// SSHCloneURL builds the SSH clone URL for a repo on a knot. Hosts with an +// explicit port need the ssh:// form — git would otherwise parse +// "git@host:port:owner/repo" with the port as part of the path. +func SSHCloneURL(host, owner, name string) string { + if strings.Contains(host, ":") { + return "ssh://git@" + host + "/" + owner + "/" + name + } + return "git@" + host + ":" + owner + "/" + name +} + // ParseRemotes parses `git remote -v` output into Remote structs. func ParseRemotes(output string) []Remote { byName := map[string]*Remote{} diff --git a/internal/authflow/flow.go b/internal/authflow/flow.go index 5b4b493..4f64b50 100644 --- a/internal/authflow/flow.go +++ b/internal/authflow/flow.go @@ -3,8 +3,10 @@ package authflow import ( "context" "fmt" + "html" "net" "net/http" + "os" "time" "github.com/bluesky-social/indigo/atproto/auth/oauth" @@ -33,13 +35,14 @@ type LoginResult struct { // 5. Exchanges code for tokens // 6. Persists session func Login(ctx context.Context, identifier string) (*LoginResult, error) { - // Find a free port for the callback server + // Bind the callback port and keep the listener for the whole flow, so the + // redirect URL registered with the auth server can't be claimed by another + // process between port selection and serving. listener, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { return nil, fmt.Errorf("failed to find available port: %w", err) } port := listener.Addr().(*net.TCPAddr).Port - listener.Close() callbackURL := fmt.Sprintf("http://127.0.0.1:%d%s", port, callbackPath) @@ -56,6 +59,7 @@ func Login(ctx context.Context, identifier string) (*LoginResult, error) { // Start the auth flow (resolves identity, sends PAR) redirectURL, err := app.StartAuthFlow(ctx, identifier) if err != nil { + listener.Close() return nil, fmt.Errorf("failed to start auth flow: %w", err) } @@ -72,7 +76,8 @@ func Login(ctx context.Context, identifier string) (*LoginResult, error) { if err != nil { resultCh <- callbackResult{err: err} w.Header().Set("Content-Type", "text/html") - fmt.Fprintf(w, "

Authentication failed

%s

You can close this tab.

", err.Error()) + // The error can echo attacker-influenced callback params; escape it. + fmt.Fprintf(w, "

Authentication failed

%s

You can close this tab.

", html.EscapeString(err.Error())) return } resultCh <- callbackResult{sessData: sessData} @@ -81,20 +86,20 @@ func Login(ctx context.Context, identifier string) (*LoginResult, error) { }) server := &http.Server{ - Addr: fmt.Sprintf("127.0.0.1:%d", port), - Handler: mux, + Handler: mux, + ReadTimeout: 30 * time.Second, } - // Start callback server + // Start callback server on the already-bound listener go func() { - if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { + if err := server.Serve(listener); err != nil && err != http.ErrServerClosed { resultCh <- callbackResult{err: fmt.Errorf("callback server error: %w", err)} } }() // Open browser if err := browser.Open(redirectURL); err != nil { - fmt.Printf("Could not open browser automatically.\nOpen this URL in your browser:\n\n %s\n\n", redirectURL) + fmt.Fprintf(os.Stderr, "Could not open browser automatically.\nOpen this URL in your browser:\n\n %s\n\n", redirectURL) } // Wait for callback (with timeout) diff --git a/internal/config/auth_config.go b/internal/config/auth_config.go index 2a83016..f695895 100644 --- a/internal/config/auth_config.go +++ b/internal/config/auth_config.go @@ -124,11 +124,12 @@ func RemoveAccount(did string) error { return SaveAccounts(filtered) } -// SessionFilePath returns the file path for a session, using a sanitized DID as filename. +// SessionFilePath returns the file path for a session, using a sanitized DID +// as filename. DID syntax already forbids path separators; sanitizeFilename is +// defense in depth. For did:plc (the common case) the result is identical to +// the historical colon-to-underscore mapping. func SessionFilePath(did string) string { - // Sanitize DID for use as filename: replace colons with underscores - safe := strings.ReplaceAll(did, ":", "_") - return filepath.Join(SessionsDir(), safe+".json") + return filepath.Join(SessionsDir(), sanitizeFilename(did)+".json") } // AuthRequestFilePath returns the path for an in-flight auth request by state. @@ -151,16 +152,6 @@ func sanitizeFilename(s string) string { }, s) } -// ValidateToken checks if a TNG_TOKEN env var is set (for CI/script usage). -func TokenFromEnv() string { - return os.Getenv("TNG_TOKEN") -} - -// FormatHandle returns just the handle portion (without any at-sign prefix). -func FormatHandle(handle string) string { - return strings.TrimPrefix(handle, "@") -} - // FormatDID returns a shortened DID for display. func FormatDID(did string) string { if len(did) > 24 { diff --git a/internal/config/auth_config_test.go b/internal/config/auth_config_test.go index 8236d58..6bbd741 100644 --- a/internal/config/auth_config_test.go +++ b/internal/config/auth_config_test.go @@ -5,23 +5,6 @@ import ( "testing" ) -func TestFormatHandle(t *testing.T) { - tests := []struct { - input string - want string - }{ - {"alice.bsky.social", "alice.bsky.social"}, - {"@alice.bsky.social", "alice.bsky.social"}, - {"", ""}, - } - - for _, tt := range tests { - if got := FormatHandle(tt.input); got != tt.want { - t.Errorf("FormatHandle(%q) = %q, want %q", tt.input, got, tt.want) - } - } -} - func TestFormatDID(t *testing.T) { tests := []struct { input string diff --git a/internal/config/config.go b/internal/config/config.go index 71f896d..0525288 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -8,14 +8,6 @@ import ( // Config holds user preferences for the CLI. type Config struct { - // Editor override - Editor string `yaml:"editor,omitempty"` - // Pager override - Pager string `yaml:"pager,omitempty"` - // Browser override - Browser string `yaml:"browser,omitempty"` - // Git protocol: ssh or https - GitProtocol string `yaml:"git_protocol,omitempty"` // Default knot for repo creation DefaultKnot string `yaml:"default_knot,omitempty"` // API host override (default: https://tangled.sh) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 95d2937..e10003b 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -16,8 +16,8 @@ func TestLoad_DefaultsWhenMissing(t *testing.T) { if cfg == nil { t.Fatal("expected non-nil config") } - if cfg.Editor != "" { - t.Errorf("editor = %q, want empty", cfg.Editor) + if cfg.DefaultKnot != "" { + t.Errorf("default_knot = %q, want empty", cfg.DefaultKnot) } } @@ -25,9 +25,7 @@ func TestLoad_ParsesYAML(t *testing.T) { dir := t.TempDir() t.Setenv("TNG_CONFIG_DIR", dir) - content := `editor: vim -git_protocol: ssh -default_knot: knot.example.com + content := `default_knot: knot.example.com api_host: https://custom.tangled.sh ` if err := os.WriteFile(filepath.Join(dir, "config.yml"), []byte(content), 0o644); err != nil { @@ -38,12 +36,6 @@ api_host: https://custom.tangled.sh if err != nil { t.Fatalf("Load: %v", err) } - if cfg.Editor != "vim" { - t.Errorf("editor = %q, want 'vim'", cfg.Editor) - } - if cfg.GitProtocol != "ssh" { - t.Errorf("git_protocol = %q, want 'ssh'", cfg.GitProtocol) - } if cfg.DefaultKnot != "knot.example.com" { t.Errorf("default_knot = %q", cfg.DefaultKnot) } @@ -73,7 +65,6 @@ func TestSaveAndLoad(t *testing.T) { t.Setenv("TNG_CONFIG_DIR", t.TempDir()) cfg := &Config{ - Editor: "nvim", DefaultKnot: "knot.test.com", } if err := cfg.Save(); err != nil { @@ -84,9 +75,6 @@ func TestSaveAndLoad(t *testing.T) { if err != nil { t.Fatalf("Load: %v", err) } - if loaded.Editor != "nvim" { - t.Errorf("editor = %q, want 'nvim'", loaded.Editor) - } if loaded.DefaultKnot != "knot.test.com" { t.Errorf("default_knot = %q, want 'knot.test.com'", loaded.DefaultKnot) } diff --git a/internal/tngcmd/cmd.go b/internal/tngcmd/cmd.go index a9c8636..6d7d407 100644 --- a/internal/tngcmd/cmd.go +++ b/internal/tngcmd/cmd.go @@ -6,6 +6,7 @@ import ( "tangled.org/eric.wien/tng-cli/pkg/cmd/factory" "tangled.org/eric.wien/tng-cli/pkg/cmd/root" + "tangled.org/eric.wien/tng-cli/pkg/cmdutil" ) const ( @@ -21,11 +22,17 @@ func Main() int { ctx := context.Background() if err := rootCmd.ExecuteContext(ctx); err != nil { - if isQuiet(err) { - return exitError + code := exitError + switch { + case cmdutil.IsAuthError(err): + code = exitAuth + case cmdutil.IsCancelError(err): + code = exitCancel } - fmt.Fprintf(f.IOStreams.ErrOut, "Error: %s\n", err) - return exitError + if !isQuiet(err) { + fmt.Fprintf(f.IOStreams.ErrOut, "Error: %s\n", err) + } + return code } return exitOK diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 4023e8a..0dbd044 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -47,10 +47,12 @@ func loginRun(cmd *cobra.Command, opts *Options) error { if !opts.IO.IsInteractive() { return fmt.Errorf("--handle is required in non-interactive mode") } - fmt.Fprint(opts.IO.Out, "Enter your handle or DID (e.g., alice.bsky.social): ") - if _, err := fmt.Fscanln(opts.IO.In, &handle); err != nil { + fmt.Fprint(opts.IO.ErrOut, "Enter your handle or DID (e.g., alice.bsky.social): ") + h, err := cmdutil.ReadLine(opts.IO.In) + if err != nil { return fmt.Errorf("failed to read handle: %w", err) } + handle = h } if handle == "" { diff --git a/pkg/cmd/factory/default.go b/pkg/cmd/factory/default.go index 2044b80..45df356 100644 --- a/pkg/cmd/factory/default.go +++ b/pkg/cmd/factory/default.go @@ -58,11 +58,6 @@ func New() *cmdutil.Factory { return cachedClient, clientErr } - // Lazy remotes resolution - f.Remotes = func() (tngctx.Remotes, error) { - return tngctx.ResolveTangledRemotes(gc) - } - // Lazy base repo resolution f.BaseRepo = func() (*gitpkg.TangledRepo, error) { return tngctx.ResolveBaseRepo(gc) diff --git a/pkg/cmd/issue/list/list.go b/pkg/cmd/issue/list/list.go index 4132659..453bd0b 100644 --- a/pkg/cmd/issue/list/list.go +++ b/pkg/cmd/issue/list/list.go @@ -73,14 +73,25 @@ func listRun(cmd *cobra.Command, opts *Options) error { return err } - // Optionally resolve to AT URI for filtering + // Resolve the repo filter. An explicit --repo that can't be resolved is an + // error; a repo auto-detected from git remotes only warns, since listing + // everything is still useful. var repoURI string if repoRef != "" { - if owner, name, err := cmdutil.ParseOwnerRepo(repoRef); err == nil { - if resolved, err := client.ResolveRepo(cmd.Context(), owner, name); err == nil { + owner, name, err := cmdutil.ParseOwnerRepo(repoRef) + if err == nil { + var resolved *api.ResolvedRepo + resolved, err = client.ResolveRepo(cmd.Context(), owner, name) + if err == nil { repoURI = resolved.URI } } + if err != nil { + if opts.Repo != "" { + return fmt.Errorf("failed to resolve %s: %w", repoRef, err) + } + fmt.Fprintf(opts.IO.ErrOut, "warning: could not resolve %s (%v); listing all your issues\n", repoRef, err) + } } issues, err := client.ListOwnIssues(cmd.Context(), repoURI) diff --git a/pkg/cmd/pr/list/list.go b/pkg/cmd/pr/list/list.go index 4ee5d0a..512c629 100644 --- a/pkg/cmd/pr/list/list.go +++ b/pkg/cmd/pr/list/list.go @@ -72,13 +72,25 @@ func listRun(cmd *cobra.Command, opts *Options) error { return err } + // Resolve the repo filter. An explicit --repo that can't be resolved is an + // error; a repo auto-detected from git remotes only warns, since listing + // everything is still useful. var repoURI string if repoRef != "" { - if owner, name, err := cmdutil.ParseOwnerRepo(repoRef); err == nil { - if resolved, err := client.ResolveRepo(cmd.Context(), owner, name); err == nil { + owner, name, err := cmdutil.ParseOwnerRepo(repoRef) + if err == nil { + var resolved *api.ResolvedRepo + resolved, err = client.ResolveRepo(cmd.Context(), owner, name) + if err == nil { repoURI = resolved.URI } } + if err != nil { + if opts.Repo != "" { + return fmt.Errorf("failed to resolve %s: %w", repoRef, err) + } + fmt.Fprintf(opts.IO.ErrOut, "warning: could not resolve %s (%v); listing all your pull requests\n", repoRef, err) + } } pulls, err := client.ListOwnPulls(cmd.Context(), repoURI) diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go index 0827bca..66247be 100644 --- a/pkg/cmd/pr/merge/merge.go +++ b/pkg/cmd/pr/merge/merge.go @@ -14,6 +14,7 @@ import ( type Options struct { IO *iostreams.IOStreams + GitClient *gitpkg.Client ApiClient func() (*api.Client, error) BaseRepo func() (*gitpkg.TangledRepo, error) Rkey string @@ -23,6 +24,7 @@ type Options struct { func NewCmdMerge(f *cmdutil.Factory) *cobra.Command { opts := &Options{ IO: f.IOStreams, + GitClient: f.GitClient, ApiClient: f.ApiClient, BaseRepo: f.BaseRepo, } @@ -89,12 +91,17 @@ func mergeRun(cmd *cobra.Command, opts *Options) error { if err := json.Unmarshal(targetRec.Value, &repoInfo); err != nil { return fmt.Errorf("failed to parse repository record: %w", err) } + if repoInfo.Knot == "" { + return fmt.Errorf("target repository record has no knot assigned") + } // Identify the merge commit's author from the active account. authorName := client.DID().String() if acct, _ := config.ActiveAccount(); acct != nil && acct.Handle != "" { authorName = acct.Handle } + // Best effort: use the local git identity for the commit's email. + authorEmail, _ := opts.GitClient.Config("user.email") knotURL := "https://" + repoInfo.Knot input := &api.MergeInput{ @@ -103,6 +110,7 @@ func mergeRun(cmd *cobra.Command, opts *Options) error { Branch: pr.TargetRef, Patch: patch, AuthorName: authorName, + AuthorEmail: authorEmail, CommitMessage: fmt.Sprintf("Merge pull request: %s", pr.Title), } diff --git a/pkg/cmd/repo/clone/clone.go b/pkg/cmd/repo/clone/clone.go index f628823..6bd6a20 100644 --- a/pkg/cmd/repo/clone/clone.go +++ b/pkg/cmd/repo/clone/clone.go @@ -80,7 +80,7 @@ func cloneRun(cmd *cobra.Command, opts *Options) error { // Strip any protocol prefix from knot for SSH URL knotHost = stripProtocol(knotHost) - cloneURL := fmt.Sprintf("git@%s:%s/%s", knotHost, owner, repoName) + cloneURL := gitpkg.SSHCloneURL(knotHost, owner, repoName) return doClone(opts, cloneURL, cs) } diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index ab72fef..cf9daf6 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -98,7 +98,6 @@ func createRun(cmd *cobra.Command, opts *Options) error { cs.SuccessIcon(), cs.Bold(opts.Name), knotURL) if opts.Clone { - // Build SSH clone URL: git@:/ knotHost := strings.TrimPrefix(knotURL, "https://") knotHost = strings.TrimPrefix(knotHost, "http://") knotHost = strings.TrimRight(knotHost, "/") @@ -109,7 +108,7 @@ func createRun(cmd *cobra.Command, opts *Options) error { owner = account.Handle } - cloneURL := fmt.Sprintf("git@%s:%s/%s", knotHost, owner, opts.Name) + cloneURL := gitpkg.SSHCloneURL(knotHost, owner, opts.Name) fmt.Fprintf(opts.IO.ErrOut, "Cloning %s...\n", cloneURL) if err := opts.GitClient.Clone(cloneURL, ""); err != nil { diff --git a/pkg/cmd/repo/delete/delete.go b/pkg/cmd/repo/delete/delete.go index 001a948..51fdc2e 100644 --- a/pkg/cmd/repo/delete/delete.go +++ b/pkg/cmd/repo/delete/delete.go @@ -55,14 +55,15 @@ func deleteRun(cmd *cobra.Command, opts *Options) error { if !opts.IO.IsInteractive() { return fmt.Errorf("cannot confirm deletion non-interactively; use --yes to confirm") } - fmt.Fprintf(opts.IO.ErrOut, "%s This will permanently delete %s. Type the repo name to confirm: ", - cs.WarningIcon(), cs.Bold(opts.Repo)) + fmt.Fprintf(opts.IO.ErrOut, "%s This will permanently delete %s. Type %s to confirm: ", + cs.WarningIcon(), cs.Bold(opts.Repo), cs.Bold(name)) - var input string - if _, err := fmt.Fscanln(opts.IO.In, &input); err != nil { + input, err := cmdutil.ReadLine(opts.IO.In) + if err != nil { return fmt.Errorf("failed to read confirmation: %w", err) } - if input != name { + // Accept the bare name or the full owner/name shown in the prompt. + if input != name && input != opts.Repo { return fmt.Errorf("confirmation did not match; aborting") } } diff --git a/pkg/cmdutil/factory.go b/pkg/cmdutil/factory.go index e28c23f..9bf00e9 100644 --- a/pkg/cmdutil/factory.go +++ b/pkg/cmdutil/factory.go @@ -2,7 +2,6 @@ package cmdutil import ( "tangled.org/eric.wien/tng-cli/api" - "tangled.org/eric.wien/tng-cli/context" gitpkg "tangled.org/eric.wien/tng-cli/git" "tangled.org/eric.wien/tng-cli/internal/config" "tangled.org/eric.wien/tng-cli/pkg/iostreams" @@ -18,7 +17,6 @@ type Factory struct { // Lazy-initialized dependencies Config func() (*config.Config, error) ApiClient func() (*api.Client, error) - Remotes func() (context.Remotes, error) BaseRepo func() (*gitpkg.TangledRepo, error) Branch func() (string, error) } diff --git a/pkg/cmdutil/prompt.go b/pkg/cmdutil/prompt.go new file mode 100644 index 0000000..0bf243a --- /dev/null +++ b/pkg/cmdutil/prompt.go @@ -0,0 +1,17 @@ +package cmdutil + +import ( + "bufio" + "io" + "strings" +) + +// ReadLine reads a single line of input and trims surrounding whitespace. +// Unlike fmt.Fscanln it returns the whole line, not just the first token. +func ReadLine(r io.Reader) (string, error) { + line, err := bufio.NewReader(r).ReadString('\n') + if err != nil && err != io.EOF { + return "", err + } + return strings.TrimSpace(line), nil +} diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index d9b456b..62a8b18 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -23,7 +23,6 @@ type IOStreams struct { func System() *IOStreams { stdoutIsTTY := isTerminal(os.Stdout) - stderrIsTTY := isTerminal(os.Stderr) stdinIsTTY := isTerminal(os.Stdin) ios := &IOStreams{ @@ -31,7 +30,7 @@ func System() *IOStreams { Out: os.Stdout, ErrOut: os.Stderr, - colorEnabled: envColorEnabled() && stdoutIsTTY, + colorEnabled: envColorEnabled() && (stdoutIsTTY || envColorForced()), isInteractive: stdinIsTTY && stdoutIsTTY, } @@ -41,8 +40,6 @@ func System() *IOStreams { } } - _ = stderrIsTTY - colorTerm := os.Getenv("COLORTERM") ios.is256Color = strings.Contains(colorTerm, "256color") || strings.Contains(os.Getenv("TERM"), "256color") ios.isTrueColor = colorTerm == "truecolor" || colorTerm == "24bit" @@ -81,16 +78,16 @@ func isTerminal(f *os.File) bool { } func envColorEnabled() bool { - if os.Getenv("NO_COLOR") != "" { - return false - } + return os.Getenv("NO_COLOR") == "" +} + +// envColorForced reports whether the user asked for color even when stdout +// isn't a TTY. NO_COLOR still wins. +func envColorForced() bool { if os.Getenv("TNG_FORCE_TTY") != "" { return true } - if os.Getenv("CLICOLOR_FORCE") != "" && os.Getenv("CLICOLOR_FORCE") != "0" { - return true - } - return true + return os.Getenv("CLICOLOR_FORCE") != "" && os.Getenv("CLICOLOR_FORCE") != "0" } // ColorScheme provides ANSI color formatting.