From dadd011dd2134fe1f26d716d87a1b02a073151e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aaron=20D=C3=B6ppner?= <58708656+aarondpn@users.noreply.github.com> Date: Mon, 15 Jun 2026 11:58:43 +0200 Subject: [PATCH] feat: add attachments command for download and inspection Add a top-level `attachments` command group (aliases `attachment`, `a`) so users and agents can download and inspect Redmine attachments without dropping to `redmine api` plus a manual curl with a hand-extracted key. - `attachments get `: metadata in table/json/csv. - `attachments download `: stream the file to disk (-d/--dir, --path, --path - for stdout, default ./). Reuses the active profile's auth and streams rather than buffering the whole file in memory. - `issues get --attachments`: list an issue's attachments with their IDs; `--download-attachments `: download all of them at once. - Surface attachments on the typed Issue model (CLI and MCP get_issue). - New read-only MCP tool `get_attachment`. Security: only attach credentials to the configured server host, so the API key and basic-auth are never leaked to an off-site host if the server or an attachment content_url redirects to external object storage. Update the bundled skill to tell the agent to download and inspect attachments (especially images), plus docs in English and Chinese, and unit and e2e tests. --- docs/astro.config.mjs | 1 + .../src/content/docs/commands/attachments.mdx | 106 ++++++++++ docs/src/content/docs/commands/issues.mdx | 33 +++- .../docs/zh-cn/commands/attachments.mdx | 105 ++++++++++ .../content/docs/zh-cn/commands/issues.mdx | 33 +++- e2e/attachments_test.go | 169 ++++++++++++++++ e2e/issues_test.go | 6 +- e2e/testdata/mcp_tools_readonly.golden.json | 4 + internal/api/attachments.go | 67 ++++++- internal/api/attachments_download_test.go | 176 +++++++++++++++++ internal/api/auth_transport_test.go | 132 +++++++++++++ internal/api/client.go | 28 ++- internal/cmd/attachment/attachment.go | 30 +++ internal/cmd/attachment/attachment_test.go | 182 ++++++++++++++++++ internal/cmd/attachment/download.go | 133 +++++++++++++ internal/cmd/attachment/get.go | 101 ++++++++++ internal/cmd/issue/get.go | 85 +++++++- internal/cmd/issue/get_attachments_test.go | 109 +++++++++++ internal/cmd/output_contract_test.go | 2 + internal/cmd/root.go | 2 + internal/cmdutil/downloads.go | 54 ++++++ internal/cmdutil/downloads_test.go | 131 +++++++++++++ internal/mcpserver/zz_generated_tools.go | 7 + internal/models/attachment.go | 16 ++ internal/models/issue.go | 1 + internal/models/wiki.go | 12 -- internal/ops/attachments.go | 19 ++ internal/ops/attachments_test.go | 34 ++++ skills/redmine-cli/SKILL.md | 25 ++- 29 files changed, 1762 insertions(+), 41 deletions(-) create mode 100644 docs/src/content/docs/commands/attachments.mdx create mode 100644 docs/src/content/docs/zh-cn/commands/attachments.mdx create mode 100644 e2e/attachments_test.go create mode 100644 internal/api/attachments_download_test.go create mode 100644 internal/api/auth_transport_test.go create mode 100644 internal/cmd/attachment/attachment.go create mode 100644 internal/cmd/attachment/attachment_test.go create mode 100644 internal/cmd/attachment/download.go create mode 100644 internal/cmd/attachment/get.go create mode 100644 internal/cmd/issue/get_attachments_test.go create mode 100644 internal/cmdutil/downloads.go create mode 100644 internal/cmdutil/downloads_test.go create mode 100644 internal/models/attachment.go create mode 100644 internal/ops/attachments.go create mode 100644 internal/ops/attachments_test.go 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.