From 2e6ecde61cddc3b23836fe66a5575112d4920f5d Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Tue, 4 Aug 2026 09:03:03 -0400 Subject: [PATCH 1/2] feat(client): route requests through the API gateway when a cloud ID is set A scoped API token -- the kind an Atlassian service account gets -- is rejected with a 401 against the site domain and must go through https://api.atlassian.com/ex/confluence/{cloudId} instead. Basic auth already works with such a token, so this is purely a URL change. Split the client's single base in two: baseURL is where requests go (the gateway when --cloud-id / CONFLUENCE_CLOUD_ID is set, else the site) and siteURL is always the Confluence site. Anything a reader sees resolves against siteURL -- printed page URLs and, critically, the base handed to convert.MdToConfluence, since rewritten links are published into the page itself and would otherwise carry the gateway host permanently. The path suffixes are identical under the gateway, so no call site changes. With no cloud ID both bases are the site and behavior is unchanged, which is what an unscoped personal token and any Data Center site need. New and Resolve take Config/Options structs rather than growing another positional string argument; the fields are URL-ish or secret and would transpose too easily unnamed. resolveNext replaces the raw baseURL + _links.next concatenation. Appending is correct -- next is a site-relative absolute path, and url.ResolveReference would silently drop the /ex/confluence/{cloudId} segment -- but it now also guards the converse, where an echoed prefix would be doubled. Verified with info through the gateway: _links.base returns the site URL, so the existing _links.base-preferring logic needs no inversion, and v1 endpoints work there too (author name lookup succeeded). Refs #52 --- _plans/017_gateway-scoped-tokens.md | 205 ++++++++++++++++++++++++++++ cmd/create/create.go | 13 +- cmd/create/create_test.go | 37 +++++ cmd/fix/fix.go | 7 +- cmd/info/info.go | 9 +- cmd/read/read.go | 5 +- cmd/root.go | 16 ++- cmd/update/update.go | 13 +- cmd/update/update_test.go | 37 +++++ internal/client/client.go | 86 ++++++++++-- internal/client/client_test.go | 87 +++++++++++- internal/client/config.go | 70 ++++++++-- internal/client/config_test.go | 73 +++++++++- 13 files changed, 603 insertions(+), 55 deletions(-) create mode 100644 _plans/017_gateway-scoped-tokens.md diff --git a/_plans/017_gateway-scoped-tokens.md b/_plans/017_gateway-scoped-tokens.md new file mode 100644 index 0000000..cfe50a3 --- /dev/null +++ b/_plans/017_gateway-scoped-tokens.md @@ -0,0 +1,205 @@ +# Plan: gateway base URL for scoped service-account API tokens + +Let markfluence authenticate as an Atlassian **service account** using a **scoped +API token**, so publishing from CI runs as the service account rather than as a +person. + +The blocker is not authentication. A scoped token still uses basic auth with +`email:token` — Atlassian's own [401 KB][401kb] shows exactly that. What breaks is +the **URL**: scoped tokens are rejected (401) against the site domain and must go +through the platform API gateway: + +``` +https://api.atlassian.com/ex/confluence/{cloudId}/wiki/api/v2/pages +``` + +The path suffix is unchanged, so every existing `baseURL + "/wiki/..."` call site +works verbatim once the base moves. The work is therefore: **split the one base URL +markfluence has today into a request base and a site base**, because the site URL is +still needed for content markfluence *writes* (link rewriting) and for URLs it +prints. + +Confirmation this is the right shape: [`pchuri/confluence-cli`][ccli] supports +scoped tokens with no dedicated credential type at all — you just point its +`--domain`/`--api-path` at the gateway and keep basic auth. + +[401kb]: https://support.atlassian.com/atlassian-cloud/kb/401-unauthorized-error-when-service-account-accesses-jira-or-confluence-api/ +[ccli]: https://github.com/pchuri/confluence-cli + +## Out of scope (deliberately) + +- **OAuth 2.0.** Atlassian's service-account OAuth is `client_credentials` (2LO), so + it *is* CI-usable — but the `client_secret` that mints the 60-minute token is + itself long-lived, so it shortens no secret we store in GitHub. It adds a token + endpoint and expiry handling and requires this same gateway split anyway. No + benefit here. +- **Bearer auth.** The bearer value *is* the scoped API token — same secret, different + header. It grants no capability basic auth lacks. Its only real payoff is not + needing the service account's email address. Worth doing later as a ~5-line + `switch` in `send`, but it is not part of what unblocks the service account, so it + lands separately once this is proven against the real token. +- **Cloud-ID auto-discovery.** `/_edge/tenant_info` returns the cloud ID but is not a + supported Atlassian API. Keep the cloud ID explicit config. + +## Decisions locked + +### `--url` stays the site URL; the gateway is derived from a new cloud ID + +The gateway URL cannot simply *replace* `CONFLUENCE_URL`, because the site URL is +still needed for two things — so it stays, and the only new fact is the cloud ID: + +| Setting | Flag | Env / `.env` | +|---|---|---| +| site URL | `--url` | `CONFLUENCE_URL` | +| username | `--username` | `CONFLUENCE_USERNAME` | +| API token | *(none — never a flag)* | `CONFLUENCE_TOKEN` | +| **cloud ID** | **`--cloud-id`** | **`CONFLUENCE_CLOUD_ID`** | + +- With a cloud ID set → requests go to `https://api.atlassian.com/ex/confluence/{cloudId}`. +- With it empty → **behavior is byte-identical to today**. This is the back-compat + guarantee: existing personal-token users and every current test are unaffected. +- **The cloud ID is not a secret.** `https://.atlassian.net/_edge/tenant_info` + returns it unauthenticated (the supported route is + `GET https://api.atlassian.com/oauth/token/accessible-resources`, whose `id` is the + cloud ID). So in CI it belongs in a repo **variable**, not a secret — and it's why it + can be a `--cloud-id` flag while the token deliberately cannot. +- Only **Cloud** sites have a cloud ID, and it identifies the *site*, not the product — + one ID serves Confluence and Jira on the same site. Data Center/Server has neither a + cloud ID nor the gateway, which is a second reason the setting must stay optional. + +Rejected: confluence-cli's flat `--api-path`. It works for them because they are +v1-only; markfluence mixes v1 (`/wiki/rest/api/content/.../child/attachment`, +`/wiki/rest/api/user`) and v2 (`/wiki/api/v2/pages`, `.../properties`), and one path +string cannot express both. + +### The client carries two bases + +`ConfluenceClient` (`internal/client/client.go:53`) gains `siteURL` alongside +`baseURL`, where **`baseURL` becomes strictly the request base**: + +- `baseURL` = gateway when a cloud ID is set, else the site URL. +- `siteURL` = always the site URL. +- New accessor `SiteURL()` beside the existing `BaseURL()`. + +`New` moves from positional args to a `Config` struct (`SiteURL`, `CloudID`, +`Username`, `Token`). Five string positional params — two of them URLs, one a secret +— transpose too easily to leave unnamed, and the struct extends cleanly when bearer +lands. Contained change: `New` has exactly **one** production caller (`Resolve`) and +two test helpers (`internal/client/client_test.go:47,140`). + +`Resolve` likewise moves to a named-field `Options` struct (`URL`, `Username`, +`CloudID`, `EnvFile`) rather than growing to a fourth positional string. Its five +call sites each pull persistent flags by name from cobra (`cmd/update/update.go:67`, +`cmd/create/create.go:94`, `cmd/fix/fix.go:43`, `cmd/info/info.go:44`, +`cmd/read/read.go:62`) and gain a `--cloud-id` pull. + +`send` (`internal/client/client.go:207`) is **untouched** — basic auth is correct for +scoped tokens. + +### Which call sites switch to `SiteURL()` + +**Content-facing — the correctness-critical one.** `MdToConfluence` receives +`c.SiteURL()` at `cmd/update/update.go:159` and `cmd/create/create.go:386`. Nothing +inside `internal/convert` changes; its `baseURL` param just receives the right value. +Getting this wrong writes `api.atlassian.com` links into **published pages** — a +silent, persistent defect that outlives the run, unlike a wrong printed URL. + +**Human-facing.** These already prefer the API's `page.Links.Base` and only fall back +to `BaseURL()`, so they largely self-heal; the fallbacks still move to `SiteURL()`: +`cmd/update/update.go:284,288`, `cmd/create/create.go:485,489`, +`cmd/info/info.go:144,146`, `cmd/fix/fix.go:177`. + +### `_links.next` must not double the gateway prefix + +`ListContentProperties` builds the next page as `c.baseURL + out.Links.Next` +(`internal/client/client.go:659`) — the only pagination site in the client. `next` is +a site-relative absolute path (`/wiki/api/v2/...`), so concatenation is what +*preserves* the `/ex/confluence/{cloudId}` prefix and is correct today. + +The risk is only if the gateway echoes `next` already carrying that prefix, which +would double it. Rather than guess the live behavior, add a small +`resolveNext(base, next)` helper handling three cases: + +1. `next` is absolute (has a scheme) → use as-is. +2. `next` already starts with `base`'s path prefix → prepend scheme+host only. +3. otherwise → concatenate onto `base` (today's behavior). + +Note `url.ResolveReference` is **wrong** here: an absolute-path reference replaces the +whole path, silently dropping `/ex/confluence/{cloudId}`. + +## Verified against the live API + +Smoke-tested with `info` through the gateway using an existing *personal* token +(gateway routing turns out to accept one, which made this testable before the service +account exists): + +- **`_links.base` returns the site URL under the gateway** — confirmed + `https://mozilla-hub.atlassian.net/wiki` from a v2 page fetch via + `api.atlassian.com`. So **no inversion is needed**: the `Links.Base`-preferring + logic in `cmd/info/info.go`, `cmd/update/update.go`, and `cmd/create/create.go` + stays as-is, and printed URLs are correct in both modes. +- **v1 endpoints work through the gateway** — `info` resolved author display names, + which is the v1 `/wiki/rest/api/user` call. + +Still open, and **only answerable with the scoped token**: + +- **The v1 attachment upload.** Atlassian documents classic scopes for v1 vs granular + for v2 while warning against mixing sets, so a scope gap would surface there, and it + is load-bearing for any page with images. A personal token is unscoped, so testing + this now would prove nothing about scopes — it has to wait for the real credential. + First real run should be an `update --dry-run` on a page with an image, then a live + one. + +### Scopes to request for the service account + +markfluence never deletes, so no delete scopes. Request the **classic** set (Atlassian's +recommendation): + +| Calls | Classic | Granular | +|---|---|---| +| GET/POST/PUT `/api/v2/pages` | `read:confluence-content.all`, `write:confluence-content` | `read:page:confluence`, `write:page:confluence` | +| GET `/api/v2/spaces` | `read:confluence-space.summary` | `read:space:confluence` | +| `/api/v2/pages/{id}/properties` (page width) | `read:confluence-props`, `write:confluence-props` | `read:content.property:confluence`, `write:content.property:confluence` | +| `/rest/api/content/{id}/child/attachment` (v1, images) | `write:confluence-file` | `read:attachment:confluence`, `write:attachment:confluence` | +| GET `/rest/api/user` (v1) | `read:confluence-user` | `read:user:confluence` | + +## Schema (`schema/json-output/v1.json`) — no change + +`url` fields already exist (lines 151, 198, 235); only their *values* could change, not +the document shape. `TestSchemaConformance` should stay green — confirm rather than +assume. + +## Testing + +- `internal/client/config_test.go`: cloud-ID resolution through the existing + flag > env > `.env` precedence; a cloud ID containing a slash or a full URL is + rejected with a useful message (guards against a pasted + `https://api.atlassian.com/ex/confluence/...`, which would otherwise 404 opaquely). +- `internal/client/client_test.go`: `New` with a cloud ID targets the gateway path + while `SiteURL()` stays the site; with no cloud ID both equal the site URL + (back-compat). +- `resolveNext`: unit-test all three branches, including the prefix-already-present + case that would otherwise double. +- **A `cmd`-level test that the converter receives the site URL in gateway mode.** + This is the one regression that silently publishes broken links, so it gets a + direct test rather than relying on client-level coverage. + +## Docs + +- `README.md`: config table (31–33), `.env` block (41–43), env-var list (~300), and a + service-account example in the GitHub Actions section (~338) — including that the + gateway needs `CONFLUENCE_CLOUD_ID` while `CONFLUENCE_URL` stays the site URL, + since that pairing is the non-obvious part. In that example `CONFLUENCE_CLOUD_ID` + must be a `vars.` entry, **not** grouped with the `secrets.` ones: it isn't + sensitive, and showing it as a secret teaches the wrong thing to anyone copying the + snippet. Document how to look it up (`_edge/tenant_info`, or + `accessible-resources` for the supported route). +- `.env.example`. +- `CLAUDE.md`: Configuration section, keeping the "token is deliberately never a + flag" note (still true), and record the scope table above so it is not re-derived. + +## Commits + +1. `feat(client): route requests through the API gateway when a cloud ID is set` — + config, two bases, `SiteURL()` at all call sites, `resolveNext`, tests. +2. `docs: document scoped service-account tokens` — README, `.env.example`, CLAUDE.md. diff --git a/cmd/create/create.go b/cmd/create/create.go index 7f9e888..696033f 100644 --- a/cmd/create/create.go +++ b/cmd/create/create.go @@ -90,8 +90,11 @@ func run(cmd *cobra.Command, args []string) error { url, _ := cmd.Flags().GetString("url") username, _ := cmd.Flags().GetString("username") + cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") - c, err := client.Resolve(url, username, envFile) + c, err := client.Resolve(client.Options{ + URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, + }) if err != nil { return fatalFail(err.Error(), jsonout.CodeConfig) } @@ -383,7 +386,9 @@ func createOne(r record, parentID string, c *client.ConfluenceClient, persist bo res := newResult(r) res.parent = nullableStr(parentID) - pageContent, err := convert.MdToConfluence(r.mdfile, c.BaseURL(), r.spaceKey, buildinfo.Stamp()) + // SiteURL, not BaseURL: rewritten links are published into the page, so they + // must point at the site even when requests go through the gateway. + pageContent, err := convert.MdToConfluence(r.mdfile, c.SiteURL(), r.spaceKey, buildinfo.Stamp()) if err != nil { return res.fail(err, jsonout.CodeConvert) } @@ -482,11 +487,11 @@ func resolveWidth(cliPageWidth string, fm map[string]string) (pagewidth.Width, e func pageURL(c *client.ConfluenceClient, page *client.Page, pageID string) string { if page.Links.WebUI == "" { - return fmt.Sprintf("%s/wiki/pages/viewpage.action?pageId=%s", c.BaseURL(), pageID) + return fmt.Sprintf("%s/wiki/pages/viewpage.action?pageId=%s", c.SiteURL(), pageID) } base := page.Links.Base if base == "" { - base = c.BaseURL() + "/wiki" + base = c.SiteURL() + "/wiki" } return base + page.Links.WebUI } diff --git a/cmd/create/create_test.go b/cmd/create/create_test.go index eafbd9e..49c2f00 100644 --- a/cmd/create/create_test.go +++ b/cmd/create/create_test.go @@ -3,6 +3,7 @@ package create import ( "testing" + "github.com/mozilla/markfluence/internal/client" "github.com/mozilla/markfluence/internal/frontmatter" "github.com/mozilla/markfluence/internal/pagewidth" ) @@ -76,3 +77,39 @@ func TestOverrideNeedsSingleFile(t *testing.T) { t.Error("no --title should not trigger the guard") } } + +// In gateway mode the request base is api.atlassian.com, which must never reach a +// reader. Every pageURL branch has to resolve against the site instead. +func TestPageURLUsesSiteNotGateway(t *testing.T) { + c := client.New(client.Config{SiteURL: "https://wiki.example.net", CloudID: "abc-123"}) + + tests := []struct { + name string + base, webu string + want string + }{ + { + "no webui link falls back to the site", + "", "", + "https://wiki.example.net/wiki/pages/viewpage.action?pageId=42", + }, + { + "webui with no base joins onto the site", + "", "/spaces/ENG/pages/42/Title", + "https://wiki.example.net/wiki/spaces/ENG/pages/42/Title", + }, + { + "the API's own base is preferred when present", + "https://wiki.example.net/wiki", "/spaces/ENG/pages/42/Title", + "https://wiki.example.net/wiki/spaces/ENG/pages/42/Title", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + page := &client.Page{ID: "42", Links: client.Links{Base: tc.base, WebUI: tc.webu}} + if got := pageURL(c, page, "42"); got != tc.want { + t.Errorf("pageURL = %q, want %q", got, tc.want) + } + }) + } +} diff --git a/cmd/fix/fix.go b/cmd/fix/fix.go index 9d2c344..ce415aa 100644 --- a/cmd/fix/fix.go +++ b/cmd/fix/fix.go @@ -39,8 +39,11 @@ func init() { func run(cmd *cobra.Command, args []string) error { url, _ := cmd.Flags().GetString("url") username, _ := cmd.Flags().GetString("username") + cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") - c, err := client.Resolve(url, username, envFile) + c, err := client.Resolve(client.Options{ + URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, + }) if err != nil { if ui.IsJSON() { _ = jsonout.EmitError(os.Stderr, "fix", err.Error(), jsonout.CodeConfig) @@ -174,7 +177,7 @@ func locatePage(fm map[string]string, c *client.ConfluenceClient) (*client.Page, fmt.Fprintf(&b, "found %d pages with title %q:", len(matches), title) for _, m := range matches { fmt.Fprintf(&b, "\n - %s: %s (%s/wiki/pages/viewpage.action?pageId=%s)", - m.ID, m.Title, c.BaseURL(), m.ID) + m.ID, m.Title, c.SiteURL(), m.ID) } b.WriteString("\nadd a page_id to the frontmatter to disambiguate") return nil, errors.New(b.String()) diff --git a/cmd/info/info.go b/cmd/info/info.go index 87c8eec..096647e 100644 --- a/cmd/info/info.go +++ b/cmd/info/info.go @@ -40,8 +40,11 @@ func init() { func run(cmd *cobra.Command, args []string) error { url, _ := cmd.Flags().GetString("url") username, _ := cmd.Flags().GetString("username") + cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") - c, err := client.Resolve(url, username, envFile) + c, err := client.Resolve(client.Options{ + URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, + }) if err != nil { return fatalFail(err.Error(), jsonout.CodeConfig) } @@ -141,9 +144,9 @@ type report struct { func buildReport(page *client.Page, c *client.ConfluenceClient, withProps bool) report { url := page.Links.Base + page.Links.WebUI if page.Links.WebUI == "" { - url = fmt.Sprintf("%s/wiki/pages/viewpage.action?pageId=%s", c.BaseURL(), page.ID) + url = fmt.Sprintf("%s/wiki/pages/viewpage.action?pageId=%s", c.SiteURL(), page.ID) } else if page.Links.Base == "" { - url = c.BaseURL() + "/wiki" + page.Links.WebUI + url = c.SiteURL() + "/wiki" + page.Links.WebUI } cache := map[string]string{} diff --git a/cmd/read/read.go b/cmd/read/read.go index 5a4bfbf..b9fade9 100644 --- a/cmd/read/read.go +++ b/cmd/read/read.go @@ -58,8 +58,11 @@ func run(cmd *cobra.Command, args []string) error { url, _ := cmd.Flags().GetString("url") username, _ := cmd.Flags().GetString("username") + cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") - c, err := client.Resolve(url, username, envFile) + c, err := client.Resolve(client.Options{ + URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, + }) if err != nil { return fatalFail(err.Error(), jsonout.CodeConfig) } diff --git a/cmd/root.go b/cmd/root.go index 9ff2422..c61e5c8 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -20,6 +20,7 @@ import ( var ( urlFlag string usernameFlag string + cloudIDFlag string envFileFlag string debugFlag bool noColorFlag bool @@ -31,9 +32,15 @@ var rootCmd = &cobra.Command{ Short: "Publish markdown to Confluence", Long: "markfluence publishes and manipulates Confluence pages from markdown files.\n\n" + "Configuration resolves with the precedence flag > environment variable >\n" + - ".env file. The base URL (--url / CONFLUENCE_URL) and username (--username /\n" + - "CONFLUENCE_USERNAME) may be set any of those ways; the API token\n" + - "(CONFLUENCE_TOKEN) comes only from the environment or .env, never a flag.", + ".env file. The site URL (--url / CONFLUENCE_URL), username (--username /\n" + + "CONFLUENCE_USERNAME), and cloud ID (--cloud-id / CONFLUENCE_CLOUD_ID) may be\n" + + "set any of those ways; the API token (CONFLUENCE_TOKEN) comes only from the\n" + + "environment or .env, never a flag.\n\n" + + "Set the cloud ID to authenticate with a scoped API token, such as one issued\n" + + "to a service account: those tokens are rejected against the site domain and\n" + + "must go through Atlassian's api.atlassian.com gateway. Leave it unset for an\n" + + "unscoped personal token. Find yours at\n" + + "https://YOUR-SITE.atlassian.net/_edge/tenant_info -- it isn't a secret.", // --version prints the build stamp ("markfluence VERSION (SHA, DATE)"), the // same string the converter substitutes for the // token. @@ -102,6 +109,9 @@ func init() { "Confluence base URL (falls back to $CONFLUENCE_URL, then .env)") rootCmd.PersistentFlags().StringVar(&usernameFlag, "username", "", "Confluence username/email (falls back to $CONFLUENCE_USERNAME, then .env)") + rootCmd.PersistentFlags().StringVar(&cloudIDFlag, "cloud-id", "", + "Atlassian cloud ID; set to use a scoped API token via the api.atlassian.com "+ + "gateway (falls back to $CONFLUENCE_CLOUD_ID, then .env)") rootCmd.PersistentFlags().StringVar(&envFileFlag, "env-file", "", "Path to an env file to read (default: ./.env in the working directory)") rootCmd.PersistentFlags().BoolVarP(&debugFlag, "debug", "d", false, diff --git a/cmd/update/update.go b/cmd/update/update.go index 5a551fa..9e68b0f 100644 --- a/cmd/update/update.go +++ b/cmd/update/update.go @@ -63,8 +63,11 @@ func run(cmd *cobra.Command, args []string) error { url, _ := cmd.Flags().GetString("url") username, _ := cmd.Flags().GetString("username") + cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") - c, err := client.Resolve(url, username, envFile) + c, err := client.Resolve(client.Options{ + URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, + }) if err != nil { if ui.IsJSON() { _ = jsonout.EmitError(os.Stderr, "update", err.Error(), jsonout.CodeConfig) @@ -156,7 +159,9 @@ func processFile(filename string, c *client.ConfluenceClient) *updateResult { } } - pageContent, err := convert.MdToConfluence(mf, c.BaseURL(), r.space, buildinfo.Stamp()) + // SiteURL, not BaseURL: rewritten links are published into the page, so they + // must point at the site even when requests go through the gateway. + pageContent, err := convert.MdToConfluence(mf, c.SiteURL(), r.space, buildinfo.Stamp()) if err != nil { return r.fail(err, jsonout.CodeConvert) } @@ -281,11 +286,11 @@ func resolveWidth(cliPageWidth string, mf *frontmatter.MarkdownFile) (pagewidth. func pageURL(c *client.ConfluenceClient, page *client.Page, pageID string) string { if page.Links.WebUI == "" { - return fmt.Sprintf("%s/wiki/pages/viewpage.action?pageId=%s", c.BaseURL(), pageID) + return fmt.Sprintf("%s/wiki/pages/viewpage.action?pageId=%s", c.SiteURL(), pageID) } base := page.Links.Base if base == "" { - base = c.BaseURL() + "/wiki" + base = c.SiteURL() + "/wiki" } return base + page.Links.WebUI } diff --git a/cmd/update/update_test.go b/cmd/update/update_test.go index 4c05e59..9b2cc81 100644 --- a/cmd/update/update_test.go +++ b/cmd/update/update_test.go @@ -3,6 +3,7 @@ package update import ( "testing" + "github.com/mozilla/markfluence/internal/client" "github.com/mozilla/markfluence/internal/frontmatter" "github.com/mozilla/markfluence/internal/pagewidth" ) @@ -91,3 +92,39 @@ func TestOverrideNeedsSingleFile(t *testing.T) { }) } } + +// In gateway mode the request base is api.atlassian.com, which must never reach a +// reader. Every pageURL branch has to resolve against the site instead. +func TestPageURLUsesSiteNotGateway(t *testing.T) { + c := client.New(client.Config{SiteURL: "https://wiki.example.net", CloudID: "abc-123"}) + + tests := []struct { + name string + base, webu string + want string + }{ + { + "no webui link falls back to the site", + "", "", + "https://wiki.example.net/wiki/pages/viewpage.action?pageId=42", + }, + { + "webui with no base joins onto the site", + "", "/spaces/ENG/pages/42/Title", + "https://wiki.example.net/wiki/spaces/ENG/pages/42/Title", + }, + { + "the API's own base is preferred when present", + "https://wiki.example.net/wiki", "/spaces/ENG/pages/42/Title", + "https://wiki.example.net/wiki/spaces/ENG/pages/42/Title", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + page := &client.Page{ID: "42", Links: client.Links{Base: tc.base, WebUI: tc.webu}} + if got := pageURL(c, page, "42"); got != tc.want { + t.Errorf("pageURL = %q, want %q", got, tc.want) + } + }) + } +} diff --git a/internal/client/client.go b/internal/client/client.go index be62c05..eb26973 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -4,6 +4,16 @@ // Requests are built as absolute URLs off the base URL. Pages and content // properties use the Confluence v2 API; attachment writes and the user lookup // use v1 (/wiki/rest/api/...) since v2 doesn't cover them. +// +// A client carries two bases. baseURL is where requests go; siteURL is the +// human-facing site the pages live on. They differ only when a cloud ID selects +// the platform API gateway (see Config): a scoped API token -- the kind a service +// account gets -- is rejected against the site domain and must go through +// https://api.atlassian.com/ex/confluence/{cloudId} instead. The path suffixes are +// identical under the gateway, so every call below is written against baseURL +// unchanged. siteURL exists because the gateway host must never reach a reader: +// it's wrong in printed URLs and, worse, would be written into published page +// content by the converter's link rewriting. package client import ( @@ -49,27 +59,60 @@ const ( // sleep is the backoff pause primitive; a package variable so tests can stub it. var sleep = time.Sleep +// gatewayPrefix is the platform API gateway a scoped token must use, joined with +// the cloud ID to form the request base. +const gatewayPrefix = "https://api.atlassian.com/ex/confluence/" + // ConfluenceClient talks to the Confluence REST API as a single authenticated user. type ConfluenceClient struct { - baseURL string + baseURL string // where requests go: the gateway when a cloud ID is set, else the site + siteURL string // always the Confluence site, for URLs a reader will see username string token string http *http.Client } -// New builds a client for baseURL authenticating as username:token. -func New(baseURL, username, token string) *ConfluenceClient { +// Config holds what a client needs to reach Confluence. The fields are named +// rather than positional because SiteURL and CloudID both address the same site +// and Token is a secret; transposing them would be easy and the failure obscure. +type Config struct { + // SiteURL is the Confluence site, e.g. https://your-org.atlassian.net. + SiteURL string + // CloudID, when set, routes requests through the platform API gateway. Leave + // it empty for site-domain requests, which is what an unscoped personal token + // and any Data Center site need. + CloudID string + // Username is the account the token belongs to (basic auth). + Username string + // Token is the API token. + Token string +} + +// New builds a client from cfg. +func New(cfg Config) *ConfluenceClient { + site := strings.TrimRight(cfg.SiteURL, "/") + base := site + if cfg.CloudID != "" { + base = gatewayPrefix + cfg.CloudID + } return &ConfluenceClient{ - baseURL: strings.TrimRight(baseURL, "/"), - username: username, - token: token, + baseURL: base, + siteURL: site, + username: cfg.Username, + token: cfg.Token, http: &http.Client{}, } } -// BaseURL returns the client's base URL (trailing slash trimmed). +// BaseURL returns the base requests are built off (trailing slash trimmed). This +// is the gateway when a cloud ID is configured, so it must not be shown to a +// reader or written into page content -- use SiteURL for that. func (c *ConfluenceClient) BaseURL() string { return c.baseURL } +// SiteURL returns the Confluence site (trailing slash trimmed), regardless of +// whether requests are routed through the gateway. +func (c *ConfluenceClient) SiteURL() string { return c.siteURL } + // HTTPError is returned when the API responds with a >= 400 status. type HTTPError struct { StatusCode int @@ -637,6 +680,29 @@ func (c *ConfluenceClient) GetContentProperty(pageID, key string) (*Property, er return &out.Results[0], nil } +// resolveNext turns a paginated response's _links.next into an absolute URL +// against base, returning "" when there is no next page. +// +// next is normally a site-relative absolute path ("/wiki/api/v2/..."), so it is +// appended to base -- which is what preserves the gateway's /ex/confluence/{cloudId} +// segment. url.ResolveReference would be wrong here: an absolute-path reference +// replaces the whole path and would silently drop that prefix. The middle branch +// guards the converse, should the gateway ever echo next with the prefix already +// applied, which plain appending would double. +func resolveNext(base, next string) string { + switch { + case next == "": + return "" + case strings.HasPrefix(next, "http://"), strings.HasPrefix(next, "https://"): + return next + } + if u, err := url.Parse(base); err == nil && u.Path != "" && strings.HasPrefix(next, u.Path) { + u.Path, u.RawQuery, u.Fragment = "", "", "" + return strings.TrimRight(u.String(), "/") + next + } + return base + next +} + // ListContentProperties returns all of a page's content properties, following // pagination. func (c *ConfluenceClient) ListContentProperties(pageID string) ([]Property, error) { @@ -655,11 +721,7 @@ func (c *ConfluenceClient) ListContentProperties(pageID string) ([]Property, err } results = append(results, out.Results...) // The next link already carries the cursor and limit as query params. - if out.Links.Next != "" { - rawURL = c.baseURL + out.Links.Next - } else { - rawURL = "" - } + rawURL = resolveNext(c.baseURL, out.Links.Next) params = nil } return results, nil diff --git a/internal/client/client_test.go b/internal/client/client_test.go index f2d1e71..7435afd 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -44,7 +44,7 @@ func newServer(t *testing.T, responses ...resp) (*ConfluenceClient, *scripted) { _, _ = w.Write([]byte(out.body)) })) t.Cleanup(srv.Close) - return New(srv.URL, "u", "t"), s + return New(Config{SiteURL: srv.URL, Username: "u", Token: "t"}), s } func eqStrings(a, b []string) bool { @@ -137,7 +137,7 @@ func countingServer(t *testing.T, handler func(w http.ResponseWriter, n int32)) handler(w, atomic.AddInt32(&n, 1)) })) t.Cleanup(srv.Close) - return New(srv.URL, "u", "t"), &n + return New(Config{SiteURL: srv.URL, Username: "u", Token: "t"}), &n } func TestSendRetriesOn429ThenSucceeds(t *testing.T) { @@ -471,23 +471,25 @@ func TestResolve(t *testing.T) { "CONFLUENCE_TOKEN=file-pass\n"), 0o644); err != nil { t.Fatal(err) } - // Clear any inherited env for a deterministic baseline. + // Clear any inherited env for a deterministic baseline. CONFLUENCE_CLOUD_ID + // matters here too: a stray one would reroute BaseURL to the gateway. t.Setenv("CONFLUENCE_URL", "") t.Setenv("CONFLUENCE_USERNAME", "") t.Setenv("CONFLUENCE_TOKEN", "") + t.Setenv("CONFLUENCE_CLOUD_ID", "") // All from .env. - c, err := Resolve("", "", "") + c, err := Resolve(Options{}) if err != nil || c.BaseURL() != "https://file.example.net" { t.Fatalf("Resolve(.env) = %v, %v", c, err) } // Flag beats env beats .env for the URL. t.Setenv("CONFLUENCE_URL", "https://env.example.net") - if c, _ := Resolve("https://flag.example.net", "", ""); c.BaseURL() != "https://flag.example.net" { + if c, _ := Resolve(Options{URL: "https://flag.example.net"}); c.BaseURL() != "https://flag.example.net" { t.Errorf("flag should win, got %q", c.BaseURL()) } - if c, _ := Resolve("", "", ""); c.BaseURL() != "https://env.example.net" { + if c, _ := Resolve(Options{}); c.BaseURL() != "https://env.example.net" { t.Errorf("env should beat .env, got %q", c.BaseURL()) } @@ -496,7 +498,78 @@ func TestResolve(t *testing.T) { if err := os.WriteFile(".env", []byte("CONFLUENCE_URL=u\nCONFLUENCE_USERNAME=x\n"), 0o644); err != nil { t.Fatal(err) } - if _, err := Resolve("u", "x", ""); err == nil { + if _, err := Resolve(Options{URL: "u", Username: "x"}); err == nil { t.Error("Resolve with no token: want error") } } + +func TestNewGatewayBase(t *testing.T) { + tests := []struct { + name string + site, cloudID string + wantBase, wantSite string + }{ + { + "no cloud ID keeps the site as the request base", + "https://wiki.example.net", "", + "https://wiki.example.net", "https://wiki.example.net", + }, + { + "cloud ID routes requests to the gateway, site unchanged", + "https://wiki.example.net", "abc-123", + gatewayPrefix + "abc-123", "https://wiki.example.net", + }, + { + "trailing slash trimmed from both", + "https://wiki.example.net/", "abc-123", + gatewayPrefix + "abc-123", "https://wiki.example.net", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + c := New(Config{SiteURL: tc.site, CloudID: tc.cloudID}) + if c.BaseURL() != tc.wantBase { + t.Errorf("BaseURL = %q, want %q", c.BaseURL(), tc.wantBase) + } + if c.SiteURL() != tc.wantSite { + t.Errorf("SiteURL = %q, want %q", c.SiteURL(), tc.wantSite) + } + }) + } +} + +func TestResolveNext(t *testing.T) { + const gw = gatewayPrefix + "abc-123" + tests := []struct { + name, base, next, want string + }{ + {"no next page", gw, "", ""}, + { + "site-relative path appends, preserving the gateway prefix", + gw, "/wiki/api/v2/pages/1/properties?cursor=X", + gw + "/wiki/api/v2/pages/1/properties?cursor=X", + }, + { + "prefix already applied is not doubled", + gw, "/ex/confluence/abc-123/wiki/api/v2/pages/1/properties?cursor=X", + gw + "/wiki/api/v2/pages/1/properties?cursor=X", + }, + { + "absolute next is used as-is", + gw, "https://elsewhere.example.net/wiki/api/v2/pages?cursor=X", + "https://elsewhere.example.net/wiki/api/v2/pages?cursor=X", + }, + { + "site base (no cloud ID) appends as before", + "https://wiki.example.net", "/wiki/api/v2/pages/1/properties?cursor=X", + "https://wiki.example.net/wiki/api/v2/pages/1/properties?cursor=X", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := resolveNext(tc.base, tc.next); got != tc.want { + t.Errorf("resolveNext(%q, %q) = %q, want %q", tc.base, tc.next, got, tc.want) + } + }) + } +} diff --git a/internal/client/config.go b/internal/client/config.go index 8a5032f..952301c 100644 --- a/internal/client/config.go +++ b/internal/client/config.go @@ -12,31 +12,51 @@ const ( urlEnv = "CONFLUENCE_URL" usernameEnv = "CONFLUENCE_USERNAME" tokenEnv = "CONFLUENCE_TOKEN" // the API token; never a command-line flag + cloudIDEnv = "CONFLUENCE_CLOUD_ID" dotenvPath = ".env" ) var spaceKeyRE = regexp.MustCompile(`^/spaces/([^/]+)/`) -// Resolve builds a client from the base URL, username, and token. Each value is -// resolved with the precedence flag > environment variable > .env file: the URL -// and username come from the urlFlag/usernameFlag values when set, then -// $CONFLUENCE_URL/$CONFLUENCE_USERNAME, then the .env file; the API token comes -// only from $CONFLUENCE_TOKEN, then .env -- never a flag. envFile selects which -// .env is read: when empty the default ./.env is read best-effort (a missing -// file is fine); when set it's an explicit path that must be readable. It -// returns a friendly error listing whatever is missing. -func Resolve(urlFlag, usernameFlag, envFile string) (*ConfluenceClient, error) { - env, err := loadEnvFile(envFile) +// Options carries the flag values Resolve needs, named so the two URL-ish fields +// can't be transposed at a call site. +type Options struct { + // URL is the --url value (the Confluence site). + URL string + // Username is the --username value. + Username string + // CloudID is the --cloud-id value; set it to route requests through the + // platform API gateway, which a scoped service-account token requires. + CloudID string + // EnvFile is the --env-file value; empty means the default ./.env. + EnvFile string +} + +// Resolve builds a client from the site URL, username, cloud ID, and token. Each +// value is resolved with the precedence flag > environment variable > .env file: +// the URL, username, and cloud ID come from opts when set, then +// $CONFLUENCE_URL/$CONFLUENCE_USERNAME/$CONFLUENCE_CLOUD_ID, then the .env file; +// the API token comes only from $CONFLUENCE_TOKEN, then .env -- never a flag. +// opts.EnvFile selects which .env is read: when empty the default ./.env is read +// best-effort (a missing file is fine); when set it's an explicit path that must +// be readable. It returns a friendly error listing whatever is missing. +// +// The cloud ID is optional: without one, requests go to the site domain exactly +// as before, which is what an unscoped personal token and any Data Center site +// need. +func Resolve(opts Options) (*ConfluenceClient, error) { + env, err := loadEnvFile(opts.EnvFile) if err != nil { return nil, err } - baseURL := resolveValue(urlFlag, urlEnv, env) - username := resolveValue(usernameFlag, usernameEnv, env) + siteURL := resolveValue(opts.URL, urlEnv, env) + username := resolveValue(opts.Username, usernameEnv, env) + cloudID := resolveValue(opts.CloudID, cloudIDEnv, env) token := resolveValue("", tokenEnv, env) var missing []string - if baseURL == "" { + if siteURL == "" { missing = append(missing, "URL (--url or "+urlEnv+")") } if username == "" { @@ -48,7 +68,29 @@ func Resolve(urlFlag, usernameFlag, envFile string) (*ConfluenceClient, error) { if len(missing) > 0 { return nil, errors.New("missing Confluence " + strings.Join(missing, ", ")) } - return New(baseURL, username, token), nil + if err := validateCloudID(cloudID); err != nil { + return nil, err + } + return New(Config{ + SiteURL: siteURL, + CloudID: cloudID, + Username: username, + Token: token, + }), nil +} + +// validateCloudID rejects a cloud ID that looks like a URL or a path fragment. +// The value is joined straight onto the gateway prefix, so pasting a whole +// gateway URL would otherwise produce an opaque 404 rather than a usable error. +func validateCloudID(cloudID string) error { + if cloudID == "" { + return nil + } + if strings.ContainsAny(cloudID, "/:") { + return fmt.Errorf("invalid Confluence cloud ID %q (--cloud-id or %s): expected just the "+ + "identifier, not a URL or path", cloudID, cloudIDEnv) + } + return nil } // resolveValue applies the flag > environment > .env precedence for one setting. diff --git a/internal/client/config_test.go b/internal/client/config_test.go index 2e4872d..9337cba 100644 --- a/internal/client/config_test.go +++ b/internal/client/config_test.go @@ -11,7 +11,7 @@ import ( // the only sources. func clearConfluenceEnv(t *testing.T) { t.Helper() - for _, k := range []string{urlEnv, usernameEnv, tokenEnv} { + for _, k := range []string{urlEnv, usernameEnv, tokenEnv, cloudIDEnv} { t.Setenv(k, "") } } @@ -28,7 +28,7 @@ func writeEnvFile(t *testing.T, body string) string { func TestResolveUsesExplicitEnvFile(t *testing.T) { clearConfluenceEnv(t) path := writeEnvFile(t, "CONFLUENCE_URL=https://wiki\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n") - c, err := Resolve("", "", path) + c, err := Resolve(Options{EnvFile: path}) if err != nil { t.Fatalf("Resolve: %v", err) } @@ -40,7 +40,7 @@ func TestResolveUsesExplicitEnvFile(t *testing.T) { func TestResolveFlagOverridesEnvFile(t *testing.T) { clearConfluenceEnv(t) path := writeEnvFile(t, "CONFLUENCE_URL=https://from-file\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n") - c, err := Resolve("https://from-flag", "", path) + c, err := Resolve(Options{URL: "https://from-flag", EnvFile: path}) if err != nil { t.Fatalf("Resolve: %v", err) } @@ -52,7 +52,7 @@ func TestResolveFlagOverridesEnvFile(t *testing.T) { func TestResolveMissingExplicitEnvFileErrors(t *testing.T) { clearConfluenceEnv(t) missing := filepath.Join(t.TempDir(), "nope.env") - if _, err := Resolve("", "", missing); err == nil { + if _, err := Resolve(Options{EnvFile: missing}); err == nil { t.Fatal("Resolve: want error for a missing --env-file path") } } @@ -62,7 +62,7 @@ func TestResolveDefaultEnvFileMissingIsFine(t *testing.T) { // No ./.env in this temp cwd, and no explicit env file: the missing default // is tolerated, so we fail only on missing config values (not a read error). t.Chdir(t.TempDir()) - _, err := Resolve("", "", "") + _, err := Resolve(Options{}) if err == nil { t.Fatal("want a missing-config error") } @@ -71,3 +71,66 @@ func TestResolveDefaultEnvFileMissingIsFine(t *testing.T) { t.Errorf("error = %q, want a missing-Confluence-config error", err) } } + +func TestResolveCloudIDPrecedence(t *testing.T) { + clearConfluenceEnv(t) + path := writeEnvFile(t, + "CONFLUENCE_URL=https://wiki\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n"+ + "CONFLUENCE_CLOUD_ID=from-file\n") + + // From .env: requests move to the gateway, the site is untouched. + c, err := Resolve(Options{EnvFile: path}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if want := gatewayPrefix + "from-file"; c.BaseURL() != want { + t.Errorf("BaseURL = %q, want %q", c.BaseURL(), want) + } + if c.SiteURL() != "https://wiki" { + t.Errorf("SiteURL = %q, want https://wiki", c.SiteURL()) + } + + // Env beats .env; flag beats env. + t.Setenv(cloudIDEnv, "from-env") + if c, _ := Resolve(Options{EnvFile: path}); c.BaseURL() != gatewayPrefix+"from-env" { + t.Errorf("env should beat .env, got %q", c.BaseURL()) + } + if c, _ := Resolve(Options{CloudID: "from-flag", EnvFile: path}); c.BaseURL() != gatewayPrefix+"from-flag" { + t.Errorf("flag should win, got %q", c.BaseURL()) + } +} + +func TestResolveRejectsURLishCloudID(t *testing.T) { + clearConfluenceEnv(t) + path := writeEnvFile(t, "CONFLUENCE_URL=https://wiki\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n") + + // Pasting a whole gateway URL (or any path fragment) is the likely mistake; + // it must fail with a usable message rather than a 404 at request time. + for _, bad := range []string{ + "https://api.atlassian.com/ex/confluence/abc", + "ex/confluence/abc", + "abc/wiki", + } { + _, err := Resolve(Options{CloudID: bad, EnvFile: path}) + if err == nil { + t.Errorf("Resolve(cloud ID %q): want an error", bad) + continue + } + if !strings.Contains(err.Error(), "invalid Confluence cloud ID") { + t.Errorf("Resolve(cloud ID %q) error = %q, want the invalid-cloud-ID message", bad, err) + } + } +} + +func TestResolveWithoutCloudIDKeepsSiteURL(t *testing.T) { + clearConfluenceEnv(t) + path := writeEnvFile(t, "CONFLUENCE_URL=https://wiki/\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n") + c, err := Resolve(Options{EnvFile: path}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + // No cloud ID: both bases are the site, exactly as before the gateway existed. + if c.BaseURL() != "https://wiki" || c.SiteURL() != "https://wiki" { + t.Errorf("BaseURL/SiteURL = %q/%q, want https://wiki for both", c.BaseURL(), c.SiteURL()) + } +} From ee80a88740858321a9e11b4b158a6ab0d2b05063 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Tue, 4 Aug 2026 09:03:11 -0400 Subject: [PATCH 2/2] docs: document scoped service-account tokens Add a "Scoped tokens and service accounts" section covering why a scoped token needs CONFLUENCE_CLOUD_ID, how to find the cloud ID, and the scopes markfluence requires. Note in the GitHub Actions section that the cloud ID belongs in a repository variable rather than a secret -- it isn't sensitive. Refs #52 --- .env.example | 7 +++++++ CLAUDE.md | 11 ++++++++--- README.md | 49 +++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 62 insertions(+), 5 deletions(-) diff --git a/.env.example b/.env.example index 01fbcdc..ad45d1d 100644 --- a/.env.example +++ b/.env.example @@ -2,3 +2,10 @@ CONFLUENCE_URL=https://your-org.atlassian.net CONFLUENCE_USERNAME=you@example.com CONFLUENCE_TOKEN=your-api-token + +# Optional. Set this only for a *scoped* API token, such as one issued to a +# service account: those must go through Atlassian's api.atlassian.com gateway +# rather than your site domain. Leave it out for a normal personal token. +# Find yours (it isn't a secret): +# curl -s https://your-org.atlassian.net/_edge/tenant_info +#CONFLUENCE_CLOUD_ID= diff --git a/CLAUDE.md b/CLAUDE.md index 8ee7b87..dbbcb4b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -19,13 +19,18 @@ Run `make test && make lint && make vet` before considering work done. ## Configuration -The CLI needs a base URL, a username, and an API token. Each resolves with the precedence **flag > environment variable > `.env` file**: +The CLI needs a site URL, a username, and an API token, plus an optional cloud ID. Each resolves with the precedence **flag > environment variable > `.env` file**: | Setting | Flag | Env / `.env` | |---|---|---| -| base URL | `--url` | `CONFLUENCE_URL` | +| site URL | `--url` | `CONFLUENCE_URL` | | username | `--username` | `CONFLUENCE_USERNAME` | | API token | *(none)* | `CONFLUENCE_TOKEN` | +| cloud ID (optional) | `--cloud-id` | `CONFLUENCE_CLOUD_ID` | + +A cloud ID routes requests through `https://api.atlassian.com/ex/confluence/{cloudId}` instead of the site domain, which is mandatory for a **scoped** API token (what an Atlassian service account gets — a scoped token 401s against the site domain). Basic auth is unchanged; only the base URL moves. Without a cloud ID, behavior is identical to before it existed, which is what an unscoped personal token and any Data Center site need. The cloud ID is not a secret (`https://SITE/_edge/tenant_info` returns it unauthenticated), which is why it may be a flag while the token may not. + +Scopes markfluence needs (it never deletes): `read:confluence-content.all`, `write:confluence-content`, `read:confluence-space.summary`, `read:confluence-props`, `write:confluence-props`, `write:confluence-file`, `read:confluence-user`. markfluence reads a `.env` from the working directory itself (a minimal built-in parser — no shell expansion), or an explicit path via the persistent `--env-file` flag (a missing explicit path is an error; a missing default `./.env` is not); `.env.example` is the template. The API token is deliberately never a command-line flag. `internal/client.Resolve` is the single place this is read and validated. @@ -43,7 +48,7 @@ Module `github.com/mozilla/markfluence` (`go 1.25`). `main.go` is a shim to `cmd - `cmd/root.go` — the cobra root: `--url`/`--username`/`--debug`/`--no-color` persistent flags, version from `internal/buildinfo`, and registration of the four subcommands. `Execute()` prints cobra-generated errors (bad args/flags) but not `ui.ErrSilent`, which marks a failure a command already reported. - `cmd/{update,create,fix,info}/` — one package per command (each exports `Cmd`), orchestrating the `internal` packages and `internal/ui` output. `create` is two-phase and transactional (validate all, then create parents-first in topological order); `fix` is read-only on the server. -- `internal/client` — `ConfluenceClient` over `net/http` with basic auth. Pages are Confluence **v2**; attachment writes and the user lookup are **v1** (`/wiki/rest/api/...`). Typed `HTTPError`, per-attempt context timeouts, centralized retry/backoff in `send` (429 for any method honoring `Retry-After`, plus 502/503/504 and network errors for idempotent methods only; exponential backoff capped), `SetContentProperty` retry-once on top (recovers a lost create-POST response), `SyncAttachments` (SHA-256-in-comment skip/update), `_links.next` pagination. `config.go` holds `Resolve` and the `.env` reader. +- `internal/client` — `ConfluenceClient` over `net/http` with basic auth. Built from a `Config` (site URL, cloud ID, username, token) via `New`; it carries **two bases**: `BaseURL()` is where requests go (the gateway when a cloud ID is set) and `SiteURL()` is always the site. Anything a reader sees uses `SiteURL()` — printed page URLs and, critically, the `baseURL` handed to `convert.MdToConfluence`, since rewritten links are published *into* the page. Pages are Confluence **v2**; attachment writes and the user lookup are **v1** (`/wiki/rest/api/...`). Typed `HTTPError`, per-attempt context timeouts, centralized retry/backoff in `send` (429 for any method honoring `Retry-After`, plus 502/503/504 and network errors for idempotent methods only; exponential backoff capped), `SetContentProperty` retry-once on top (recovers a lost create-POST response), `SyncAttachments` (SHA-256-in-comment skip/update), `_links.next` pagination. `config.go` holds `Resolve` and the `.env` reader. - `internal/convert` — the converter (the crux). `MdToConfluence(md *frontmatter.MarkdownFile, baseURL, spaceKey, version string) (*ConfluencePage, error)`. It parses with goldmark (GFM) and renders through a custom `storageRenderer` registered at priority 100 (below the default HTML=1000 and table=500 renderers) that emits Confluence storage format. `shield.go` renames raw `ac:`/`ri:` tags to colon-free sentinels around the goldmark step so pasted storage passes through; `callouts.go` is an AST transformer + blockquote renderer for GitHub alerts; `images.go`, `links.go` (sibling-file scans, GitHub/Confluence slugs, doc-link + anchor rewriting), `tables.go` (the `` tag only, stamped with Confluence's `data-layout="align-start"` so tables auto-size to their content and left-align; rows and cells still fall through to the GFM renderer), and `renderer.go` (code macros, text soft-break→space, images, links) do the rest. The `` and `` token substitutions happen **inside** `MdToConfluence`. - `internal/frontmatter` — flat YAML frontmatter parse/quote/`UpdateField`, and the `MarkdownFile` type (`Parse`/`ParseFile`, exported `Filename`/`Content`/`Frontmatter`/`Body`, and `Title`/`PageID`/`Space`/`Parent` accessors that normalize missing/blank/`"null"`). - `internal/pagewidth` — the `page_width` `Width` enum (`narrow`/`wide`/`max`, default `max`), `Declared`, the vocab↔content-property maps, `WidthFromProperties`, and `Apply`/`Read` against the client. diff --git a/README.md b/README.md index 777da17..ed451ac 100644 --- a/README.md +++ b/README.md @@ -23,14 +23,15 @@ TBD — published to a tap on the first release. ## Configure -markfluence needs a base URL, a username, and an API token. Each is resolved +markfluence needs a site URL, a username, and an API token. Each is resolved with the precedence **flag > environment variable > `.env` file**: | Setting | Flag | Environment / `.env` | | --- | --- | --- | -| Base URL | `--url` | `CONFLUENCE_URL` | +| Site URL | `--url` | `CONFLUENCE_URL` | | Username | `--username` | `CONFLUENCE_USERNAME` | | API token | *(none — never a flag)* | `CONFLUENCE_TOKEN` | +| Cloud ID *(optional)* | `--cloud-id` | `CONFLUENCE_CLOUD_ID` | markfluence reads a `.env` file from the current directory automatically (no need to `source` it), or from an explicit path via `--env-file PATH` (a persistent flag @@ -48,6 +49,40 @@ from the environment or `.env`. (Optional): `alias mf=markfluence` +### Scoped tokens and service accounts + +For a normal personal API token, you can leave `CONFLUENCE_CLOUD_ID` unset. + +For a **scoped** API token for an Atlassian [service account][svcacct], you +need to set `CONFLUENCE_CLOUD_ID`. You would use this to publish from CI as a +service account rather than as a person. Scoped tokens are rejected with a +**401** against your site domain; they must go through Atlassian's +`api.atlassian.com` gateway, and the cloud ID is what addresses your site +there. `CONFLUENCE_URL` still holds the site URL: markfluence needs it to write +correct links into the pages it publishes. + +Find your cloud ID — it is **not** a secret: + +```console +$ curl -s https://your-org.atlassian.net/_edge/tenant_info +{"cloudId":"d8febd08-5555-5555-5555-db37c2369ce5"} +``` + +The scopes markfluence needs: + +| Used for | Classic scope | +| --- | --- | +| Reading, creating, and updating pages | `read:confluence-content.all`, `write:confluence-content` | +| Resolving space keys | `read:confluence-space.summary` | +| Page width (content properties) | `read:confluence-props`, `write:confluence-props` | +| Image attachments | `write:confluence-file` | +| Author names in `info` | `read:confluence-user` | + +Currently, markfluence doesn't support deleting anything, so it doesn't need +delete scopes. This might change in the future. + +[svcacct]: https://support.atlassian.com/user-management/docs/understand-service-accounts/ + ## Usage ```sh @@ -301,6 +336,13 @@ markfluence reads them straight from the environment — no `.env` in CI. - `CONFLUENCE_URL` - `CONFLUENCE_USERNAME` +Prefer a [service account][svcacct] over a personal token here, so published pages +aren't authored by an individual and publishing doesn't break when that person +rotates their token or moves on. That means a **scoped** token, which also needs +`CONFLUENCE_CLOUD_ID` (see [Scoped tokens and service +accounts](#scoped-tokens-and-service-accounts)). The cloud ID is not sensitive, so +make it a repository **variable** rather than a secret. + [secrets]: https://docs.github.com/en/actions/security-guides/using-secrets-in-github-actions ### Workflow @@ -338,6 +380,9 @@ jobs: CONFLUENCE_URL: ${{ secrets.CONFLUENCE_URL }} CONFLUENCE_USERNAME: ${{ secrets.CONFLUENCE_USERNAME }} CONFLUENCE_TOKEN: ${{ secrets.CONFLUENCE_TOKEN }} + # A variable, not a secret: the cloud ID is public. Omit it if you're + # using an unscoped personal token. + CONFLUENCE_CLOUD_ID: ${{ vars.CONFLUENCE_CLOUD_ID }} run: markfluence update --page-id=12345 --force docs/some_doc.md ```