Support storage URL downloads in files download#283
Conversation
github-actions
bot
added
commands
There was a problem hiding this comment.
Pull request overview
Adds first-class support for downloading inline rich-text attachments (storage blob URLs) via basecamp files download, by rewriting storage URLs to the API host for OAuth-authenticated access, and introduces a rich text helper to extract <bc-attachment> metadata.
Changes:
- Extend
files downloadto detectstorage.X.basecamp.com/.../blobs/.../download/...URLs and download them via the API host without requiring--in. - Add
richtext.ExtractAttachments()andInlineAttachmentfor parsing downloadable<bc-attachment>elements out of rich text HTML (excluding mentions). - Add/extend tests and update the Basecamp skill documentation with the new workflow.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
internal/commands/files.go |
Adds storage-URL download path, shared file-writing helper, and URL rewrite/download helpers. |
internal/commands/files_test.go |
Adds unit tests for storage URL detection/rewriting and filename parsing. |
internal/richtext/richtext.go |
Adds InlineAttachment and ExtractAttachments() to parse <bc-attachment> tags (excluding mentions). |
internal/richtext/richtext_test.go |
Adds tests covering attachment extraction scenarios (empty/no attachments/mixed/mention exclusion). |
skills/basecamp/SKILL.md |
Documents downloading inline attachments via storage URLs in quick reference and workflows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/richtext/richtext.go">
<violation number="1" location="internal/richtext/richtext.go:666">
P2: Extracted attribute values are not HTML-entity decoded. If Basecamp ever emits `&` or other entities inside `href`, `filename`, etc., the downstream download URL or output filename will be incorrect. Apply `unescapeHTML` (already in this file) to each extracted value.</violation>
</file>
<file name="internal/commands/files.go">
<violation number="1" location="internal/commands/files.go:1540">
P1: `isStorageURL` does not require `https` scheme. An `http://` storage URL would pass validation and `downloadStorageURL` would send the OAuth bearer token over plaintext HTTP. Add `u.Scheme == "https"` to the check.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/commands/files.go">
<violation number="1" location="internal/commands/files.go:1643">
P2: `http.Client.Timeout` on the S3 download client covers the full body read, which happens after this function returns. Large files that take >5 minutes to stream from S3 will fail with a deadline error. Since the request already uses `ctx` for cancellation, drop the client-level timeout or increase it substantially.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Just an observation from real-world use on my end: Once this lands, the SKILL.md could benefit from a short "Working with Inline Images" section that walks an AI agent through the full loop: fetch a recording's JSON, extract Right now the skill documents the CLI commands but doesn't connect the dots for an agent that can see images but doesn't know it should look for them. A brief recipe would close that gap and prevent agents from silently skipping visual context — mockups, screenshots, annotated designs — that's often the most important part of a Basecamp todo for us. |
|
@brigleb great points - just the sort of usage the skill needs to cover, linking together steps. Suggests that the CLI could even automate this with e.g. @jorgemanrubia, I'm going to shift some of this over to the SDK as well. |
Inline attachments in rich text content (card descriptions, comments, messages) use storage blob URLs like `https://storage.3.basecamp.com/{account}/blobs/{uuid}/download/{filename}`. These URLs appear in `<bc-attachment>` HTML elements in the `content` field of recordings. Until now, `files download` only handled upload IDs. This extends `files download` to accept storage URLs directly so attachments embedded in rich text can be downloaded without manual URL rewriting or token juggling. The storage host (`storage.X.basecamp.com`) uses cookie-based session auth, which the CLI doesn't have. The fix: rewrite the URL to the API host (`X.basecampapi.com`) which accepts OAuth bearer tokens and redirects to a signed S3 URL. Changes: - `files download` detects storage URLs and routes them through the API host with OAuth authentication, following the redirect to S3 - `richtext.ExtractAttachments()` parses `<bc-attachment>` elements from HTML, extracting href, filename, filesize, content-type, and sgid (excludes mentions) - Shared `writeDownloadToFile` helper consolidates path sanitization and file writing between both download paths - Helper functions: `isStorageURL`, `parseStorageFilename`, `storageToAPIURL`, `downloadStorageURL`
- Use strings.EqualFold for mention content-type comparison - HTML-entity decode extracted attachment attribute values - Handle filepath.Abs errors in writeDownloadToFile - Require https scheme in isStorageURL to prevent token leak - Add 5-minute timeout to HTTP clients - Handle all 3xx redirects (not just 301/302) with relative URL resolution
http.Client.Timeout covers the full body read, but the response body is streamed after downloadStorageURL returns. Large files would hit the deadline. The request context already provides cancellation.
SDK v0.6.0 adds AccountClient.DownloadURL with full hook chain, URL rewriting, auth stripping, and shared fetchSignedDownload helper. Collapse the manual HTTP wiring to a single SDK call and delete downloadStorageURL, storageToAPIURL, parseStorageFilename, and the HTTPTransport/HTTPTimeout fields from App that existed solely to support them.
jeremy
force-pushed
the
jm/files-download-storage-urls
branch
from
92cecac to
500c37a
Compare
Summary
Inline attachments in rich text content (card descriptions, comments, messages) use storage blob URLs that appear in
<bc-attachment>HTML elements. Until now,files downloadonly handled upload IDs — there was no way to download these inline attachments through the CLI.Motivation: when working with recordings via the API (e.g.
basecamp cards show <url> --json), thecontentfield contains<bc-attachment>elements withhrefattributes pointing to storage URLs likehttps://storage.3.basecamp.com/{account}/blobs/{uuid}/download/{filename}. Downloading these required manually rewriting the URL to the API host and adding OAuth headers. This change makes it a single command:No
--inflag needed — the URL is self-contained.How it works: the storage host (
storage.X.basecamp.com) uses cookie-based session auth, which the CLI doesn't have. The command rewrites the URL to the API host (X.basecampapi.com), authenticates with the OAuth bearer token, and follows the redirect to a signed S3 URL. The Authorization header is stripped before following the redirect to avoid leaking the token to S3.Also adds
richtext.ExtractAttachments()— parses<bc-attachment>elements from HTML and extracts href, filename, filesize, content-type, and sgid (excluding mentions). This is useful for any code that needs to find downloadable attachments in rich text content.Changes
internal/commands/files.go—files downloaddetects storage URLs and routes them through the API host. SharedwriteDownloadToFilehelper consolidates path sanitization between both download paths. New helpers:isStorageURL,parseStorageFilename,storageToAPIURL,downloadStorageURL.internal/commands/files_test.go— Tests forisStorageURL,parseStorageFilename,storageToAPIURL.internal/richtext/richtext.go—InlineAttachmentstruct andExtractAttachments()function.internal/richtext/richtext_test.go— Tests forExtractAttachments(empty, no attachments, mention excluded, no-href excluded, single file, mixed).skills/basecamp/SKILL.md— Documents storage URL download in Quick Reference, Download File workflow, and Files resource reference.Summary by cubic
Adds support for storage blob URLs in
files download, so inline rich text attachments download directly without URL rewrites or--in. Also addsrichtext.ExtractAttachments()and moves downloads to the SDK’sDownloadURLfor safer handling.New Features
files downloadaccepts storage URLs (e.g.https://storage.../blobs/.../download/<file>); handled via SDKDownloadURLwhich rewrites to*.basecampapi.com, uses OAuth, follows the signed S3 redirect, and strips Authorization before S3.richtext.ExtractAttachments()parses<bc-attachment>elements and returns href, filename, filesize, content-type, and sgid; skips mentions.Refactors
github.com/basecamp/basecamp-sdk/go v0.6.0, usingAccount().DownloadURLand a shared HTTP transport; removed custom HTTP plumbing.writeDownloadToFilefor unified, safe file writes; hardenedisStorageURL(requires https, validatesstorage.*.basecamp.com) with tests.tools clone: pass optional title viaCloneToolOptionstoTools().Create().Written for commit 500c37a. Summary will update on new commits.