diff --git a/docs/astro.config.mjs b/docs/astro.config.mjs index 09a1ac1..5e0329c 100644 --- a/docs/astro.config.mjs +++ b/docs/astro.config.mjs @@ -41,6 +41,7 @@ export default defineConfig({ { label: 'Memberships', translations: { 'zh-CN': '成员关系' }, slug: 'commands/memberships' }, { label: 'Versions', translations: { 'zh-CN': '版本' }, slug: 'commands/versions' }, { label: 'Files', translations: { 'zh-CN': '项目文件' }, slug: 'commands/files' }, + { label: 'Attachments', translations: { 'zh-CN': '附件' }, slug: 'commands/attachments' }, { label: 'Time Entries', translations: { 'zh-CN': '工时记录' }, slug: 'commands/time' }, { label: 'Users', translations: { 'zh-CN': '用户' }, slug: 'commands/users' }, { label: 'My Account', translations: { 'zh-CN': '我的账户' }, slug: 'commands/my_account' }, diff --git a/docs/src/content/docs/commands/attachments.mdx b/docs/src/content/docs/commands/attachments.mdx new file mode 100644 index 0000000..efba7ac --- /dev/null +++ b/docs/src/content/docs/commands/attachments.mdx @@ -0,0 +1,106 @@ +--- +title: Attachments +description: Inspect attachment metadata and download attachment files without curl or manual API-key handling. +--- + +import { CardGrid, LinkCard, Tabs, TabItem, Aside, Badge } from '@astrojs/starlight/components'; + +The `attachments` command (aliases `attachment`, `a`) surfaces Redmine's [Attachments API](https://www.redmine.org/projects/redmine/wiki/Rest_api#Attachments) so you can inspect an attachment's metadata and download its bytes. Downloads reuse the active profile's server and API key, streaming straight to disk - no `curl`, no manually extracted key. + + + + + + + + +## get + +```bash +redmine attachments get [flags] +``` + +Aliases: `show`, `view`, `metadata`. Fetches the attachment's ID, filename, size, content type, description, author, and creation time via `GET /attachments/:id.json`. Table output also shows the download URL (when present); `json` returns every field, including `content_url`. + +| Flag | Description | +|------|-------------| +| `-o, --output` | Output format: `table`, `json`, `csv` | + + + + ```bash + redmine attachments get 42 + ``` + + + ```bash + redmine attachments get 42 -o json + ``` + + + ```bash + redmine attachments get 42 -o csv + ``` + + + +## download + +```bash +redmine attachments download [flags] +``` + +Aliases: `dl`, `get-file`. Streams the attachment's raw bytes to disk (or stdout), authenticating with the active profile. The response is streamed, never buffered entirely in memory, so large files are safe. + +| Flag | Description | +|------|-------------| +| `-d, --dir` | Directory to save into, using the attachment's real filename (created if missing) | +| `--path` | Exact destination file path. Use `-` to stream to stdout | +| `-o, --output` | Confirmation format: `json` prints a result object; otherwise a human line is written to stderr | + +With no destination flag, the file is saved as `./` in the current directory. + + + + + + ```bash + # Saves ./ + redmine attachments download 42 + ``` + + + ```bash + redmine attachments download 42 --dir ./downloads + ``` + + + ```bash + redmine attachments download 42 --path ./logo.png + ``` + + + ```bash + redmine attachments download 42 --path - | file - + ``` + + + ```bash + redmine attachments download 42 --dir ./downloads -o json + ``` + + + + diff --git a/docs/src/content/docs/commands/issues.mdx b/docs/src/content/docs/commands/issues.mdx index 785da4c..80deb28 100644 --- a/docs/src/content/docs/commands/issues.mdx +++ b/docs/src/content/docs/commands/issues.mdx @@ -91,11 +91,34 @@ Aliases: `show`, `view`. | Flag | Description | |------|-------------| -| `--include` | Include related data: `journals`, `children`, `relations` | -| `--journals` | Shorthand for `--include journals` | -| `--children` | Shorthand for `--include children` | -| `--relations` | Shorthand for `--include relations` | -| `-o, --output` | Output format | +| `--include` | Include related data: `journals`, `children`, `relations`, `attachments` | +| `--journals` | Shorthand for `--include journals` | +| `--children` | Shorthand for `--include children` | +| `--relations` | Shorthand for `--include relations` | +| `--attachments` | List the issue's attachments (id, filename, size, content type) | +| `--download-attachments` | Download every attachment of the issue into the given directory | +| `-o, --output` | Output format | + + + + ```bash + # List attachment IDs so you can download one + redmine issues get 123 --attachments + ``` + + + ```bash + # Pull every attachment into ./issue-123 + redmine issues get 123 --download-attachments ./issue-123 + ``` + + + + ## create diff --git a/docs/src/content/docs/zh-cn/commands/attachments.mdx b/docs/src/content/docs/zh-cn/commands/attachments.mdx new file mode 100644 index 0000000..4421ace --- /dev/null +++ b/docs/src/content/docs/zh-cn/commands/attachments.mdx @@ -0,0 +1,105 @@ +--- +title: 附件 +description: 查看附件元数据并下载附件文件，无需 curl，也无需手动处理 API 密钥。 +--- + +import { CardGrid, LinkCard, Tabs, TabItem, Aside, Badge } from '@astrojs/starlight/components'; + +`attachments` 命令（别名 `attachment`、`a`）封装了 Redmine 的[附件 API](https://www.redmine.org/projects/redmine/wiki/Rest_api#Attachments)，让你既能查看附件的元数据，也能下载其字节内容。下载会复用当前 profile 的服务器地址和 API 密钥，并直接流式写入磁盘 - 无需 `curl`，也无需手动提取密钥。 + + + + + + + + +## get + +```bash +redmine attachments get [flags] +``` + +别名：`show`、`view`、`metadata`。通过 `GET /attachments/:id.json` 获取附件的 ID、文件名、大小、内容类型、描述、作者和创建时间。表格输出还会显示下载 URL（若存在）；`json` 会返回所有字段，包括 `content_url`。 + +| 标志 | 描述 | +|------|------| +| `-o, --output` | 输出格式：`table`、`json`、`csv` | + + + + ```bash + redmine attachments get 42 + ``` + + + ```bash + redmine attachments get 42 -o json + ``` + + + ```bash + redmine attachments get 42 -o csv + ``` + + + +## download + +```bash +redmine attachments download [flags] +``` + +别名：`dl`、`get-file`。使用当前 profile 进行认证，将附件的原始字节流式写入磁盘（或 stdout）。响应是流式处理的，不会整体缓存在内存中，因此下载大文件也很安全。 + +| 标志 | 描述 | +|------|------| +| `-d, --dir` | 保存到的目录，使用附件的真实文件名（目录不存在时自动创建） | +| `--path` | 精确的目标文件路径。使用 `-` 可流式输出到 stdout | +| `-o, --output` | 确认信息的格式：`json` 打印结果对象；否则向 stderr 写入一行可读信息 | + +若不指定目标标志，文件会以 `./<文件名>` 保存到当前目录。 + + + + + + ```bash + # 保存为 ./<文件名> + redmine attachments download 42 + ``` + + + ```bash + redmine attachments download 42 --dir ./downloads + ``` + + + ```bash + redmine attachments download 42 --path ./logo.png + ``` + + + ```bash + redmine attachments download 42 --path - | file - + ``` + + + ```bash + redmine attachments download 42 --dir ./downloads -o json + ``` + + + + diff --git a/docs/src/content/docs/zh-cn/commands/issues.mdx b/docs/src/content/docs/zh-cn/commands/issues.mdx index d1d7071..e96f152 100644 --- a/docs/src/content/docs/zh-cn/commands/issues.mdx +++ b/docs/src/content/docs/zh-cn/commands/issues.mdx @@ -91,11 +91,34 @@ redmine issues get [flags] | 标志 | 描述 | |------|------| -| `--include` | 包含关联数据：`journals`、`children`、`relations` | -| `--journals` | `--include journals` 的简写 | -| `--children` | `--include children` 的简写 | -| `--relations` | `--include relations` 的简写 | -| `-o, --output` | 输出格式 | +| `--include` | 包含关联数据：`journals`、`children`、`relations`、`attachments` | +| `--journals` | `--include journals` 的简写 | +| `--children` | `--include children` 的简写 | +| `--relations` | `--include relations` 的简写 | +| `--attachments` | 列出工单的附件（id、文件名、大小、内容类型） | +| `--download-attachments` | 将工单的所有附件下载到指定目录 | +| `-o, --output` | 输出格式 | + + + + ```bash + # 列出附件 ID，以便下载某个附件 + redmine issues get 123 --attachments + ``` + + + ```bash + # 将所有附件下载到 ./issue-123 + redmine issues get 123 --download-attachments ./issue-123 + ``` + + + + ## create diff --git a/e2e/attachments_test.go b/e2e/attachments_test.go new file mode 100644 index 0000000..f62f742 --- /dev/null +++ b/e2e/attachments_test.go @@ -0,0 +1,169 @@ +//go:build e2e + +package e2e + +import ( + "bytes" + "os" + "path/filepath" + "strconv" + "testing" + "time" +) + +// pngFixture returns a small payload that begins with the real PNG signature +// and contains NUL and high bytes, so a byte-identical round-trip proves +// binary integrity (no text decoding / newline mangling). +func pngFixture(uniq string) []byte { + return []byte("\x89PNG\r\n\x1a\n\x00\x01\x02\x03\xff\xfe redmine-cli e2e " + uniq + "\n") +} + +// createIssueWithAttachment creates an issue carrying a single attachment and +// returns the issue ID, the on-disk filename, and the exact bytes uploaded. +func createIssueWithAttachment(t *testing.T, r *cliRunner, projectIdentifier string) (issueID int, filename string, payload []byte) { + t.Helper() + uniq := strconv.FormatInt(time.Now().UnixNano(), 36) + filename = "logo-" + uniq + ".png" + payload = pngFixture(uniq) + path := filepath.Join(t.TempDir(), filename) + if err := os.WriteFile(path, payload, 0o600); err != nil { + t.Fatalf("write attach file: %v", err) + } + + var created struct { + ID int `json:"id"` + } + r.runJSON(t, &created, "issues", "create", + "--project", projectIdentifier, + "--tracker", firstTrackerName(t, r), + "--subject", "E2E attachment "+uniq, + "--attach", path) + if created.ID == 0 { + t.Fatal("issues create returned no ID") + } + return created.ID, filename, payload +} + +type attachmentEntry struct { + ID int `json:"id"` + Filename string `json:"filename"` + Filesize int64 `json:"filesize"` + ContentType string `json:"content_type"` +} + +// findIssueAttachmentID discovers the attachment ID via the new +// `issues get --attachments` flag, asserting the attachment surfaces in the +// typed JSON output. +func findIssueAttachmentID(t *testing.T, r *cliRunner, issueID int, filename string) attachmentEntry { + t.Helper() + var resp struct { + Attachments []attachmentEntry `json:"attachments"` + } + r.runJSON(t, &resp, "issues", "get", issueIDArg(issueID), "--attachments") + for _, att := range resp.Attachments { + if att.Filename == filename { + return att + } + } + t.Fatalf("attachment %q not surfaced by `issues get --attachments`: %+v", filename, resp.Attachments) + return attachmentEntry{} +} + +// TestAttachments_Lifecycle exercises the full discover -> metadata -> download +// flow and asserts binary integrity of the downloaded file. +func TestAttachments_Lifecycle(t *testing.T) { + requireE2E(t) + r := newCLIRunner(t, e2eBaseURL(), e2eAPIKey()) + proj := createTestProject(t, r) + + issueID, filename, payload := createIssueWithAttachment(t, r, proj.Identifier) + + att := findIssueAttachmentID(t, r, issueID, filename) + if att.ID == 0 { + t.Fatal("discovered attachment has zero ID") + } + if att.Filesize != int64(len(payload)) { + t.Errorf("filesize = %d, want %d", att.Filesize, len(payload)) + } + + // Metadata via `attachments get`. + var meta attachmentEntry + r.runJSON(t, &meta, "attachments", "get", strconv.Itoa(att.ID)) + if meta.Filename != filename || meta.Filesize != int64(len(payload)) { + t.Errorf("attachments get = %+v, want filename=%q size=%d", meta, filename, len(payload)) + } + + // Download into a directory using the real filename. + dir := t.TempDir() + var dl struct { + ID int `json:"id"` + Filename string `json:"filename"` + Path string `json:"path"` + Bytes int64 `json:"bytes"` + } + r.runJSON(t, &dl, "attachments", "download", strconv.Itoa(att.ID), "--dir", dir) + if dl.Path != filepath.Join(dir, filename) { + t.Errorf("download path = %q, want %q", dl.Path, filepath.Join(dir, filename)) + } + if dl.Bytes != int64(len(payload)) { + t.Errorf("downloaded bytes = %d, want %d", dl.Bytes, len(payload)) + } + + got, err := os.ReadFile(dl.Path) + if err != nil { + t.Fatalf("read downloaded file: %v", err) + } + if !bytes.Equal(got, payload) { + t.Fatalf("downloaded file not byte-identical to the uploaded PNG\nwant %v\ngot %v", payload, got) + } +} + +// TestAttachments_DownloadToStdout verifies `--path -` streams raw bytes to +// stdout without corruption, even though the runner injects --output json. +func TestAttachments_DownloadToStdout(t *testing.T) { + requireE2E(t) + r := newCLIRunner(t, e2eBaseURL(), e2eAPIKey()) + proj := createTestProject(t, r) + + issueID, filename, payload := createIssueWithAttachment(t, r, proj.Identifier) + att := findIssueAttachmentID(t, r, issueID, filename) + + stdout, _, err := r.runRaw("attachments", "download", strconv.Itoa(att.ID), "--path", "-") + if err != nil { + t.Fatalf("download to stdout failed: %v", err) + } + if !bytes.Equal(stdout, payload) { + t.Fatalf("stdout not byte-identical to the uploaded PNG\nwant %v\ngot %v", payload, stdout) + } +} + +// TestAttachments_IssueDownloadAll covers `issues get --download-attachments`. +func TestAttachments_IssueDownloadAll(t *testing.T) { + requireE2E(t) + r := newCLIRunner(t, e2eBaseURL(), e2eAPIKey()) + proj := createTestProject(t, r) + + issueID, filename, payload := createIssueWithAttachment(t, r, proj.Identifier) + + dir := filepath.Join(t.TempDir(), "issue-attachments") + // The issue JSON still lands on stdout; the download is a stderr-reported + // side effect, so we only need a successful exit here. + r.run(t, "issues", "get", issueIDArg(issueID), "--download-attachments", dir) + + got, err := os.ReadFile(filepath.Join(dir, filename)) + if err != nil { + t.Fatalf("expected downloaded attachment in %s: %v", dir, err) + } + if !bytes.Equal(got, payload) { + t.Fatal("issue-downloaded attachment not byte-identical to source") + } +} + +// TestAttachments_GetMissing asserts a clear error for an unknown attachment. +func TestAttachments_GetMissing(t *testing.T) { + requireE2E(t) + r := newCLIRunner(t, e2eBaseURL(), e2eAPIKey()) + + stdout, _ := r.runExpectError(t, "attachments", "get", "999999999") + requireErrorEnvelopeMessage(t, stdout) +} diff --git a/e2e/issues_test.go b/e2e/issues_test.go index 3d754cf..d8dd358 100644 --- a/e2e/issues_test.go +++ b/e2e/issues_test.go @@ -160,9 +160,9 @@ func TestIssues_Assign(t *testing.T) { } // TestIssues_Attachment verifies the multipart upload path: create an issue -// with --attach, then fetch via the raw api with ?include=attachments (the -// typed Issue model doesn't surface attachments) and assert the file is -// present. +// with --attach, then fetch via the raw api with ?include=attachments and +// assert the file is present. (The typed `issues get --attachments` flow is +// covered separately in attachments_test.go.) func TestIssues_Attachment(t *testing.T) { requireE2E(t) r := newCLIRunner(t, e2eBaseURL(), e2eAPIKey()) diff --git a/e2e/testdata/mcp_tools_readonly.golden.json b/e2e/testdata/mcp_tools_readonly.golden.json index 6b7855e..62ac551 100644 --- a/e2e/testdata/mcp_tools_readonly.golden.json +++ b/e2e/testdata/mcp_tools_readonly.golden.json @@ -1,4 +1,8 @@ [ + { + "name": "get_attachment", + "description": "Fetch metadata for a Redmine attachment by ID: filename, size, content type, description, author, and download URL. Discover attachment IDs via get_issue with includes=[\"attachments\"]." + }, { "name": "get_custom_field", "description": "Fetch a single custom field definition by ID. Admin-only endpoint." diff --git a/internal/api/attachments.go b/internal/api/attachments.go index 626e1fc..25f5dc3 100644 --- a/internal/api/attachments.go +++ b/internal/api/attachments.go @@ -7,16 +7,81 @@ import ( "io" "net/http" "net/url" + "strconv" + "strings" "time" "github.com/aarondpn/redmine-cli/v2/internal/debug" + "github.com/aarondpn/redmine-cli/v2/internal/models" ) -// AttachmentService handles file upload API calls. +// AttachmentService handles attachment metadata, upload, and download API +// calls. type AttachmentService struct { client *Client } +// Get retrieves the metadata for a single attachment by ID via +// GET /attachments/:id.json (filename, content type, size, author, +// content_url, ...). +func (s *AttachmentService) Get(ctx context.Context, id int) (*models.Attachment, error) { + var resp struct { + Attachment models.Attachment `json:"attachment"` + } + if err := s.client.Get(ctx, fmt.Sprintf("/attachments/%d.json", id), nil, &resp); err != nil { + return nil, err + } + return &resp.Attachment, nil +} + +// Download streams the raw bytes of an attachment to w without buffering the +// whole file in memory. It returns the number of bytes written. +// +// The download URL is resolved from the attachment's content_url when that URL +// is hosted on the configured Redmine server (so subdirectory installs keep +// working), and otherwise falls back to the canonical +// /attachments/download/:id/:filename path built from the base URL. This guard +// ensures the API key (added to every request by authTransport) is never sent +// to a host other than the configured server. +func (s *AttachmentService) Download(ctx context.Context, att *models.Attachment, w io.Writer) (int64, error) { + u := s.downloadURL(att) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil) + if err != nil { + return 0, err + } + + start := time.Now() + resp, err := s.client.httpClient.Do(req) + duration := time.Since(start) + if err != nil { + s.client.debugLog.Printf("HTTP %s %s -> error (%s): %v", req.Method, debug.ScrubURL(req.URL.String()), duration.Round(time.Millisecond), err) + return 0, fmt.Errorf("download request failed: %w", err) + } + defer resp.Body.Close() + + s.client.debugLog.Printf("HTTP %s %s -> %d (%s)", req.Method, debug.ScrubURL(req.URL.String()), resp.StatusCode, duration.Round(time.Millisecond)) + + if resp.StatusCode >= 400 { + return 0, parseErrorResponse(resp) + } + + n, err := io.Copy(w, resp.Body) + if err != nil { + return n, fmt.Errorf("streaming attachment %d: %w", att.ID, err) + } + return n, nil +} + +// downloadURL returns the absolute URL to stream the attachment bytes from. +// See Download for the host-matching rationale. +func (s *AttachmentService) downloadURL(att *models.Attachment) string { + if att.ContentURL != "" && strings.HasPrefix(att.ContentURL, s.client.baseURL+"/") { + return att.ContentURL + } + return s.client.baseURL + "/attachments/download/" + strconv.Itoa(att.ID) + "/" + url.PathEscape(att.Filename) +} + // Upload streams body (of known size) to Redmine's /uploads.json endpoint and // returns the upload token. filename is sent as a query parameter per the // Redmine REST docs so the server records the original name. diff --git a/internal/api/attachments_download_test.go b/internal/api/attachments_download_test.go new file mode 100644 index 0000000..901b08a --- /dev/null +++ b/internal/api/attachments_download_test.go @@ -0,0 +1,176 @@ +package api + +import ( + "bytes" + "context" + "errors" + "net/http" + "net/http/httptest" + "testing" + + "github.com/aarondpn/redmine-cli/v2/internal/models" +) + +func TestAttachmentGet_Success(t *testing.T) { + var gotPath string + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"attachment":{"id":7,"filename":"diagram.png","filesize":2048,"content_type":"image/png","description":"arch","content_url":"http://example/attachments/download/7/diagram.png","author":{"id":3,"name":"Ada"},"created_on":"2026-01-02T03:04:05Z"}}`)) + })) + defer ts.Close() + + c := newTestClient(ts) + c.Attachments = &AttachmentService{client: c} + + att, err := c.Attachments.Get(context.Background(), 7) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if gotPath != "/attachments/7.json" { + t.Errorf("path = %q, want /attachments/7.json", gotPath) + } + if att.ID != 7 || att.Filename != "diagram.png" || att.Filesize != 2048 { + t.Errorf("attachment = %+v, want id=7 filename=diagram.png size=2048", att) + } + if att.ContentType != "image/png" || att.Author.Name != "Ada" { + t.Errorf("attachment metadata = %+v, want content_type/image author Ada", att) + } +} + +func TestAttachmentGet_NotFound(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + })) + defer ts.Close() + + c := newTestClient(ts) + c.Attachments = &AttachmentService{client: c} + + _, err := c.Attachments.Get(context.Background(), 999) + var apiErr *APIError + if !errors.As(err, &apiErr) || !apiErr.IsNotFound() { + t.Fatalf("err = %v, want APIError 404", err) + } +} + +func TestAttachmentDownload_StreamsFromContentURL(t *testing.T) { + want := []byte("\x89PNG\r\n\x1a\n raw binary \x00\x01\x02 bytes") + var gotPath string + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + w.Header().Set("Content-Type", "image/png") + _, _ = w.Write(want) + })) + defer ts.Close() + + c := newTestClient(ts) + c.Attachments = &AttachmentService{client: c} + + att := &models.Attachment{ + ID: 7, + Filename: "diagram.png", + ContentURL: ts.URL + "/attachments/download/7/diagram.png", + } + + var buf bytes.Buffer + n, err := c.Attachments.Download(context.Background(), att, &buf) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if gotPath != "/attachments/download/7/diagram.png" { + t.Errorf("download path = %q, want /attachments/download/7/diagram.png", gotPath) + } + if n != int64(len(want)) { + t.Errorf("bytes = %d, want %d", n, len(want)) + } + if !bytes.Equal(buf.Bytes(), want) { + t.Errorf("body = %v, want %v (binary integrity)", buf.Bytes(), want) + } +} + +func TestAttachmentDownload_FallsBackToCanonicalPath(t *testing.T) { + want := []byte("plain content") + var gotPath string + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + _, _ = w.Write(want) + })) + defer ts.Close() + + c := newTestClient(ts) + c.Attachments = &AttachmentService{client: c} + + // No content_url: Download must build /attachments/download/:id/:filename + // against the base URL and URL-escape the filename. + att := &models.Attachment{ID: 9, Filename: "release notes.txt"} + + var buf bytes.Buffer + if _, err := c.Attachments.Download(context.Background(), att, &buf); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if gotPath != "/attachments/download/9/release notes.txt" { + t.Errorf("download path = %q, want decoded /attachments/download/9/release notes.txt", gotPath) + } + if !bytes.Equal(buf.Bytes(), want) { + t.Errorf("body = %q, want %q", buf.Bytes(), want) + } +} + +func TestAttachmentDownload_ErrorStatus(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + })) + defer ts.Close() + + c := newTestClient(ts) + c.Attachments = &AttachmentService{client: c} + + att := &models.Attachment{ID: 1, Filename: "x.bin", ContentURL: ts.URL + "/attachments/download/1/x.bin"} + _, err := c.Attachments.Download(context.Background(), att, &bytes.Buffer{}) + var apiErr *APIError + if !errors.As(err, &apiErr) || !apiErr.IsForbidden() { + t.Fatalf("err = %v, want APIError 403", err) + } +} + +// TestAttachmentDownloadURL exercises the host-matching guard that keeps the +// API key (added to every request by authTransport) from being sent to a host +// other than the configured Redmine server. +func TestAttachmentDownloadURL(t *testing.T) { + s := &AttachmentService{client: &Client{baseURL: "https://redmine.example.com/redmine"}} + + cases := []struct { + name string + att *models.Attachment + want string + }{ + { + name: "content_url on configured server is used verbatim", + att: &models.Attachment{ID: 1, Filename: "a.png", ContentURL: "https://redmine.example.com/redmine/attachments/download/1/a.png"}, + want: "https://redmine.example.com/redmine/attachments/download/1/a.png", + }, + { + name: "foreign content_url falls back to canonical base-URL path", + att: &models.Attachment{ID: 2, Filename: "b.png", ContentURL: "https://evil.example.net/steal"}, + want: "https://redmine.example.com/redmine/attachments/download/2/b.png", + }, + { + name: "empty content_url builds canonical path", + att: &models.Attachment{ID: 3, Filename: "c.png"}, + want: "https://redmine.example.com/redmine/attachments/download/3/c.png", + }, + { + name: "filename is URL-escaped in the built path", + att: &models.Attachment{ID: 4, Filename: "my file (1).png"}, + want: "https://redmine.example.com/redmine/attachments/download/4/my%20file%20%281%29.png", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := s.downloadURL(tc.att); got != tc.want { + t.Errorf("downloadURL = %q, want %q", got, tc.want) + } + }) + } +} diff --git a/internal/api/auth_transport_test.go b/internal/api/auth_transport_test.go new file mode 100644 index 0000000..aa7f35c --- /dev/null +++ b/internal/api/auth_transport_test.go @@ -0,0 +1,132 @@ +package api + +import ( + "bytes" + "context" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/aarondpn/redmine-cli/v2/internal/debug" + "github.com/aarondpn/redmine-cli/v2/internal/models" +) + +func hostOf(t *testing.T, raw string) string { + t.Helper() + u, err := url.Parse(raw) + if err != nil { + t.Fatalf("parse %q: %v", raw, err) + } + return u.Host +} + +func TestAuthTransport_AttachesKeyOnConfiguredHost(t *testing.T) { + var gotKey string + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotKey = r.Header.Get("X-Redmine-API-Key") + _, _ = w.Write([]byte("{}")) + })) + defer ts.Close() + + client := &http.Client{Transport: &authTransport{ + base: http.DefaultTransport, authMethod: "apikey", apiKey: "K", host: hostOf(t, ts.URL), + }} + req, _ := http.NewRequest(http.MethodGet, ts.URL, nil) + resp, err := client.Do(req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + resp.Body.Close() + if gotKey != "K" { + t.Errorf("X-Redmine-API-Key = %q, want K (configured host must receive auth)", gotKey) + } +} + +func TestAuthTransport_StripsKeyOnForeignHost(t *testing.T) { + var gotKey string + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotKey = r.Header.Get("X-Redmine-API-Key") + _, _ = w.Write([]byte("{}")) + })) + defer ts.Close() + + // Configured host is deliberately different from the server we call. + client := &http.Client{Transport: &authTransport{ + base: http.DefaultTransport, authMethod: "apikey", apiKey: "K", host: "redmine.internal:9999", + }} + req, _ := http.NewRequest(http.MethodGet, ts.URL, nil) + resp, err := client.Do(req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + resp.Body.Close() + if gotKey != "" { + t.Errorf("X-Redmine-API-Key = %q, want empty (must not leak to a foreign host)", gotKey) + } +} + +func TestAuthTransport_StripsBasicAuthOnForeignHost(t *testing.T) { + var gotUser string + var hadAuth bool + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotUser, _, hadAuth = r.BasicAuth() + _, _ = w.Write([]byte("{}")) + })) + defer ts.Close() + + client := &http.Client{Transport: &authTransport{ + base: http.DefaultTransport, authMethod: "basic", username: "u", password: "p", host: "redmine.internal:9999", + }} + req, _ := http.NewRequest(http.MethodGet, ts.URL, nil) + resp, err := client.Do(req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + resp.Body.Close() + if hadAuth { + t.Errorf("basic auth leaked to foreign host (user=%q)", gotUser) + } +} + +// TestAttachmentDownload_CrossHostRedirectDoesNotLeakKey reproduces the +// realistic case where an attachment's content_url 302-redirects to external +// object storage. The API key must reach the configured server but never the +// off-host redirect target. +func TestAttachmentDownload_CrossHostRedirectDoesNotLeakKey(t *testing.T) { + var evilGotKey string + evil := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + evilGotKey = r.Header.Get("X-Redmine-API-Key") + _, _ = w.Write([]byte("payload")) + })) + defer evil.Close() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("X-Redmine-API-Key") != "SECRET" { + t.Errorf("configured host missing API key: %q", r.Header.Get("X-Redmine-API-Key")) + } + http.Redirect(w, r, evil.URL+"/object-storage", http.StatusFound) + })) + defer srv.Close() + + c := &Client{ + httpClient: &http.Client{Transport: &authTransport{ + base: http.DefaultTransport, authMethod: "apikey", apiKey: "SECRET", host: hostOf(t, srv.URL), + }}, + baseURL: srv.URL, + debugLog: debug.New(nil), + } + c.Attachments = &AttachmentService{client: c} + + att := &models.Attachment{ID: 1, Filename: "x.bin", ContentURL: srv.URL + "/attachments/download/1/x.bin"} + var buf bytes.Buffer + if _, err := c.Attachments.Download(context.Background(), att, &buf); err != nil { + t.Fatalf("download failed: %v", err) + } + if evilGotKey != "" { + t.Fatalf("API key leaked to off-host redirect target: %q", evilGotKey) + } + if buf.String() != "payload" { + t.Errorf("body = %q, want payload (redirect must still be followed)", buf.String()) + } +} diff --git a/internal/api/client.go b/internal/api/client.go index 1abe1e2..bb43be6 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -63,6 +63,13 @@ type authTransport struct { apiKey string username string password string + // host is the configured server host (host[:port]). Credentials are only + // attached when the request targets this host, so a redirect off-site + // (e.g. an attachment content_url that 302s to external object storage) + // never receives the API key or basic-auth credentials. An empty host + // means "always attach" (used by tests that construct the transport + // directly without a configured server). + host string } func (t *authTransport) RoundTrip(req *http.Request) (*http.Response, error) { @@ -71,11 +78,16 @@ func (t *authTransport) RoundTrip(req *http.Request) (*http.Response, error) { req.Header.Set("Content-Type", "application/json") } - switch t.authMethod { - case "basic": - req.SetBasicAuth(t.username, t.password) - default: - req.Header.Set("X-Redmine-API-Key", t.apiKey) + // Only attach credentials to the configured server's host. This keeps the + // API key (and basic-auth) from leaking to a third party if the server, or + // an attachment's content_url, redirects to an off-site host. + if t.host == "" || strings.EqualFold(req.URL.Host, t.host) { + switch t.authMethod { + case "basic": + req.SetBasicAuth(t.username, t.password) + default: + req.Header.Set("X-Redmine-API-Key", t.apiKey) + } } base := t.base @@ -93,12 +105,18 @@ func NewClient(cfg *config.Config, log *debug.Logger) (*Client, error) { baseURL := strings.TrimRight(cfg.Server, "/") + var host string + if u, err := url.Parse(baseURL); err == nil { + host = u.Host + } + transport := &authTransport{ base: http.DefaultTransport, authMethod: cfg.AuthMethod, apiKey: cfg.APIKey, username: cfg.Username, password: cfg.Password, + host: host, } c := &Client{ diff --git a/internal/cmd/attachment/attachment.go b/internal/cmd/attachment/attachment.go new file mode 100644 index 0000000..0784d56 --- /dev/null +++ b/internal/cmd/attachment/attachment.go @@ -0,0 +1,30 @@ +// Package attachment implements the `redmine attachments` command group, which +// surfaces the Redmine REST Attachments API so users (and agents) can inspect +// attachment metadata and download the raw bytes without dropping down to +// `redmine api` plus a manual `curl` with a hand-extracted API key. +package attachment + +import ( + "github.com/spf13/cobra" + + "github.com/aarondpn/redmine-cli/v2/internal/cmdutil" +) + +// NewCmdAttachments creates the parent attachments command. +func NewCmdAttachments(f *cmdutil.Factory) *cobra.Command { + cmd := &cobra.Command{ + Use: "attachments", + Aliases: []string{"attachment", "a"}, + Short: "Inspect and download Redmine attachments", + Long: "Inspect attachment metadata and download attachment files.\n\n" + + "Discover attachment IDs with `redmine issues get --attachments`, then\n" + + "fetch metadata with `redmine attachments get ` or download the file with\n" + + "`redmine attachments download `. Downloads reuse the active profile's\n" + + "server and API key, so there is no need to extract the key or shell out to curl.", + } + + cmd.AddCommand(newCmdGet(f)) + cmd.AddCommand(newCmdDownload(f)) + + return cmd +} diff --git a/internal/cmd/attachment/attachment_test.go b/internal/cmd/attachment/attachment_test.go new file mode 100644 index 0000000..3b27455 --- /dev/null +++ b/internal/cmd/attachment/attachment_test.go @@ -0,0 +1,182 @@ +package attachment + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/aarondpn/redmine-cli/v2/internal/testutil" +) + +// pngBytes is a tiny payload containing NUL and other non-text bytes so the +// tests double as a binary-integrity check. +var pngBytes = []byte{0x89, 'P', 'N', 'G', '\r', '\n', 0x1a, '\n', 0x00, 0x01, 0x02, 0xff} + +// attachmentServer serves metadata at /attachments/.json and raw bytes at +// the content_url it advertises. id 55 -> logo.png. +func attachmentServer(t *testing.T) *httptest.Server { + t.Helper() + var srv *httptest.Server + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/attachments/55.json": + w.Header().Set("Content-Type", "application/json") + fmt.Fprintf(w, `{"attachment":{"id":55,"filename":"logo.png","filesize":%d,"content_type":"image/png","description":"the logo","content_url":%q,"author":{"id":1,"name":"Ada"},"created_on":"2026-01-01T00:00:00Z"}}`, + len(pngBytes), srv.URL+"/attachments/download/55/logo.png") + case "/attachments/download/55/logo.png": + _, _ = w.Write(pngBytes) + default: + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + w.WriteHeader(http.StatusNotFound) + } + })) + t.Cleanup(srv.Close) + return srv +} + +func TestCmdAttachmentGet_JSON(t *testing.T) { + srv := attachmentServer(t) + f := testutil.NewFactory(t, srv.URL) + cmd := newCmdGet(f) + cmd.SetArgs([]string{"55", "--output", "json"}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + out := testutil.Stdout(f) + if !strings.Contains(out, `"filename": "logo.png"`) || !strings.Contains(out, `"content_type": "image/png"`) { + t.Fatalf("json stdout = %q, want filename and content_type", out) + } +} + +func TestCmdAttachmentGet_TableAndCSV(t *testing.T) { + for _, format := range []string{"table", "csv"} { + t.Run(format, func(t *testing.T) { + srv := attachmentServer(t) + f := testutil.NewFactory(t, srv.URL) + cmd := newCmdGet(f) + cmd.SetArgs([]string{"55", "--output", format}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + if out := testutil.Stdout(f); !strings.Contains(out, "logo.png") { + t.Fatalf("%s stdout = %q, want logo.png", format, out) + } + }) + } +} + +func TestCmdAttachmentGet_InvalidID(t *testing.T) { + f := testutil.NewFactory(t, "http://127.0.0.1:0") + cmd := newCmdGet(f) + cmd.SetArgs([]string{"not-a-number"}) + if err := cmd.Execute(); err == nil { + t.Fatal("expected error for non-numeric id") + } +} + +func TestCmdAttachmentDownload_ToDirJSON(t *testing.T) { + srv := attachmentServer(t) + f := testutil.NewFactory(t, srv.URL) + dir := t.TempDir() + cmd := newCmdDownload(f) + cmd.SetArgs([]string{"55", "--dir", dir, "--output", "json"}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + + var res struct { + ID int `json:"id"` + Filename string `json:"filename"` + Path string `json:"path"` + Bytes int64 `json:"bytes"` + } + if err := json.Unmarshal([]byte(testutil.Stdout(f)), &res); err != nil { + t.Fatalf("decode result: %v\nstdout: %s", err, testutil.Stdout(f)) + } + wantPath := filepath.Join(dir, "logo.png") + if res.Path != wantPath || res.Bytes != int64(len(pngBytes)) || res.Filename != "logo.png" { + t.Fatalf("result = %+v, want path=%q bytes=%d", res, wantPath, len(pngBytes)) + } + got, err := os.ReadFile(wantPath) + if err != nil { + t.Fatalf("read downloaded file: %v", err) + } + if string(got) != string(pngBytes) { + t.Errorf("downloaded bytes differ from source (binary integrity)") + } +} + +func TestCmdAttachmentDownload_DefaultsToCurrentDir(t *testing.T) { + srv := attachmentServer(t) + f := testutil.NewFactory(t, srv.URL) + t.Chdir(t.TempDir()) // bare `download ` saves into the working directory + + cmd := newCmdDownload(f) + cmd.SetArgs([]string{"55"}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + got, err := os.ReadFile("logo.png") + if err != nil { + t.Fatalf("expected ./logo.png in the working directory: %v", err) + } + if string(got) != string(pngBytes) { + t.Error("downloaded bytes differ from source") + } +} + +func TestCmdAttachmentDownload_ToExplicitPath(t *testing.T) { + srv := attachmentServer(t) + f := testutil.NewFactory(t, srv.URL) + dest := filepath.Join(t.TempDir(), "renamed.png") + cmd := newCmdDownload(f) + cmd.SetArgs([]string{"55", "--path", dest}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + got, err := os.ReadFile(dest) + if err != nil { + t.Fatalf("read downloaded file: %v", err) + } + if string(got) != string(pngBytes) { + t.Error("downloaded bytes differ from source") + } + // Default (table) output writes the confirmation to stderr, never stdout. + if testutil.Stdout(f) != "" { + t.Errorf("stdout = %q, want empty (confirmation goes to stderr)", testutil.Stdout(f)) + } + if !strings.Contains(testutil.Stderr(f), "renamed.png") { + t.Errorf("stderr = %q, want confirmation mentioning the path", testutil.Stderr(f)) + } +} + +func TestCmdAttachmentDownload_ToStdout(t *testing.T) { + srv := attachmentServer(t) + f := testutil.NewFactory(t, srv.URL) + cmd := newCmdDownload(f) + // --output json must NOT corrupt the piped binary on stdout. + cmd.SetArgs([]string{"55", "--path", "-", "--output", "json"}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + if testutil.Stdout(f) != string(pngBytes) { + t.Errorf("stdout did not receive the raw bytes verbatim") + } + if !strings.Contains(testutil.Stderr(f), "stdout") { + t.Errorf("stderr = %q, want confirmation mentioning stdout", testutil.Stderr(f)) + } +} + +func TestCmdAttachmentDownload_DirAndPathMutuallyExclusive(t *testing.T) { + f := testutil.NewFactory(t, "http://127.0.0.1:0") + cmd := newCmdDownload(f) + cmd.SetArgs([]string{"55", "--dir", "/tmp/x", "--path", "/tmp/y"}) + if err := cmd.Execute(); err == nil { + t.Fatal("expected error when --dir and --path are combined") + } +} diff --git a/internal/cmd/attachment/download.go b/internal/cmd/attachment/download.go new file mode 100644 index 0000000..92258b7 --- /dev/null +++ b/internal/cmd/attachment/download.go @@ -0,0 +1,133 @@ +package attachment + +import ( + "context" + "fmt" + "strconv" + + "github.com/spf13/cobra" + + "github.com/aarondpn/redmine-cli/v2/internal/cmdutil" + "github.com/aarondpn/redmine-cli/v2/internal/ops" + "github.com/aarondpn/redmine-cli/v2/internal/output" +) + +// downloadResult is the JSON payload emitted (with -o json) after a successful +// download to a file or directory. +type downloadResult struct { + ID int `json:"id"` + Filename string `json:"filename"` + ContentType string `json:"content_type,omitempty"` + Path string `json:"path"` + Bytes int64 `json:"bytes"` +} + +func newCmdDownload(f *cmdutil.Factory) *cobra.Command { + var ( + dir string + path string + format string + ) + + cmd := &cobra.Command{ + Use: "download ", + Aliases: []string{"dl", "get-file"}, + Short: "Download an attachment file", + Long: "Download the raw bytes of an attachment, authenticating with the active\n" + + "profile's server and API key (no manual key handling, no curl).\n\n" + + "By default the file is saved in the current directory under its real\n" + + "filename. Use --dir to choose a directory, --path to choose an exact file\n" + + "path, or `--path -` to stream the bytes to stdout for piping. The file is\n" + + "streamed to disk, never buffered entirely in memory.\n\n" + + "Note: for this command -o/--output controls the confirmation output\n" + + "(json prints a result object; otherwise a human-readable line is written to\n" + + "stderr). It does NOT change where the bytes go; use --path/--dir for that.", + Example: ` # Save to ./ in the current directory + redmine attachments download 42 + + # Save into a directory, keeping the real filename + redmine attachments download 42 --dir ./downloads + + # Save to an explicit path + redmine attachments download 42 --path ./logo.png + + # Stream to stdout (pipe into another tool) + redmine attachments download 42 --path - > logo.png + + # Print a JSON result describing the saved file + redmine attachments download 42 --dir ./downloads -o json`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + id, err := strconv.Atoi(args[0]) + if err != nil { + return fmt.Errorf("invalid attachment ID: %s", args[0]) + } + + client, err := f.ApiClient() + if err != nil { + return err + } + printer := f.Printer(format) + + ctx := context.Background() + stop := printer.Spinner("Downloading attachment...") + att, err := ops.GetAttachment(ctx, client, ops.GetAttachmentInput{ID: id}) + if err != nil { + stop() + return fmt.Errorf("failed to get attachment %d: %w", id, err) + } + + // Stream to stdout: stdout carries the raw bytes, so the + // confirmation always goes to stderr and no JSON envelope is + // emitted (it would corrupt the piped output). + if path == "-" { + n, err := client.Attachments.Download(ctx, att, f.IOStreams.Out) + stop() + if err != nil { + return fmt.Errorf("failed to download attachment %d: %w", id, err) + } + fmt.Fprintf(f.IOStreams.ErrOut, "Downloaded %q (%d bytes) to stdout\n", att.Filename, n) + return nil + } + + var ( + dest string + bytes int64 + ) + switch { + case path != "": + dest = path + bytes, err = cmdutil.SaveAttachmentToFile(ctx, client, att, path) + case dir != "": + dest, bytes, err = cmdutil.SaveAttachmentToDir(ctx, client, att, dir) + default: + dest, bytes, err = cmdutil.SaveAttachmentToDir(ctx, client, att, ".") + } + stop() + if err != nil { + return fmt.Errorf("failed to download attachment %d: %w", id, err) + } + + result := downloadResult{ + ID: att.ID, + Filename: att.Filename, + ContentType: att.ContentType, + Path: dest, + Bytes: bytes, + } + if printer.Format() == output.FormatJSON { + printer.JSON(result) + return nil + } + printer.Success(fmt.Sprintf("Downloaded %q (%d bytes) to %s", att.Filename, bytes, dest)) + return nil + }, + } + + cmd.Flags().StringVarP(&dir, "dir", "d", "", "Directory to save into, using the attachment's real filename") + cmd.Flags().StringVar(&path, "path", "", "Exact destination file path (use - for stdout)") + cmd.MarkFlagsMutuallyExclusive("dir", "path") + cmdutil.AddOutputFlag(cmd, &format) + + return cmd +} diff --git a/internal/cmd/attachment/get.go b/internal/cmd/attachment/get.go new file mode 100644 index 0000000..6a2b771 --- /dev/null +++ b/internal/cmd/attachment/get.go @@ -0,0 +1,101 @@ +package attachment + +import ( + "context" + "fmt" + "strconv" + + "github.com/spf13/cobra" + + "github.com/aarondpn/redmine-cli/v2/internal/cmdutil" + "github.com/aarondpn/redmine-cli/v2/internal/models" + "github.com/aarondpn/redmine-cli/v2/internal/ops" + "github.com/aarondpn/redmine-cli/v2/internal/output" +) + +func newCmdGet(f *cmdutil.Factory) *cobra.Command { + var format string + + cmd := &cobra.Command{ + Use: "get ", + Aliases: []string{"show", "view", "metadata"}, + Short: "Show attachment metadata", + Long: "Display metadata for a single attachment: filename, size, content type, " + + "description, author, creation time, and download URL.\n\n" + + "Use `redmine attachments download ` to fetch the file itself.", + Example: ` # Show metadata in a table + redmine attachments get 42 + + # JSON output (for scripting) + redmine attachments get 42 -o json + + # CSV output + redmine attachments get 42 -o csv`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + id, err := strconv.Atoi(args[0]) + if err != nil { + return fmt.Errorf("invalid attachment ID: %s", args[0]) + } + + client, err := f.ApiClient() + if err != nil { + return err + } + printer := f.Printer(format) + + stop := printer.Spinner("Fetching attachment...") + att, err := ops.GetAttachment(context.Background(), client, ops.GetAttachmentInput{ID: id}) + stop() + if err != nil { + return fmt.Errorf("failed to get attachment %d: %w", id, err) + } + + switch printer.Format() { + case output.FormatJSON: + printer.JSON(att) + case output.FormatCSV: + printer.CSV(attachmentHeaders, [][]string{attachmentRow(att)}) + default: + renderAttachmentDetail(printer, att) + } + return nil + }, + } + + cmdutil.AddOutputFlag(cmd, &format) + + return cmd +} + +var attachmentHeaders = []string{"ID", "Filename", "Size", "Content-Type", "Author", "Created", "Description"} + +func attachmentRow(att *models.Attachment) []string { + return []string{ + strconv.Itoa(att.ID), + att.Filename, + strconv.FormatInt(att.Filesize, 10), + att.ContentType, + att.Author.Name, + att.CreatedOn, + att.Description, + } +} + +func renderAttachmentDetail(printer output.Printer, att *models.Attachment) { + pairs := []output.KeyValue{ + {Key: "ID", Value: output.StyleID.Render(strconv.Itoa(att.ID))}, + {Key: "Filename", Value: att.Filename}, + {Key: "Size", Value: fmt.Sprintf("%d bytes", att.Filesize)}, + {Key: "Content-Type", Value: att.ContentType}, + {Key: "Author", Value: att.Author.Name}, + {Key: "Created", Value: att.CreatedOn}, + } + if att.Description != "" { + pairs = append(pairs, output.KeyValue{Key: "Description", Value: att.Description}) + } + if att.ContentURL != "" { + pairs = append(pairs, output.KeyValue{Key: "Content URL", Value: att.ContentURL}) + } + printer.Detail(pairs) +} diff --git a/internal/cmd/issue/get.go b/internal/cmd/issue/get.go index ba093aa..0e5a0e5 100644 --- a/internal/cmd/issue/get.go +++ b/internal/cmd/issue/get.go @@ -8,7 +8,9 @@ import ( "github.com/spf13/cobra" + "github.com/aarondpn/redmine-cli/v2/internal/api" "github.com/aarondpn/redmine-cli/v2/internal/cmdutil" + "github.com/aarondpn/redmine-cli/v2/internal/models" "github.com/aarondpn/redmine-cli/v2/internal/ops" "github.com/aarondpn/redmine-cli/v2/internal/output" ) @@ -16,20 +18,38 @@ import ( // NewCmdGet creates the issues get command. func NewCmdGet(f *cmdutil.Factory) *cobra.Command { var ( - include string - journals bool - children bool - relations bool - format string + include string + journals bool + children bool + relations bool + attachments bool + downloadAttachments string + format string ) cmd := &cobra.Command{ Use: "get ", Aliases: []string{"show", "view"}, Short: "Get issue details", - Long: "Display detailed information about a specific issue.", - Args: cobra.ExactArgs(1), + Long: "Display detailed information about a specific issue.\n\n" + + "Use --attachments to list the issue's attachments (with their IDs, so you can\n" + + "download one via `redmine attachments download `), or --download-attachments\n" + + " to download every attachment of the issue into in one step.", + Example: ` # Show an issue + redmine issues get 123 + + # Include comments/history + redmine issues get 123 --journals + + # List the issue's attachments (id, filename, size, type) + redmine issues get 123 --attachments + + # Download every attachment into ./issue-123 + redmine issues get 123 --download-attachments ./issue-123`, + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + ctx := context.Background() + id, err := strconv.Atoi(args[0]) if err != nil { return fmt.Errorf("invalid issue ID: %s", args[0]) @@ -53,15 +73,26 @@ func NewCmdGet(f *cmdutil.Factory) *cobra.Command { if relations { includes = append(includes, "relations") } + if attachments || downloadAttachments != "" { + includes = append(includes, "attachments") + } printer := f.Printer(format) stop := printer.Spinner("Fetching issue...") - issue, err := ops.GetIssue(context.Background(), client, ops.GetIssueInput{ID: id, Includes: includes}) + issue, err := ops.GetIssue(ctx, client, ops.GetIssueInput{ID: id, Includes: includes}) stop() if err != nil { return fmt.Errorf("failed to get issue %s: %w", fmt.Sprintf("#%d", id), err) } + // Side-effect: download every attachment into the target dir. + // Confirmations go to stderr so JSON stdout stays the issue body. + if downloadAttachments != "" { + if err := downloadIssueAttachments(ctx, client, printer, issue, downloadAttachments); err != nil { + return err + } + } + if printer.Format() == output.FormatJSON { printer.JSON(issue) return nil @@ -111,15 +142,51 @@ func NewCmdGet(f *cmdutil.Factory) *cobra.Command { } } + if len(issue.Attachments) > 0 { + fmt.Println() + fmt.Println("Attachments:") + rows := make([][]string, len(issue.Attachments)) + for i, a := range issue.Attachments { + rows[i] = []string{ + strconv.Itoa(a.ID), + a.Filename, + strconv.FormatInt(a.Filesize, 10), + a.ContentType, + } + } + printer.Table([]string{"ID", "Filename", "Size", "Content-Type"}, rows) + } + return nil }, } - cmd.Flags().StringVar(&include, "include", "", "Include related data: journals,children,relations") + cmd.Flags().StringVar(&include, "include", "", "Include related data: journals,children,relations,attachments") cmd.Flags().BoolVar(&journals, "journals", false, "Include issue history/comments (shorthand for --include journals)") cmd.Flags().BoolVar(&children, "children", false, "Include child issues (shorthand for --include children)") cmd.Flags().BoolVar(&relations, "relations", false, "Include issue relations (shorthand for --include relations)") + cmd.Flags().BoolVar(&attachments, "attachments", false, "Include the issue's attachments (shorthand for --include attachments)") + cmd.Flags().StringVar(&downloadAttachments, "download-attachments", "", "Download every attachment of the issue into the given directory") cmdutil.AddOutputFlag(cmd, &format) return cmd } + +// downloadIssueAttachments streams every attachment of the issue into dir, +// reporting progress to stderr (via the printer) so it never pollutes JSON +// output on stdout. +func downloadIssueAttachments(ctx context.Context, client *api.Client, printer output.Printer, issue *models.Issue, dir string) error { + if len(issue.Attachments) == 0 { + printer.Warning(fmt.Sprintf("Issue #%d has no attachments to download", issue.ID)) + return nil + } + for i := range issue.Attachments { + att := &issue.Attachments[i] + path, n, err := cmdutil.SaveAttachmentToDir(ctx, client, att, dir) + if err != nil { + return fmt.Errorf("downloading attachment %d (%s): %w", att.ID, att.Filename, err) + } + printer.Success(fmt.Sprintf("Downloaded %q (%d bytes) to %s", att.Filename, n, path)) + } + return nil +} diff --git a/internal/cmd/issue/get_attachments_test.go b/internal/cmd/issue/get_attachments_test.go new file mode 100644 index 0000000..9720b25 --- /dev/null +++ b/internal/cmd/issue/get_attachments_test.go @@ -0,0 +1,109 @@ +package issue + +import ( + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/aarondpn/redmine-cli/v2/internal/testutil" +) + +var issuePNG = []byte{0x89, 'P', 'N', 'G', 0x00, 0x01, 0x02, 0xff} + +// issueWithAttachmentsServer serves issue #1 with one attachment (when +// include=attachments is requested) plus the raw bytes at the content_url. +func issueWithAttachmentsServer(t *testing.T) *httptest.Server { + t.Helper() + var srv *httptest.Server + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/issues/1.json": + if !strings.Contains(r.URL.RawQuery, "attachments") { + t.Errorf("expected include=attachments, got query %q", r.URL.RawQuery) + } + w.Header().Set("Content-Type", "application/json") + fmt.Fprintf(w, `{"issue":{"id":1,"project":{"id":1,"name":"P"},"tracker":{"id":1,"name":"T"},"status":{"id":1,"name":"New"},"priority":{"id":1,"name":"Normal"},"author":{"id":1,"name":"A"},"subject":"s","attachments":[{"id":55,"filename":"shot.png","filesize":%d,"content_type":"image/png","content_url":%q,"author":{"id":1,"name":"A"},"created_on":"2026-01-01T00:00:00Z"}]}}`, + len(issuePNG), srv.URL+"/attachments/download/55/shot.png") + case "/attachments/download/55/shot.png": + _, _ = w.Write(issuePNG) + default: + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + w.WriteHeader(http.StatusNotFound) + } + })) + t.Cleanup(srv.Close) + return srv +} + +func TestCmdIssueGet_AttachmentsIncludedInJSON(t *testing.T) { + srv := issueWithAttachmentsServer(t) + f := testutil.NewFactory(t, srv.URL) + cmd := NewCmdGet(f) + cmd.SetArgs([]string{"1", "--attachments", "--output", "json"}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + out := testutil.Stdout(f) + if !strings.Contains(out, `"filename": "shot.png"`) || !strings.Contains(out, `"id": 55`) { + t.Fatalf("json stdout = %q, want attachment shot.png id 55", out) + } +} + +func TestCmdIssueGet_DownloadAttachments_NoAttachments(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/issues/2.json" { + t.Errorf("unexpected request %s", r.URL.Path) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"issue":{"id":2,"project":{"id":1,"name":"P"},"tracker":{"id":1,"name":"T"},"status":{"id":1,"name":"New"},"priority":{"id":1,"name":"Normal"},"author":{"id":1,"name":"A"},"subject":"s","attachments":[]}}`)) + })) + defer srv.Close() + + f := testutil.NewFactory(t, srv.URL) + dir := t.TempDir() + cmd := NewCmdGet(f) + cmd.SetArgs([]string{"2", "--download-attachments", dir, "--output", "json"}) + if err := cmd.Execute(); err != nil { + t.Fatalf("expected success for an issue with no attachments: %v", err) + } + if !strings.Contains(testutil.Stderr(f), "no attachments to download") { + t.Errorf("stderr = %q, want a no-attachments warning", testutil.Stderr(f)) + } + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + if len(entries) != 0 { + t.Errorf("expected no files written, got %d", len(entries)) + } +} + +func TestCmdIssueGet_DownloadAttachments(t *testing.T) { + srv := issueWithAttachmentsServer(t) + f := testutil.NewFactory(t, srv.URL) + dir := t.TempDir() + cmd := NewCmdGet(f) + cmd.SetArgs([]string{"1", "--download-attachments", dir, "--output", "json"}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + + got, err := os.ReadFile(filepath.Join(dir, "shot.png")) + if err != nil { + t.Fatalf("expected downloaded attachment: %v", err) + } + if string(got) != string(issuePNG) { + t.Error("downloaded attachment bytes differ from source") + } + // The issue JSON still lands on stdout; download confirmation on stderr. + if !strings.Contains(testutil.Stdout(f), `"subject": "s"`) { + t.Errorf("stdout should still carry the issue JSON, got %q", testutil.Stdout(f)) + } + if !strings.Contains(testutil.Stderr(f), "shot.png") { + t.Errorf("stderr = %q, want download confirmation", testutil.Stderr(f)) + } +} diff --git a/internal/cmd/output_contract_test.go b/internal/cmd/output_contract_test.go index 3ec7b4a..020dda4 100644 --- a/internal/cmd/output_contract_test.go +++ b/internal/cmd/output_contract_test.go @@ -29,6 +29,8 @@ type commandContract struct { // failure instead of a review surprise. var commandOutputContracts = map[string]commandContract{ "redmine api": {Mode: outputContractStructured}, + "redmine attachments download": {Mode: outputContractStructured}, + "redmine attachments get": {Mode: outputContractStructured}, "redmine auth list": {Mode: outputContractStructured}, "redmine auth login": {Mode: outputContractInteractive, Reason: "runs an interactive Huh form to collect credentials"}, "redmine auth logout": {Mode: outputContractInteractive, Reason: "may prompt for interactive confirmation before deleting a profile"}, diff --git a/internal/cmd/root.go b/internal/cmd/root.go index cf3948d..f2d2362 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -6,6 +6,7 @@ import ( "github.com/spf13/cobra" apicmd "github.com/aarondpn/redmine-cli/v2/internal/cmd/api" + "github.com/aarondpn/redmine-cli/v2/internal/cmd/attachment" "github.com/aarondpn/redmine-cli/v2/internal/cmd/auth" "github.com/aarondpn/redmine-cli/v2/internal/cmd/category" "github.com/aarondpn/redmine-cli/v2/internal/cmd/completion" @@ -94,6 +95,7 @@ func NewRootCmdWithFactory(version string) (*cobra.Command, *cmdutil.Factory) { cmd.AddCommand(apicmd.NewCmdAPI(f)) cmd.AddCommand(auth.NewCmdAuth(f)) cmd.AddCommand(issue.NewCmdIssue(f)) + cmd.AddCommand(attachment.NewCmdAttachments(f)) cmd.AddCommand(group.NewCmdGroup(f)) cmd.AddCommand(membership.NewCmdMemberships(f)) cmd.AddCommand(project.NewCmdProject(f)) diff --git a/internal/cmdutil/downloads.go b/internal/cmdutil/downloads.go new file mode 100644 index 0000000..4abc474 --- /dev/null +++ b/internal/cmdutil/downloads.go @@ -0,0 +1,54 @@ +package cmdutil + +import ( + "context" + "fmt" + "os" + "path/filepath" + + "github.com/aarondpn/redmine-cli/v2/internal/api" + "github.com/aarondpn/redmine-cli/v2/internal/models" +) + +// SaveAttachmentToDir streams an attachment into dir using its real filename +// and returns the path written and the number of bytes streamed. The directory +// is created if it does not exist. The server-supplied filename is reduced to +// its base component so a malicious or malformed name (e.g. one containing +// "../") cannot escape dir. +func SaveAttachmentToDir(ctx context.Context, client *api.Client, att *models.Attachment, dir string) (string, int64, error) { + if err := os.MkdirAll(dir, 0o755); err != nil { + return "", 0, fmt.Errorf("creating directory %s: %w", dir, err) + } + name := filepath.Base(att.Filename) + // filepath.Base can only yield ".", "..", the separator, or "" for a + // degenerate name; all of them would write outside dir (or to dir itself), + // so fall back to a synthetic, contained name. + if name == "." || name == ".." || name == string(os.PathSeparator) || name == "" { + name = fmt.Sprintf("attachment-%d", att.ID) + } + path := filepath.Join(dir, name) + n, err := SaveAttachmentToFile(ctx, client, att, path) + return path, n, err +} + +// SaveAttachmentToFile streams an attachment to the exact path given and +// returns the number of bytes written. The parent directory must already +// exist. The file is created (or truncated) with 0o644 permissions. +func SaveAttachmentToFile(ctx context.Context, client *api.Client, att *models.Attachment, path string) (int64, error) { + f, err := os.Create(path) + if err != nil { + return 0, fmt.Errorf("creating %s: %w", path, err) + } + + n, err := client.Attachments.Download(ctx, att, f) + if closeErr := f.Close(); closeErr != nil && err == nil { + err = fmt.Errorf("closing %s: %w", path, closeErr) + } + if err != nil { + // Best-effort cleanup of the partial file so a failed download does + // not leave a truncated artifact behind. + _ = os.Remove(path) + return n, err + } + return n, nil +} diff --git a/internal/cmdutil/downloads_test.go b/internal/cmdutil/downloads_test.go new file mode 100644 index 0000000..c23ce69 --- /dev/null +++ b/internal/cmdutil/downloads_test.go @@ -0,0 +1,131 @@ +package cmdutil + +import ( + "context" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "github.com/aarondpn/redmine-cli/v2/internal/api" + "github.com/aarondpn/redmine-cli/v2/internal/debug" + "github.com/aarondpn/redmine-cli/v2/internal/models" +) + +func newDownloadClient(t *testing.T, body []byte, status int) *api.Client { + t.Helper() + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if status != 0 && status != http.StatusOK { + w.WriteHeader(status) + return + } + _, _ = w.Write(body) + })) + t.Cleanup(ts.Close) + return api.NewTestClient(ts.Client(), ts.URL, debug.New(nil)) +} + +func attFor(client *api.Client, filename string) *models.Attachment { + // content_url is left empty so Download builds the canonical path against + // the test client's base URL (any path resolves to the stub handler). + return &models.Attachment{ID: 1, Filename: filename} +} + +func TestSaveAttachmentToDir_WritesRealFilename(t *testing.T) { + want := []byte("file body bytes") + client := newDownloadClient(t, want, http.StatusOK) + dir := t.TempDir() + + path, n, err := SaveAttachmentToDir(context.Background(), client, attFor(client, "report.pdf"), dir) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got := filepath.Join(dir, "report.pdf"); path != got { + t.Errorf("path = %q, want %q", path, got) + } + if n != int64(len(want)) { + t.Errorf("bytes = %d, want %d", n, len(want)) + } + got, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read written file: %v", err) + } + if string(got) != string(want) { + t.Errorf("content = %q, want %q", got, want) + } +} + +// TestSaveAttachmentToDir_SanitizesFilename guards against a server-supplied +// filename escaping the destination directory via path traversal. +func TestSaveAttachmentToDir_SanitizesFilename(t *testing.T) { + client := newDownloadClient(t, []byte("x"), http.StatusOK) + dir := t.TempDir() + + path, _, err := SaveAttachmentToDir(context.Background(), client, attFor(client, "../../../etc/evil"), dir) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if filepath.Dir(path) != filepath.Clean(dir) { + t.Errorf("written outside dir: path=%q dir=%q", path, dir) + } + if filepath.Base(path) != "evil" { + t.Errorf("base = %q, want sanitized to evil", filepath.Base(path)) + } +} + +// TestSaveAttachmentToDir_FallsBackForDegenerateNames covers names that +// filepath.Base reduces to a value that would escape dir or name the directory +// itself ("..", "/", ""); each must fall back to a contained synthetic name. +func TestSaveAttachmentToDir_FallsBackForDegenerateNames(t *testing.T) { + for _, name := range []string{"..", "../..", "/", ""} { + t.Run("name="+name, func(t *testing.T) { + client := newDownloadClient(t, []byte("x"), http.StatusOK) + dir := t.TempDir() + + path, _, err := SaveAttachmentToDir(context.Background(), client, attFor(client, name), dir) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if filepath.Dir(path) != filepath.Clean(dir) { + t.Errorf("written outside dir: path=%q dir=%q", path, dir) + } + if filepath.Base(path) != "attachment-1" { + t.Errorf("base = %q, want synthetic attachment-1", filepath.Base(path)) + } + if _, err := os.Stat(path); err != nil { + t.Errorf("expected file at %q: %v", path, err) + } + }) + } +} + +// TestSaveAttachmentToDir_CreatesDir confirms the destination directory is +// created when missing (used by both `attachments download --dir` and +// `issues get --download-attachments`). +func TestSaveAttachmentToDir_CreatesDir(t *testing.T) { + client := newDownloadClient(t, []byte("data"), http.StatusOK) + dir := filepath.Join(t.TempDir(), "nested", "created") + + path, _, err := SaveAttachmentToDir(context.Background(), client, attFor(client, "a.bin"), dir) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if _, err := os.Stat(path); err != nil { + t.Errorf("expected file at %q: %v", path, err) + } +} + +// TestSaveAttachmentToFile_CleansUpPartialOnError verifies a failed download +// does not leave a truncated artifact behind. +func TestSaveAttachmentToFile_CleansUpPartialOnError(t *testing.T) { + client := newDownloadClient(t, nil, http.StatusForbidden) + path := filepath.Join(t.TempDir(), "partial.bin") + + if _, err := SaveAttachmentToFile(context.Background(), client, attFor(client, "partial.bin"), path); err == nil { + t.Fatal("expected error on 403 download") + } + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Errorf("partial file should have been removed, stat err = %v", err) + } +} diff --git a/internal/mcpserver/zz_generated_tools.go b/internal/mcpserver/zz_generated_tools.go index 1b88d4f..e9cd5dd 100644 --- a/internal/mcpserver/zz_generated_tools.go +++ b/internal/mcpserver/zz_generated_tools.go @@ -106,6 +106,12 @@ func registerGeneratedTools(s *mcp.Server, client *api.Client, opts Options) { Writes: true, Call: ops.DeleteIssueRelation, }) + registerToolSpec(s, client, opts, toolSpec[ops.GetAttachmentInput, *models.Attachment]{ + Name: "get_attachment", + Description: "Fetch metadata for a Redmine attachment by ID: filename, size, content type, description, author, and download URL. Discover attachment IDs via get_issue with includes=[\"attachments\"].", + Category: "issues", + Call: ops.GetAttachment, + }) registerToolSpec(s, client, opts, toolSpec[ops.GetIssueInput, *models.Issue]{ Name: "get_issue", Description: "Fetch a single Redmine issue by ID.", @@ -487,6 +493,7 @@ func generatedToolDescriptors() []ToolDescriptor { {Name: "create_issue_relation", Description: "Create a relation between two issues. Requires --enable-writes.", Group: "issues", Writes: true}, {Name: "delete_issue", Description: "Delete a Redmine issue. Destructive. Requires --enable-writes.", Group: "issues", Writes: true}, {Name: "delete_issue_relation", Description: "Delete an issue relation by its relation ID. Requires --enable-writes.", Group: "issues", Writes: true}, + {Name: "get_attachment", Description: "Fetch metadata for a Redmine attachment by ID: filename, size, content type, description, author, and download URL. Discover attachment IDs via get_issue with includes=[\"attachments\"].", Group: "issues", Writes: false}, {Name: "get_issue", Description: "Fetch a single Redmine issue by ID.", Group: "issues", Writes: false}, {Name: "get_issue_relation", Description: "Fetch a single issue relation by its relation ID.", Group: "issues", Writes: false}, {Name: "list_issue_relations", Description: "List relations attached to an issue.", Group: "issues", Writes: false}, diff --git a/internal/models/attachment.go b/internal/models/attachment.go new file mode 100644 index 0000000..70dff71 --- /dev/null +++ b/internal/models/attachment.go @@ -0,0 +1,16 @@ +package models + +// Attachment represents a file attached to a Redmine resource (issue, wiki +// page, ...). It is returned by the /attachments/:id.json endpoint and by the +// attachments[] array on issues and wiki pages (include=attachments). +type Attachment struct { + ID int `json:"id"` + Filename string `json:"filename"` + Filesize int64 `json:"filesize"` + ContentType string `json:"content_type"` + Description string `json:"description"` + ContentURL string `json:"content_url"` + ThumbnailURL string `json:"thumbnail_url,omitempty"` + Author IDName `json:"author"` + CreatedOn string `json:"created_on"` +} diff --git a/internal/models/issue.go b/internal/models/issue.go index 70d5c08..6af4e4c 100644 --- a/internal/models/issue.go +++ b/internal/models/issue.go @@ -34,6 +34,7 @@ type Issue struct { Children []IDRef `json:"children,omitempty"` Watchers []IDName `json:"watchers,omitempty"` Relations []IssueRelation `json:"relations,omitempty"` + Attachments []Attachment `json:"attachments,omitempty"` } // IssueRelation represents a relation between two issues. Returned both via diff --git a/internal/models/wiki.go b/internal/models/wiki.go index 3c427fa..4d4a7f2 100644 --- a/internal/models/wiki.go +++ b/internal/models/wiki.go @@ -18,18 +18,6 @@ type WikiPageTitle struct { Title string `json:"title"` } -// Attachment represents a file attached to a wiki page. -type Attachment struct { - ID int `json:"id"` - Filename string `json:"filename"` - Filesize int64 `json:"filesize"` - ContentType string `json:"content_type"` - Description string `json:"description"` - ContentURL string `json:"content_url"` - Author IDName `json:"author"` - CreatedOn string `json:"created_on"` -} - // WikiPageIndex represents a wiki page entry in the index listing. type WikiPageIndex struct { Title string `json:"title"` diff --git a/internal/ops/attachments.go b/internal/ops/attachments.go new file mode 100644 index 0000000..c3478a1 --- /dev/null +++ b/internal/ops/attachments.go @@ -0,0 +1,19 @@ +package ops + +import ( + "context" + + "github.com/aarondpn/redmine-cli/v2/internal/api" + "github.com/aarondpn/redmine-cli/v2/internal/models" +) + +type GetAttachmentInput struct { + ID int `json:"id" jsonschema:"Numeric attachment ID. Discover IDs via get_issue with includes=[\"attachments\"]."` +} + +//mcpgen:tool get_attachment +//mcpgen:description Fetch metadata for a Redmine attachment by ID: filename, size, content type, description, author, and download URL. Discover attachment IDs via get_issue with includes=["attachments"]. +//mcpgen:category issues +func GetAttachment(ctx context.Context, client *api.Client, input GetAttachmentInput) (*models.Attachment, error) { + return client.Attachments.Get(ctx, input.ID) +} diff --git a/internal/ops/attachments_test.go b/internal/ops/attachments_test.go new file mode 100644 index 0000000..cbe9d6d --- /dev/null +++ b/internal/ops/attachments_test.go @@ -0,0 +1,34 @@ +package ops + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/aarondpn/redmine-cli/v2/internal/api" + "github.com/aarondpn/redmine-cli/v2/internal/debug" +) + +func TestGetAttachment_DecodesResponse(t *testing.T) { + var gotPath string + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"attachment":{"id":42,"filename":"screenshot.png","filesize":1234,"content_type":"image/png","author":{"id":1,"name":"Grace"},"created_on":"2026-05-01T00:00:00Z"}}`)) + })) + defer ts.Close() + + client := api.NewTestClient(ts.Client(), ts.URL, debug.New(nil)) + + att, err := GetAttachment(context.Background(), client, GetAttachmentInput{ID: 42}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if gotPath != "/attachments/42.json" { + t.Errorf("path = %q, want /attachments/42.json", gotPath) + } + if att.ID != 42 || att.Filename != "screenshot.png" || att.ContentType != "image/png" { + t.Errorf("attachment = %+v, want id=42 screenshot.png image/png", att) + } +} diff --git a/skills/redmine-cli/SKILL.md b/skills/redmine-cli/SKILL.md index 8a6d40f..9296a5c 100644 --- a/skills/redmine-cli/SKILL.md +++ b/skills/redmine-cli/SKILL.md @@ -14,6 +14,7 @@ Only these top-level commands exist. Do NOT invent subcommands that aren't liste | Command | Purpose | |---------|---------| | `issues` | Create, list, get, update, close, reopen, assign, comment, delete, search, browse issues; manage watchers and relations (`issues watchers …`, `issues relations …`) | +| `attachments` | Inspect attachment metadata (`attachments get `) and download attachment files (`attachments download `) using the active profile's auth | | `queries` | List Redmine saved queries; reuse them via `issues list --query` / `--query-id` | | `projects` | List, get, create, update, archive, unarchive, delete projects; list project members. `--include` on list/get exposes trackers, modules, categories, custom fields, and time-entry activities (Redmine 5.0+ for archive). | | `time` | Log, list, get, update, delete, summarize time entries | @@ -90,10 +91,32 @@ When you create an issue, project, user, or other resource, the CLI returns the Get the server URL from `redmine config` (or from the JSON output's hints). Always mention the URL or the `open` command after a successful create so the user can quickly navigate to the new resource. +## Attachments: Always Download and Inspect Them + +Issues often carry attachments (screenshots, diagrams, logs, PDFs) that contain +information not present in the text. **Whenever an issue has attachments, +download them and inspect their contents before answering** - especially +images, which frequently hold the actual error, mockup, or detail the ticket is +about. + +1. **Discover attachment IDs**: `redmine issues get --attachments` lists each + attachment's `id`, `filename`, `size`, and `content_type`. With `-o json` the + issue's `attachments[]` array is included in the output. +2. **Download one file**: `redmine attachments download -d ` saves it + under its real filename (or `--path ` for an exact path, `--path -` to + stream to stdout). No `curl`, no manual API-key handling - it reuses the active + profile's auth. +3. **Download everything at once**: `redmine issues get --download-attachments ` + pulls every attachment of the issue into `` in one step. +4. **Inspect metadata only** (no download): `redmine attachments get `. + +After downloading an image, open/read it and use what it shows. Do not answer a +question about a ticket with attachments without first looking at them. + ## Non-Obvious Behaviors - `redmine issues list` defaults to `--status open`. Use `--status closed`, `--status "*"`, or a specific status name. -- `redmine issues get --journals` includes comments/history. Also available: `--children`, `--relations`. +- `redmine issues get --journals` includes comments/history. Also available: `--children`, `--relations`, `--attachments`. - `redmine issues update` only sends flags you explicitly pass — omitted flags are not changed. - If `--project` is omitted, the configured default project is used (set via `redmine auth login`). - Projects can accumulate hundreds of versions, most of them closed or locked. When you need a version for a new issue, time entry, or similar workflow, always start from `redmine versions list --open` so the shortlist stays small and you don't pick a version that can no longer accept work.