From 68c1e3e0e6bef821cd34e7850cfd3f8a9a3d97bc Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Mon, 7 Sep 2026 11:25:40 -0400 Subject: [PATCH 1/8] feat(ui): add SecurityWarn, the one warning --json does not silence Every helper here is a no-op in JSON mode on the premise stated at jsonMode: the content is carried in the structured payload instead. That premise does not hold for a warning about the caller's credentials -- it names neither a page nor a file, so no results entry can carry it, and the envelope has no top-level warnings field. Routed through Warn it would disappear in exactly the automated runs most likely to have a world-readable .env. Named narrowly on purpose. This is a hole in "stdout is the payload and everything else is quiet under --json", so it is for credential hygiene and nothing else; ordinary warnings belong in the payload, which is what Warn enforces. Stderr was never part of the JSON contract. Refs #136 --- internal/ui/ui.go | 18 +++++++++++++ internal/ui/ui_test.go | 61 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 internal/ui/ui_test.go diff --git a/internal/ui/ui.go b/internal/ui/ui.go index 159bfe8..5ae7743 100644 --- a/internal/ui/ui.go +++ b/internal/ui/ui.go @@ -134,6 +134,24 @@ func Hint(msg string) { fmt.Fprintln(os.Stderr, "\n "+msg) } +// SecurityWarn prints a warning to stderr that is *not* silenced in JSON mode. +// +// Every other helper here goes quiet under --json on the premise stated at +// jsonMode: the content is carried in the structured payload instead. That is +// false for a warning about the caller's credentials -- it concerns neither a +// file nor a page, so no results entry can carry it, and the envelope has no +// top-level warnings field. Routed through Warn it would disappear in exactly +// the automated runs most likely to have a world-readable .env. +// +// Deliberately narrow, and named to stay that way: this is a hole in the rule +// that stdout is the payload and everything else is quiet under --json, so it +// is for a credential-hygiene warning and nothing else. Ordinary warnings +// belong in the payload, which is what Warn enforces. Stderr is never part of +// the JSON contract, so nothing a consumer parses is affected. +func SecurityWarn(msg string) { + fmt.Fprintln(os.Stderr, yellow.Render(" ! ")+msg) +} + // Dim prints a dimmed line. No-op in JSON mode. func Dim(msg string) { if jsonMode { diff --git a/internal/ui/ui_test.go b/internal/ui/ui_test.go new file mode 100644 index 0000000..d711798 --- /dev/null +++ b/internal/ui/ui_test.go @@ -0,0 +1,61 @@ +package ui + +import ( + "io" + "os" + "strings" + "testing" +) + +// captureStderr runs fn with os.Stderr redirected, returning what it wrote. +func captureStderr(t *testing.T, fn func()) string { + t.Helper() + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + old := os.Stderr + os.Stderr = w + fn() + os.Stderr = old + if err := w.Close(); err != nil { + t.Fatal(err) + } + out, err := io.ReadAll(r) + if err != nil { + t.Fatal(err) + } + return string(out) +} + +// TestSecurityWarnIsNotSilencedByJSONMode is the whole reason this helper +// exists rather than reusing Warn. Every other helper here goes quiet under +// --json because its content is carried in the structured payload instead; a +// credential warning has no payload to be carried in, so silencing it would +// drop it in exactly the automated runs most likely to need it. A refactor +// that makes JSON mode silence everything should fail here. +func TestSecurityWarnIsNotSilencedByJSONMode(t *testing.T) { + SetJSON(true) + t.Cleanup(func() { SetJSON(false) }) + + out := captureStderr(t, func() { SecurityWarn(".env is readable by others") }) + if !strings.Contains(out, ".env is readable by others") { + t.Errorf("stderr = %q, want the warning even in JSON mode", out) + } + + // The contrast: ordinary warnings stay silent, so this is a deliberate + // exception and not a hole in the rule. + quiet := captureStderr(t, func() { Warn("an ordinary warning") }) + if quiet != "" { + t.Errorf("Warn wrote %q under --json, want nothing", quiet) + } +} + +// TestSecurityWarnWritesToStderr keeps it off stdout, which under --json is a +// JSON document and under human output may be a table on its way into a pipe. +func TestSecurityWarnWritesToStderr(t *testing.T) { + out := captureStderr(t, func() { SecurityWarn("mind the mode") }) + if !strings.Contains(out, "mind the mode") { + t.Errorf("stderr = %q, want the warning", out) + } +} From 8bd45a6eaacabc4e98962a90b39e8337175806ff Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Mon, 7 Sep 2026 11:25:41 -0400 Subject: [PATCH 2/8] feat(client): warn when the .env holding the token is readable by others The .env reader handed back whatever it found without looking at the file's mode, so a .env left at the 0644 an editor or a redirect produces made the API token readable by every account on the machine. The token is deliberately never a flag because it is the one value that must not be casually visible; the file it lives in is the thing to check. Two halves to the rule. The mode: any group or other bit set (mode.Perm()&0o077), so 0600 and the more restrictive 0400 stay quiet and the user execute bit is ignored. And the file must actually contain CONFLUENCE_TOKEN -- a .env holding only the URL and username leaks nothing (the cloud ID is documented as not a secret either), and a warning that fires on a file with no secret in it is how a security warning becomes something people learn to scroll past. loadDotenv has already parsed the file, so the gate is free. It stats rather than lstats: a .env symlinked to a 0600 file is safe, and the link's own 0777 would cry wolf every run. It lives in loadDotenv because that is the one function both the discovered .env and an explicit --env-file pass through. And it reaches the reader through a package-level hook wired beside SetRetryLogger, for the reason that one documents -- twelve commands build a client through Resolve with an identical literal, and internal/client produces no output. A group- or world-writable .env with no token in it is knowingly not covered, though CONFLUENCE_URL resolves from there too and rewriting it would redirect the token to another host. Recorded in #136. Refs #136 --- cmd/root.go | 2 + internal/client/config.go | 48 ++++++++++++ internal/client/config_test.go | 132 +++++++++++++++++++++++++++++++++ 3 files changed, 182 insertions(+) diff --git a/cmd/root.go b/cmd/root.go index b726600..b51bd52 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -67,6 +67,8 @@ var rootCmd = &cobra.Command{ ui.SetDebug(debugFlag) ui.SetJSON(jsonFlag) client.SetRetryLogger(logRetry) + // Not silenced under --json: see ui.SecurityWarn. + client.SetSecurityWarner(ui.SecurityWarn) return nil }, // Bare `markfluence` prints help; subcommands carry the work. diff --git a/internal/client/config.go b/internal/client/config.go index b275d64..bd775a1 100644 --- a/internal/client/config.go +++ b/internal/client/config.go @@ -161,6 +161,50 @@ func loadEnvFile(envFile string, roots *project.Cache) (map[string]string, error return env, nil } +// securityWarner receives a credential-hygiene warning. Package-level and set +// once from the command layer for the same reason SetRetryLogger is +// (retrylog.go): twelve commands build a client through Resolve with an +// identical literal, so anything passed per-call is something the thirteenth +// silently forgets -- and internal/client deliberately produces no output and +// imports no ui. +var securityWarner func(string) + +// SetSecurityWarner installs fn as the credential-hygiene reporter, replacing +// any previous one. Pass nil to silence it. +func SetSecurityWarner(fn func(string)) { securityWarner = fn } + +// warnLoosePermissions reports a .env that anyone but its owner can reach, +// when that file is the one holding the API token. +// +// The token gate is what keeps this worth reading. A .env carrying only +// CONFLUENCE_URL and CONFLUENCE_USERNAME at 0644 leaks nothing -- neither is a +// secret, and the cloud ID is documented as not one either -- and a warning +// that fires on a file with no secret in it is how a security warning becomes +// something people learn to scroll past. +// +// os.Stat, not Lstat: a .env symlinked to a 0600 file is perfectly safe, and +// the link's own 0777 would cry wolf on every run. The user execute bit is +// ignored for the same reason -- 0700 is odd, but it is not a leak. +// +// A stat failure is silent. The file was just read, so a failure here is +// exotic, and a warning about the inability to warn is noise. +func warnLoosePermissions(path string, env map[string]string) { + if securityWarner == nil || env[tokenEnv] == "" { + return + } + fi, err := os.Stat(path) + if err != nil { + return + } + perm := fi.Mode().Perm() + if perm&0o077 == 0 { + return + } + securityWarner(fmt.Sprintf( + "%s is readable by others (mode %#o) and holds your API token; run: chmod 600 %s", + path, perm, path)) +} + // loadDotenv reads a simple .env file into a map: KEY=value lines, with blank // lines and # comments skipped, an optional leading "export ", and optional // surrounding single or double quotes stripped. Values are taken verbatim (no @@ -183,6 +227,10 @@ func loadDotenv(path string) (map[string]string, error) { } out[strings.TrimSpace(key)] = unquote(strings.TrimSpace(value)) } + // Here rather than in loadEnvFile: this is the one function both the + // discovered .env and an explicit --env-file go through, and the check + // needs the parsed contents to know whether a token is in there. + warnLoosePermissions(path, out) return out, nil } diff --git a/internal/client/config_test.go b/internal/client/config_test.go index cd188c3..6660bc4 100644 --- a/internal/client/config_test.go +++ b/internal/client/config_test.go @@ -225,3 +225,135 @@ func TestSpaceKeyFromWebUI(t *testing.T) { t.Errorf("space = %q, want empty for an empty webui", got) } } + +// captureSecurityWarnings installs a recording warner for the duration of a +// test, restoring whatever was there before -- the hook is package-level, so a +// test that leaks one changes the next test's behavior. +func captureSecurityWarnings(t *testing.T) *[]string { + t.Helper() + var got []string + prev := securityWarner + securityWarner = func(msg string) { got = append(got, msg) } + t.Cleanup(func() { securityWarner = prev }) + return &got +} + +// TestWarnLoosePermissions covers both halves of the rule: the mode, and the +// token gate that keeps the warning off files with no secret in them. +func TestWarnLoosePermissions(t *testing.T) { + const withToken = "CONFLUENCE_URL=https://wiki\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n" + const noToken = "CONFLUENCE_URL=https://wiki\nCONFLUENCE_USERNAME=bot\n" + + tests := []struct { + name string + body string + mode os.FileMode + want bool + }{ + {"world-readable with a token", withToken, 0o644, true}, + {"group-readable with a token", withToken, 0o640, true}, + {"world-writable with a token", withToken, 0o622, true}, + {"wide open with a token", withToken, 0o666, true}, + {"owner-only", withToken, 0o600, false}, + // More restrictive than required, not less: warning here would be + // nonsense. + {"owner read-only", withToken, 0o400, false}, + // The user execute bit is odd but leaks nothing. + {"owner rwx", withToken, 0o700, false}, + // The gate: no secret in the file, so its mode is nobody's business. + {"world-readable without a token", noToken, 0o644, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := captureSecurityWarnings(t) + path := filepath.Join(t.TempDir(), ".env") + if err := os.WriteFile(path, []byte(tt.body), tt.mode); err != nil { + t.Fatal(err) + } + // WriteFile applies the umask, so set the mode explicitly. + if err := os.Chmod(path, tt.mode); err != nil { + t.Fatal(err) + } + if _, err := loadDotenv(path); err != nil { + t.Fatalf("loadDotenv: %v", err) + } + if fired := len(*got) > 0; fired != tt.want { + t.Errorf("warned = %v, want %v (%v)", fired, tt.want, *got) + } + }) + } +} + +// TestWarnLoosePermissionsMessage pins what the reader is told: the path, the +// mode in the form chmod takes, why it matters, and the exact remedy. +func TestWarnLoosePermissionsMessage(t *testing.T) { + got := captureSecurityWarnings(t) + path := filepath.Join(t.TempDir(), ".env") + if err := os.WriteFile(path, []byte("CONFLUENCE_TOKEN=secret\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chmod(path, 0o644); err != nil { + t.Fatal(err) + } + if _, err := loadDotenv(path); err != nil { + t.Fatalf("loadDotenv: %v", err) + } + if len(*got) != 1 { + t.Fatalf("warnings = %v, want exactly one", *got) + } + for _, want := range []string{path, "mode 0644", "holds your API token", "chmod 600 " + path} { + if !strings.Contains((*got)[0], want) { + t.Errorf("message %q missing %q", (*got)[0], want) + } + } +} + +// TestWarnLoosePermissionsFollowsASymlink is why the check stats rather than +// lstats: a link's own mode is 0777 on every system that has them, so lstat +// would warn about a target that is perfectly safe. +func TestWarnLoosePermissionsFollowsASymlink(t *testing.T) { + got := captureSecurityWarnings(t) + dir := t.TempDir() + target := filepath.Join(dir, "real.env") + if err := os.WriteFile(target, []byte("CONFLUENCE_TOKEN=secret\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.Chmod(target, 0o600); err != nil { + t.Fatal(err) + } + link := filepath.Join(dir, ".env") + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + if _, err := loadDotenv(link); err != nil { + t.Fatalf("loadDotenv: %v", err) + } + if len(*got) != 0 { + t.Errorf("warnings = %v, want none: the link points at a 0600 file", *got) + } +} + +// TestResolveWarnsThroughTheDiscoveredEnvFile exercises the real path a command +// takes -- Resolve, not loadDotenv -- so the check cannot be wired only to the +// explicit --env-file branch. +func TestResolveWarnsThroughTheDiscoveredEnvFile(t *testing.T) { + clearConfluenceEnv(t) + got := captureSecurityWarnings(t) + dir := t.TempDir() + path := filepath.Join(dir, ".env") + body := "CONFLUENCE_URL=https://wiki\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n" + if err := os.WriteFile(path, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chmod(path, 0o644); err != nil { + t.Fatal(err) + } + t.Chdir(dir) + + if _, err := Resolve(ResolveOptions{}); err != nil { + t.Fatalf("Resolve: %v", err) + } + if len(*got) != 1 { + t.Errorf("warnings = %v, want exactly one", *got) + } +} From 8def8560a3196840e75d7ca9aba8b5e4946cf49d Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Mon, 7 Sep 2026 11:25:41 -0400 Subject: [PATCH 3/8] docs: document the .env permission expectation README's configuration section: chmod 600 .env after copying the example, what the warning covers, and why it survives --json when nothing else does. .env.example says it at the point someone copies the file. CLAUDE.md records both halves of the rule, the stat-not-lstat choice, and the writable case left uncovered. Refs #136 --- .env.example | 2 ++ CLAUDE.md | 4 ++-- README.md | 15 +++++++++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/.env.example b/.env.example index ad45d1d..da1cf73 100644 --- a/.env.example +++ b/.env.example @@ -1,4 +1,6 @@ # Copy to .env and fill in. Required by markfluence. +# Then: chmod 600 .env -- it holds your API token, and markfluence warns if +# anyone else can read or write it. CONFLUENCE_URL=https://your-org.atlassian.net CONFLUENCE_USERNAME=you@example.com CONFLUENCE_TOKEN=your-api-token diff --git a/CLAUDE.md b/CLAUDE.md index bb8c8ae..5fc5eb1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -64,14 +64,14 @@ Module `github.com/mozilla/markfluence` (`go 1.25`). `main.go` is a shim to `cmd - `internal/attachfile` — `Resolve` (where an attachment goes under a destination root, **including the traversal clamp**) and `Write` (download it there, honoring force/dry-run). Shared by `attachment-download` and `export`; the clamp must never exist in two copies. - `internal/pagetree` — `Walk`, the traversal of pages *and folders* under a node, plus `WalkSpace` (the same traversal seeded from a space's root pages, via `client.ListSpaceRootPages`) and `AllDepths`. Both go through one `walker`, so the depth rule and the visited guard exist in a single copy. It is a package rather than command-local because listing a subtree and exporting one (#59) need the identical walk, and its rules must not exist in two copies: siblings arrive from two requests (`/child/page`, `/child/folder`) and are **merged by `extensions.position`**, or the output loses the order Confluence displays; a folder **counts as a level** like a page, which is only reasonable because folders are reported rather than silently traversed; and the walk descends folders even when only pages matter, since a folder may hold the only pages in a subtree. `nodeURL` uses `SiteURL()` — a v1 child row carries `webui` but no `base`. A visited set guards the unbounded case. - `internal/pageref` — `Resolve`, the single page-argument resolver: a numeric id, a Confluence page **or folder** URL (`pagePathRE` matches both `/pages/` and `/folder/`, since `children` takes a folder and a folder URL is what a browser hands you — the id is all it returns, so a command that can only use a page reports its own not-found), or a `.md` file whose frontmatter has a `page_id` (stat'd first, so `123.md` is a file). Every command taking a page uses it. `message.go` also owns the wording for the two ways a *frontmatter* `page_id` is wrong — `NotFoundMessage` (caller supplies the remedy, which differs per command) and `NotNumericMessage` — because `create`, `update`, and `fix` all report them and a reader should recognize the same problem across all three. They return strings, not errors: `create` wraps the text in its typed `pageIDFailure` (which also carries the `--json` fields), the others want a plain error. Anything checking a `page_id` before a request uses `IsDigits`, since the API answers a non-numeric id with a 400 whose body says nothing useful. -- `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/...`). A **folder** — the Cloud content type that can parent a page — has its own v2 route, `GetFolderOrNil` against `/wiki/api/v2/folders/{id}`, because every v2 *page* route answers a folder id with 404; enumerating children, if it is ever added, must be v1, since v2 cannot list inside a folder at all and its page-children route silently omits folders ([docs/confluence/folders.md](docs/confluence/folders.md)). Typed `HTTPError`, per-attempt context timeouts, centralized retry/backoff in `send`. `HTTPError.Error()` appends a **hint** for the three auth failures whose status misleads, matched on the response *body* rather than deduced from the status and always **appended** to it, never replacing it. The one that matters: **a rejected credential is a 404 on every v2 route**, so a revoked token used to make `read` answer `page ... not found` about a page that exists. `RejectedCredential` tells it apart by the fact that every genuine v2 404 *names* what it could not find and the auth one does not, `notFound` gates the three `…OrNil` helpers on it so they stop reading it as "absent", and `jsonout.CodeFor` checks it before the status switch so `--json` reports `AUTH` rather than `NOT_FOUND`. **Two error types on the request path, and one predicate for them**: an `*HTTPError` once a response has a status, an unexported `requestError` when there is none (a transport failure, a request that would not build, a body that would not decode), and `FromRequest` answers whether an error is either. That is what lets a caller tell a server failure from a local one — `jsonout.CodeOr(err, fallback)` is the whole point of it, since `CodeFor` alone reports every non-`HTTPError` as `NETWORK` and so turns `no title given` into a network problem (#133). The rule is deliberately scoped to the request: `DownloadAttachment` writing to the caller's writer, `uploadAttachment` opening the caller's file, and `Resolve` reading the environment stay untyped, because tagging them would misreport an unreadable file as a network failure. The wrapper carries no message of its own, so `Error()` is the inner text verbatim and nothing a reader sees changed. A 403 that is not one of the two measured credential phrasings gets no hint, because that is what a genuine permission denial looks like ([docs/confluence/api.md](docs/confluence/api.md#scopes)). **Retry rules**: 429 for any method; 502/503/504 for idempotent methods; **any other 5xx only when the response carries `Retry-After`** — that is how a 500 becomes retryable, and it is why `parseRetryAfter` reports the header's *presence* apart from its delay (`Retry-After: 0` means "retry now", not "no header"). The exponential delay is jittered, a server-supplied `Retry-After` never is. Decisions go to a package-level hook (`SetRetryLogger`, set once in `root.go` beside `ui.SetDebug`) and fire whichever way they went, because `internal/client` prints nothing and a silent twelve-minute retry storm is otherwise indistinguishable from a hang. **A versioned PUT is not as idempotent as its method**: `SetContentProperty` retry-once on top (recovers a lost create-POST response) and `UpdatePage`'s `updateLanded` both exist for the same reason — a write whose response was lost gets re-sent, and the re-sent version is refused. `updateLanded` requires version *and* title *and* body to match what was sent, since a concurrent edit could have produced the version alone and claiming success over someone else's content is worse than a false failure ([docs/confluence/api.md](docs/confluence/api.md)). `SyncAttachments` (skip/update by a SHA-256 recorded in the attachment's comment, alongside the source path so `read` recovers image paths exactly; only the current comment form is parsed — an attachment stamped by a markfluence predating a comment-format change reads as unmanaged and is re-uploaded once, the same as any hand-uploaded file — except that a *recorded path disagreeing with the local source* is an update even when the checksum matches, so a mangled path repairs itself instead of surviving every later publish; a comment with no source recorded at all is not a disagreement. Every text part of the upload form must go through `writeTextField`, never `multipart.Writer.WriteField`, which emits no charset and gets decoded as Latin-1), `_links.next` pagination. **Three pagination schemes, and picking the wrong one truncates silently.** v1 *child/attachment* collections page through the generic `listV1` helper by `start`/`limit` offset, never `_links.next` (absent when the results fit one page, so it cannot terminate a loop); `ListAttachments`, `ListChildPages`, and `ListChildFolders` all go through it. v2 collections page through `listV2` by the cursor in `_links.next`, which is a `/wiki`-prefixed absolute path `resolveNext` handles unchanged; `ListContentProperties` and `SearchPagesByTitle` share it. **`/wiki/rest/api/search` is neither**: it ignores `start` outright, its `next` is context-relative so it needs the `/wiki` prefix `resolveNext` does not add, a short page does *not* mean the end, and `totalSize` can be nonzero against an empty `results` — so `searchCQL` terminates only on a missing `next` and nothing may branch on `totalSize` ([docs/confluence/search.md](docs/confluence/search.md)). `searchCQLBounded` adds a row bound under it (`SearchCQL` is that call with no bound, which is why `find` is unaffected): it asks for `max+1` and reports the surplus as `more`, since `totalSize` cannot supply a count. Full text goes through `SearchText`/`SearchRawCQL`, which return the cleaned `SearchMatch` the way `FindByTitle` returns `TitleMatch` — and **every field of a match comes from the row's `content` object**, because the row-level `title` is HTML-escaped *and* wrapped in `@@@hl@@@` markers where `content.title` is neither. The `excerpt` exists only at row level, so `cleanExcerpt` strips those markers, unescapes once, and collapses to one line — in the client, so the human and `--json` paths cannot disagree about it. `excerpt=highlight` is passed explicitly and **re-attached when following the cursor** (the `next` link carries `cql` and `limit` but not `excerpt`, and `doJSON` appends params with a bare `?`); an unrecognized value there yields an empty excerpt with a 200, so a rename by Atlassian degrades to no excerpts rather than an error. A row with no `content` object is skipped and **counted** — `type = space` answers with hundreds of them, and a silent skip would report a successful empty result. A bare v1 child row already carries `webui`, `status`, and `extensions.position`, so child listing needs no `expand`. `DownloadAttachment` goes through `send` (inheriting retry/backoff) against `_links.download`; **never** add a `CheckRedirect` that forwards headers — it would leak site credentials to Atlassian's media host, which neither needs nor wants them. `config.go` holds `Resolve` and the `.env` reader. Why each of these is shaped this way, with the evidence: [docs/confluence/api.md](docs/confluence/api.md) and [attachments.md](docs/confluence/attachments.md). +- `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/...`). A **folder** — the Cloud content type that can parent a page — has its own v2 route, `GetFolderOrNil` against `/wiki/api/v2/folders/{id}`, because every v2 *page* route answers a folder id with 404; enumerating children, if it is ever added, must be v1, since v2 cannot list inside a folder at all and its page-children route silently omits folders ([docs/confluence/folders.md](docs/confluence/folders.md)). Typed `HTTPError`, per-attempt context timeouts, centralized retry/backoff in `send`. `HTTPError.Error()` appends a **hint** for the three auth failures whose status misleads, matched on the response *body* rather than deduced from the status and always **appended** to it, never replacing it. The one that matters: **a rejected credential is a 404 on every v2 route**, so a revoked token used to make `read` answer `page ... not found` about a page that exists. `RejectedCredential` tells it apart by the fact that every genuine v2 404 *names* what it could not find and the auth one does not, `notFound` gates the three `…OrNil` helpers on it so they stop reading it as "absent", and `jsonout.CodeFor` checks it before the status switch so `--json` reports `AUTH` rather than `NOT_FOUND`. **Two error types on the request path, and one predicate for them**: an `*HTTPError` once a response has a status, an unexported `requestError` when there is none (a transport failure, a request that would not build, a body that would not decode), and `FromRequest` answers whether an error is either. That is what lets a caller tell a server failure from a local one — `jsonout.CodeOr(err, fallback)` is the whole point of it, since `CodeFor` alone reports every non-`HTTPError` as `NETWORK` and so turns `no title given` into a network problem (#133). The rule is deliberately scoped to the request: `DownloadAttachment` writing to the caller's writer, `uploadAttachment` opening the caller's file, and `Resolve` reading the environment stay untyped, because tagging them would misreport an unreadable file as a network failure. The wrapper carries no message of its own, so `Error()` is the inner text verbatim and nothing a reader sees changed. A 403 that is not one of the two measured credential phrasings gets no hint, because that is what a genuine permission denial looks like ([docs/confluence/api.md](docs/confluence/api.md#scopes)). **Retry rules**: 429 for any method; 502/503/504 for idempotent methods; **any other 5xx only when the response carries `Retry-After`** — that is how a 500 becomes retryable, and it is why `parseRetryAfter` reports the header's *presence* apart from its delay (`Retry-After: 0` means "retry now", not "no header"). The exponential delay is jittered, a server-supplied `Retry-After` never is. Decisions go to a package-level hook (`SetRetryLogger`, set once in `root.go` beside `ui.SetDebug`) and fire whichever way they went, because `internal/client` prints nothing and a silent twelve-minute retry storm is otherwise indistinguishable from a hang. **A versioned PUT is not as idempotent as its method**: `SetContentProperty` retry-once on top (recovers a lost create-POST response) and `UpdatePage`'s `updateLanded` both exist for the same reason — a write whose response was lost gets re-sent, and the re-sent version is refused. `updateLanded` requires version *and* title *and* body to match what was sent, since a concurrent edit could have produced the version alone and claiming success over someone else's content is worse than a false failure ([docs/confluence/api.md](docs/confluence/api.md)). `SyncAttachments` (skip/update by a SHA-256 recorded in the attachment's comment, alongside the source path so `read` recovers image paths exactly; only the current comment form is parsed — an attachment stamped by a markfluence predating a comment-format change reads as unmanaged and is re-uploaded once, the same as any hand-uploaded file — except that a *recorded path disagreeing with the local source* is an update even when the checksum matches, so a mangled path repairs itself instead of surviving every later publish; a comment with no source recorded at all is not a disagreement. Every text part of the upload form must go through `writeTextField`, never `multipart.Writer.WriteField`, which emits no charset and gets decoded as Latin-1), `_links.next` pagination. **Three pagination schemes, and picking the wrong one truncates silently.** v1 *child/attachment* collections page through the generic `listV1` helper by `start`/`limit` offset, never `_links.next` (absent when the results fit one page, so it cannot terminate a loop); `ListAttachments`, `ListChildPages`, and `ListChildFolders` all go through it. v2 collections page through `listV2` by the cursor in `_links.next`, which is a `/wiki`-prefixed absolute path `resolveNext` handles unchanged; `ListContentProperties` and `SearchPagesByTitle` share it. **`/wiki/rest/api/search` is neither**: it ignores `start` outright, its `next` is context-relative so it needs the `/wiki` prefix `resolveNext` does not add, a short page does *not* mean the end, and `totalSize` can be nonzero against an empty `results` — so `searchCQL` terminates only on a missing `next` and nothing may branch on `totalSize` ([docs/confluence/search.md](docs/confluence/search.md)). `searchCQLBounded` adds a row bound under it (`SearchCQL` is that call with no bound, which is why `find` is unaffected): it asks for `max+1` and reports the surplus as `more`, since `totalSize` cannot supply a count. Full text goes through `SearchText`/`SearchRawCQL`, which return the cleaned `SearchMatch` the way `FindByTitle` returns `TitleMatch` — and **every field of a match comes from the row's `content` object**, because the row-level `title` is HTML-escaped *and* wrapped in `@@@hl@@@` markers where `content.title` is neither. The `excerpt` exists only at row level, so `cleanExcerpt` strips those markers, unescapes once, and collapses to one line — in the client, so the human and `--json` paths cannot disagree about it. `excerpt=highlight` is passed explicitly and **re-attached when following the cursor** (the `next` link carries `cql` and `limit` but not `excerpt`, and `doJSON` appends params with a bare `?`); an unrecognized value there yields an empty excerpt with a 200, so a rename by Atlassian degrades to no excerpts rather than an error. A row with no `content` object is skipped and **counted** — `type = space` answers with hundreds of them, and a silent skip would report a successful empty result. A bare v1 child row already carries `webui`, `status`, and `extensions.position`, so child listing needs no `expand`. `DownloadAttachment` goes through `send` (inheriting retry/backoff) against `_links.download`; **never** add a `CheckRedirect` that forwards headers — it would leak site credentials to Atlassian's media host, which neither needs nor wants them. `config.go` holds `Resolve` and the `.env` reader, plus the **`.env` permission warning** (#136): a `.env` reachable by anyone but its owner (`mode.Perm()&0o077`) *and* containing `CONFLUENCE_TOKEN` earns a warning naming the file, its mode, and the `chmod`. Both halves matter — a `.env` holding only the URL and username leaks nothing, and a warning that fires on a file with no secret in it is how one becomes something people scroll past. It stats rather than lstats (a link's own `0777` would cry wolf over a `0600` target), lives in `loadDotenv` because that is the one function both the discovered `.env` and `--env-file` pass through, and reaches the reader through `SetSecurityWarner` for the same reason `SetRetryLogger` exists. A group/world-*writable* `.env` with no token in it is knowingly **not** covered, though `CONFLUENCE_URL` resolves from there too and rewriting it would redirect the token: see #136. Why each of these is shaped this way, with the evidence: [docs/confluence/api.md](docs/confluence/api.md) and [attachments.md](docs/confluence/attachments.md). - `internal/convert` — the converter (the crux). `MdToConfluence(md *frontmatter.MarkdownFile, root *project.Root, index *linkindex.Index, baseURL, spaceKey, version string) (*ConfluencePage, error)`. `root` bounds which images and parent references may be read (S1/S2) and is what an image's recorded `Source` is relative to; `index` is the tree-wide link/anchor index for `root` (`internal/linkindex.Build`), built once and shared across every file converted under it rather than rebuilt per conversion — both are discovered/built by the caller (`internal/project`/`internal/linkindex`), which is why this package stays client-free. 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; `aclink.go` is the *inverse* direction's one element with enough shape to need its own file — ``, which the editor writes for every internal link and `MdToConfluence` never emits, so nothing in the regression suite covers it. One rule decides its whole mapping: **convert when the markdown republishes to a link resolving to the same target, pass the storage through when it would not** — so a page link and a space link convert, while a mention (80% of all real usage), an attachment link (only images are uploaded, so a relative href would be dead) and an unresolvable target stay raw, which the shield republishes byte-identical. A page target is a **title, never an id**, so `PageLinkTargets` reports what needs resolving and `StorageOptions.PageLinks` carries the answers back. An `ac:anchor` is **percent-encoded** where `confluenceSlug` output is not: decode it before matching a heading, leave it encoded inside a URL. A same-page anchor recovers its heading from the document rather than inverting the slug, which is impossible — `confluenceSlug` turns both a space and a hyphen into `-`. The survey the mapping rests on, and the `xml.HTMLAutoClose` trap that made `` crash the parser outright (#88), are in [docs/confluence/links-and-anchors.md](docs/confluence/links-and-anchors.md); `attachname.go` owns the source-path→attachment-name mapping, which is now the path's **base name** and nothing else (#59/`_plans/029`): the name is the attachment's identity, so an encoded path moved the name every time the file moved and orphaned the old attachment, and the path is recorded in the comment anyway. The mapping is therefore lossy, and what the bijection used to buy is an explicit refusal — two assets in one document whose base names agree return a typed `NameCollisionError` from `MdToConfluence`, which is a *failure* and not a `Broken` entry, since nothing blocks a publish on `Broken`. `check` catches that error and reports it as `Broken` anyway, because there it is a document defect like a dead link rather than a converter failure. A stored name is never interpreted in the other direction either: `sourceFor` reads the recorded path or uses the name verbatim. What names Confluence accepts is in [docs/confluence/attachments.md](docs/confluence/attachments.md); `destination.go` owns the **other** codec, destination↔path (`decodeDestination`/`encodeDestination`), shared by images *and* doc links — a markdown destination is a URL, so decode inbound (**before** `withinRoot`, or an encoded `..%2F` slips the clamp) and encode outbound in `storage_to_md.go` (or `export` emits markdown that no longer parses, and `sourceFor`'s absolute-path refusal is undone by the next read); an undecodable destination is a literal `%` in a filename, not an error; the reasoning is in [docs/confluence/links-and-anchors.md](docs/confluence/links-and-anchors.md); `images.go` (resolution stays page-relative like GitHub; the documentation root — cwd — bounds what may be published, and an image above it is `IMAGE BROKEN`), `links.go` (GitHub/Confluence slugs, doc-link + anchor rewriting against `internal/linkindex`'s tree-wide index; `resolveDocKey` resolves a destination to the index's root-relative key and reports `escapes` — a purely lexical check on the *query* side, since the index itself needs no clamp: an escaping key can never be in it, built by walking downward from root). A doc-link target is one of four severities, #42: missing entirely or escaping root is **Broken** (`LINK BROKEN: … (not found|outside the documentation root)`) and replaces the whole `` element — tags and visible text alike — with that literal message, matching `images.go`'s precedent for a missing image (`renderLink` needs a small per-node flag, `linkBrokenText`, since goldmark still invokes a container node's renderer on the matching leaving call regardless of `WalkSkipChildren` on entering, and there is no `` to write in the broken case); existing on disk with no `page_id` yet is unchanged — a **warning**, the normal state of an unpublished tree; a `#fragment` matching no heading on an otherwise-resolving target also **warns**, gated on `linkindex.Index.FileExists` so a missing/escaping target isn't double-reported. `tables.go` (the `` tag, stamped with `data-layout="align-start"` so tables auto-size and left-align — this must stay if column widths are ever emitted, or a `` silently induces a layout; plus cells: an AST transformer consumes a leading `` comment in a cell and `renderTableCell` emits it as `data-highlight-colour`; `storage_to_md.go`'s `cellTexts` reverses this, reading `data-highlight-colour` back into a `bg:` marker (`cellBGNames`, the reverse of `tables.go`'s swatch map — a hex outside the 21 swatches round-trips as the literal hex, and where two names share a hex the British spelling wins, matching Confluence's own `-colour`), and a column's GFM alignment becomes a `

` wrapper **inside** the cell — never the `align` attribute the GFM renderer would emit, which is the one form Confluence discards. Only center and right are emitted: Confluence has no explicit left, so `:---` publishes bare and `read` recovers it as `---`. Since alignment is per-paragraph there and per-column in GFM, `columnSeparators` in `storage_to_md.go` takes each column's most common declared alignment (ties to the first seen) and drops the rest. Rows still fall through to the GFM renderer. A multi-line cell is one `

` per line — Enter in the editor starts a new `

`, it does not insert a `
` — so `renderCellLines` in `storage_to_md.go` joins sibling `

` children with a literal `
` rather than nothing: a GFM table row is exactly one physical line, so a real newline isn't an option, and the same substitution catches a bare mid-line `
` (Shift+Enter) that would otherwise render as the two-space hard break valid in ordinary block content but not inside a table row. A `

` tag, stamped with `data-layout="align-start"` so tables auto-size and left-align — this must stay if column widths are ever emitted, or a `` silently induces a layout; plus cells: an AST transformer consumes a leading `` comment in a cell and `renderTableCell` emits it as `data-highlight-colour`; `storage_to_md.go`'s `cellTexts` reverses this, reading `data-highlight-colour` back into a `bg:` marker (`cellBGNames`, the reverse of `tables.go`'s swatch map — a hex outside the 21 swatches round-trips as the literal hex, and where two names share a hex the British spelling wins, matching Confluence's own `-colour`), and a column's GFM alignment becomes a `

` wrapper **inside** the cell — never the `align` attribute the GFM renderer would emit, which is the one form Confluence discards. Only center and right are emitted: Confluence has no explicit left, so `:---` publishes bare and `read` recovers it as `---`. Since alignment is per-paragraph there and per-column in GFM, `columnSeparators` in `storage_to_md.go` takes each column's most common declared alignment (ties to the first seen) and drops the rest. Rows still fall through to the GFM renderer. A multi-line cell is one `

` per line — Enter in the editor starts a new `

`, it does not insert a `
` — so `renderCellLines` in `storage_to_md.go` joins sibling `

` children with a literal `
` rather than nothing: a GFM table row is exactly one physical line, so a real newline isn't an option, and the same substitution catches a bare mid-line `
` (Shift+Enter) that would otherwise render as the two-space hard break valid in ordinary block content but not inside a table row. A `