From 9f0f5752191b5463f6ee3d04406fff782862c168 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Wed, 23 Sep 2026 14:28:54 -0400 Subject: [PATCH 1/6] docs: the credentials-init plan Answers #193: a command that prompts for the credentials, checks them, and writes the user's credentials file. --- _plans/051_credentials-init.md | 345 +++++++++++++++++++++++++++++++++ 1 file changed, 345 insertions(+) create mode 100644 _plans/051_credentials-init.md diff --git a/_plans/051_credentials-init.md b/_plans/051_credentials-init.md new file mode 100644 index 0000000..1b5ced2 --- /dev/null +++ b/_plans/051_credentials-init.md @@ -0,0 +1,345 @@ +# 051: `credentials-init`, a command that writes the credentials file + +Answers #193. After #188 (plan 050), people create +`~/.config/markfluence/credentials` by hand. This plan adds +`markfluence credentials-init`, which prompts for the settings, checks them +against Confluence, and writes the file with the right permissions. + +It writes only the credentials file. Project setup (`markfluence.yaml`, a +GitHub Actions workflow) stays with #5's `init`: the credentials file belongs +to one machine and `markfluence.yaml` to one project, and a person fixing auth +from `~` must not end up with a `~/markfluence.yaml` that becomes the +documentation root for every Markdown file under their home directory. + +## Decisions + +**D1. The name is `credentials-init`.** Multi-word commands put the noun first +(`attachment-list`, `user-info`), and markfluence has no command groups. `init` +is #5's. `login` and `auth-login` describe a session that does not exist: +nothing logs in, and there would be no `logout` to pair with it. + +**D2. It writes only the default credentials file** (`credentialsPath`, D2 of +plan 050). No flag for another path: a file for a second site is a copy and an +edit, and a target flag could be pointed into a repository by mistake. Other +targets are in Not in scope until someone needs one. + +`--env-file` is refused (`cmd.Flags().Changed("env-file")`). It cannot name +the target (above), and it means nothing else here. + +When there is no credentials path (`credentialsPath` returns "" for a relative +or empty `HOME`, or an `os.UserHomeDir` failure), the command refuses with +exit 2 and says why: there is nowhere to write. + +**D3. The prompts.** In order: + +1. **Site URL.** Normalized to scheme and host: surrounding space is trimmed, + `https://` is added when there is no scheme, and a path is dropped, with a + note when one was, so a pasted page URL works. Every request appends + `/wiki/...` to the site URL, so a kept `/wiki` would double. A scheme other + than `https` is refused: the token goes out as basic auth. +2. **Username.** +3. **API token**, read without echo (D6). + +Every answer is trimmed (`strings.TrimSpace`), so a pasted trailing space or +newline never reaches the file. A blank answer is refused and asked again, +except where there is a current value to keep (D4). End of input (Ctrl-D) at +any prompt aborts: exit 1, nothing written. Without that, a blank-answer loop +reading a closed stdin would spin forever, and `ReadPassword` reports an +empty line at EOF as `io.EOF`. The cloud ID is not a prompt (D5). + +Prompts go to stderr, and results to stdout through `internal/ui`. + +**D4. An existing file prefills the prompts, and is rewritten.** Each prompt +shows the current value as its default and Enter keeps it; the token's default +is shown as `(Enter keeps the current token)`, never any part of the token. +Running it again to rotate a token is then one paste. The file is rewritten +whole, so if the existing one holds a comment or a key other than the four +`CONFLUENCE_*` settings, the command says, before it asks anything, that those +lines will not be kept. + +The existing file is read with the same rules as `Resolve` (`loadCredentials`): +missing is fine, a dangling symbolic link or an unreadable file is an error and +nothing is written. + +Reading a loose existing file goes through `loadDotenv`, which would raise the +#136 "run chmod 600" warning just before the command rewrites the file `0600` +anyway. The command silences the warner for that read (`SetSecurityWarner(nil)` +around it, then reinstating the previous one, which needs a getter or a +returned restore function) and instead says the rewrite will make the file +`0600`. + +Values are never prefilled from the environment or `--env-file`. The file +holds what was typed. + +**D5. The cloud ID is fetched and saved for every `*.atlassian.net` site.** +Only there: `gatewayPrefix` is hardcoded to `api.atlassian.com`, which +Government and isolated Cloud sites do not use, and `tenant_info` is verified +only on `*.atlassian.net`. Those sites are in scope though untested +(CLAUDE.md), and a saved cloud ID would send every request to the wrong +gateway, where the check would then refuse credentials that work. For any +other host the command saves no cloud ID and says why. + +After the URL prompt, the command asks `GET {site}/_edge/tenant_info`, which answers `{"cloudId": "…"}` +with no credentials (docs/confluence/api.md, verified 2026-08-07), and shows +the result. There is no question about whether the token is scoped: an +unscoped personal token returns 200 through the gateway (docs/confluence/api.md, +verified 2026-08-07, and every write measurement since went through it with a +personal token), so saving a cloud ID costs nothing, and nobody has to know +what one is. + +The request is sent **without** an `Authorization` header: it does not need +one, and at that point no username or token has been typed. + +If the site answers but gives no usable cloud ID (a non-200, a body without +`cloudId`, or a value `validateCloudID` refuses), the command warns and goes on +without one: an unscoped token still works, and a scoped one can be fixed by +running the command again. If the site cannot be reached, it says so and goes +on without one; the check (D7) will fail the same way and ask. + +A prefilled cloud ID is never kept: it is fetched again, because the URL may +have changed. + +**D6. The token is read with `charmbracelet/x/term`.** `ReadPassword` and +`IsTerminal` from `github.com/charmbracelet/x/term` v0.2.1, which lipgloss +already brings into the build, so no new code ships in the binary. It becomes +the seventh direct dependency in `go.mod`. It is pre-1.0 and lipgloss decides +its version in practice; `golang.org/x/term` was the alternative, and would add +a module to the build. + +`ReadPassword` turns echo off and restores it in a `defer`, but it leaves +`ISIG` set, so Ctrl-C kills the process before the `defer` runs and leaves the +terminal with echo off. The command saves the terminal state with `GetState` +before reading the token and, for `SIGINT`, `SIGTERM`, and `SIGHUP` during the +read, restores it with `Restore`, prints a newline, and exits 130. It calls +`signal.Stop` after the read, so the rest of the run keeps default signal +handling. + +Plain lines are read from stdin a byte at a time, not through a `bufio.Reader`: +a buffered reader would swallow typed-ahead input that `ReadPassword`, which +reads the file descriptor directly, then never sees. + +**D7. The check before saving.** The command prints `Checking the +credentials…`, builds a client from the typed values and the fetched cloud ID, +and calls `CurrentUser`, the request `user-info` makes. On success it reports +the account's display name and account id. The line first, because `send` +retries an unreachable host for about 2¾ minutes (5 attempts at the 30-second +read timeout, plus backoff), and without it that is a silent hang. + +A failure is classified by the response's shape, never by its status alone, +because the same status arrives for unrelated reasons +(docs/confluence/api.md, "A scope failure is a 401, not a 403"). The markers +are the ones `HTTPError.hint` already matches (client.go), so the check and +the hint cannot disagree: + +| response | meaning | action | +|---|---|---| +| `HTTPError.RejectedCredential()` | wrong or revoked username or token | refuse | +| 401 from the site domain, not JSON | a scoped token sent without a cloud ID (only reachable when D5 saved none) | refuse, and say the token looks scoped, and why no cloud ID was saved | +| 401 with `scope does not match` | the token **authenticated**, but lacks `read:confluence-user` | ask `Save anyway? [y/N]`, saying the credentials are good but `user-info` will not work with this token | +| any other 401 or 403 | refused, in a shape not yet measured | refuse | +| 404 that is not `RejectedCredential` | the URL is not a Confluence site | refuse | +| no response, 5xx or 429 after retries, or a body that would not decode | unknown: the values may well be right | ask `Save anyway? [y/N]` | + +A 200 whose account has an empty `accountId`, or is `type: anonymous`, is +refused: the site answered without authenticating anyone. + +**Refuse** writes nothing (an existing file stays byte-identical), names the +URL and the username, and exits 1: a mistyped token never reaches the disk. +**Ask**: yes writes the file and says it was not checked, no writes nothing +and exits 1. + +The shape check is in `internal/client`, as `HTTPError` methods beside +`RejectedCredential` (for example `ScopeMismatch()` and `SiteRejectedAuth()`), +refactoring `hint` to use them, so the markers exist once. The check decides +with `var he *client.HTTPError; errors.As(err, &he)`. What the gateway answers +for a wrong token on `/user/current` is not measured yet, which is why "any +other 401 or 403" has a row (see Checks). + +**D8. How the file is written.** `client.WriteCredentials(path, values)`: + +- The directory is created `0700` if missing; an existing directory's mode is + left alone. +- The file is written to a temporary file in the same directory + (`os.CreateTemp`, which creates it `0600`), synced, and renamed over the + target, so an interrupted write never leaves a half file, and a loose + existing file comes out `0600`. The temporary file is removed on every + failure path. +- When the target is a symbolic link (a dotfiles repository), the write goes + to the link's target, with the temporary file beside it, so the link + survives. It resolves the path with `filepath.EvalSymlinks` only when + `os.Lstat` shows a link, since `EvalSymlinks` fails on a path that does not + exist yet. +- The keys are written in the order URL, username, token, cloud ID, below a + one-line comment naming the command and `docs/credentials.md`. The quoting + rule is defined as a round trip rather than as a list of characters: a value + `v` is written bare when `unquote(strings.TrimSpace(v)) == v`, and + double-quoted otherwise, which `unquote` strips exactly. `TrimSpace` strips + all Unicode space (U+00A0 included), and a list would miss some. A value + holding `\n` or `\r` is refused. +- It then reads the file back through the parser `Resolve` uses, with the + warner silenced (the file is `0600`, but a verification read must never + warn), and compares, the way the frontmatter writer verifies its own + output. A mismatch is an error. + +It lives in `internal/client` beside `loadDotenv` so the writer and the reader +of the format are in one package. + +**D9. After saving, it warns about what the environment does to the file.** +The warnings follow `Resolve`'s actual rules, not a blanket "overrides": + +- URL, username, and token all exported: the file is not read at all, so none + of it takes effect until they are unset. +- `CONFLUENCE_URL` or `CONFLUENCE_TOKEN` exported (but not both with the + username): every command fails the same-source rule until it is unset. +- `CONFLUENCE_USERNAME` exported: it is used instead of the file's. +- `CONFLUENCE_CLOUD_ID` exported: it is ignored, since the URL comes from the + file, and `Resolve` will warn about it on every run. + +It does not refuse: an exported username alone is harmless. + +**D10. It refuses to run without a terminal, and refuses `--json`.** If stdin +or stderr is not a terminal, it fails at once (exit 2, before any prompt) with +a message pointing to `docs/credentials.md` and noting that CI uses +environment variables. Stdin, because reading answers from a pipe would bring +back the shell history problem (`echo $TOKEN |`) and make the prompt order an +interface. Stderr, because the prompts go there, and with `2>log` the person +would be answering prompts they cannot see. + +`--json` is refused (exit 2) and the command goes on `cmd`'s `noJSONEnvelope` +list: a person answering prompts is the only consumer, so a schema branch would +be an interface with no user. + +Every refusal in D2 and D10 is a plain returned error, not `ui.Error` plus a +silent exit. `Execute` already turns a plain error into exit 2, printed as a +human line or, under `--json`, as an `errorObject` on stderr. Under `--json` +every `ui` helper is a no-op, so the `ui.Error` route would refuse `--json` +while printing nothing at all. + +**D11. The credential errors name the command.** When the credentials file +applies (`credPath != ""`), the "missing Confluence …" error from `Resolve` +adds `run markfluence credentials-init, or` before the places list. The error +that sends a person to the docs is the main way they discover the command. Not +in the two half-pair forms (only the URL or only the token set), which name +the one place the other setting can go, and where the command would write a +file that then fails the same-source rule. The same-source error does not +change either: running the command does not fix it. In CI the credentials path +is usually set too, so the suggestion appears there, which is harmless: the +command refuses without a terminal and says what to do instead. + +## Implementation + +`internal/client`: + +- `CredentialsPath() string`: `credentialsPath`, exported. +- `ReadCredentials(path string) (map[string]string, error)`: `loadCredentials`, + exported. It stays the one reading of the file for both callers. +- `WriteCredentials(path string, values map[string]string) error` (D8), in a + new `credentials.go` with the path and read functions moved beside it. +- `FetchCloudID(siteURL string) (string, error)`: the unauthenticated + `tenant_info` request (D5), through its own `http.Client` with `Timeout` + set to the read timeout. Not through `send`, which always sets basic auth; + losing `send`'s retries and retry logging is acceptable because D5 degrades + to "no cloud ID". It returns the `requestError` or `*HTTPError` shapes + `FromRequest` knows. +- `HTTPError.ScopeMismatch()` and `HTTPError.SiteRejectedAuth()` (D7), with + `hint` rewritten to use them. +- A way to silence the warner around a read and restore it (D4, D8): for + example `SetSecurityWarner` returning the previous function. + +`cmd/credentialsinit/` exports `Cmd`. `run` is a thin shell over +`runInit(io prompter, deps)`, where `prompter` is `Line(prompt, def string)` +and `Secret(prompt string, hasCurrent bool)`, and `deps` carries +`fetchCloudID`, `verify(client.Config) (*client.User, error)`, the path, and +`getenv`. The terminal `prompter` is the only code that touches the tty, so +every decision above is testable with a scripted one. + +`cmd/root.go` registers it, and its `Long` mentions it next to the credentials +file. + +## Files + +| file | change | +|---|---| +| `cmd/credentialsinit/credentialsinit.go` | new command | +| `cmd/credentialsinit/credentialsinit_test.go`, `main_test.go` | tests; `TestMain` calls `testenv.RunIsolated` | +| `cmd/root.go` | register; `Long` names the command | +| `cmd/root_test.go` | `noJSONEnvelope` entry | +| `internal/client/credentials.go` | new: path, read, write, `FetchCloudID` | +| `internal/client/config.go` | move the path/read functions out; D11's wording; `Resolve`'s closing comment ("Without a cloud ID … an unscoped personal token needs") | +| `internal/client/client.go` | `ScopeMismatch`/`SiteRejectedAuth`; `hint` uses them, and the site-rejected hint names `credentials-init`; the `Config.CloudID` comment says leaving it empty is for Data Center, not required by a personal token | +| `internal/client/*_test.go` | tests below; `config_test.go` pins the exact "missing" wording and changes with D11 | +| `docs/confluence/api.md` | the gateway's answer for a wrong token on `/user/current` (Checks) | +| `go.mod`, `go.sum` | `charmbracelet/x/term` direct | +| `docs/credentials.md` | setup starts with the command; manual steps stay as the alternative; cloud ID wording (D5) | +| `README.md` | Configure names the command; the scoped-token section says the command fills the cloud ID | +| `.env.example` | the cloud ID comment no longer says to leave it out for a personal token | +| `docs/commands/` | `make docs` | +| `CLAUDE.md` | Configuration (the command; "leave it unset for an unscoped token" becomes "harmless for one"), a Layout bullet, the four exports, the seventh dependency | + +## Tests + +`internal/client`: + +- `WriteCredentials`: file `0600`, new directory `0700`, existing directory + mode untouched; a loose existing file becomes `0600`; a symbolic link is + written through and survives; round-trip of values with a leading space, a + trailing U+00A0, a leading quote, a value both starting and ending with a + quote, `#`, `=`, and `export `; `\n` and `\r` are refused and the old file is + untouched; no temporary file is left behind on either path; writing raises + no permission warning. +- `ScopeMismatch`/`SiteRejectedAuth`/`RejectedCredential` against the measured + bodies, and `hint`'s output unchanged for the existing cases. +- `FetchCloudID`: the id from a 200; no `Authorization` header on the request; + a 404, a body without `cloudId`, and a value with `/` are errors; + an unreachable server is a `FromRequest` error that is not an `HTTPError`. +- The "missing" error names `credentials-init` when there is a credentials + path, and does not when there is none. + +`cmd/credentialsinit` (scripted prompter, stub deps): + +- Stdin not a terminal, or stderr not a terminal: exit 2, no prompt, nothing + written. `--json` (the refusal arrives as an `errorObject` on stderr), + `--env-file`, and no credentials path: exit 2, nothing written. +- EOF at each prompt: exit 1, nothing written, no loop. +- A pasted token with a trailing space is saved trimmed. +- A loose existing file: no chmod warning during the prefill read, and the + rewrite leaves it `0600`. +- A new file: the values typed plus the fetched cloud ID are written. +- URL normalization: a bare host, a trailing slash, and a page URL all become + `https://host`; `http://` is refused and asked again. +- Prefill: Enter at every prompt keeps the URL, username, and token, and the + cloud ID is refetched, not kept; an existing comment triggers the notice. +- `tenant_info` with no id: warns and writes no cloud ID. +- A host not under `.atlassian.net`: no `tenant_info` request, no cloud ID, + and a note saying why. +- The check, one case per row of D7's table: each refusal writes nothing and + leaves an existing file byte-identical; each "ask" with "n" writes nothing + and with "y" writes and says unchecked; an anonymous account is refused. +- The environment, one case per D9 bullet, and none gives no warning. +- An unreadable existing file fails before any prompt. + +`cmd`: `TestSubcommandsDocumentThemselves` and `TestSubcommandsCompleteArgs` +cover `Long`, `Example`, and completion (no arguments, so +`completion.Values()`). + +## Checks + +- `make check`. +- **Live:** run `./bin/markfluence credentials-init` against the Mozilla + instance with `XDG_CONFIG_HOME` pointed at a temp directory: a good token + saves and reports the account; a wrong token through the gateway, to measure + what `/user/current` answers there (D7), recorded in docs/confluence/api.md; + then `user-info` with that file. One request per case, no loops, on the + shared instance. +- Ctrl-C at the token prompt leaves echo on (D6), by hand. + +## Not in scope + +- **Writing anywhere but the default path** (D2), including honoring + `--env-file` as a target. +- **Non-interactive input**: flags or piped answers (D10). +- **The OS keychain and named profiles**, for #188's reasons. +- **Project setup** (`markfluence.yaml`, a workflow): #5. +- **Checking that the username matches the account** `CurrentUser` returns: + Atlassian hides the email address by privacy setting, so the comparison + would often have nothing to compare. The command shows the account instead. From 075d5ff86270793be8f45d884cb2ff1fb0c28a83 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Wed, 23 Sep 2026 14:39:08 -0400 Subject: [PATCH 2/6] feat(client): write the credentials file, and fetch a site's cloud ID The pieces credentials-init (#193) needs, kept in internal/client beside the reader so one package owns the format: - CredentialsPath and ReadCredentials, exported, moved to credentials.go. ReadCredentials raises no permission warning, since its caller is about to rewrite the file 0600; loadDotenv is now readDotenv plus the warning. - WriteCredentials: temp file and rename at 0600, writing through a symbolic link, quoting a value exactly when readDotenv would not read it back unchanged, then reading the result back and comparing. - UnkeptLines, the lines a rewrite would drop, and DisplayPath. - FetchCloudID: the unauthenticated tenant_info request, outside send, which always sets basic auth. - HTTPError.ScopeMismatch and SiteRejectedAuth, the shapes hint already matched, so a caller classifying a failure cannot disagree with it. The missing-settings error now suggests credentials-init when there is a credentials file to write, and not in the half-pair forms, where a new file would fail the same-source rule. --- internal/client/client.go | 26 ++- internal/client/config.go | 102 ++------- internal/client/config_test.go | 14 +- internal/client/credentials.go | 276 +++++++++++++++++++++++ internal/client/credentials_test.go | 336 ++++++++++++++++++++++++++++ 5 files changed, 664 insertions(+), 90 deletions(-) create mode 100644 internal/client/credentials.go create mode 100644 internal/client/credentials_test.go diff --git a/internal/client/client.go b/internal/client/client.go index 153b986..8f20a4b 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -103,9 +103,9 @@ type ConfluenceClient struct { 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, when set, routes requests through the platform API gateway, which + // a scoped token requires and an unscoped personal token also accepts. Leave + // it empty for site-domain requests. CloudID string // Username is the account the token belongs to (basic auth). Username string @@ -205,14 +205,14 @@ func (e *HTTPError) Error() string { // would send someone to reissue a credential that is working fine. func (e *HTTPError) hint() string { switch { - case e.StatusCode == http.StatusUnauthorized && strings.Contains(e.Body, bodyScopeMismatch): + case e.ScopeMismatch(): return "hint: the API token is valid but carries no scope for this call. Scopes are fixed " + "when a token is issued, so this needs a new token rather than an edit to this one -- " + "the list markfluence needs is in README.md." - case e.StatusCode == http.StatusUnauthorized && !e.viaGateway() && !jsonBody(e.Body): + case e.SiteRejectedAuth(): return "hint: the site domain rejected this before it reached the API. A scoped " + "(service-account) token has to go through the platform gateway -- set " + - "CONFLUENCE_CLOUD_ID." + "CONFLUENCE_CLOUD_ID, or run markfluence credentials-init, which finds it." case e.RejectedCredential(): return "hint: the credentials were rejected. Check CONFLUENCE_USERNAME and " + "CONFLUENCE_TOKEN -- this is what a wrong or revoked token returns, and on a v2 route " + @@ -221,6 +221,20 @@ func (e *HTTPError) hint() string { return "" } +// ScopeMismatch reports whether the gateway refused a call the token carries no +// scope for. The token authenticated -- this is not a bad credential, and a +// caller deciding whether credentials work must not read it as one. +func (e *HTTPError) ScopeMismatch() bool { + return e.StatusCode == http.StatusUnauthorized && strings.Contains(e.Body, bodyScopeMismatch) +} + +// SiteRejectedAuth reports whether the site domain refused the request before +// it reached the API: a 401 that is a servlet's HTML page rather than the API's +// JSON. That is what a scoped token sent without a cloud ID gets. +func (e *HTTPError) SiteRejectedAuth() bool { + return e.StatusCode == http.StatusUnauthorized && !e.viaGateway() && !jsonBody(e.Body) +} + // RejectedCredential reports whether the response is the API refusing the // credentials outright, which it does with three different statuses depending // on the route: 403 on v1, and 404 with an unnamed target on v2. diff --git a/internal/client/config.go b/internal/client/config.go index 5625ba8..be088fc 100644 --- a/internal/client/config.go +++ b/internal/client/config.go @@ -1,14 +1,10 @@ package client import ( - "errors" "fmt" - "io/fs" "os" - "path/filepath" "regexp" "strings" - "syscall" ) const ( @@ -45,7 +41,7 @@ func lookup(sources []source, key string) (string, int) { // Resolve builds a client from the site URL, username, token, and optional // cloud ID. Each resolves key by key from three sources, highest first: the // file named by --env-file (envFile; it must be readable), the CONFLUENCE_* -// environment variables, and the user's credentials file (credentialsPath). +// environment variables, and the user's credentials file (CredentialsPath). // Credentials come only from sources the user chose: nothing is discovered // from the working directory, so a checkout cannot supply them (#188). // @@ -68,8 +64,8 @@ func lookup(sources []source, key string) (string, int) { // unset, so a broken or loose file cannot fail or warn on a run that never // uses it. // -// Without a cloud ID, requests go to the site domain, which is what an -// unscoped personal token needs. +// Without a cloud ID, requests go to the site domain. An unscoped personal +// token works either way; a scoped one needs the cloud ID. func Resolve(envFile string) (*ConfluenceClient, error) { var sources []source if envFile != "" { @@ -81,9 +77,9 @@ func Resolve(envFile string) (*ConfluenceClient, error) { } sources = append(sources, environment()) - credPath := credentialsPath() + credPath := CredentialsPath() if !complete(sources) && credPath != "" { - creds, err := loadCredentials(credPath) + creds, err := loadCredentials(credPath, loadDotenv) if err != nil { return nil, err } @@ -119,6 +115,11 @@ func Resolve(envFile string) (*ConfluenceClient, error) { places = sources[tokenFrom].label + ", where " + tokenEnv + " is" case token == "" && siteURL != "": places = sources[urlFrom].label + ", where " + urlEnv + " is" + case credPath != "": + // The command writes the credentials file, so it is only worth + // naming when there is one to write -- and not in the two cases + // above, where a new file would fail the same-source rule. + setThem = "run markfluence credentials-init, or " + setThem } return nil, fmt.Errorf("missing Confluence %s: %s%s. See %s", strings.Join(missing, ", "), setThem, places, credentialsDoc) @@ -181,71 +182,6 @@ func complete(sources []source) bool { return true } -// credentialsPath is where the user's credentials file lives: -// $XDG_CONFIG_HOME/markfluence/credentials, else -// $HOME/.config/markfluence/credentials, on every platform. os.UserConfigDir is -// not used because on macOS it answers ~/Library/Application Support, which -// command-line users do not look in. -// -// A relative XDG_CONFIG_HOME is ignored, as the XDG spec says, and a relative -// or empty HOME means there is no credentials file ("" here). Either would -// make the file depend on the working directory, which is exactly what #188 -// removed. -func credentialsPath() string { - if x := os.Getenv("XDG_CONFIG_HOME"); filepath.IsAbs(x) { - return filepath.Join(x, "markfluence", "credentials") - } - home, err := os.UserHomeDir() - if err != nil || !filepath.IsAbs(home) { - return "" - } - return filepath.Join(home, ".config", "markfluence", "credentials") -} - -// loadCredentials reads the credentials file, returning nil when there is -// none. A missing file, or a path that runs through something that is not a -// directory, means the user has not made one; a dangling symbolic link does -// not. Any other failure means they -// made one that cannot be used, and is an error: falling through silently -// would make a token rotation look like it had no effect. -func loadCredentials(path string) (map[string]string, error) { - env, err := loadDotenv(path) - switch { - case err == nil: - return env, nil - case errors.Is(err, fs.ErrNotExist) && isSymlink(path): - return nil, fmt.Errorf("reading credentials file %s: it is a symbolic link to a file that "+ - "does not exist", displayPath(path)) - case errors.Is(err, fs.ErrNotExist), errors.Is(err, syscall.ENOTDIR): - return nil, nil - default: - return nil, fmt.Errorf("reading credentials file %s: %w", displayPath(path), err) - } -} - -// isSymlink reports whether path itself is a symbolic link. A dangling link -// at the credentials path reads as "no such file", but it is a file the user -// made -- into a dotfiles repository that moved, or a volume not mounted -- so -// it must not read as "you have no credentials file". -func isSymlink(path string) bool { - fi, err := os.Lstat(path) - return err == nil && fi.Mode()&fs.ModeSymlink != 0 -} - -// displayPath shows a path under the home directory as ~/..., the form the -// docs use, so an error names the file the way a reader knows it. -func displayPath(path string) string { - home, err := os.UserHomeDir() - if err != nil || home == "" { - return path - } - if rel, err := filepath.Rel(home, path); err == nil && rel != ".." && - !strings.HasPrefix(rel, ".."+string(filepath.Separator)) { - return "~" + string(filepath.Separator) + rel - } - return path -} - // placesToSet lists where credentials can go, for the "missing" error. func placesToSet(credPath string) string { if credPath == "" { @@ -347,6 +283,20 @@ func shellArg(path string) string { // surrounding single or double quotes stripped. Values are taken verbatim (no // shell expansion). It errors if the file can't be read. func loadDotenv(path string) (map[string]string, error) { + out, err := readDotenv(path) + if err != nil { + return nil, err + } + // Here rather than in Resolve: this is the one function both the + // credentials file 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 +} + +// readDotenv is loadDotenv without the permission warning, for a reader that +// is about to rewrite the file 0600 or has just written it. +func readDotenv(path string) (map[string]string, error) { out := map[string]string{} data, err := os.ReadFile(path) if err != nil { @@ -364,10 +314,6 @@ func loadDotenv(path string) (map[string]string, error) { } out[strings.TrimSpace(key)] = unquote(strings.TrimSpace(value)) } - // Here rather than in Resolve: this is the one function both the - // credentials file 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 9c27ab4..4737b8e 100644 --- a/internal/client/config_test.go +++ b/internal/client/config_test.go @@ -106,8 +106,8 @@ func TestCredentialsPath(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Setenv("XDG_CONFIG_HOME", tt.xdg) t.Setenv("HOME", tt.home) - if got := credentialsPath(); got != tt.want { - t.Errorf("credentialsPath() = %q, want %q", got, tt.want) + if got := CredentialsPath(); got != tt.want { + t.Errorf("CredentialsPath() = %q, want %q", got, tt.want) } }) } @@ -309,7 +309,8 @@ func TestResolveMissingNamesEverySource(t *testing.T) { t.Fatal("want a missing-settings error") } for _, want := range []string{"missing Confluence URL (CONFLUENCE_URL)", "token (CONFLUENCE_TOKEN)", - "set them in one place", "the environment", filepath.Join(cfg, "markfluence", "credentials"), + "run markfluence credentials-init, or set them in one place", "the environment", + filepath.Join(cfg, "markfluence", "credentials"), "--env-file", credentialsDoc} { if !strings.Contains(err.Error(), want) { t.Errorf("error %q missing %q", err, want) @@ -364,7 +365,7 @@ func TestResolveMissingHalfOfThePair(t *testing.T) { _, err := Resolve("") if err == nil || !strings.Contains(err.Error(), "set it in ") || !strings.Contains(err.Error(), "markfluence/credentials, where CONFLUENCE_TOKEN is.") || - strings.Contains(err.Error(), "the environment") { + strings.Contains(err.Error(), "the environment") || strings.Contains(err.Error(), "credentials-init") { t.Errorf("token in the credentials file: err = %v, want only its place offered for the URL", err) } @@ -375,8 +376,9 @@ func TestResolveMissingHalfOfThePair(t *testing.T) { t.Fatal(err) } _, err = Resolve("") - if want := "set it in the environment, where CONFLUENCE_URL is."; err == nil || !strings.Contains(err.Error(), want) { - t.Errorf("URL in the environment: err = %v, want %q", err, want) + if want := "set it in the environment, where CONFLUENCE_URL is."; err == nil || !strings.Contains(err.Error(), want) || + strings.Contains(err.Error(), "credentials-init") { + t.Errorf("URL in the environment: err = %v, want %q and no credentials-init", err, want) } } diff --git a/internal/client/credentials.go b/internal/client/credentials.go new file mode 100644 index 0000000..5ff6938 --- /dev/null +++ b/internal/client/credentials.go @@ -0,0 +1,276 @@ +package client + +import ( + "encoding/json" + "errors" + "fmt" + "io" + "io/fs" + "net/http" + "os" + "path/filepath" + "strings" + "syscall" +) + +// credentialKeys are the settings the credentials file holds, in the order +// WriteCredentials writes them. +var credentialKeys = []string{urlEnv, usernameEnv, tokenEnv, cloudIDEnv} + +// credentialsHeader opens a file WriteCredentials writes. One line, so +// UnkeptLines can recognize it and a rewrite of a written file is quiet. +const credentialsHeader = "# Written by markfluence credentials-init. See " + credentialsDoc + "\n" + +// CredentialsPath is where the user's credentials file lives: +// $XDG_CONFIG_HOME/markfluence/credentials, else +// $HOME/.config/markfluence/credentials, on every platform. os.UserConfigDir is +// not used because on macOS it answers ~/Library/Application Support, which +// command-line users do not look in. +// +// A relative XDG_CONFIG_HOME is ignored, as the XDG spec says, and a relative +// or empty HOME means there is no credentials file ("" here). Either would +// make the file depend on the working directory, which is exactly what #188 +// removed. +func CredentialsPath() string { + if x := os.Getenv("XDG_CONFIG_HOME"); filepath.IsAbs(x) { + return filepath.Join(x, "markfluence", "credentials") + } + home, err := os.UserHomeDir() + if err != nil || !filepath.IsAbs(home) { + return "" + } + return filepath.Join(home, ".config", "markfluence", "credentials") +} + +// ReadCredentials reads the credentials file at path with the rules Resolve +// uses, returning nil when there is none. It raises no permission warning: its +// caller is credentials-init, which is about to rewrite the file 0600. +func ReadCredentials(path string) (map[string]string, error) { + return loadCredentials(path, readDotenv) +} + +// loadCredentials reads the credentials file with read, returning nil when +// there is none. A missing file, or a path that runs through something that is +// not a directory, means the user has not made one; a dangling symbolic link +// does not. Any other failure means they made one that cannot be used, and is +// an error: falling through silently would make a token rotation look like it +// had no effect. +func loadCredentials(path string, read func(string) (map[string]string, error)) (map[string]string, error) { + env, err := read(path) + switch { + case err == nil: + return env, nil + case errors.Is(err, fs.ErrNotExist) && isSymlink(path): + return nil, fmt.Errorf("reading credentials file %s: it is a symbolic link to a file that "+ + "does not exist", displayPath(path)) + case errors.Is(err, fs.ErrNotExist), errors.Is(err, syscall.ENOTDIR): + return nil, nil + default: + return nil, fmt.Errorf("reading credentials file %s: %w", displayPath(path), err) + } +} + +// isSymlink reports whether path itself is a symbolic link. A dangling link +// at the credentials path reads as "no such file", but it is a file the user +// made -- into a dotfiles repository that moved, or a volume not mounted -- so +// it must not read as "you have no credentials file". +func isSymlink(path string) bool { + fi, err := os.Lstat(path) + return err == nil && fi.Mode()&fs.ModeSymlink != 0 +} + +// DisplayPath shows a path under the home directory as ~/..., the form the +// docs use, so a message names the file the way a reader knows it. +func DisplayPath(path string) string { return displayPath(path) } + +func displayPath(path string) string { + home, err := os.UserHomeDir() + if err != nil || home == "" { + return path + } + if rel, err := filepath.Rel(home, path); err == nil && rel != ".." && + !strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "~" + string(filepath.Separator) + rel + } + return path +} + +// WriteCredentials writes values -- keyed by the CONFLUENCE_* names, an empty +// value omitted -- as the credentials file at path, mode 0600, creating its +// directory 0700 if it is missing. +// +// The write is a temporary file renamed over the target, so an interrupted +// write never leaves half a file, and a loose existing file comes out 0600. +// When path is a symbolic link (a dotfiles repository), the link's target is +// written and the link survives. +// +// It reads the result back through the parser Resolve uses and compares, the +// way the frontmatter writer verifies its own output: a file that says +// something other than what was asked for is worse than an error. +func WriteCredentials(path string, values map[string]string) error { + var b strings.Builder + b.WriteString(credentialsHeader) + want := map[string]string{} + for _, k := range credentialKeys { + v := values[k] + if v == "" { + continue + } + if strings.ContainsAny(v, "\r\n") { + return fmt.Errorf("%s holds a line break, which a credentials file cannot", k) + } + fmt.Fprintf(&b, "%s=%s\n", k, envValue(v)) + want[k] = v + } + for k := range values { + if !isCredentialKey(k) { + return fmt.Errorf("%s is not a credentials setting", k) + } + } + + target := path + if isSymlink(path) { + t, err := filepath.EvalSymlinks(path) + if err != nil { + return fmt.Errorf("resolving %s: %w", displayPath(path), err) + } + target = t + } + dir := filepath.Dir(target) + if err := os.MkdirAll(dir, 0o700); err != nil { + return fmt.Errorf("creating %s: %w", displayPath(dir), err) + } + if err := replaceFile(target, b.String()); err != nil { + return fmt.Errorf("writing %s: %w", displayPath(path), err) + } + + got, err := readDotenv(target) + if err != nil { + return fmt.Errorf("reading back %s: %w", displayPath(path), err) + } + if len(got) != len(want) { + return fmt.Errorf("%s did not read back as written", displayPath(path)) + } + for k, v := range want { + if got[k] != v { + return fmt.Errorf("%s did not read back as written: %s differs", displayPath(path), k) + } + } + return nil +} + +// UnkeptLines counts the lines of the credentials file at path that +// WriteCredentials would not keep: comments, other than its own header, and +// keys that are not credentials settings. A missing file has none. +// credentials-init says so before it rewrites a file somebody wrote by hand. +func UnkeptLines(path string) (int, error) { + data, err := os.ReadFile(path) + if errors.Is(err, fs.ErrNotExist) { + return 0, nil + } + if err != nil { + return 0, err + } + n := 0 + for _, line := range strings.Split(string(data), "\n") { + line = strings.TrimSpace(line) + switch { + case line == "", line+"\n" == credentialsHeader: + case strings.HasPrefix(line, "#"): + n++ + default: + key, _, _ := strings.Cut(strings.TrimPrefix(line, "export "), "=") + if !isCredentialKey(strings.TrimSpace(key)) { + n++ + } + } + } + return n, nil +} + +// replaceFile writes content to a temporary file beside target and renames it +// over target. os.CreateTemp creates the file 0600. +func replaceFile(target, content string) error { + f, err := os.CreateTemp(filepath.Dir(target), ".credentials-*") + if err != nil { + return err + } + tmp := f.Name() + _, err = f.WriteString(content) + if err == nil { + err = f.Sync() + } + if cerr := f.Close(); err == nil { + err = cerr + } + if err == nil { + err = os.Rename(tmp, target) + } + if err != nil { + _ = os.Remove(tmp) + } + return err +} + +func isCredentialKey(k string) bool { + for _, c := range credentialKeys { + if k == c { + return true + } + } + return false +} + +// envValue spells v so that readDotenv reads it back unchanged. The rule is the +// round trip itself rather than a list of characters: readDotenv trims all +// Unicode space and strips one pair of matching quotes, so v is written bare +// exactly when that leaves it alone, and double-quoted otherwise -- which +// unquote strips exactly, whatever v holds. +func envValue(v string) string { + if unquote(strings.TrimSpace(v)) == v { + return v + } + return `"` + v + `"` +} + +// FetchCloudID asks a Confluence Cloud site for its cloud ID, through +// GET {site}/_edge/tenant_info, which answers {"cloudId": "..."} with no +// credentials (docs/confluence/api.md). +// +// Not through send, which always sets basic auth: the route needs none, and +// its caller has no token yet. That loses send's retries and retry logging, +// which is acceptable because the caller goes on without a cloud ID when this +// fails. Its errors are the shapes FromRequest knows. +func FetchCloudID(siteURL string) (string, error) { + u := strings.TrimRight(siteURL, "/") + "/_edge/tenant_info" + req, err := http.NewRequest(http.MethodGet, u, nil) + if err != nil { + return "", wrapRequest(err) + } + req.Header.Set("Accept", "application/json") + resp, err := (&http.Client{Timeout: timeoutRead}).Do(req) + if err != nil { + return "", wrapRequest(err) + } + defer func() { _ = resp.Body.Close() }() + body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<16)) + if err != nil { + return "", wrapRequest(err) + } + if resp.StatusCode != http.StatusOK { + return "", &HTTPError{StatusCode: resp.StatusCode, Method: http.MethodGet, URL: u, Body: string(body)} + } + var out struct { + CloudID string `json:"cloudId"` + } + if err := json.Unmarshal(body, &out); err != nil { + return "", wrapRequest(fmt.Errorf("GET %s: %w", u, err)) + } + if out.CloudID == "" { + return "", wrapRequest(fmt.Errorf("GET %s: the answer has no cloudId", u)) + } + if err := validateCloudID(out.CloudID, u); err != nil { + return "", wrapRequest(err) + } + return out.CloudID, nil +} diff --git a/internal/client/credentials_test.go b/internal/client/credentials_test.go new file mode 100644 index 0000000..d355a04 --- /dev/null +++ b/internal/client/credentials_test.go @@ -0,0 +1,336 @@ +package client + +import ( + "errors" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +// credentialsTarget is a credentials path under a fresh directory that does +// not exist yet, so the write has to create it. +func credentialsTarget(t *testing.T) string { + t.Helper() + return filepath.Join(t.TempDir(), "markfluence", "credentials") +} + +func mode(t *testing.T, path string) os.FileMode { + t.Helper() + fi, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + return fi.Mode().Perm() +} + +// noTempFiles fails if a write left its temporary file behind. +func noTempFiles(t *testing.T, dir string) { + t.Helper() + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + for _, e := range entries { + if strings.HasPrefix(e.Name(), ".credentials-") { + t.Errorf("temporary file %s left behind", e.Name()) + } + } +} + +func TestWriteCredentialsModes(t *testing.T) { + path := credentialsTarget(t) + values := map[string]string{urlEnv: "https://wiki", usernameEnv: "bot", tokenEnv: "secret"} + if err := WriteCredentials(path, values); err != nil { + t.Fatal(err) + } + if m := mode(t, path); m != 0o600 { + t.Errorf("file mode = %#o, want 0600", m) + } + if m := mode(t, filepath.Dir(path)); m != 0o700 { + t.Errorf("new directory mode = %#o, want 0700", m) + } + noTempFiles(t, filepath.Dir(path)) + + got, err := ReadCredentials(path) + if err != nil { + t.Fatal(err) + } + for k, v := range values { + if got[k] != v { + t.Errorf("%s = %q, want %q", k, got[k], v) + } + } + if _, ok := got[cloudIDEnv]; ok { + t.Errorf("an empty cloud ID was written: %v", got) + } +} + +// TestWriteCredentialsLeavesAnExistingDirectoryAlone: the directory's mode is +// the user's business once it exists. +func TestWriteCredentialsLeavesAnExistingDirectoryAlone(t *testing.T) { + path := credentialsTarget(t) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Chmod(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := WriteCredentials(path, map[string]string{tokenEnv: "secret"}); err != nil { + t.Fatal(err) + } + if m := mode(t, filepath.Dir(path)); m != 0o755 { + t.Errorf("existing directory mode = %#o, want 0755 unchanged", m) + } +} + +// TestWriteCredentialsTightensALooseFile: rewriting a 0644 file leaves it 0600, +// and says nothing about the old mode on the way. +func TestWriteCredentialsTightensALooseFile(t *testing.T) { + warnings := captureSecurityWarnings(t) + path := credentialsTarget(t) + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(full), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chmod(path, 0o644); err != nil { + t.Fatal(err) + } + if _, err := ReadCredentials(path); err != nil { + t.Fatal(err) + } + if err := WriteCredentials(path, map[string]string{tokenEnv: "new"}); err != nil { + t.Fatal(err) + } + if m := mode(t, path); m != 0o600 { + t.Errorf("mode = %#o, want 0600", m) + } + if len(*warnings) != 0 { + t.Errorf("warnings = %q, want none from reading or writing", *warnings) + } +} + +// TestWriteCredentialsWritesThroughASymlink: a credentials file kept in a +// dotfiles repository and linked into place stays linked. +func TestWriteCredentialsWritesThroughASymlink(t *testing.T) { + path := credentialsTarget(t) + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + real := filepath.Join(t.TempDir(), "dotfiles-credentials") + if err := os.WriteFile(real, []byte(full), 0o600); err != nil { + t.Fatal(err) + } + if err := os.Symlink(real, path); err != nil { + t.Fatal(err) + } + if err := WriteCredentials(path, map[string]string{tokenEnv: "rotated"}); err != nil { + t.Fatal(err) + } + if !isSymlink(path) { + t.Fatal("the symbolic link was replaced by a file") + } + got, err := ReadCredentials(real) + if err != nil { + t.Fatal(err) + } + if got[tokenEnv] != "rotated" { + t.Errorf("link target token = %q, want rotated", got[tokenEnv]) + } + noTempFiles(t, filepath.Dir(path)) + noTempFiles(t, filepath.Dir(real)) +} + +// TestWriteCredentialsRoundTrips: every value reads back exactly, including the +// ones the parser would otherwise trim or unquote. +func TestWriteCredentialsRoundTrips(t *testing.T) { + for _, v := range []string{ + "plain", + " leading space", + "trailing nbsp ", + `"quoted"`, + `'single'`, + `"`, + `"half`, + "has#hash", + "a=b=c", + "export x", + `back\slash`, + } { + path := credentialsTarget(t) + if err := WriteCredentials(path, map[string]string{tokenEnv: v}); err != nil { + t.Errorf("%q: %v", v, err) + continue + } + got, err := ReadCredentials(path) + if err != nil { + t.Fatal(err) + } + if got[tokenEnv] != v { + t.Errorf("wrote %q, read back %q", v, got[tokenEnv]) + } + } +} + +// TestWriteCredentialsRefusesALineBreak: a value the format cannot hold is an +// error, and the file already there is untouched. +func TestWriteCredentialsRefusesALineBreak(t *testing.T) { + for _, v := range []string{"a\nb", "a\rb"} { + path := credentialsTarget(t) + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(full), 0o600); err != nil { + t.Fatal(err) + } + if err := WriteCredentials(path, map[string]string{tokenEnv: v}); err == nil { + t.Errorf("%q: want an error", v) + } + if b, _ := os.ReadFile(path); string(b) != full { + t.Errorf("%q: the existing file changed to %q", v, b) + } + noTempFiles(t, filepath.Dir(path)) + } +} + +func TestWriteCredentialsRefusesAnUnknownKey(t *testing.T) { + path := credentialsTarget(t) + if err := WriteCredentials(path, map[string]string{"CONFLUENCE_URLL": "x"}); err == nil { + t.Error("want an error for a key that is not a setting") + } + if _, err := os.Stat(path); !errors.Is(err, os.ErrNotExist) { + t.Errorf("a file was written: %v", err) + } +} + +func TestReadCredentialsMissingIsNil(t *testing.T) { + got, err := ReadCredentials(credentialsTarget(t)) + if err != nil || got != nil { + t.Errorf("ReadCredentials = %v, %v; want nil, nil", got, err) + } +} + +func TestFetchCloudID(t *testing.T) { + var auth []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + auth = append(auth, r.Header.Get("Authorization")) + if r.URL.Path != "/_edge/tenant_info" { + http.NotFound(w, r) + return + } + _, _ = w.Write([]byte(`{"cloudId":"d8febd08-5555-5555-5555-db37c2369ce5"}`)) + })) + defer srv.Close() + + got, err := FetchCloudID(srv.URL + "/") + if err != nil { + t.Fatal(err) + } + if got != "d8febd08-5555-5555-5555-db37c2369ce5" { + t.Errorf("cloud ID = %q", got) + } + if len(auth) != 1 || auth[0] != "" { + t.Errorf("Authorization headers = %q, want one request with none", auth) + } +} + +func TestFetchCloudIDFailures(t *testing.T) { + tests := []struct { + name string + status int + body string + }{ + {"not found", http.StatusNotFound, "no"}, + {"no cloudId", http.StatusOK, `{"other":"x"}`}, + {"not JSON", http.StatusOK, ``}, + {"a URL for a cloud ID", http.StatusOK, `{"cloudId":"https://x/y"}`}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(tt.status) + _, _ = w.Write([]byte(tt.body)) + })) + defer srv.Close() + if id, err := FetchCloudID(srv.URL); err == nil || !FromRequest(err) { + t.Errorf("FetchCloudID = %q, %v; want a request error", id, err) + } + }) + } +} + +// TestFetchCloudIDUnreachable: no response at all is a request error with no +// status, which is what lets a caller tell "offline" from "refused". +func TestFetchCloudIDUnreachable(t *testing.T) { + srv := httptest.NewServer(http.NotFoundHandler()) + u := srv.URL + srv.Close() + _, err := FetchCloudID(u) + var he *HTTPError + if err == nil || !FromRequest(err) || errors.As(err, &he) { + t.Errorf("err = %v, want a request error that is not an HTTPError", err) + } +} + +// TestHTTPErrorShapePredicates: each predicate answers for its own measured +// shape and no other, since credentials-init decides between refusing and +// asking on them. +func TestHTTPErrorShapePredicates(t *testing.T) { + const ( + gw = gatewayPrefix + "cloud-id/wiki/rest/api/user/current" + site = "https://example.atlassian.net/wiki/rest/api/user/current" + ) + tests := []struct { + name string + err HTTPError + scope, siteAuth, rejected bool + }{ + {"scope mismatch", HTTPError{StatusCode: 401, URL: gw, + Body: `{"code":401,"message":"Unauthorized; scope does not match"}`}, true, false, false}, + {"site HTML 401", HTTPError{StatusCode: 401, URL: site, + Body: `HTTP Status 401`}, false, true, false}, + {"gateway HTML 401", HTTPError{StatusCode: 401, URL: gw, + Body: `HTTP Status 401`}, false, false, false}, + {"v1 rejected", HTTPError{StatusCode: 403, URL: site, + Body: `{"message":"caller cannot access Confluence"}`}, false, false, true}, + {"plain JSON 401", HTTPError{StatusCode: 401, URL: site, + Body: `{"code":401,"message":"Unauthorized"}`}, false, false, false}, + } + for _, tt := range tests { + if got := tt.err.ScopeMismatch(); got != tt.scope { + t.Errorf("%s: ScopeMismatch = %v", tt.name, got) + } + if got := tt.err.SiteRejectedAuth(); got != tt.siteAuth { + t.Errorf("%s: SiteRejectedAuth = %v", tt.name, got) + } + if got := tt.err.RejectedCredential(); got != tt.rejected { + t.Errorf("%s: RejectedCredential = %v", tt.name, got) + } + } +} + +// TestUnkeptLines: a hand-written file's comments and stray keys are counted, +// and a file WriteCredentials wrote has none. +func TestUnkeptLines(t *testing.T) { + path := credentialsTarget(t) + if n, err := UnkeptLines(path); n != 0 || err != nil { + t.Errorf("missing file: %d, %v; want 0, nil", n, err) + } + if err := WriteCredentials(path, map[string]string{urlEnv: "https://wiki", tokenEnv: "s"}); err != nil { + t.Fatal(err) + } + if n, err := UnkeptLines(path); n != 0 || err != nil { + t.Errorf("written file: %d, %v; want 0, nil", n, err) + } + body := "# my token, rotated in May\nexport CONFLUENCE_URL=https://wiki\n\nOTHER=1\nCONFLUENCE_TOKEN=s\n" + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + if n, err := UnkeptLines(path); n != 2 || err != nil { + t.Errorf("hand-written file: %d, %v; want 2, nil", n, err) + } +} From 9deb03d98e6d5c2102417242c237594d07f6d198 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Wed, 23 Sep 2026 14:39:09 -0400 Subject: [PATCH 3/6] feat: add credentials-init, which writes the credentials file Answers #193. credentials-init prompts for the site URL, username, and token (read without echo), fetches the cloud ID for an atlassian.net site, checks the credentials with the request user-info makes, and writes the user's credentials file 0600. It writes nothing in the working directory. A failed check is classified by the response's shape: a rejected credential, a site-domain HTML 401, another 401/403, or a non-credential 404 writes nothing; a scope mismatch, a server error, or no response asks whether to save anyway. An existing file prefills every prompt. It refuses without a terminal on stdin or stderr, and refuses --json and --env-file, each as a plain error so --json still reports it. charmbracelet/x/term becomes a direct dependency; lipgloss already built it in. The plan is amended where the implementation found a simpler shape. --- CLAUDE.md | 9 +- _plans/051_credentials-init.md | 12 +- cmd/credentialsinit/credentialsinit.go | 383 ++++++++++++++ cmd/credentialsinit/credentialsinit_test.go | 487 ++++++++++++++++++ cmd/credentialsinit/main_test.go | 12 + cmd/credentialsinit/terminal.go | 85 +++ cmd/root.go | 4 +- cmd/root_test.go | 7 +- docs/commands/markfluence.md | 3 +- docs/commands/markfluence_credentials-init.md | 65 +++ go.mod | 2 +- 11 files changed, 1057 insertions(+), 12 deletions(-) create mode 100644 cmd/credentialsinit/credentialsinit.go create mode 100644 cmd/credentialsinit/credentialsinit_test.go create mode 100644 cmd/credentialsinit/main_test.go create mode 100644 cmd/credentialsinit/terminal.go create mode 100644 docs/commands/markfluence_credentials-init.md diff --git a/CLAUDE.md b/CLAUDE.md index d741ede..8160964 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -2,7 +2,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. -markfluence is a Go CLI (cobra + `net/http` + [goldmark](https://github.com/yuin/goldmark) + lipgloss) for publishing Markdown to Confluence. The dependency list is deliberately short — six direct, and [go-udiff](https://github.com/aymanbagabas/go-udiff) (`diff`'s unified output, zero transitive dependencies of its own) was the last one added, so adding a seventh is a decision rather than a detail. It was originally a Python tool; the Go rewrite is now the sole implementation. +markfluence is a Go CLI (cobra + `net/http` + [goldmark](https://github.com/yuin/goldmark) + lipgloss) for publishing Markdown to Confluence. The dependency list is deliberately short — seven direct. The last two added were [go-udiff](https://github.com/aymanbagabas/go-udiff) (`diff`'s unified output, zero transitive dependencies of its own) and [charmbracelet/x/term](https://github.com/charmbracelet/x) (`credentials-init`'s no-echo token prompt, #193), which lipgloss already brought into the build, so it added no code to the binary. Adding an eighth is a decision rather than a detail. It was originally a Python tool; the Go rewrite is now the sole implementation. ## Commands @@ -33,9 +33,9 @@ A change that adds a package, exports a function, adds a make target, or introdu The CLI needs a site URL (`CONFLUENCE_URL`), a username (`CONFLUENCE_USERNAME`), and an API token (`CONFLUENCE_TOKEN`), plus an optional cloud ID (`CONFLUENCE_CLOUD_ID`). Each resolves key by key from three sources, highest first: **the file named by `--env-file` > the environment > the user's credentials file** (`$XDG_CONFIG_HOME/markfluence/credentials` when that is absolute, else `$HOME/.config/markfluence/credentials`, on every platform — `os.UserConfigDir` would answer `~/Library/Application Support` on macOS). There are **no credential flags** (#188): `--url`/`--username`/`--cloud-id` existed and were removed, because a flag for three of four settings always meant credentials from two places. -A cloud ID routes requests through the platform API gateway instead of the site domain, which a **scoped** API token requires. Basic auth is unchanged; only the base URL moves. Leave it unset for an unscoped personal token. **Data Center/Server is out of scope** (README): pages go through v2, which DC does not have, and DC behavior could never be verified by the method [docs/confluence/](docs/confluence/) holds everything else to, since nobody here has an instance. Government/isolated Cloud is *not* out of scope, only untested — same APIs, with one known risk, an emitted Atlassian host that is not the site. The cloud ID is not a secret, but it still resolves from the URL's source, since it names one site. Gateway details, the scope list, and what is verified vs. assumed: [docs/confluence/api.md](docs/confluence/api.md). +A cloud ID routes requests through the platform API gateway instead of the site domain, which a **scoped** API token requires. Basic auth is unchanged; only the base URL moves. An unscoped personal token works either way (it answers 200 through the gateway too), which is why `credentials-init` saves one for every `*.atlassian.net` site without asking what kind of token it has — and for no other host, since `gatewayPrefix` is `api.atlassian.com` and Government/isolated Cloud use a different gateway. **Data Center/Server is out of scope** (README): pages go through v2, which DC does not have, and DC behavior could never be verified by the method [docs/confluence/](docs/confluence/) holds everything else to, since nobody here has an instance. Government/isolated Cloud is *not* out of scope, only untested — same APIs, with one known risk, an emitted Atlassian host that is not the site. The cloud ID is not a secret, but it still resolves from the URL's source, since it names one site. Gateway details, the scope list, and what is verified vs. assumed: [docs/confluence/api.md](docs/confluence/api.md). -**Credentials come only from sources the user chose.** Nothing is discovered from the working directory or a project: a walked-up `.env` let a cloned checkout, or an ancestor directory you do not control, set `CONFLUENCE_URL` and send your token wherever it liked. Both files use the env format (a minimal built-in parser — no shell expansion); `.env.example` is the template. A missing explicit `--env-file` is an error; a missing credentials file is not, but one that exists and cannot be read is (`ENOTDIR` counts as missing). The credentials file is read **only when a higher source leaves a setting unset**, so a broken or loose one cannot fail or warn on a run that never uses it. Three rules keep a token from going somewhere unintended, and they are the part to preserve: **the URL and the token must come from the same source** or `Resolve` fails naming both (key-by-key merging would otherwise send one source's token to another's URL — which also means `CONFLUENCE_URL=… markfluence …` alone fails when the token lives in the credentials file, by design); **the cloud ID is read only from the URL's source** and ignored anywhere else, because it names one site the way the URL does, and it is ignored rather than refused because a higher source cannot say "no cloud ID" (an empty value is unset) — but one ignored from a place *above* the URL's earns a warning, since that is someone who exported it on purpose and would otherwise get an unexplained 401, while one below it is the ordinary other-instance case and stays quiet; and **the username may come from anywhere**. `internal/client.Resolve(envFile)` is the single place this is read and validated. Tests must never see a developer's real credentials file: every package whose code calls `Resolve`, and `internal/client` itself, has a `TestMain` calling `testenv.RunIsolated`. +**Credentials come only from sources the user chose.** Nothing is discovered from the working directory or a project: a walked-up `.env` let a cloned checkout, or an ancestor directory you do not control, set `CONFLUENCE_URL` and send your token wherever it liked. Both files use the env format (a minimal built-in parser — no shell expansion); `.env.example` is the template, and `markfluence credentials-init` (#193) writes the credentials file for a person at a terminal. A missing explicit `--env-file` is an error; a missing credentials file is not, but one that exists and cannot be read is (`ENOTDIR` counts as missing). The credentials file is read **only when a higher source leaves a setting unset**, so a broken or loose one cannot fail or warn on a run that never uses it. Three rules keep a token from going somewhere unintended, and they are the part to preserve: **the URL and the token must come from the same source** or `Resolve` fails naming both (key-by-key merging would otherwise send one source's token to another's URL — which also means `CONFLUENCE_URL=… markfluence …` alone fails when the token lives in the credentials file, by design); **the cloud ID is read only from the URL's source** and ignored anywhere else, because it names one site the way the URL does, and it is ignored rather than refused because a higher source cannot say "no cloud ID" (an empty value is unset) — but one ignored from a place *above* the URL's earns a warning, since that is someone who exported it on purpose and would otherwise get an unexplained 401, while one below it is the ordinary other-instance case and stays quiet; and **the username may come from anywhere**. `internal/client.Resolve(envFile)` is the single place this is read and validated. Tests must never see a developer's real credentials file: every package whose code calls `Resolve`, and `internal/client` itself, has a `TestMain` calling `testenv.RunIsolated`. ## Commit conventions @@ -55,6 +55,7 @@ Module `github.com/mozilla/markfluence` (`go 1.25`). `main.go` is a shim to `cmd - `cmd/root.go` — the cobra root: `--env-file`/`--root`/`--debug`/`--no-color`/`--json` persistent flags, version from `internal/buildinfo`, and registration of every subcommand. `Execute()` prints cobra-generated errors (bad args/flags) but not `ui.ErrSilent`, which marks a failure a command already reported. - `cmd/{update,create,check,diff,pageinfo,spaceinfo,read,export,children,find,search,userfind,userinfo}/` — one package per command (each exports `Cmd`), orchestrating the `internal` packages and `internal/ui` output. `create` is three-phase and transactional (preflight all, reserve parents-first in topological order, then publish); `check` is read-only on both the server and disk and touches neither, since it never constructs one (see its own bullet below). `create` accepts a **page or a folder** as `parent`: `checkParentInSpace` asks the page route, then `/folders/{id}`, and finding nothing as a page proves nothing until both have been asked — that single missing fallback was #68, since Confluence itself accepts a folder `parentId` with no other accommodation. It also validates a frontmatter `page_id` **first** (before space/parent/title lookups: most specific error, three fewer API calls) and treats all three outcomes as failures — non-numeric, resolves to nothing, or already taken. The middle one is the bug from #18: publishing anyway would create a second page and overwrite the id, so an id that can't be explained is never "create a new page". Both "already exists" errors (page_id taken, title clash) link the page in the way; the `pageIDFailure` typed error carries `page_id`/`url` so `--json` reports them as fields, which is the only case where a failed result names a page. Preflight also **converts every file and throws the page away**, keeping only the error (#127/S7): a defect the converter refuses is a property of the file on disk, so asking before the reserve phase is what keeps it from leaving a content-less page and a `page_id` the author has to undo by hand. The result cannot be reused by `publishOne` — reserve seeds the batch's ids into the shared link index in between, so an in-set link renders unresolved in preflight and resolves in publish, and phase 1's `Broken`/`Warnings` are discarded for that reason. The *error* is identical across the two, for a narrower reason than "the converter ignores the index": whether `renderImage` runs at all does depend on it (`renderLink` skips a broken link's children, and `Broken` is decided by `FileExists`), but reserve only calls `SetPage`, which writes `idx.pages` alone — nothing there can raise an error or change one's text, and `FileExists`/`Anchor` read `idx.anchors`, fixed at `Build` time. Making `SetPage` also mark a file as existing would break it; pinned by `TestErrorDoesNotDependOnTheIndex`. It is called **last**, after every server check, so `page_id`-first precedence is untouched, and its failure carries `CodeConvert` on the `failure` struct rather than `abort()`'s old hardcoded `VALIDATION`. +- `cmd/credentialsinit/` — `credentials-init` (#193, `_plans/051`): prompt for the URL, username and token, check them, and write the credentials file. It writes **only** that file, never anything in the working directory — project setup is #5's `init`, and a person fixing auth from `~` must not get a `~/markfluence.yaml` that becomes the root for every Markdown file under their home. Four things decide its shape. **A failed check is classified by the response's shape, never its status**: a rejected credential, a site-domain HTML 401 (a scoped token with no cloud ID), any other 401/403, and a non-credential 404 refuse and write nothing, while a scope-mismatch 401 (the token *authenticated* but lacks `read:confluence-user`), a 5xx, or no response at all asks `Save anyway?` — the shapes are `HTTPError`'s own predicates, so the check and the error hint cannot disagree. **The cloud ID is fetched, never asked for**, and only for `*.atlassian.net` (see Configuration). **It refuses rather than degrades**: no terminal on stdin *or* stderr (the prompts go to stderr), `--json`, `--env-file`, and no credentials path are each a plain returned error, because `Execute` turns that into an `errorObject` under `--json`, where a `ui.Error` would print nothing; it is on `noJSONEnvelope`. And **the terminal is the only code touching a tty** (`terminal.go`): `runInit` takes a `prompter` and a `deps` of stubs, so every decision is tested with a scripted prompter. `terminal.go` reads lines a byte at a time (a `bufio.Reader` would swallow typed-ahead input `ReadPassword` then never sees) and restores the terminal on SIGINT/SIGTERM/SIGHUP during the token read, since `ReadPassword` leaves `ISIG` set and its own `defer` never runs on a signal. An existing file prefills every prompt (never showing the token) and is rewritten whole, saying first how many lines it will not keep (`client.UnkeptLines`). - `cmd/check/` — `check` (#42): validate one or more Markdown FILEs against the converter and frontmatter rules with **no network access, no credentials, and no writes** — the first command whose `run()` never constructs a `client.ConfluenceClient` (`root.go`'s `PersistentPreRunE` doesn't force one into existence either, so nothing upstream requires it). It builds `root`/`index` per file exactly like `update`/`create` (`internal/project.Cache`/`internal/linkindex.Cache`), against hardcoded `baseURL`/`spaceKey` (the regression suite's own `https://wiki.example.net`/`ENG`) rather than flags — both are read only to build a rewritten doc-link's *text*, and nothing in `Broken`/`Warnings` reads either, so hardcoding them costs nothing and makes `check` byte-identical across machines. Frontmatter validation is deliberately narrow: an unparseable/unterminated block (`frontmatter.ErrUnterminatedFrontmatter`, plus everything real YAML now refuses — a nested value, a `|` block, a duplicate key, a tab indent, a reserved indicator), an invalid `page_width` (`pagewidth.Declared`), a present-but-non-numeric `page_id` (`pageref.IsDigits`), a **present-but-empty `title`**, and a **present-but-empty `page_status`** — never whether `page_id`/`space`/`parent` are set at all, since `check` cannot know whether the caller is about to `create` or `update`, and a false positive there is worse than a miss. `title` is the one exception to that reasoning and only in its present-but-empty form: `create` and `update` both reject it, so no verb makes it valid and there is no false positive to have. An *absent* title stays unreported, since `update` accepts one and keeps the live page's title. `page_status` is the first field whose *vocabulary* `check` cannot reach at all — a space's statuses are that space's own configuration, read from Confluence — so it checks the shape and its `Long` says so, because a green `check` otherwise reads as a promise the status will publish. A `broken` result is `ok: false` with `error`/`code` both left `null`: unlike every other failure, `broken`/`warnings` already say everything there is to say, so `code: VALIDATION` is reserved for `status: failed` (a file that never reached the converter at all). `--show-html` surfaces `ConfluencePage.HTML`/`Attachments` — nothing else in the CLI ever prints either — as `debug: {html, attachments} | null`; `html` stays compact/unindented in `--json` (matching what `update`/`create` would literally publish) while human output indents it by nesting depth (`indentHTML`, a per-line indent based on tag-open/close counting, not a whitespace-normalizing reformat, since the renderer already breaks lines at every structural boundary and reformatting within a line could alter meaningful inline text). - `cmd/diff/` — `diff` (#154): what differs between one file and its page, fetching the page and rendering it to Markdown the way `export` does. Writes nothing, to disk or to Confluence. **One file**, deliberately the opposite of `update`/`check` — a diff over a tree is output nobody reads, and #148's `status` is the tree-wide view. Four things decide its shape. **The output is two things on two streams**: stdout carries the body as a unified diff and *nothing else*, so `diff FILE > my.diff` is a patch — applied with `patch -R -p1`, since the file on disk is the `+++` side, or a plain `patch -p1` from `diff --reverse` — while stderr carries the frontmatter half as a per-field report (`frontmatterReport`). The split is not presentational — a value difference is a one-line fact rather than a hunk, and only a report can carry the **provenance** (`pagemeta.Resolved.Origin`, via `sourceLabel`) that makes "the title differs" say which file to edit; keeping stdout to one document is `children --space`'s hint rule with a stronger case. **The frontmatter block is shared between the two diffed documents byte for byte** rather than diffed (`documents`), which is what makes hunk line numbers file-relative rather than body-relative (a `patch` "succeeded at 23 (offset 18 lines)" fudge otherwise), keeps the local side the file's *exact bytes* so a patch can apply to it — never normalized, so a missing final newline stays the `\ No newline at end of file` marker unified diff has for it — and means no patch can rewrite a `page_id`; the author's blank line after the delimiter belongs to that shared prefix, or a file with two diffs against a rendered side with one. A **label is recognised by its position and never by its prefix** (`bodyLines`/`headerLines`): a removed body line is `-` plus its content, so `--json is a flag` arrives as `---json is a flag` and a prefix test read it as a file label — dropping it from the counts and colouring it as one, which Markdown thematic breaks and flag-heavy documentation hit constantly. An **empty page body is compared as an empty body** rather than refused the way `read` refuses one: a folder cannot reach that point (v2 answers a folder id with 404), so the reachable case is a genuinely empty page — what `create` leaves for a body-less file — and the whole local file is simply an addition. **Only fields the file declares are compared**, because those are the ones publishing asserts (an absent `labels`/`page_width` leaves the page's alone per L9, an absent `title` keeps the live one), which is the single rule that removes most of what #154 accepted as unavoidable frontmatter noise; a page-side read that *fails* is reported `comparable: false` and does **not** count as a difference, since `read`/`export`'s omit-on-failure would here claim publishing adds a label that may already be there. `parent` is compared as a **resolved id**, not a spelling — a file from an export tree names its parent by relative `.md` path — and `page_status` for the same reason, since the match is case-insensitive and only an id ever travels, so two spellings of one status are not a difference; and `page_width` consults the project file's own setting too, since that is a real declaration (#100) `update` acts on, labelled distinctly (`markfluence.yaml (project default)`) because it is a different place in the file than a `pages:` entry. And **exit codes are `diff(1)`'s**: `0` identical, `1` differs, `2` *any* trouble, including the operational failures every other command reports as `1`. A deliberate departure from `docs/json-output.md`'s contract, and the reason the command is worth having in CI; `--reverse` swaps the sides so the patch applies Confluence→file without `patch -R`. The diff labels are root-relative, **except when no `markfluence.yaml` exists anywhere**: `project.Discover` then falls back to the starting directory, which here is the file's own, so a root-relative path would be the bare base name and drop the `docs/` a reader typed — `reportPath` gates on `root.File != ""` and uses the path as typed otherwise. Colour goes through `internal/ui`'s `DiffAdded`/`DiffRemoved`/`DiffHunk` applied line by line to the library's own output (`renderDiff`) rather than re-rendering its structured hunks, since the `@@ -a,b +c,d` arithmetic is exactly what a library was chosen to get right; the styler is a parameter for `cmd/search`'s reason — lipgloss emits nothing when stdout is not a terminal, so a test wired to the real style proves nothing. - `cmd/export/` — `export`: `pagedoc` for the body, `attachfile` for the attachments. **`--depth` exports a subtree** (`0` default / a number / `all`), **`--space KEY` a whole space** (requiring an explicit `--depth`, since the default would export nothing and defaulting to `all` would make a typo walk a whole space), and a **folder** may be the target — a folder and a space have no file of their own, so their children become the top level, which is why `layout`'s `rootRef` carries the id children hang off *separately* from whether anything is written for it: the walk's top-level nodes report a folder as their parent but report nothing for a space, and conflating the two placed every page at the destination root. `layout` owns every path an export writes (mirrored hierarchy: `.md` plus a `/` for children and unrecorded attachments, a folder as a bare directory) and the `-` suffix for a group of siblings that slug the same — applied to *every* member so a filename never depends on walk order, and disambiguating rather than refusing because a space nobody can retitle would otherwise be unexportable over a punctuation variant (an exported filename is ergonomic; identity is `page_id`, per L8). `parent:` is a relative path to the parent's own `.md` so the tree publishes into fresh pages, except for the export root and a page whose parent is a folder, which keep an id. Two things the layout buys that are easy to miss: page directories are unique, which is what makes page-scoped attachment placement collision-free — so `pagedoc.Placement` must carry the *disambiguated* directory (`AttachmentDirFor`), or two colliding siblings silently share one attachment file, which no checksum catches because a native attachment has none. `destClaims` reserves every page's destination before any attachment is written, since a recorded `path=` is server data that can name a page's own file and a parent's attachments are written before its children exist — otherwise the attachment lands first and the page is reported `skipped (exists)`. A page already on disk skips its *render* but not its attachment pass, so a retry resumes a run that died mid-download. `markfluence.yaml` is planted at `dest` for a multi-page export **before the first page**, because a partial tree with no marker republishes every shared asset as `IMAGE BROKEN`. Markdown only. An attachment with a recorded `path=` lands there; one without is page-scoped. Still no `--attachments-dir`, but for a different reason than before the naming change: moving an asset no longer renames its attachment, so it is no longer unsafe — it would simply reintroduce the collision a base name has to refuse, since two pages' `diagram.png` cannot share a directory. Only referenced attachments are exported, found by scanning raw storage for `ri:filename` (not just `ac:image`, which is all the converter special-cases, so a link target or a macro-internal reference would otherwise be dropped). A reference with no attachment is a warning, not a failure. @@ -75,7 +76,7 @@ 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 that **declares** a `page_id` — in its own frontmatter or in a `pages:` entry for it (#139), resolved through `pagemeta` after discovering the root from the file's own directory (stat'd first, so `123.md` is a file). Every command taking a page uses it, which is why the manifest lookup lives here rather than at the seven call sites: without it the page argument meant one thing to `update` and another to `page-info`/`read`/`children`/`export`/`attachment-*`, so a file `update` could publish could not be named to any of them. A **disagreement** between the two locations is fatal here — it is the question being asked — while a **malformed** `markfluence.yaml` is not: a project file this resolver never consults must not make `page-info 123` fail, and the commands that bound reads by the root report it themselves. `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` and `update` report both and `check` reports the non-numeric one, and a reader should recognize the same problem across all of them. 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)). Page **status** — the title lozenge — is v1 only and lives in `state.go` (`PageState`/`AvailableStates`/`SetPageState`, plus `StateVocabulary`, which keeps the space's statuses and the caller's own custom ones in separate fields because only the first are valid for a file); v2 carries no state field on a page in any form and there is no expansion that adds one, so a lozenge is one extra request per page, always. `AvailableStates` must be asked about the page the status is going on — its answer varies by caller *and* page, and it needs edit permission on that page. There is deliberately **no `ClearPageState`**: the `DELETE` route exists, but nothing can reach it until a clearing spelling does, and an unused write method is a loaded gun. 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. **Four 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` (whose loop is `walkV2`, shared rather than copied so a counting caller can stream — a second implementation of v2 paging is how one of them comes to terminate on a short page), 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. **`/wiki/rest/api/space` is a fifth, and the one that punishes the obvious choice**: it pages by `start`/`limit` offset exactly as the child collections do, and **a short page is not the end** — asked for 250 from `start=0` it answered 200, and `start=200` then answered 250 more, against 525 spaces. `listV1` stops on that short page, so `WalkSpaceOperations` has its own loop terminating on an **empty** page, advancing by rows *returned* rather than by the limit asked for, bounded by `maxSpacePages` since an empty page is the only end signal offset paging has here. The first version of the probe that found this trusted the short page and reported 200 spaces with total confidence ([users.md](docs/confluence/users.md)). **`/wiki/rest/api/search/user` is a fourth**, and the one that looks most like an existing scheme while not being it: it pages by `start`/`limit` offset exactly as `listV1` does, so `listV1` is the obvious home for it and is a trap — the route **caps a page at 100 rows while echoing back whatever limit was requested** (101, 250 and 500 all answer 100), and `listV1` asks for `v1PageSize = 250` and reads a short page as the end of the collection, so it would truncate every result set past 100 with no error at all. `SearchUsers` lives in its own `users.go` with `userPageSize = 100` and the measurement beside it for that reason, and `TestPageCapDoesNotTruncate` is the regression. It also carries `maxUserPages`, `searchCQLBounded`'s guard for the same hazard reached a different way: a short page is the *only* end signal offset paging here has, so a server that clamped `start` — or ignored it the way `/wiki/rest/api/search` ignores it outright — would return a full page forever and an unbounded walk would collect rows until it ran out of memory. Its `totalSize` is a *third* kind of wrong: not absent like v1's and not an estimate like `/search`'s, but the row count of the page just fetched, so `limit=3` answers 3 and `limit=500` answers 100 against 304 real matches. `user.go` holds the two identity routes (`CurrentUser`, `UserInfo` — both `read:confluence-user`, both seeing a deactivated account the directory cannot) and `WalkSpaceOperations`; `space.go` holds `GetSpace` (one v1 request answering identity, the caller's own operations, description, labels and the homepage *with its title*), `SpaceStateSettings` (space-admin only, so a 403 that is not a rejected credential is `(nil, nil)` rather than an error) and `WalkSpacePages`. Both space routes decode the space `id` as a `json.Number`: v1 reports it as a **number** where every v2 route reports a string, and `homepage.id` in the same response is a string. 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-file reader, plus the **permission warning** (#136): a *regular* credentials file or `--env-file` reachable by anyone but its owner (`mode.Perm()&0o077`; a pipe from `--env-file <(pass show …)` reports 0440 and no chmod can fix it) *and* containing `CONFLUENCE_TOKEN` earns a warning naming the file, its mode, and the `chmod`. Both halves matter — a file 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 credentials file and `--env-file` pass through, and reaches the reader through `SetSecurityWarner` for the same reason `SetRetryLogger` exists — wired to `cmd/root.go`'s `reportSecurityWarning`, which prints it (human mode) *and* records it via `jsonout.AddWarning`, since stderr under `--json` is a schema-validated document with no room for a stray line. A group/world-*writable* file with no token in it is knowingly **not** covered: the same-source rule means a URL rewritten there can no longer be paired with a token from somewhere else, and the cloud ID follows the URL. 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)). Page **status** — the title lozenge — is v1 only and lives in `state.go` (`PageState`/`AvailableStates`/`SetPageState`, plus `StateVocabulary`, which keeps the space's statuses and the caller's own custom ones in separate fields because only the first are valid for a file); v2 carries no state field on a page in any form and there is no expansion that adds one, so a lozenge is one extra request per page, always. `AvailableStates` must be asked about the page the status is going on — its answer varies by caller *and* page, and it needs edit permission on that page. There is deliberately **no `ClearPageState`**: the `DELETE` route exists, but nothing can reach it until a clearing spelling does, and an unused write method is a loaded gun. 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. **Four 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` (whose loop is `walkV2`, shared rather than copied so a counting caller can stream — a second implementation of v2 paging is how one of them comes to terminate on a short page), 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. **`/wiki/rest/api/space` is a fifth, and the one that punishes the obvious choice**: it pages by `start`/`limit` offset exactly as the child collections do, and **a short page is not the end** — asked for 250 from `start=0` it answered 200, and `start=200` then answered 250 more, against 525 spaces. `listV1` stops on that short page, so `WalkSpaceOperations` has its own loop terminating on an **empty** page, advancing by rows *returned* rather than by the limit asked for, bounded by `maxSpacePages` since an empty page is the only end signal offset paging has here. The first version of the probe that found this trusted the short page and reported 200 spaces with total confidence ([users.md](docs/confluence/users.md)). **`/wiki/rest/api/search/user` is a fourth**, and the one that looks most like an existing scheme while not being it: it pages by `start`/`limit` offset exactly as `listV1` does, so `listV1` is the obvious home for it and is a trap — the route **caps a page at 100 rows while echoing back whatever limit was requested** (101, 250 and 500 all answer 100), and `listV1` asks for `v1PageSize = 250` and reads a short page as the end of the collection, so it would truncate every result set past 100 with no error at all. `SearchUsers` lives in its own `users.go` with `userPageSize = 100` and the measurement beside it for that reason, and `TestPageCapDoesNotTruncate` is the regression. It also carries `maxUserPages`, `searchCQLBounded`'s guard for the same hazard reached a different way: a short page is the *only* end signal offset paging here has, so a server that clamped `start` — or ignored it the way `/wiki/rest/api/search` ignores it outright — would return a full page forever and an unbounded walk would collect rows until it ran out of memory. Its `totalSize` is a *third* kind of wrong: not absent like v1's and not an estimate like `/search`'s, but the row count of the page just fetched, so `limit=3` answers 3 and `limit=500` answers 100 against 304 real matches. `user.go` holds the two identity routes (`CurrentUser`, `UserInfo` — both `read:confluence-user`, both seeing a deactivated account the directory cannot) and `WalkSpaceOperations`; `space.go` holds `GetSpace` (one v1 request answering identity, the caller's own operations, description, labels and the homepage *with its title*), `SpaceStateSettings` (space-admin only, so a 403 that is not a rejected credential is `(nil, nil)` rather than an error) and `WalkSpacePages`. Both space routes decode the space `id` as a `json.Number`: v1 reports it as a **number** where every v2 route reports a string, and `homepage.id` in the same response is a string. 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. `credentials.go` holds the credentials file's own functions — `CredentialsPath`, `ReadCredentials` (the rules `Resolve` uses, minus the permission warning, for a caller about to rewrite the file), `WriteCredentials` (temp-file-and-rename at `0600`, writing through a symbolic link, quoting a value exactly when `readDotenv` would not read it back unchanged, then reading the result back and comparing), `UnkeptLines`, `DisplayPath`, and `FetchCloudID`, the unauthenticated `tenant_info` request, which bypasses `send` because `send` always sets basic auth. `HTTPError.ScopeMismatch`/`SiteRejectedAuth`, beside `RejectedCredential`, are the shapes `hint` matches, exported for `credentials-init`. `config.go` holds `Resolve` and the env-file reader (`loadDotenv`, which warns, over `readDotenv`, which does not), plus the **permission warning** (#136): a *regular* credentials file or `--env-file` reachable by anyone but its owner (`mode.Perm()&0o077`; a pipe from `--env-file <(pass show …)` reports 0440 and no chmod can fix it) *and* containing `CONFLUENCE_TOKEN` earns a warning naming the file, its mode, and the `chmod`. Both halves matter — a file 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 credentials file and `--env-file` pass through, and reaches the reader through `SetSecurityWarner` for the same reason `SetRetryLogger` exists — wired to `cmd/root.go`'s `reportSecurityWarning`, which prints it (human mode) *and* records it via `jsonout.AddWarning`, since stderr under `--json` is a schema-validated document with no room for a stray line. A group/world-*writable* file with no token in it is knowingly **not** covered: the same-source rule means a URL rewritten there can no longer be paired with a token from somewhere else, and the cloud ID follows the URL. 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 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; `mention.go` owns the user-mention mapping in both directions (#91): a mention is 80% of all `` usage, and it converts to `[@Display Name](https://home.atlassian.com/people/{accountId})`. Three things decide its shape, each measured rather than reasoned. **The URL is Atlassian Home, not the site** — Confluence's own renderer still emits `{site}/wiki/people/{id}`, which no longer resolves usefully in a browser, so a mention in Markdown names *no site*, needs nothing from configuration, and is therefore recognisable by `check` with no client at all. **Matching is on the path, ignoring host and query**, because several spellings of one target circulate (the Home URL, the modal's `?cloudId=` copy, the `/o/{orgId}` redirect, both Confluence forms, root-relative) and none of `cloudId`/`ref`/the org segment identifies the person — only the id does, and `ri:user` stores nothing else. **The `@` on the link text is the marker**, load-bearing rather than decoration: the URL cannot tell "mention this person" from "link to their profile", so without it anyone writing the second would silently get the first. The account id is **not** pattern-validated (two shapes are live on one instance, so a pattern tight enough for one rejects the other) and `ri:local-id` is never emitted (a mention carrying only the id resolves to the same person, verified via ADF). `MentionMarkdown` is the shared builder for a mention's whole Markdown line, exported because `user-find` (#143) prints exactly it and a second copy there would be a second place to get the `@` marker and name escaping right. `ConfluencePage.Mentions` reports the ids the *forward* direction emitted so the caller can warn about one that names nobody — the `Attachments` arrangement, and necessary because Confluence accepts any id and renders `@Unlicensed user` rather than failing, and the profile URL 200s either way. An unresolvable mention still renders as a link, `[@Unlicensed user](…)` — that wording mirrors Confluence because the only ids reaching it are the ones the page labels that way: a **deactivated account resolves normally** and keeps its name (measured across every mention on a real page — 18 of them, six departed, all 200, returning e.g. `Mark Reid (Deactivated)`), so a departed colleague never takes that branch. Name resolution is `pagedoc.UserCache`, a per-run cross-page cache, and the tri-state is the part to preserve: `client.LookupUser` separates a name from `ErrNoSuchUser` from an unaskable question, `StorageOptions.UserNames` carries that as name / `""` / absent, and only a **confirmed** absence renders the placeholder. Flattening those would write a fabricated name over a real one the moment a VPN dropped mid-export, across a whole tree, into a file that then looks authoritative — which is also why the cache remembers a 404 but not a timeout (one is an answer, the other is not) and why `MentionWarnings` warns only about a confirmed absence; `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 and a space link convert, and 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); Plain text used as a Markdown link's text goes through `escapeLinkText` (`\`, `[`, `]`), applied to the *raw* sources only — a page title, a space key, an anchor, a display name — and via `inlineTextForLink` to a body whose every descendant is a text node. Never to already-rendered output: an `ac:link-body` holding markup has been converted to Markdown already, and escaping it yields a literal `\*\*bold\*\*`. Both directions are tested, because a fix at either extreme passes one and fails the other. `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 `