From 7cad23bd588b861582df729dbd6fe6c2f561ea4d Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Wed, 23 Sep 2026 12:04:02 -0400 Subject: [PATCH 1/7] docs: the credentials-file plan Plans #188: a user-scoped credentials file replaces .env discovery, and the --url, --username, and --cloud-id flags go away. --- _plans/050_credentials-file.md | 248 +++++++++++++++++++++++++++++++++ 1 file changed, 248 insertions(+) create mode 100644 _plans/050_credentials-file.md diff --git a/_plans/050_credentials-file.md b/_plans/050_credentials-file.md new file mode 100644 index 0000000..9f61af5 --- /dev/null +++ b/_plans/050_credentials-file.md @@ -0,0 +1,248 @@ +# 050: a user-scoped credentials file + +Answers #188. Credentials come only from sources the user chose. A +user-scoped credentials file replaces the per-project `.env`, and the +`--url`, `--username`, and `--cloud-id` flags go away. + +This plan adds the implementation detail, the order of the work, and how to +check it. + +## Decisions + +**D1. Three sources, highest precedence first.** Each setting resolves key by +key from: + +1. the file named by `--env-file PATH`, +2. the environment (`CONFLUENCE_URL`, `CONFLUENCE_USERNAME`, + `CONFLUENCE_TOKEN`, `CONFLUENCE_CLOUD_ID`), +3. the credentials file. + +An empty value counts as unset, as it does now (`resolveValue`), so +`CONFLUENCE_TOKEN=` in a higher source lets a lower source supply the token. +The explicit file ranks above the environment on purpose: a token exported in +a shell profile must not reach the URL in a file named for another instance. + +**D2. Where the credentials file is.** `$XDG_CONFIG_HOME/markfluence/credentials` +when `XDG_CONFIG_HOME` is set **and absolute**, else +`$HOME/.config/markfluence/credentials`, on every platform. The XDG spec says to +ignore a relative value, and a relative one would make the file depend on the +working directory, which is the problem #188 removes. The same holds for +`HOME`: `os.UserHomeDir` returns it as set, so a relative or empty `HOME` (or +an `os.UserHomeDir` failure, as in some CI containers) means there is no +credentials file. That is not an error. + +**D3. The credentials file is read only when it is needed, and a missing one is +fine.** It is read only if `--env-file` and the environment leave a key unset, +so a broken or loose file does not fail or warn on a run that never uses it. +When it is read, `fs.ErrNotExist` and `ENOTDIR` mean there is no file. Any +other failure (a directory at the path, no read permission) means the user +made one and markfluence cannot use it, so the command fails and names the +path. Silently falling through would make a token rotation look like it had +no effect. + +**D4. The URL and the token must come from the same source.** If both are set +and their sources differ, `Resolve` fails before it builds a client: + +> CONFLUENCE_URL comes from the environment, but CONFLUENCE_TOKEN comes from +> ~/.config/markfluence/credentials. Set both in the same place. + +A source is named as `--env-file PATH`, `the environment`, or the credentials +file's path with the home directory shown as `~`. The error names the source +that *supplied* each value, so an empty key in a higher source is not blamed. +The rule applies whether or not a cloud ID is set. With a cloud ID the token +goes to Atlassian's gateway and not to the URL, so the rule protects less +there, but one rule is easier to state and to test than two. All 15 callers +already map a `Resolve` error to `CodeConfig`, so the error exits 2 and is an +`errorObject` under `--json` with no extra work. + +So setting `CONFLUENCE_URL` alone for one command fails when the token lives +in the credentials file. The one-off forms are `--env-file`, or the URL and +the token together in the environment. + +The cloud ID is part of the credentials: it names one site, as the URL does. It +is read only from the source that supplied the URL, and a cloud ID in any other +source is ignored. It is ignored rather than an error because a higher source +cannot say "no cloud ID": an empty value counts as unset. The username may come +from any source, since a wrong one fails with a 401. + +**D5. `Resolve` takes only the env-file path.** The signature becomes +`client.Resolve(envFile string)`. `ResolveOptions` goes: three of its fields +were the removed flags, and `Roots` existed only so the `.env` walk could reuse +a command's `project.Cache`. With the walk gone, `loadEnvFile`, `dotenvDir`, +and `dotenvPath` go too, and `internal/client` no longer imports +`internal/project` (`config.go` is its only importer). + +**D6. A malformed `markfluence.yaml` above the working directory no longer +fails every command.** Today `Resolve` discovers the root from the working +directory, so a project file it cannot parse fails even `read 123` (#100's +"abort immediately", in `loadEnvFile`'s comment in +`internal/client/config.go`). After this change nothing discovers a root from +the working directory. A command that reads a project file still reports it: +the per-file commands, `pageref` on a `.md` argument, and `export --dest`. +`read 123` and `search` never needed the file, so they stop failing on it. +This is intended. + +**D7. `--root` stops affecting credentials, and `roots` loses the working +directory's root.** `--root` moved the `.env` lookup for `create`, `update`, +`diff`, and `attachment-upload`. Now it bounds per-file roots only, which is +all its name says. + +A second effect is visible in output. Passing `Roots` to `Resolve` put the +working directory's root into the command's cache. So `create`, `update`, and +`attachment-upload` printed it as `root:`, listed it in `--json`'s `roots`, and +reported its settings (`ReportSettings`), even when no file argument lives +under it. After this change they report only the roots of the files they were +given. That is what `roots` claims to mean. `docs/root-model.md` gets +checked for text that describes the old reporting. + +**D8. The "missing" error names every source, and says "one place".** For +example: `missing Confluence URL (CONFLUENCE_URL), token (CONFLUENCE_TOKEN): +set them in one place: the environment, +~/.config/markfluence/credentials, or a file named by --env-file`. Someone +whose `.env` stopped working learns where to put it from the error itself, and +the wording does not steer them into D4. There is no special hint for a `.env` +in the working directory: markfluence is unreleased, so there is nobody to +migrate. + +**D9. The permission warning (#136) covers the credentials file.** It already +runs inside `loadDotenv`, which every source file goes through, so this needs +no new code. With D3 it fires only when the file is actually read. Its wording, +and the docs that say "your `.env`", become "the file that holds your API +token". + +**D10. Tests cannot see the developer's credentials file.** After this change +a real `~/.config/markfluence/credentials` would feed every test that reaches +`Resolve`. It would fill in settings a test left unset, turn a "missing token" +test into a D4 error, and a real cloud ID would send a test's requests to the +gateway instead of its `httptest` server. + +Isolating tests one by one is fragile, because the next test added forgets it. +So each affected package gets a `TestMain` that sets `XDG_CONFIG_HOME` and +`HOME` to an empty temp directory and unsets the four `CONFLUENCE_*` variables +before any test runs. A test then `t.Setenv`s only what it needs. No test in +the repo uses `t.Parallel`, so this is safe. + +`internal/clienttest` provides the helper for the `cmd` packages. It cannot +serve `internal/client`: those tests are `package client`, and `clienttest` +imports `client`, which would be a cycle. So `internal/client` has its own +`TestMain` in `config_test.go`. + +## Work, in commit order + +1. **`test: isolate credential resolution from the home directory`.** Add the + `clienttest` helper and a `TestMain` to every package whose tests reach + `Resolve`: `cmd` (root_test.go), `cmd/pageinfo`, `read`, `diff`, `userinfo`, + `find`, `children`, `create`, `userfind`, `spaceinfo`, and + `internal/client`. With no `markfluence.yaml`, `.env` is read from the + working directory, which in a test is the package directory, and no package + directory has a `.env`. So no test reads one today, and this commit changes + no behaviour. It lands first + so the next commit's diff is only the change itself. +2. **`feat(client)!: read credentials from a user-scoped file`.** + - `internal/client/config.go`: D1 to D5, D8, and `validateCloudID`'s + message, which names `--cloud-id`. + - `cmd/root.go`: remove the three flags. Rewrite `Long` and the + `--env-file` usage for the new sources. + - The 15 commands that call `Resolve`: drop the three `GetString` calls and + pass `envFile`. + - Tests: `config_test.go`, plus `client_test.go`'s `TestResolve` and + `TestResolveValuePrecedence`, which test `resolveValue`'s flag argument. + The per-package `testCmd` helpers stop registering `url`, `username`, and + `cloud-id` and set `CONFLUENCE_URL` instead. + - `make docs`. +3. **`docs: document the credentials file`.** + - `README.md` Configure section, and the line near 675 about where `.env` + is read. + - `CONTRIBUTING.md`: it tells contributors to put a `.env` in the working + directory. + - `.env.example`: becomes the template for the credentials file (or for an + `--env-file`). It keeps its name, because `.gitignore` still ignores + `.env` for anyone who uses one with `--env-file` or direnv. + - `SECURITY.md`. + - `docs/root-model.md`: remove "Where markfluence reads `.env`". Fix the + NOTE near line 128 that gives the precedence as "flag, environment, + `.env`". Rewrite the paragraph that says markfluence reports the root "and + thus where it read `.env` from". Check the `roots` description against + D7. + - `docs/json-output.md`, the `warnings` comments in + `internal/jsonout/jsonout.go`, and the `warnings` descriptions in + `schema/json-output/v1.json`. Only descriptions change, so + `schema_version` stays. + - `docs/confluence/README.md` and `docs/confluence/api.md`. + - `internal/project`: the comment in `project.go` that gives "locate `.env`" + as a reason `Discover` runs from the working directory, and `config.go`'s + statement of the credential precedence. + - `CLAUDE.md`: the Configuration section, the `cmd/root.go` bullet, the + `internal/project` bullet ("called from two starting points for two + reasons" is no longer true), and the `internal/client` bullet's `.env` + warning text. +4. **Code review** of the branch, verify each finding, then fix them in one + commit. +5. **Open a PR** that fixes #188. + +## Tests for step 2 + +In `internal/client/config_test.go`: + +- each precedence pair: `--env-file` beats the environment, the environment + beats the credentials file, and the credentials file is used when nothing + else sets a key; +- `XDG_CONFIG_HOME` absolute is used, relative is ignored, and unset falls back + to `$HOME/.config`; a relative or empty `HOME` means no file; +- a missing credentials file is fine; `ENOTDIR` is fine; a directory at the + path fails and names the path; +- the credentials file is not read, and does not warn or fail, when higher + sources set every key; +- same-source: URL from the environment with the token from the file fails, + URL from `--env-file` with the token from the environment fails, and both + from one source passes; a username from another source passes; +- the cloud ID comes only from the URL's source: a cloud ID in the credentials + file is ignored when the URL comes from `--env-file` or the environment, and + is used when the URL comes from the credentials file too; +- an empty `CONFLUENCE_TOKEN=` in the `--env-file`, with the token in the + environment, names the environment as the token's source; +- a `.env` in the working directory, and one at a discovered project root, are + **not** read (these replace the tests that asserted they were); +- a malformed `markfluence.yaml` above the working directory does not fail + `Resolve` (this replaces `TestResolveFailsOnAMalformedProjectFile`); +- the permission warning fires for a loose credentials file; +- the "missing" error names the credentials file's path. + +`cmd/root_test.go`'s flag list loses `url`, and a test pins that `--url`, +`--username`, and `--cloud-id` are unknown flags. + +## Checks before the PR + +- `make check` passes. +- `grep -rn -- '--url\|--username\|--cloud-id' --exclude-dir=_plans .` finds + nothing but unrelated hits. +- Every remaining mention of `.env` outside `_plans/` describes a file named + with `--env-file`, direnv, or the `.gitignore` entry. +- The GitHub Action and the demo keep working. Both pass all four settings as + environment variables and use none of the removed flags (checked + 2026-09-23), so neither needs a change. + +## Interaction with other work + +`_plans/049` (help text in STE100, #165) rewrites `cmd/root.go`'s `Long` and +every flag usage, including the three this removes. This change should land +first, so 049 rewrites text that still exists. + +Outside the repository: + +- The maintainer's checkout has a `.env` that the live probes use. It moves to + `~/.config/markfluence/credentials`. +- `~/.claude/skills/markfluence/SKILL.md` tells users to put a `.env` in the + current directory. It needs the same update after this lands. + +## Not in scope + +- The OS keychain and named profiles, for the issue's reasons. +- A hint when a `.env` sits in the working directory (D8). +- A `--debug` line that reports which source supplied each setting. The + same-source error names sources where it matters, and `user-info` answers + which account a token belongs to. +- A command that writes the credentials file (#193). People create it by hand, + so the README shows its whole contents inline, since a Homebrew install + ships no `.env.example`, and gives the `mkdir` and `chmod 600` steps. +- A user settings file (the issue's "separate config file"). From 316d2ce64da51b6dfa8f5615483f68f77699ef4f Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Wed, 23 Sep 2026 12:30:59 -0400 Subject: [PATCH 2/7] test: isolate credential resolution from the home directory Every package whose tests reach client.Resolve now runs them with HOME and XDG_CONFIG_HOME pointing at an empty temporary directory and the CONFLUENCE_* variables unset. Changes no behaviour today; it makes #188's user-scoped credentials file invisible to tests, which would otherwise fill in settings a test left unset. It is done in TestMain rather than per test so a test added later cannot forget it. --- cmd/children/main_test.go | 12 ++++++++ cmd/create/main_test.go | 12 ++++++++ cmd/diff/main_test.go | 12 ++++++++ cmd/find/main_test.go | 12 ++++++++ cmd/main_test.go | 12 ++++++++ cmd/pageinfo/main_test.go | 12 ++++++++ cmd/read/main_test.go | 12 ++++++++ cmd/spaceinfo/main_test.go | 12 ++++++++ cmd/userfind/main_test.go | 12 ++++++++ cmd/userinfo/main_test.go | 12 ++++++++ internal/client/client_test.go | 27 +++++++++++++++++- internal/clienttest/clienttest.go | 47 +++++++++++++++++++++++++++++++ 12 files changed, 193 insertions(+), 1 deletion(-) create mode 100644 cmd/children/main_test.go create mode 100644 cmd/create/main_test.go create mode 100644 cmd/diff/main_test.go create mode 100644 cmd/find/main_test.go create mode 100644 cmd/main_test.go create mode 100644 cmd/pageinfo/main_test.go create mode 100644 cmd/read/main_test.go create mode 100644 cmd/spaceinfo/main_test.go create mode 100644 cmd/userfind/main_test.go create mode 100644 cmd/userinfo/main_test.go diff --git a/cmd/children/main_test.go b/cmd/children/main_test.go new file mode 100644 index 0000000..e17b828 --- /dev/null +++ b/cmd/children/main_test.go @@ -0,0 +1,12 @@ +package children + +import ( + "os" + "testing" + + "github.com/mozilla/markfluence/internal/clienttest" +) + +// TestMain hides the developer's own credentials from this package's tests; +// see clienttest.RunIsolated. +func TestMain(m *testing.M) { os.Exit(clienttest.RunIsolated(m)) } diff --git a/cmd/create/main_test.go b/cmd/create/main_test.go new file mode 100644 index 0000000..5e62340 --- /dev/null +++ b/cmd/create/main_test.go @@ -0,0 +1,12 @@ +package create + +import ( + "os" + "testing" + + "github.com/mozilla/markfluence/internal/clienttest" +) + +// TestMain hides the developer's own credentials from this package's tests; +// see clienttest.RunIsolated. +func TestMain(m *testing.M) { os.Exit(clienttest.RunIsolated(m)) } diff --git a/cmd/diff/main_test.go b/cmd/diff/main_test.go new file mode 100644 index 0000000..8452de9 --- /dev/null +++ b/cmd/diff/main_test.go @@ -0,0 +1,12 @@ +package diff + +import ( + "os" + "testing" + + "github.com/mozilla/markfluence/internal/clienttest" +) + +// TestMain hides the developer's own credentials from this package's tests; +// see clienttest.RunIsolated. +func TestMain(m *testing.M) { os.Exit(clienttest.RunIsolated(m)) } diff --git a/cmd/find/main_test.go b/cmd/find/main_test.go new file mode 100644 index 0000000..1fea16d --- /dev/null +++ b/cmd/find/main_test.go @@ -0,0 +1,12 @@ +package find + +import ( + "os" + "testing" + + "github.com/mozilla/markfluence/internal/clienttest" +) + +// TestMain hides the developer's own credentials from this package's tests; +// see clienttest.RunIsolated. +func TestMain(m *testing.M) { os.Exit(clienttest.RunIsolated(m)) } diff --git a/cmd/main_test.go b/cmd/main_test.go new file mode 100644 index 0000000..15e7f79 --- /dev/null +++ b/cmd/main_test.go @@ -0,0 +1,12 @@ +package cmd + +import ( + "os" + "testing" + + "github.com/mozilla/markfluence/internal/clienttest" +) + +// TestMain hides the developer's own credentials from this package's tests; +// see clienttest.RunIsolated. +func TestMain(m *testing.M) { os.Exit(clienttest.RunIsolated(m)) } diff --git a/cmd/pageinfo/main_test.go b/cmd/pageinfo/main_test.go new file mode 100644 index 0000000..be6aa2c --- /dev/null +++ b/cmd/pageinfo/main_test.go @@ -0,0 +1,12 @@ +package pageinfo + +import ( + "os" + "testing" + + "github.com/mozilla/markfluence/internal/clienttest" +) + +// TestMain hides the developer's own credentials from this package's tests; +// see clienttest.RunIsolated. +func TestMain(m *testing.M) { os.Exit(clienttest.RunIsolated(m)) } diff --git a/cmd/read/main_test.go b/cmd/read/main_test.go new file mode 100644 index 0000000..8098a01 --- /dev/null +++ b/cmd/read/main_test.go @@ -0,0 +1,12 @@ +package read + +import ( + "os" + "testing" + + "github.com/mozilla/markfluence/internal/clienttest" +) + +// TestMain hides the developer's own credentials from this package's tests; +// see clienttest.RunIsolated. +func TestMain(m *testing.M) { os.Exit(clienttest.RunIsolated(m)) } diff --git a/cmd/spaceinfo/main_test.go b/cmd/spaceinfo/main_test.go new file mode 100644 index 0000000..14408a0 --- /dev/null +++ b/cmd/spaceinfo/main_test.go @@ -0,0 +1,12 @@ +package spaceinfo + +import ( + "os" + "testing" + + "github.com/mozilla/markfluence/internal/clienttest" +) + +// TestMain hides the developer's own credentials from this package's tests; +// see clienttest.RunIsolated. +func TestMain(m *testing.M) { os.Exit(clienttest.RunIsolated(m)) } diff --git a/cmd/userfind/main_test.go b/cmd/userfind/main_test.go new file mode 100644 index 0000000..8213e31 --- /dev/null +++ b/cmd/userfind/main_test.go @@ -0,0 +1,12 @@ +package userfind + +import ( + "os" + "testing" + + "github.com/mozilla/markfluence/internal/clienttest" +) + +// TestMain hides the developer's own credentials from this package's tests; +// see clienttest.RunIsolated. +func TestMain(m *testing.M) { os.Exit(clienttest.RunIsolated(m)) } diff --git a/cmd/userinfo/main_test.go b/cmd/userinfo/main_test.go new file mode 100644 index 0000000..d3808ce --- /dev/null +++ b/cmd/userinfo/main_test.go @@ -0,0 +1,12 @@ +package userinfo + +import ( + "os" + "testing" + + "github.com/mozilla/markfluence/internal/clienttest" +) + +// TestMain hides the developer's own credentials from this package's tests; +// see clienttest.RunIsolated. +func TestMain(m *testing.M) { os.Exit(clienttest.RunIsolated(m)) } diff --git a/internal/client/client_test.go b/internal/client/client_test.go index 99fcf7b..58bf8c4 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -25,7 +25,32 @@ func TestMain(m *testing.M) { // Backoff assertions want exact durations; TestJitterDelay exercises the // real spreading function directly. jitter = func(d time.Duration) time.Duration { return d } - os.Exit(m.Run()) + os.Exit(runIsolated(m)) +} + +// runIsolated is clienttest.RunIsolated, which this package cannot import: +// clienttest imports client, and these tests are package client. It hides the +// developer's own credentials from every test here. +func runIsolated(m *testing.M) int { + home, err := os.MkdirTemp("", "markfluence-test-home-") + if err != nil { + fmt.Fprintln(os.Stderr, "client tests:", err) + return 1 + } + defer func() { _ = os.RemoveAll(home) }() + for k, v := range map[string]string{"HOME": home, "XDG_CONFIG_HOME": filepath.Join(home, ".config")} { + if err := os.Setenv(k, v); err != nil { + fmt.Fprintln(os.Stderr, "client tests:", err) + return 1 + } + } + for _, k := range []string{urlEnv, usernameEnv, tokenEnv, cloudIDEnv} { + if err := os.Unsetenv(k); err != nil { + fmt.Fprintln(os.Stderr, "client tests:", err) + return 1 + } + } + return m.Run() } // scripted is a test server that returns canned responses in order and records diff --git a/internal/clienttest/clienttest.go b/internal/clienttest/clienttest.go index df204a1..b928154 100644 --- a/internal/clienttest/clienttest.go +++ b/internal/clienttest/clienttest.go @@ -8,8 +8,11 @@ package clienttest import ( + "fmt" "net/http" "net/http/httptest" + "os" + "path/filepath" "testing" "github.com/mozilla/markfluence/internal/client" @@ -25,3 +28,47 @@ func New(t *testing.T, handler http.HandlerFunc) *client.ConfluenceClient { t.Cleanup(srv.Close) return client.New(client.Config{SiteURL: srv.URL, Username: "u", Token: "t"}) } + +// credentialEnv lists the environment variables client.Resolve reads. It +// repeats internal/client's own names rather than exporting them, since they +// are a published interface and cannot change without breaking every user. +var credentialEnv = []string{ + "CONFLUENCE_URL", "CONFLUENCE_USERNAME", "CONFLUENCE_TOKEN", "CONFLUENCE_CLOUD_ID", +} + +// RunIsolated runs a package's tests with no view of the developer's own +// credentials, and returns the exit code for TestMain to pass to os.Exit: +// +// func TestMain(m *testing.M) { os.Exit(clienttest.RunIsolated(m)) } +// +// HOME and XDG_CONFIG_HOME point at an empty temporary directory, and the +// CONFLUENCE_* variables are unset, so a test sets exactly the settings it +// means to. Without it, a real credentials file or an exported token fills in +// whatever a test left unset: a "missing token" test passes or fails depending +// on who runs it, and a real cloud ID sends a test's requests to Atlassian's +// gateway instead of its httptest server. +// +// Every package whose tests reach client.Resolve calls it from TestMain, and +// not per test, so a test added later cannot forget it. +func RunIsolated(m *testing.M) int { + home, err := os.MkdirTemp("", "markfluence-test-home-") + if err != nil { + fmt.Fprintln(os.Stderr, "clienttest:", err) + return 1 + } + defer func() { _ = os.RemoveAll(home) }() + env := map[string]string{"HOME": home, "XDG_CONFIG_HOME": filepath.Join(home, ".config")} + for k, v := range env { + if err := os.Setenv(k, v); err != nil { + fmt.Fprintln(os.Stderr, "clienttest:", err) + return 1 + } + } + for _, k := range credentialEnv { + if err := os.Unsetenv(k); err != nil { + fmt.Fprintln(os.Stderr, "clienttest:", err) + return 1 + } + } + return m.Run() +} From ad6a0152d5c0e1b01c30a80cb93f43100d0e5c54 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Wed, 23 Sep 2026 12:35:44 -0400 Subject: [PATCH 3/7] feat(client)!: read credentials from a user-scoped file Credentials now come only from sources the user chose (#188), highest first: the file named by --env-file, the CONFLUENCE_* environment variables, and a credentials file at $XDG_CONFIG_HOME/markfluence/credentials, else $HOME/.config/markfluence/credentials. - .env is no longer discovered by walking up from the working directory, so a checkout cannot supply credentials. - The --url, --username, and --cloud-id flags are gone. - The URL and the token must come from the same source, or the command fails naming both. - The cloud ID is read only from the URL's source. - The credentials file is read only when a higher source leaves a setting unset; a missing one is fine, an unreadable one is an error. Resolve now takes only the env-file path. Nothing discovers a root from the working directory any more, so --root no longer affects credentials, and a malformed markfluence.yaml no longer fails commands that never read it. BREAKING CHANGE: a project .env is no longer read, and the --url, --username, and --cloud-id flags are removed. Move credentials to ~/.config/markfluence/credentials or name the file with --env-file. --- cmd/attachmentdownload/attachmentdownload.go | 7 +- cmd/attachmentlist/attachmentlist.go | 7 +- cmd/attachmentupload/attachmentupload.go | 7 +- cmd/children/children.go | 7 +- cmd/children/children_test.go | 12 +- cmd/create/create.go | 7 +- cmd/create/run_test.go | 14 +- cmd/diff/diff.go | 7 +- cmd/diff/diff_test.go | 12 +- cmd/export/export.go | 7 +- cmd/find/find.go | 7 +- cmd/find/find_test.go | 13 +- cmd/pageinfo/pageinfo.go | 7 +- cmd/pageinfo/pageinfo_test.go | 12 +- cmd/read/read.go | 7 +- cmd/read/read_test.go | 12 +- cmd/root.go | 50 ++- cmd/root_test.go | 19 +- cmd/search/search.go | 7 +- cmd/spaceinfo/spaceinfo.go | 7 +- cmd/update/update.go | 7 +- cmd/userfind/userfind.go | 7 +- cmd/userfind/userfind_test.go | 8 +- cmd/userinfo/userinfo.go | 7 +- docs/commands/markfluence.md | 32 +- .../markfluence_attachment-download.md | 5 +- docs/commands/markfluence_attachment-list.md | 5 +- .../commands/markfluence_attachment-upload.md | 5 +- docs/commands/markfluence_check.md | 5 +- docs/commands/markfluence_children.md | 5 +- docs/commands/markfluence_create.md | 5 +- docs/commands/markfluence_diff.md | 5 +- docs/commands/markfluence_export.md | 5 +- docs/commands/markfluence_find.md | 5 +- docs/commands/markfluence_page-info.md | 5 +- docs/commands/markfluence_read.md | 5 +- docs/commands/markfluence_schema.md | 5 +- docs/commands/markfluence_search.md | 5 +- docs/commands/markfluence_space-info.md | 5 +- docs/commands/markfluence_update.md | 5 +- docs/commands/markfluence_user-find.md | 5 +- docs/commands/markfluence_user-info.md | 5 +- internal/client/client_test.go | 57 --- internal/client/config.go | 292 +++++++------ internal/client/config_test.go | 392 ++++++++++-------- 45 files changed, 515 insertions(+), 600 deletions(-) diff --git a/cmd/attachmentdownload/attachmentdownload.go b/cmd/attachmentdownload/attachmentdownload.go index c92c7aa..37ae917 100644 --- a/cmd/attachmentdownload/attachmentdownload.go +++ b/cmd/attachmentdownload/attachmentdownload.go @@ -74,13 +74,8 @@ func init() { } func run(cmd *cobra.Command, args []string) error { - url, _ := cmd.Flags().GetString("url") - username, _ := cmd.Flags().GetString("username") - cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") - c, err := client.Resolve(client.ResolveOptions{ - URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, - }) + c, err := client.Resolve(envFile) if err != nil { return fatalFail(err.Error(), jsonout.CodeConfig) } diff --git a/cmd/attachmentlist/attachmentlist.go b/cmd/attachmentlist/attachmentlist.go index 77bce7c..876f67a 100644 --- a/cmd/attachmentlist/attachmentlist.go +++ b/cmd/attachmentlist/attachmentlist.go @@ -42,13 +42,8 @@ var Cmd = &cobra.Command{ } func run(cmd *cobra.Command, args []string) error { - url, _ := cmd.Flags().GetString("url") - username, _ := cmd.Flags().GetString("username") - cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") - c, err := client.Resolve(client.ResolveOptions{ - URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, - }) + c, err := client.Resolve(envFile) if err != nil { return fatalFail(err.Error(), jsonout.CodeConfig) } diff --git a/cmd/attachmentupload/attachmentupload.go b/cmd/attachmentupload/attachmentupload.go index 80553bc..361cb32 100644 --- a/cmd/attachmentupload/attachmentupload.go +++ b/cmd/attachmentupload/attachmentupload.go @@ -75,16 +75,11 @@ func run(cmd *cobra.Command, args []string) error { jsonout.CodeValidation) } - url, _ := cmd.Flags().GetString("url") - username, _ := cmd.Flags().GetString("username") - cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") rootOverride, _ := cmd.Flags().GetString("root") roots := project.NewCache(rootOverride) defer roots.Close() - c, err := client.Resolve(client.ResolveOptions{ - URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, Roots: roots, - }) + c, err := client.Resolve(envFile) if err != nil { return fatalFail(err.Error(), jsonout.CodeConfig) } diff --git a/cmd/children/children.go b/cmd/children/children.go index d57b5cc..165f129 100644 --- a/cmd/children/children.go +++ b/cmd/children/children.go @@ -75,9 +75,6 @@ func init() { } func run(cmd *cobra.Command, args []string) error { - url, _ := cmd.Flags().GetString("url") - username, _ := cmd.Flags().GetString("username") - cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") // Before the credential check: neither of these needs a server to be @@ -90,9 +87,7 @@ func run(cmd *cobra.Command, args []string) error { return fatalFail(err.Error(), jsonout.CodeValidation) } - c, err := client.Resolve(client.ResolveOptions{ - URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, - }) + c, err := client.Resolve(envFile) if err != nil { return fatalFail(err.Error(), jsonout.CodeConfig) } diff --git a/cmd/children/children_test.go b/cmd/children/children_test.go index 0cbd258..c1744e2 100644 --- a/cmd/children/children_test.go +++ b/cmd/children/children_test.go @@ -15,17 +15,15 @@ import ( "github.com/spf13/cobra" ) -// testCmd builds a bare *cobra.Command carrying the flags run() reads, -// pointed at url. It doesn't go through the real root command tree, and -// CONFLUENCE_TOKEN (never a flag) comes from the environment instead, as it -// would in a real invocation. +// testCmd builds a bare *cobra.Command carrying the flags run() reads, and +// sets credentials for url in the environment, where a real invocation finds +// them. It doesn't go through the real root command tree. func testCmd(t *testing.T, url string) *cobra.Command { t.Helper() + t.Setenv("CONFLUENCE_URL", url) + t.Setenv("CONFLUENCE_USERNAME", "u") t.Setenv("CONFLUENCE_TOKEN", "t") c := &cobra.Command{} - c.Flags().String("url", url, "") - c.Flags().String("username", "u", "") - c.Flags().String("cloud-id", "", "") c.Flags().String("env-file", "", "") // run reads --depth's *value* from the package-level flag var, but asks the // command whether it was set at all, so the flag has to exist here too. diff --git a/cmd/create/create.go b/cmd/create/create.go index 0ac8dd7..c982b43 100644 --- a/cmd/create/create.go +++ b/cmd/create/create.go @@ -379,16 +379,11 @@ func run(cmd *cobra.Command, args []string) error { } doPersist := wantPersist(persistOpt, noPersistOpt) - url, _ := cmd.Flags().GetString("url") - username, _ := cmd.Flags().GetString("username") - cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") rootOverride, _ := cmd.Flags().GetString("root") roots := project.NewCache(rootOverride) defer roots.Close() - c, err := client.Resolve(client.ResolveOptions{ - URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, Roots: roots, - }) + c, err := client.Resolve(envFile) if err != nil { return fatalFail(err.Error(), jsonout.CodeConfig) } diff --git a/cmd/create/run_test.go b/cmd/create/run_test.go index 9deb950..a3c4f14 100644 --- a/cmd/create/run_test.go +++ b/cmd/create/run_test.go @@ -540,18 +540,16 @@ func TestCreateBatchIgnoresDirectoryNesting(t *testing.T) { } // testCmd builds a bare *cobra.Command carrying the flags run() reads itself, -// pointed at url. It doesn't go through the real root command tree, and -// CONFLUENCE_TOKEN (never a flag) comes from the environment instead, as it -// would in a real invocation. --root is pinned to the fixture directory so a -// fixture's images resolve inside a root the test chose, rather than whatever -// Discover's fallback happens to pick. +// and sets credentials for url in the environment, where a real invocation +// finds them. It doesn't go through the real root command tree. --root is +// pinned to the fixture directory so a fixture's images resolve inside a root +// the test chose, rather than whatever Discover's fallback happens to pick. func testCmd(t *testing.T, url, root string) *cobra.Command { t.Helper() + t.Setenv("CONFLUENCE_URL", url) + t.Setenv("CONFLUENCE_USERNAME", "u") t.Setenv("CONFLUENCE_TOKEN", "t") c := &cobra.Command{} - c.Flags().String("url", url, "") - c.Flags().String("username", "u", "") - c.Flags().String("cloud-id", "", "") c.Flags().String("env-file", "", "") c.Flags().String("root", root, "") return c diff --git a/cmd/diff/diff.go b/cmd/diff/diff.go index 47e3566..64d0284 100644 --- a/cmd/diff/diff.go +++ b/cmd/diff/diff.go @@ -170,13 +170,8 @@ func run(cmd *cobra.Command, args []string) error { return fatalFail(pageref.NotNumericMessage(pageID), jsonout.CodeValidation) } - url, _ := cmd.Flags().GetString("url") - username, _ := cmd.Flags().GetString("username") - cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") - c, err := client.Resolve(client.ResolveOptions{ - URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, Roots: roots, - }) + c, err := client.Resolve(envFile) if err != nil { return fatalFail(err.Error(), jsonout.CodeConfig) } diff --git a/cmd/diff/diff_test.go b/cmd/diff/diff_test.go index 7080f9f..5131290 100644 --- a/cmd/diff/diff_test.go +++ b/cmd/diff/diff_test.go @@ -146,17 +146,15 @@ func projectDir(t *testing.T, cfg string, files map[string]string) string { return dir } -// testCmd builds a bare *cobra.Command carrying the flags run() reads, pointed -// at url. It does not go through the real root command tree, and -// CONFLUENCE_TOKEN (never a flag) comes from the environment as it would in a -// real invocation. +// testCmd builds a bare *cobra.Command carrying the flags run() reads, and +// sets credentials for url in the environment, where a real invocation finds +// them. It doesn't go through the real root command tree. func testCmd(t *testing.T, url string) *cobra.Command { t.Helper() + t.Setenv("CONFLUENCE_URL", url) + t.Setenv("CONFLUENCE_USERNAME", "u") t.Setenv("CONFLUENCE_TOKEN", "t") c := &cobra.Command{} - c.Flags().String("url", url, "") - c.Flags().String("username", "u", "") - c.Flags().String("cloud-id", "", "") c.Flags().String("env-file", "", "") c.Flags().String("root", "", "") c.Flags().Bool("reverse", false, "") diff --git a/cmd/export/export.go b/cmd/export/export.go index 52dd73a..ca1a9f2 100644 --- a/cmd/export/export.go +++ b/cmd/export/export.go @@ -117,9 +117,6 @@ func init() { } func run(cmd *cobra.Command, args []string) error { - url, _ := cmd.Flags().GetString("url") - username, _ := cmd.Flags().GetString("username") - cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") // Before the credential check: none of these needs a server to be // recognized as a usage error, and reporting a missing token for a command @@ -135,9 +132,7 @@ func run(cmd *cobra.Command, args []string) error { return fatalFail(err.Error(), jsonout.CodeValidation) } - c, err := client.Resolve(client.ResolveOptions{ - URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, - }) + c, err := client.Resolve(envFile) if err != nil { return fatalFail(err.Error(), jsonout.CodeConfig) } diff --git a/cmd/find/find.go b/cmd/find/find.go index 62da7ee..9fbdcd9 100644 --- a/cmd/find/find.go +++ b/cmd/find/find.go @@ -55,9 +55,6 @@ func init() { } func run(cmd *cobra.Command, args []string) error { - url, _ := cmd.Flags().GetString("url") - username, _ := cmd.Flags().GetString("username") - cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") // Before the credential check: an empty title is a usage error and needs no @@ -69,9 +66,7 @@ func run(cmd *cobra.Command, args []string) error { return fatalFail("no title given: TITLE must not be empty", jsonout.CodeValidation) } - c, err := client.Resolve(client.ResolveOptions{ - URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, - }) + c, err := client.Resolve(envFile) if err != nil { return fatalFail(err.Error(), jsonout.CodeConfig) } diff --git a/cmd/find/find_test.go b/cmd/find/find_test.go index 22b3bca..2c57346 100644 --- a/cmd/find/find_test.go +++ b/cmd/find/find_test.go @@ -14,18 +14,15 @@ import ( "github.com/spf13/cobra" ) -// testCmd builds a bare *cobra.Command carrying the flags run() reads, -// pointed at url. It doesn't go through the real root command (which would -// need a full command tree and CONFLUENCE_TOKEN can't be a flag), so the -// token is supplied via the environment instead, exactly as a real -// invocation would. +// testCmd builds a bare *cobra.Command carrying the flags run() reads, and +// sets credentials for url in the environment, where a real invocation finds +// them. It doesn't go through the real root command tree. func testCmd(t *testing.T, url string) *cobra.Command { t.Helper() + t.Setenv("CONFLUENCE_URL", url) + t.Setenv("CONFLUENCE_USERNAME", "u") t.Setenv("CONFLUENCE_TOKEN", "t") c := &cobra.Command{} - c.Flags().String("url", url, "") - c.Flags().String("username", "u", "") - c.Flags().String("cloud-id", "", "") c.Flags().String("env-file", "", "") return c } diff --git a/cmd/pageinfo/pageinfo.go b/cmd/pageinfo/pageinfo.go index b65b68f..71bdf8f 100644 --- a/cmd/pageinfo/pageinfo.go +++ b/cmd/pageinfo/pageinfo.go @@ -61,13 +61,8 @@ func init() { } func run(cmd *cobra.Command, args []string) error { - url, _ := cmd.Flags().GetString("url") - username, _ := cmd.Flags().GetString("username") - cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") - c, err := client.Resolve(client.ResolveOptions{ - URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, - }) + c, err := client.Resolve(envFile) if err != nil { return fatalFail(err.Error(), jsonout.CodeConfig) } diff --git a/cmd/pageinfo/pageinfo_test.go b/cmd/pageinfo/pageinfo_test.go index 42d0d3e..890de48 100644 --- a/cmd/pageinfo/pageinfo_test.go +++ b/cmd/pageinfo/pageinfo_test.go @@ -16,17 +16,15 @@ import ( "github.com/spf13/cobra" ) -// testCmd builds a bare *cobra.Command carrying the flags run() reads, -// pointed at url. It doesn't go through the real root command tree, and -// CONFLUENCE_TOKEN (never a flag) comes from the environment instead, as it -// would in a real invocation. +// testCmd builds a bare *cobra.Command carrying the flags run() reads, and +// sets credentials for url in the environment, where a real invocation finds +// them. It doesn't go through the real root command tree. func testCmd(t *testing.T, url string) *cobra.Command { t.Helper() + t.Setenv("CONFLUENCE_URL", url) + t.Setenv("CONFLUENCE_USERNAME", "u") t.Setenv("CONFLUENCE_TOKEN", "t") c := &cobra.Command{} - c.Flags().String("url", url, "") - c.Flags().String("username", "u", "") - c.Flags().String("cloud-id", "", "") c.Flags().String("env-file", "", "") return c } diff --git a/cmd/read/read.go b/cmd/read/read.go index b6072c9..91084ec 100644 --- a/cmd/read/read.go +++ b/cmd/read/read.go @@ -79,13 +79,8 @@ func run(cmd *cobra.Command, args []string) error { return fatalFail(err.Error(), jsonout.CodeValidation) } - url, _ := cmd.Flags().GetString("url") - username, _ := cmd.Flags().GetString("username") - cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") - c, err := client.Resolve(client.ResolveOptions{ - URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, - }) + c, err := client.Resolve(envFile) if err != nil { return fatalFail(err.Error(), jsonout.CodeConfig) } diff --git a/cmd/read/read_test.go b/cmd/read/read_test.go index 895a9e6..889689f 100644 --- a/cmd/read/read_test.go +++ b/cmd/read/read_test.go @@ -13,17 +13,15 @@ import ( "github.com/spf13/cobra" ) -// testCmd builds a bare *cobra.Command carrying the flags run() reads, -// pointed at url. It doesn't go through the real root command tree, and -// CONFLUENCE_TOKEN (never a flag) comes from the environment instead, as it -// would in a real invocation. +// testCmd builds a bare *cobra.Command carrying the flags run() reads, and +// sets credentials for url in the environment, where a real invocation finds +// them. It doesn't go through the real root command tree. func testCmd(t *testing.T, url string) *cobra.Command { t.Helper() + t.Setenv("CONFLUENCE_URL", url) + t.Setenv("CONFLUENCE_USERNAME", "u") t.Setenv("CONFLUENCE_TOKEN", "t") c := &cobra.Command{} - c.Flags().String("url", url, "") - c.Flags().String("username", "u", "") - c.Flags().String("cloud-id", "", "") c.Flags().String("env-file", "", "") return c } diff --git a/cmd/root.go b/cmd/root.go index dfe7266..e30cd31 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -33,27 +33,34 @@ import ( ) var ( - urlFlag string - usernameFlag string - cloudIDFlag string - envFileFlag string - rootFlag string - debugFlag bool - noColorFlag bool - jsonFlag bool + envFileFlag string + rootFlag string + debugFlag bool + noColorFlag bool + jsonFlag bool ) var rootCmd = &cobra.Command{ Use: "markfluence", Short: "Publish Markdown to Confluence", Long: "markfluence publishes and manipulates Confluence pages from Markdown files.\n\n" + - "It needs a site URL, a username, and an API token. It reads each one from a flag\n" + - "first, then from an environment variable, then from a .env file:\n\n" + - " site URL --url CONFLUENCE_URL\n" + - " username --username CONFLUENCE_USERNAME\n" + - " API token (no flag) CONFLUENCE_TOKEN\n" + - " cloud ID --cloud-id CONFLUENCE_CLOUD_ID\n\n" + - "The API token is never a flag, so it cannot get into your shell history.\n\n" + + "It needs a site URL, a username, and an API token, and for a scoped token a\n" + + "cloud ID:\n\n" + + " CONFLUENCE_URL the site, such as https://YOUR-SITE.atlassian.net\n" + + " CONFLUENCE_USERNAME your email address\n" + + " CONFLUENCE_TOKEN your API token\n" + + " CONFLUENCE_CLOUD_ID optional; only for a scoped API token\n\n" + + "markfluence reads each one from these places, and uses the first it finds:\n\n" + + " 1. the file that --env-file names\n" + + " 2. the environment variable\n" + + " 3. your credentials file, ~/.config/markfluence/credentials\n" + + " ($XDG_CONFIG_HOME/markfluence/credentials if you set XDG_CONFIG_HOME)\n\n" + + "The two files hold KEY=value lines. Put the URL and the token in the same\n" + + "place: markfluence refuses to send a token to a URL from a different place.\n" + + "It reads the cloud ID only from the place that gives the URL.\n\n" + + "There is no flag for any of these, so the token cannot get into your shell\n" + + "history. To use a different site for one command, name a file with\n" + + "--env-file.\n\n" + "Set the cloud ID only for a scoped API token, such as the token of a service\n" + "account. Confluence refuses a scoped token at your site URL, so markfluence\n" + "sends it through the api.atlassian.com gateway, which needs the cloud ID. To\n" + @@ -148,18 +155,9 @@ func jsonRequested(args []string) bool { } func init() { - rootCmd.PersistentFlags().StringVar(&urlFlag, "url", "", - "Confluence site URL. If not set, markfluence uses $CONFLUENCE_URL, then .env") - rootCmd.PersistentFlags().StringVar(&usernameFlag, "username", "", - "Confluence username (your email address). If not set, markfluence uses "+ - "$CONFLUENCE_USERNAME, then .env") - rootCmd.PersistentFlags().StringVar(&cloudIDFlag, "cloud-id", "", - "Atlassian cloud ID. Set it only for a scoped API token. If not set, "+ - "markfluence uses $CONFLUENCE_CLOUD_ID, then .env") rootCmd.PersistentFlags().StringVar(&envFileFlag, "env-file", "", - "Env file to read credentials from. The default is .env in the documentation "+ - "root of the working directory, or in the working directory if there is no "+ - "markfluence.yaml") + "File to read credentials from, before the environment and your credentials "+ + "file") rootCmd.PersistentFlags().StringVar(&rootFlag, "root", "", "Documentation root for every file. The default is the nearest directory "+ "above each file that has a markfluence.yaml, or the directory of the file "+ diff --git a/cmd/root_test.go b/cmd/root_test.go index 6316056..2bbc566 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -21,13 +21,24 @@ func TestRootCommandWiring(t *testing.T) { if rootCmd.Use != "markfluence" { t.Errorf("rootCmd.Use = %q, want %q", rootCmd.Use, "markfluence") } - for _, flag := range []string{"url", "debug", "no-color", "json", "env-file", "root"} { + for _, flag := range []string{"debug", "no-color", "json", "env-file", "root"} { if rootCmd.PersistentFlags().Lookup(flag) == nil { t.Errorf("persistent flag --%s not registered", flag) } } } +// TestNoCredentialFlags: credentials come from an env file, the environment, +// or the credentials file, and never from a flag (#188). A flag for the URL +// alone would always mean credentials from two places. +func TestNoCredentialFlags(t *testing.T) { + for _, flag := range []string{"url", "username", "cloud-id"} { + if rootCmd.PersistentFlags().Lookup(flag) != nil { + t.Errorf("persistent flag --%s is registered, want no credential flags", flag) + } + } +} + // TestSchemaCommandRegistered checks the one registration // TestCommandEnumMatchesRegisteredCommands can't: every other subcommand's // registration is implied by its presence in the schema's command enum (that @@ -175,7 +186,7 @@ func TestSubcommandsDocumentThemselves(t *testing.T) { } } -// TestSecurityWarnerIsWired pins the one line that makes the .env permission +// TestSecurityWarnerIsWired pins the one line that makes the permission // warning exist at runtime. Everything else about it is tested in // internal/client (the predicate) and internal/ui (the output), each against // its own double -- so deleting the SetSecurityWarner call in @@ -204,7 +215,7 @@ func TestSecurityWarnerIsWired(t *testing.T) { } old := os.Stderr os.Stderr = w - _, resolveErr := client.Resolve(client.ResolveOptions{EnvFile: path}) + _, resolveErr := client.Resolve(path) os.Stderr = old if err := w.Close(); err != nil { t.Fatal(err) @@ -217,7 +228,7 @@ func TestSecurityWarnerIsWired(t *testing.T) { t.Fatalf("Resolve: %v", resolveErr) } if !strings.Contains(string(out), "holds your API token") { - t.Errorf("stderr = %q, want the .env permission warning: is SetSecurityWarner still wired?", out) + t.Errorf("stderr = %q, want the permission warning: is SetSecurityWarner still wired?", out) } } diff --git a/cmd/search/search.go b/cmd/search/search.go index d5f1c5e..e606057 100644 --- a/cmd/search/search.go +++ b/cmd/search/search.go @@ -95,9 +95,6 @@ func init() { } func run(cmd *cobra.Command, args []string) error { - url, _ := cmd.Flags().GetString("url") - username, _ := cmd.Flags().GetString("username") - cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") // Everything in this block is a usage error and needs no server to recognize. @@ -119,9 +116,7 @@ func run(cmd *cobra.Command, args []string) error { return fatalFail(err.Error(), jsonout.CodeValidation) } - c, err := client.Resolve(client.ResolveOptions{ - URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, - }) + c, err := client.Resolve(envFile) if err != nil { return fatalFail(err.Error(), jsonout.CodeConfig) } diff --git a/cmd/spaceinfo/spaceinfo.go b/cmd/spaceinfo/spaceinfo.go index b34ecfa..1b2d0d8 100644 --- a/cmd/spaceinfo/spaceinfo.go +++ b/cmd/spaceinfo/spaceinfo.go @@ -77,13 +77,8 @@ func run(cmd *cobra.Command, args []string) error { // for a blank argument names the wrong problem. return fatalFail("no space key given", jsonout.CodeValidation) } - url, _ := cmd.Flags().GetString("url") - username, _ := cmd.Flags().GetString("username") - cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") - c, err := client.Resolve(client.ResolveOptions{ - URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, - }) + c, err := client.Resolve(envFile) if err != nil { return fatalFail(err.Error(), jsonout.CodeConfig) } diff --git a/cmd/update/update.go b/cmd/update/update.go index 3cf3e15..53a4322 100644 --- a/cmd/update/update.go +++ b/cmd/update/update.go @@ -118,16 +118,11 @@ func init() { } func run(cmd *cobra.Command, args []string) error { - url, _ := cmd.Flags().GetString("url") - username, _ := cmd.Flags().GetString("username") - cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") rootOverride, _ := cmd.Flags().GetString("root") roots := project.NewCache(rootOverride) defer roots.Close() - c, err := client.Resolve(client.ResolveOptions{ - URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, Roots: roots, - }) + c, err := client.Resolve(envFile) if err != nil { if ui.IsJSON() { _ = jsonout.EmitError(os.Stderr, "update", err.Error(), jsonout.CodeConfig) diff --git a/cmd/userfind/userfind.go b/cmd/userfind/userfind.go index 04f1258..1d6cc85 100644 --- a/cmd/userfind/userfind.go +++ b/cmd/userfind/userfind.go @@ -72,9 +72,6 @@ func init() { } func run(cmd *cobra.Command, args []string) error { - url, _ := cmd.Flags().GetString("url") - username, _ := cmd.Flags().GetString("username") - cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") // Both checks are usage errors needing no server. The empty-name one @@ -90,9 +87,7 @@ func run(cmd *cobra.Command, args []string) error { return fatalFail(err.Error(), jsonout.CodeValidation) } - c, err := client.Resolve(client.ResolveOptions{ - URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, - }) + c, err := client.Resolve(envFile) if err != nil { return fatalFail(err.Error(), jsonout.CodeConfig) } diff --git a/cmd/userfind/userfind_test.go b/cmd/userfind/userfind_test.go index 8348684..1c0f597 100644 --- a/cmd/userfind/userfind_test.go +++ b/cmd/userfind/userfind_test.go @@ -53,11 +53,10 @@ func people(pairs ...[2]string) string { func testCmd(t *testing.T, url string) *cobra.Command { t.Helper() + t.Setenv("CONFLUENCE_URL", url) + t.Setenv("CONFLUENCE_USERNAME", "u") t.Setenv("CONFLUENCE_TOKEN", "t") c := &cobra.Command{} - c.Flags().String("url", url, "") - c.Flags().String("username", "u", "") - c.Flags().String("cloud-id", "", "") c.Flags().String("env-file", "", "") return c } @@ -70,8 +69,7 @@ type outcome struct { // runFind executes the command against a stub, capturing both streams. // -// It runs from an empty directory so the repository's own .env cannot be read -// as this test's configuration. +// It runs from an empty directory; TestMain keeps any real credentials out. func runFind(t *testing.T, s stub, limit string, args ...string) outcome { t.Helper() c := clienttest.New(t, s.handler(t)) diff --git a/cmd/userinfo/userinfo.go b/cmd/userinfo/userinfo.go index 5d0a1dd..83f55ad 100644 --- a/cmd/userinfo/userinfo.go +++ b/cmd/userinfo/userinfo.go @@ -67,13 +67,8 @@ func run(cmd *cobra.Command, args []string) error { return fatalFail("no account id given", jsonout.CodeValidation) } } - url, _ := cmd.Flags().GetString("url") - username, _ := cmd.Flags().GetString("username") - cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") - c, err := client.Resolve(client.ResolveOptions{ - URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, - }) + c, err := client.Resolve(envFile) if err != nil { return fatalFail(err.Error(), jsonout.CodeConfig) } diff --git a/docs/commands/markfluence.md b/docs/commands/markfluence.md index 64a8d8e..acfa408 100644 --- a/docs/commands/markfluence.md +++ b/docs/commands/markfluence.md @@ -6,15 +6,28 @@ Publish Markdown to Confluence markfluence publishes and manipulates Confluence pages from Markdown files. -It needs a site URL, a username, and an API token. It reads each one from a flag -first, then from an environment variable, then from a .env file: +It needs a site URL, a username, and an API token, and for a scoped token a +cloud ID: - site URL --url CONFLUENCE_URL - username --username CONFLUENCE_USERNAME - API token (no flag) CONFLUENCE_TOKEN - cloud ID --cloud-id CONFLUENCE_CLOUD_ID + CONFLUENCE_URL the site, such as https://YOUR-SITE.atlassian.net + CONFLUENCE_USERNAME your email address + CONFLUENCE_TOKEN your API token + CONFLUENCE_CLOUD_ID optional; only for a scoped API token -The API token is never a flag, so it cannot get into your shell history. +markfluence reads each one from these places, and uses the first it finds: + + 1. the file that --env-file names + 2. the environment variable + 3. your credentials file, ~/.config/markfluence/credentials + ($XDG_CONFIG_HOME/markfluence/credentials if you set XDG_CONFIG_HOME) + +The two files hold KEY=value lines. Put the URL and the token in the same +place: markfluence refuses to send a token to a URL from a different place. +It reads the cloud ID only from the place that gives the URL. + +There is no flag for any of these, so the token cannot get into your shell +history. To use a different site for one command, name a file with +--env-file. Set the cloud ID only for a scoped API token, such as the token of a service account. Confluence refuses a scoped token at your site URL, so markfluence @@ -29,15 +42,12 @@ markfluence [flags] ### Options ``` - --cloud-id string Atlassian cloud ID. Set it only for a scoped API token. If not set, markfluence uses $CONFLUENCE_CLOUD_ID, then .env -d, --debug Print debug details, such as each retry decision - --env-file string Env file to read credentials from. The default is .env in the documentation root of the working directory, or in the working directory if there is no markfluence.yaml + --env-file string File to read credentials from, before the environment and your credentials file -h, --help help for markfluence --json Write JSON, and no human output. A result goes to stdout. A fatal error goes to stderr as a JSON error object --no-color Print output with no color --root string Documentation root for every file. The default is the nearest directory above each file that has a markfluence.yaml, or the directory of the file if there is none - --url string Confluence site URL. If not set, markfluence uses $CONFLUENCE_URL, then .env - --username string Confluence username (your email address). If not set, markfluence uses $CONFLUENCE_USERNAME, then .env ``` ### SEE ALSO diff --git a/docs/commands/markfluence_attachment-download.md b/docs/commands/markfluence_attachment-download.md index 0e77f32..906193e 100644 --- a/docs/commands/markfluence_attachment-download.md +++ b/docs/commands/markfluence_attachment-download.md @@ -61,14 +61,11 @@ markfluence attachment-download PAGE [NAME...] [flags] ### Options inherited from parent commands ``` - --cloud-id string Atlassian cloud ID. Set it only for a scoped API token. If not set, markfluence uses $CONFLUENCE_CLOUD_ID, then .env -d, --debug Print debug details, such as each retry decision - --env-file string Env file to read credentials from. The default is .env in the documentation root of the working directory, or in the working directory if there is no markfluence.yaml + --env-file string File to read credentials from, before the environment and your credentials file --json Write JSON, and no human output. A result goes to stdout. A fatal error goes to stderr as a JSON error object --no-color Print output with no color --root string Documentation root for every file. The default is the nearest directory above each file that has a markfluence.yaml, or the directory of the file if there is none - --url string Confluence site URL. If not set, markfluence uses $CONFLUENCE_URL, then .env - --username string Confluence username (your email address). If not set, markfluence uses $CONFLUENCE_USERNAME, then .env ``` ### SEE ALSO diff --git a/docs/commands/markfluence_attachment-list.md b/docs/commands/markfluence_attachment-list.md index e4b9e7e..56b24da 100644 --- a/docs/commands/markfluence_attachment-list.md +++ b/docs/commands/markfluence_attachment-list.md @@ -41,14 +41,11 @@ markfluence attachment-list PAGE [flags] ### Options inherited from parent commands ``` - --cloud-id string Atlassian cloud ID. Set it only for a scoped API token. If not set, markfluence uses $CONFLUENCE_CLOUD_ID, then .env -d, --debug Print debug details, such as each retry decision - --env-file string Env file to read credentials from. The default is .env in the documentation root of the working directory, or in the working directory if there is no markfluence.yaml + --env-file string File to read credentials from, before the environment and your credentials file --json Write JSON, and no human output. A result goes to stdout. A fatal error goes to stderr as a JSON error object --no-color Print output with no color --root string Documentation root for every file. The default is the nearest directory above each file that has a markfluence.yaml, or the directory of the file if there is none - --url string Confluence site URL. If not set, markfluence uses $CONFLUENCE_URL, then .env - --username string Confluence username (your email address). If not set, markfluence uses $CONFLUENCE_USERNAME, then .env ``` ### SEE ALSO diff --git a/docs/commands/markfluence_attachment-upload.md b/docs/commands/markfluence_attachment-upload.md index 370fa3b..09eacf1 100644 --- a/docs/commands/markfluence_attachment-upload.md +++ b/docs/commands/markfluence_attachment-upload.md @@ -56,14 +56,11 @@ markfluence attachment-upload PAGE FILE... [flags] ### Options inherited from parent commands ``` - --cloud-id string Atlassian cloud ID. Set it only for a scoped API token. If not set, markfluence uses $CONFLUENCE_CLOUD_ID, then .env -d, --debug Print debug details, such as each retry decision - --env-file string Env file to read credentials from. The default is .env in the documentation root of the working directory, or in the working directory if there is no markfluence.yaml + --env-file string File to read credentials from, before the environment and your credentials file --json Write JSON, and no human output. A result goes to stdout. A fatal error goes to stderr as a JSON error object --no-color Print output with no color --root string Documentation root for every file. The default is the nearest directory above each file that has a markfluence.yaml, or the directory of the file if there is none - --url string Confluence site URL. If not set, markfluence uses $CONFLUENCE_URL, then .env - --username string Confluence username (your email address). If not set, markfluence uses $CONFLUENCE_USERNAME, then .env ``` ### SEE ALSO diff --git a/docs/commands/markfluence_check.md b/docs/commands/markfluence_check.md index 96cdf9d..5756e28 100644 --- a/docs/commands/markfluence_check.md +++ b/docs/commands/markfluence_check.md @@ -65,14 +65,11 @@ markfluence check FILE... [flags] ### Options inherited from parent commands ``` - --cloud-id string Atlassian cloud ID. Set it only for a scoped API token. If not set, markfluence uses $CONFLUENCE_CLOUD_ID, then .env -d, --debug Print debug details, such as each retry decision - --env-file string Env file to read credentials from. The default is .env in the documentation root of the working directory, or in the working directory if there is no markfluence.yaml + --env-file string File to read credentials from, before the environment and your credentials file --json Write JSON, and no human output. A result goes to stdout. A fatal error goes to stderr as a JSON error object --no-color Print output with no color --root string Documentation root for every file. The default is the nearest directory above each file that has a markfluence.yaml, or the directory of the file if there is none - --url string Confluence site URL. If not set, markfluence uses $CONFLUENCE_URL, then .env - --username string Confluence username (your email address). If not set, markfluence uses $CONFLUENCE_USERNAME, then .env ``` ### SEE ALSO diff --git a/docs/commands/markfluence_children.md b/docs/commands/markfluence_children.md index 1d568dd..66a76b6 100644 --- a/docs/commands/markfluence_children.md +++ b/docs/commands/markfluence_children.md @@ -60,14 +60,11 @@ markfluence children [PAGE] [flags] ### Options inherited from parent commands ``` - --cloud-id string Atlassian cloud ID. Set it only for a scoped API token. If not set, markfluence uses $CONFLUENCE_CLOUD_ID, then .env -d, --debug Print debug details, such as each retry decision - --env-file string Env file to read credentials from. The default is .env in the documentation root of the working directory, or in the working directory if there is no markfluence.yaml + --env-file string File to read credentials from, before the environment and your credentials file --json Write JSON, and no human output. A result goes to stdout. A fatal error goes to stderr as a JSON error object --no-color Print output with no color --root string Documentation root for every file. The default is the nearest directory above each file that has a markfluence.yaml, or the directory of the file if there is none - --url string Confluence site URL. If not set, markfluence uses $CONFLUENCE_URL, then .env - --username string Confluence username (your email address). If not set, markfluence uses $CONFLUENCE_USERNAME, then .env ``` ### SEE ALSO diff --git a/docs/commands/markfluence_create.md b/docs/commands/markfluence_create.md index 24c55bc..e2fb54c 100644 --- a/docs/commands/markfluence_create.md +++ b/docs/commands/markfluence_create.md @@ -97,14 +97,11 @@ markfluence create FILE... [flags] ### Options inherited from parent commands ``` - --cloud-id string Atlassian cloud ID. Set it only for a scoped API token. If not set, markfluence uses $CONFLUENCE_CLOUD_ID, then .env -d, --debug Print debug details, such as each retry decision - --env-file string Env file to read credentials from. The default is .env in the documentation root of the working directory, or in the working directory if there is no markfluence.yaml + --env-file string File to read credentials from, before the environment and your credentials file --json Write JSON, and no human output. A result goes to stdout. A fatal error goes to stderr as a JSON error object --no-color Print output with no color --root string Documentation root for every file. The default is the nearest directory above each file that has a markfluence.yaml, or the directory of the file if there is none - --url string Confluence site URL. If not set, markfluence uses $CONFLUENCE_URL, then .env - --username string Confluence username (your email address). If not set, markfluence uses $CONFLUENCE_USERNAME, then .env ``` ### SEE ALSO diff --git a/docs/commands/markfluence_diff.md b/docs/commands/markfluence_diff.md index 05eb456..0e68b1e 100644 --- a/docs/commands/markfluence_diff.md +++ b/docs/commands/markfluence_diff.md @@ -109,14 +109,11 @@ markfluence diff FILE [flags] ### Options inherited from parent commands ``` - --cloud-id string Atlassian cloud ID. Set it only for a scoped API token. If not set, markfluence uses $CONFLUENCE_CLOUD_ID, then .env -d, --debug Print debug details, such as each retry decision - --env-file string Env file to read credentials from. The default is .env in the documentation root of the working directory, or in the working directory if there is no markfluence.yaml + --env-file string File to read credentials from, before the environment and your credentials file --json Write JSON, and no human output. A result goes to stdout. A fatal error goes to stderr as a JSON error object --no-color Print output with no color --root string Documentation root for every file. The default is the nearest directory above each file that has a markfluence.yaml, or the directory of the file if there is none - --url string Confluence site URL. If not set, markfluence uses $CONFLUENCE_URL, then .env - --username string Confluence username (your email address). If not set, markfluence uses $CONFLUENCE_USERNAME, then .env ``` ### SEE ALSO diff --git a/docs/commands/markfluence_export.md b/docs/commands/markfluence_export.md index 9b323c7..d45632b 100644 --- a/docs/commands/markfluence_export.md +++ b/docs/commands/markfluence_export.md @@ -78,14 +78,11 @@ markfluence export [PAGE] [flags] ### Options inherited from parent commands ``` - --cloud-id string Atlassian cloud ID. Set it only for a scoped API token. If not set, markfluence uses $CONFLUENCE_CLOUD_ID, then .env -d, --debug Print debug details, such as each retry decision - --env-file string Env file to read credentials from. The default is .env in the documentation root of the working directory, or in the working directory if there is no markfluence.yaml + --env-file string File to read credentials from, before the environment and your credentials file --json Write JSON, and no human output. A result goes to stdout. A fatal error goes to stderr as a JSON error object --no-color Print output with no color --root string Documentation root for every file. The default is the nearest directory above each file that has a markfluence.yaml, or the directory of the file if there is none - --url string Confluence site URL. If not set, markfluence uses $CONFLUENCE_URL, then .env - --username string Confluence username (your email address). If not set, markfluence uses $CONFLUENCE_USERNAME, then .env ``` ### SEE ALSO diff --git a/docs/commands/markfluence_find.md b/docs/commands/markfluence_find.md index 5bfd77a..fff491d 100644 --- a/docs/commands/markfluence_find.md +++ b/docs/commands/markfluence_find.md @@ -47,14 +47,11 @@ markfluence find TITLE [flags] ### Options inherited from parent commands ``` - --cloud-id string Atlassian cloud ID. Set it only for a scoped API token. If not set, markfluence uses $CONFLUENCE_CLOUD_ID, then .env -d, --debug Print debug details, such as each retry decision - --env-file string Env file to read credentials from. The default is .env in the documentation root of the working directory, or in the working directory if there is no markfluence.yaml + --env-file string File to read credentials from, before the environment and your credentials file --json Write JSON, and no human output. A result goes to stdout. A fatal error goes to stderr as a JSON error object --no-color Print output with no color --root string Documentation root for every file. The default is the nearest directory above each file that has a markfluence.yaml, or the directory of the file if there is none - --url string Confluence site URL. If not set, markfluence uses $CONFLUENCE_URL, then .env - --username string Confluence username (your email address). If not set, markfluence uses $CONFLUENCE_USERNAME, then .env ``` ### SEE ALSO diff --git a/docs/commands/markfluence_page-info.md b/docs/commands/markfluence_page-info.md index 5fc63f8..b4827a9 100644 --- a/docs/commands/markfluence_page-info.md +++ b/docs/commands/markfluence_page-info.md @@ -50,14 +50,11 @@ markfluence page-info PAGE [flags] ### Options inherited from parent commands ``` - --cloud-id string Atlassian cloud ID. Set it only for a scoped API token. If not set, markfluence uses $CONFLUENCE_CLOUD_ID, then .env -d, --debug Print debug details, such as each retry decision - --env-file string Env file to read credentials from. The default is .env in the documentation root of the working directory, or in the working directory if there is no markfluence.yaml + --env-file string File to read credentials from, before the environment and your credentials file --json Write JSON, and no human output. A result goes to stdout. A fatal error goes to stderr as a JSON error object --no-color Print output with no color --root string Documentation root for every file. The default is the nearest directory above each file that has a markfluence.yaml, or the directory of the file if there is none - --url string Confluence site URL. If not set, markfluence uses $CONFLUENCE_URL, then .env - --username string Confluence username (your email address). If not set, markfluence uses $CONFLUENCE_USERNAME, then .env ``` ### SEE ALSO diff --git a/docs/commands/markfluence_read.md b/docs/commands/markfluence_read.md index 783bf95..df8364a 100644 --- a/docs/commands/markfluence_read.md +++ b/docs/commands/markfluence_read.md @@ -60,14 +60,11 @@ markfluence read PAGE [flags] ### Options inherited from parent commands ``` - --cloud-id string Atlassian cloud ID. Set it only for a scoped API token. If not set, markfluence uses $CONFLUENCE_CLOUD_ID, then .env -d, --debug Print debug details, such as each retry decision - --env-file string Env file to read credentials from. The default is .env in the documentation root of the working directory, or in the working directory if there is no markfluence.yaml + --env-file string File to read credentials from, before the environment and your credentials file --json Write JSON, and no human output. A result goes to stdout. A fatal error goes to stderr as a JSON error object --no-color Print output with no color --root string Documentation root for every file. The default is the nearest directory above each file that has a markfluence.yaml, or the directory of the file if there is none - --url string Confluence site URL. If not set, markfluence uses $CONFLUENCE_URL, then .env - --username string Confluence username (your email address). If not set, markfluence uses $CONFLUENCE_USERNAME, then .env ``` ### SEE ALSO diff --git a/docs/commands/markfluence_schema.md b/docs/commands/markfluence_schema.md index eedbfbe..b3452cb 100644 --- a/docs/commands/markfluence_schema.md +++ b/docs/commands/markfluence_schema.md @@ -38,14 +38,11 @@ markfluence schema [flags] ### Options inherited from parent commands ``` - --cloud-id string Atlassian cloud ID. Set it only for a scoped API token. If not set, markfluence uses $CONFLUENCE_CLOUD_ID, then .env -d, --debug Print debug details, such as each retry decision - --env-file string Env file to read credentials from. The default is .env in the documentation root of the working directory, or in the working directory if there is no markfluence.yaml + --env-file string File to read credentials from, before the environment and your credentials file --json Write JSON, and no human output. A result goes to stdout. A fatal error goes to stderr as a JSON error object --no-color Print output with no color --root string Documentation root for every file. The default is the nearest directory above each file that has a markfluence.yaml, or the directory of the file if there is none - --url string Confluence site URL. If not set, markfluence uses $CONFLUENCE_URL, then .env - --username string Confluence username (your email address). If not set, markfluence uses $CONFLUENCE_USERNAME, then .env ``` ### SEE ALSO diff --git a/docs/commands/markfluence_search.md b/docs/commands/markfluence_search.md index 4cbf2b5..90968cf 100644 --- a/docs/commands/markfluence_search.md +++ b/docs/commands/markfluence_search.md @@ -53,14 +53,11 @@ markfluence search QUERY [flags] ### Options inherited from parent commands ``` - --cloud-id string Atlassian cloud ID. Set it only for a scoped API token. If not set, markfluence uses $CONFLUENCE_CLOUD_ID, then .env -d, --debug Print debug details, such as each retry decision - --env-file string Env file to read credentials from. The default is .env in the documentation root of the working directory, or in the working directory if there is no markfluence.yaml + --env-file string File to read credentials from, before the environment and your credentials file --json Write JSON, and no human output. A result goes to stdout. A fatal error goes to stderr as a JSON error object --no-color Print output with no color --root string Documentation root for every file. The default is the nearest directory above each file that has a markfluence.yaml, or the directory of the file if there is none - --url string Confluence site URL. If not set, markfluence uses $CONFLUENCE_URL, then .env - --username string Confluence username (your email address). If not set, markfluence uses $CONFLUENCE_USERNAME, then .env ``` ### SEE ALSO diff --git a/docs/commands/markfluence_space-info.md b/docs/commands/markfluence_space-info.md index 2b023c0..a3c5230 100644 --- a/docs/commands/markfluence_space-info.md +++ b/docs/commands/markfluence_space-info.md @@ -66,14 +66,11 @@ markfluence space-info KEY [flags] ### Options inherited from parent commands ``` - --cloud-id string Atlassian cloud ID. Set it only for a scoped API token. If not set, markfluence uses $CONFLUENCE_CLOUD_ID, then .env -d, --debug Print debug details, such as each retry decision - --env-file string Env file to read credentials from. The default is .env in the documentation root of the working directory, or in the working directory if there is no markfluence.yaml + --env-file string File to read credentials from, before the environment and your credentials file --json Write JSON, and no human output. A result goes to stdout. A fatal error goes to stderr as a JSON error object --no-color Print output with no color --root string Documentation root for every file. The default is the nearest directory above each file that has a markfluence.yaml, or the directory of the file if there is none - --url string Confluence site URL. If not set, markfluence uses $CONFLUENCE_URL, then .env - --username string Confluence username (your email address). If not set, markfluence uses $CONFLUENCE_USERNAME, then .env ``` ### SEE ALSO diff --git a/docs/commands/markfluence_update.md b/docs/commands/markfluence_update.md index bac0a33..333d93a 100644 --- a/docs/commands/markfluence_update.md +++ b/docs/commands/markfluence_update.md @@ -109,14 +109,11 @@ markfluence update FILE... [flags] ### Options inherited from parent commands ``` - --cloud-id string Atlassian cloud ID. Set it only for a scoped API token. If not set, markfluence uses $CONFLUENCE_CLOUD_ID, then .env -d, --debug Print debug details, such as each retry decision - --env-file string Env file to read credentials from. The default is .env in the documentation root of the working directory, or in the working directory if there is no markfluence.yaml + --env-file string File to read credentials from, before the environment and your credentials file --json Write JSON, and no human output. A result goes to stdout. A fatal error goes to stderr as a JSON error object --no-color Print output with no color --root string Documentation root for every file. The default is the nearest directory above each file that has a markfluence.yaml, or the directory of the file if there is none - --url string Confluence site URL. If not set, markfluence uses $CONFLUENCE_URL, then .env - --username string Confluence username (your email address). If not set, markfluence uses $CONFLUENCE_USERNAME, then .env ``` ### SEE ALSO diff --git a/docs/commands/markfluence_user-find.md b/docs/commands/markfluence_user-find.md index 1acd49a..f24a114 100644 --- a/docs/commands/markfluence_user-find.md +++ b/docs/commands/markfluence_user-find.md @@ -52,14 +52,11 @@ markfluence user-find NAME [flags] ### Options inherited from parent commands ``` - --cloud-id string Atlassian cloud ID. Set it only for a scoped API token. If not set, markfluence uses $CONFLUENCE_CLOUD_ID, then .env -d, --debug Print debug details, such as each retry decision - --env-file string Env file to read credentials from. The default is .env in the documentation root of the working directory, or in the working directory if there is no markfluence.yaml + --env-file string File to read credentials from, before the environment and your credentials file --json Write JSON, and no human output. A result goes to stdout. A fatal error goes to stderr as a JSON error object --no-color Print output with no color --root string Documentation root for every file. The default is the nearest directory above each file that has a markfluence.yaml, or the directory of the file if there is none - --url string Confluence site URL. If not set, markfluence uses $CONFLUENCE_URL, then .env - --username string Confluence username (your email address). If not set, markfluence uses $CONFLUENCE_USERNAME, then .env ``` ### SEE ALSO diff --git a/docs/commands/markfluence_user-info.md b/docs/commands/markfluence_user-info.md index be93025..b761a4f 100644 --- a/docs/commands/markfluence_user-info.md +++ b/docs/commands/markfluence_user-info.md @@ -66,14 +66,11 @@ markfluence user-info [ACCOUNT_ID] [flags] ### Options inherited from parent commands ``` - --cloud-id string Atlassian cloud ID. Set it only for a scoped API token. If not set, markfluence uses $CONFLUENCE_CLOUD_ID, then .env -d, --debug Print debug details, such as each retry decision - --env-file string Env file to read credentials from. The default is .env in the documentation root of the working directory, or in the working directory if there is no markfluence.yaml + --env-file string File to read credentials from, before the environment and your credentials file --json Write JSON, and no human output. A result goes to stdout. A fatal error goes to stderr as a JSON error object --no-color Print output with no color --root string Documentation root for every file. The default is the nearest directory above each file that has a markfluence.yaml, or the directory of the file if there is none - --url string Confluence site URL. If not set, markfluence uses $CONFLUENCE_URL, then .env - --username string Confluence username (your email address). If not set, markfluence uses $CONFLUENCE_USERNAME, then .env ``` ### SEE ALSO diff --git a/internal/client/client_test.go b/internal/client/client_test.go index 58bf8c4..1305f05 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -1051,63 +1051,6 @@ func TestLoadDotenv(t *testing.T) { } } -func TestResolveValuePrecedence(t *testing.T) { - dotenv := map[string]string{"K": "from-dotenv"} - t.Setenv("K", "from-env") - if got := resolveValue("from-flag", "K", dotenv); got != "from-flag" { - t.Errorf("flag should win, got %q", got) - } - if got := resolveValue("", "K", dotenv); got != "from-env" { - t.Errorf("env should beat .env, got %q", got) - } - t.Setenv("K", "") - if got := resolveValue("", "K", dotenv); got != "from-dotenv" { - t.Errorf(".env should be the fallback, got %q", got) - } -} - -func TestResolve(t *testing.T) { - // Work in a temp dir so Resolve reads our .env, not the repo's. - dir := t.TempDir() - t.Chdir(dir) - if err := os.WriteFile(".env", []byte( - "CONFLUENCE_URL=https://file.example.net\n"+ - "CONFLUENCE_USERNAME=file-user\n"+ - "CONFLUENCE_TOKEN=file-pass\n"), 0o644); err != nil { - t.Fatal(err) - } - // Clear any inherited env for a deterministic baseline. CONFLUENCE_CLOUD_ID - // matters here too: a stray one would reroute BaseURL to the gateway. - t.Setenv("CONFLUENCE_URL", "") - t.Setenv("CONFLUENCE_USERNAME", "") - t.Setenv("CONFLUENCE_TOKEN", "") - t.Setenv("CONFLUENCE_CLOUD_ID", "") - - // All from .env. - c, err := Resolve(ResolveOptions{}) - if err != nil || c.BaseURL() != "https://file.example.net" { - t.Fatalf("Resolve(.env) = %v, %v", c, err) - } - - // Flag beats env beats .env for the URL. - t.Setenv("CONFLUENCE_URL", "https://env.example.net") - if c, _ := Resolve(ResolveOptions{URL: "https://flag.example.net"}); c.BaseURL() != "https://flag.example.net" { - t.Errorf("flag should win, got %q", c.BaseURL()) - } - if c, _ := Resolve(ResolveOptions{}); c.BaseURL() != "https://env.example.net" { - t.Errorf("env should beat .env, got %q", c.BaseURL()) - } - - // Missing token (no flag for it) is an error. - t.Setenv("CONFLUENCE_TOKEN", "") - if err := os.WriteFile(".env", []byte("CONFLUENCE_URL=u\nCONFLUENCE_USERNAME=x\n"), 0o644); err != nil { - t.Fatal(err) - } - if _, err := Resolve(ResolveOptions{URL: "u", Username: "x"}); err == nil { - t.Error("Resolve with no token: want error") - } -} - func TestNewGatewayBase(t *testing.T) { tests := []struct { name string diff --git a/internal/client/config.go b/internal/client/config.go index 93abd87..409a7fd 100644 --- a/internal/client/config.go +++ b/internal/client/config.go @@ -3,12 +3,12 @@ package client import ( "errors" "fmt" + "io/fs" "os" "path/filepath" "regexp" "strings" - - "github.com/mozilla/markfluence/internal/project" + "syscall" ) const ( @@ -16,72 +16,101 @@ const ( usernameEnv = "CONFLUENCE_USERNAME" tokenEnv = "CONFLUENCE_TOKEN" // the API token; never a command-line flag cloudIDEnv = "CONFLUENCE_CLOUD_ID" - dotenvPath = ".env" ) var spaceKeyRE = regexp.MustCompile(`^/spaces/([^/]+)/`) -// ResolveOptions carries the flag values Resolve needs, named so the two -// URL-ish fields can't be transposed at a call site. -type ResolveOptions struct { - // URL is the --url value (the Confluence site). - URL string - // Username is the --username value. - Username string - // CloudID is the --cloud-id value; set it to route requests through the - // platform API gateway, which a scoped service-account token requires. - CloudID string - // EnvFile is the --env-file value; empty means .env at the discovered - // project root (see loadEnvFile). - EnvFile string - // Roots, when set, is the caller's own per-file project.Cache -- built - // before Resolve is called, and passed back in so the .env lookup's - // directory resolution reuses it instead of a second, independent - // Discover/os.OpenRoot pass. The ordinary case is the two passes landing - // on the same root; sharing the cache is what makes that cost one - // discovery instead of two. Resolve does not close anything found this - // way -- the cache still owns it, for the caller's later per-file work. - Roots *project.Cache +// source is one place credentials come from, and the name an error uses for it. +type source struct { + label string + values map[string]string +} + +// lookup returns a setting's value and the index of the source that supplied +// it, or -1. An empty value counts as unset, so a lower source can supply it. +func lookup(sources []source, key string) (string, int) { + for i, s := range sources { + if v := s.values[key]; v != "" { + return v, i + } + } + return "", -1 } -// Resolve builds a client from the site URL, username, cloud ID, and token. Each -// value is resolved with the precedence flag > environment variable > .env file: -// the URL, username, and cloud ID come from opts when set, then -// $CONFLUENCE_URL/$CONFLUENCE_USERNAME/$CONFLUENCE_CLOUD_ID, then the .env file; -// the API token comes only from $CONFLUENCE_TOKEN, then .env -- never a flag. -// opts.EnvFile selects which .env is read: when empty, .env at the discovered -// project root is read best-effort (a missing file is fine; see loadEnvFile -// for what "discovered" means here); when set it's an explicit path that must -// be readable. It returns a friendly error listing whatever is missing. +// 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). +// Credentials come only from sources the user chose: nothing is discovered +// from the working directory, so a checkout cannot supply them (#188). // -// The cloud ID is optional: without one, requests go to the site domain exactly -// as before, which is what an unscoped personal token and any Data Center site -// need. -func Resolve(opts ResolveOptions) (*ConfluenceClient, error) { - env, err := loadEnvFile(opts.EnvFile, opts.Roots) - if err != nil { - return nil, err +// The explicit file ranks above the environment on purpose: a token exported +// in a shell profile must not reach the URL in a file named for another +// instance. +// +// Three rules keep a token from going somewhere it was not meant for: +// +// - 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 source's URL. +// - The cloud ID is read only from the URL's source, and ignored anywhere +// else. It names one site, as the URL does, so a cloud ID from another +// source never names this URL's site. Ignored rather than an error because +// a higher source cannot say "no cloud ID": an empty value is unset. +// - The username may come from any source; a wrong one fails with a 401. +// +// The credentials file is read only when the higher sources leave a setting +// 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. +func Resolve(envFile string) (*ConfluenceClient, error) { + var sources []source + if envFile != "" { + env, err := loadDotenv(envFile) + if err != nil { + return nil, fmt.Errorf("reading env file %q: %w", envFile, err) + } + sources = append(sources, source{label: "--env-file " + envFile, values: env}) + } + sources = append(sources, environment()) + + credPath := credentialsPath() + if !complete(sources) && credPath != "" { + creds, err := loadCredentials(credPath) + if err != nil { + return nil, err + } + if creds != nil { + sources = append(sources, source{label: displayPath(credPath), values: creds}) + } } - siteURL := resolveValue(opts.URL, urlEnv, env) - username := resolveValue(opts.Username, usernameEnv, env) - cloudID := resolveValue(opts.CloudID, cloudIDEnv, env) - token := resolveValue("", tokenEnv, env) + siteURL, urlFrom := lookup(sources, urlEnv) + username, _ := lookup(sources, usernameEnv) + token, tokenFrom := lookup(sources, tokenEnv) var missing []string if siteURL == "" { - missing = append(missing, "URL (--url or "+urlEnv+")") + missing = append(missing, "URL ("+urlEnv+")") } if username == "" { - missing = append(missing, "username (--username or "+usernameEnv+")") + missing = append(missing, "username ("+usernameEnv+")") } if token == "" { missing = append(missing, "token ("+tokenEnv+")") } if len(missing) > 0 { - return nil, errors.New("missing Confluence " + strings.Join(missing, ", ")) + return nil, fmt.Errorf("missing Confluence %s: set them in one place: %s", + strings.Join(missing, ", "), placesToSet(credPath)) + } + if urlFrom != tokenFrom { + return nil, fmt.Errorf("%s comes from %s, but %s comes from %s. Set both in the same place", + urlEnv, sources[urlFrom].label, tokenEnv, sources[tokenFrom].label) } - if err := validateCloudID(cloudID); err != nil { + cloudID := sources[urlFrom].values[cloudIDEnv] + if err := validateCloudID(cloudID, sources[urlFrom].label); err != nil { return nil, err } return New(Config{ @@ -92,103 +121,105 @@ func Resolve(opts ResolveOptions) (*ConfluenceClient, error) { }), nil } -// validateCloudID rejects a cloud ID that looks like a URL or a path fragment. -// The value is joined straight onto the gateway prefix, so pasting a whole -// gateway URL would otherwise produce an opaque 404 rather than a usable error. -func validateCloudID(cloudID string) error { - if cloudID == "" { - return nil +// environment is the CONFLUENCE_* variables as a source. +func environment() source { + values := map[string]string{} + for _, k := range []string{urlEnv, usernameEnv, tokenEnv, cloudIDEnv} { + values[k] = os.Getenv(k) } - if strings.ContainsAny(cloudID, "/:") { - return fmt.Errorf("invalid Confluence cloud ID %q (--cloud-id or %s): expected just the "+ - "identifier, not a URL or path", cloudID, cloudIDEnv) + return source{label: "the environment", values: values} +} + +// complete reports whether sources already supply the URL, username, and +// token. The cloud ID is not asked about: it comes only from the URL's source, +// which is already among these when the URL is. +func complete(sources []source) bool { + for _, k := range []string{urlEnv, usernameEnv, tokenEnv} { + if _, i := lookup(sources, k); i < 0 { + return false + } } - return nil + return true } -// resolveValue applies the flag > environment > .env precedence for one setting. -func resolveValue(flagVal, envKey string, dotenv map[string]string) string { - if flagVal != "" { - return flagVal +// 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") } - if v := os.Getenv(envKey); v != "" { - return v + home, err := os.UserHomeDir() + if err != nil || !filepath.IsAbs(home) { + return "" } - return dotenv[envKey] + return filepath.Join(home, ".config", "markfluence", "credentials") } -// loadEnvFile resolves which .env to read and parses it. An explicit envFile -// (from --env-file) must be readable, so a read failure is an error, and it -// overrides everything below absolutely -- including roots. With no explicit -// path, .env is read from the project root -- the directory holding -// markfluence.yaml, found by walking up from the working directory, or the -// working directory itself when there is none. When roots is given (a -// caller's own per-file project.Cache, built before Resolve is called), its -// Resolve is used for that walk instead of a bare project.Discover call, so -// a caller with its own --root override applies it here too, and doesn't pay -// for a second discovery (and a second os.OpenRoot) of the identical root; -// roots owns closing the handle, so none happens here. This is its own -// discovery pass, separate from the per-file root the converter uses: it -// starts at the working directory rather than a Markdown file's directory, -// runs once before any file is touched, and doesn't bound anything -- it -// only answers "where is .env." A missing .env, wherever it lands, is fine -// and yields an empty map, matching prior behavior. -// -// A discovery *failure* is not fine, and used to be swallowed. Since the -// project file is parsed (#100), one that cannot be understood makes discovery -// fail -- and degrading to the working directory there would read a different -// .env than the project's, silently, and would leave a command with no -// per-file root of its own (read, search, info) never reporting the malformed -// file at all. #100 settles that as: abort immediately. -func loadEnvFile(envFile string, roots *project.Cache) (map[string]string, error) { - if envFile != "" { - env, err := loadDotenv(envFile) - if err != nil { - return nil, fmt.Errorf("reading env file %q: %w", envFile, err) - } +// 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. 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), errors.Is(err, syscall.ENOTDIR): + return nil, nil + default: + return nil, fmt.Errorf("reading credentials file %s: %w", path, err) } +} - dir := "." - if cwd, err := os.Getwd(); err == nil { - found, err := dotenvDir(cwd, roots) - if err != nil { - return nil, err - } - dir = found +// 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 +} - env, err := loadDotenv(filepath.Join(dir, dotenvPath)) - if err != nil { - return map[string]string{}, nil // a missing .env is fine +// placesToSet lists where credentials can go, for the "missing" error. +func placesToSet(credPath string) string { + if credPath == "" { + return "the environment, or a file named by --env-file" } - return env, nil + return "the environment, " + displayPath(credPath) + ", or a file named by --env-file" } -// dotenvDir reports the directory .env is read from: the caller's own cache -// when it has one, so a --root override and the walk it already paid for both -// apply here too, and a fresh walk otherwise. The cache owns closing its -// handle; the fresh walk's is ours. -func dotenvDir(cwd string, roots *project.Cache) (string, error) { - if roots != nil { - root, err := roots.Resolve(cwd) - if err != nil { - return "", err - } - return root.Dir, nil +// validateCloudID rejects a cloud ID that looks like a URL or a path fragment. +// The value is joined straight onto the gateway prefix, so pasting a whole +// gateway URL would otherwise produce an opaque 404 rather than a usable error. +func validateCloudID(cloudID, from string) error { + if cloudID == "" { + return nil } - root, err := project.Discover(cwd) - if err != nil { - return "", err + if strings.ContainsAny(cloudID, "/:") { + return fmt.Errorf("invalid Confluence cloud ID %q (%s, from %s): expected just the "+ + "identifier, not a URL or path", cloudID, cloudIDEnv, from) } - defer func() { _ = root.FS.Close() }() - return root.Dir, nil + return nil } // securityWarner receives a credential-hygiene warning. Package-level and set // once from the command layer for the same reason SetRetryLogger is -// (retrylog.go): twelve commands build a client through Resolve with an -// identical literal, so anything passed per-call is something the thirteenth +// (retrylog.go): fifteen commands build a client through Resolve with an +// identical call, so anything passed per-call is something the sixteenth // silently forgets -- and internal/client deliberately produces no output and // imports no ui. var securityWarner func(string) @@ -197,16 +228,17 @@ var securityWarner func(string) // any previous one. Pass nil to silence it. func SetSecurityWarner(fn func(string)) { securityWarner = fn } -// warnLoosePermissions reports a .env that anyone but its owner can reach, -// when that file is the one holding the API token. +// warnLoosePermissions reports a credentials file (the user's own, or one named +// by --env-file) that anyone but its owner can reach, when it holds the API +// token. // -// The token gate is what keeps this worth reading. A .env carrying only +// The token gate is what keeps this worth reading. A file carrying only // CONFLUENCE_URL and CONFLUENCE_USERNAME at 0644 leaks nothing -- neither is a // secret, and the cloud ID is documented as not one either -- and a warning // that fires on a file with no secret in it is how a security warning becomes // something people learn to scroll past. // -// os.Stat, not Lstat: a .env symlinked to a 0600 file is perfectly safe, and +// os.Stat, not Lstat: a file symlinked to a 0600 file is perfectly safe, and // the link's own 0777 would cry wolf on every run. The user execute bit is // ignored for the same reason -- 0700 is odd, but it is not a leak. // @@ -254,7 +286,7 @@ func shellArg(path string) string { return path } -// loadDotenv reads a simple .env file into a map: KEY=value lines, with blank +// loadDotenv reads a simple env file into a map: KEY=value lines, with blank // lines and # comments skipped, an optional leading "export ", and optional // surrounding single or double quotes stripped. Values are taken verbatim (no // shell expansion). It errors if the file can't be read. @@ -276,8 +308,8 @@ func loadDotenv(path string) (map[string]string, error) { } out[strings.TrimSpace(key)] = unquote(strings.TrimSpace(value)) } - // Here rather than in loadEnvFile: this is the one function both the - // discovered .env and an explicit --env-file go through, and the check + // 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 e2e4951..a3711c1 100644 --- a/internal/client/config_test.go +++ b/internal/client/config_test.go @@ -5,12 +5,13 @@ import ( "path/filepath" "strings" "testing" - - "github.com/mozilla/markfluence/internal/project" ) -// clearConfluenceEnv unsets the CONFLUENCE_* vars for a test so .env / flags are -// the only sources. +// full is a complete set of credentials in env-file form. +const full = "CONFLUENCE_URL=https://wiki\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n" + +// clearConfluenceEnv unsets the CONFLUENCE_* vars for a test. TestMain already +// does this for the package; a test calls it again only to undo its own Setenv. func clearConfluenceEnv(t *testing.T) { t.Helper() for _, k := range []string{urlEnv, usernameEnv, tokenEnv, cloudIDEnv} { @@ -21,16 +22,30 @@ func clearConfluenceEnv(t *testing.T) { func writeEnvFile(t *testing.T, body string) string { t.Helper() path := filepath.Join(t.TempDir(), "custom.env") - if err := os.WriteFile(path, []byte(body), 0o644); err != nil { + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + return path +} + +// withCredentialsFile points XDG_CONFIG_HOME at a fresh directory and writes +// body as the credentials file there, returning its path. +func withCredentialsFile(t *testing.T, body string) string { + t.Helper() + cfg := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", cfg) + path := filepath.Join(cfg, "markfluence", "credentials") + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { t.Fatal(err) } return path } func TestResolveUsesExplicitEnvFile(t *testing.T) { - clearConfluenceEnv(t) - path := writeEnvFile(t, "CONFLUENCE_URL=https://wiki\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n") - c, err := Resolve(ResolveOptions{EnvFile: path}) + c, err := Resolve(writeEnvFile(t, full)) if err != nil { t.Fatalf("Resolve: %v", err) } @@ -39,147 +54,195 @@ func TestResolveUsesExplicitEnvFile(t *testing.T) { } } -func TestResolveFlagOverridesEnvFile(t *testing.T) { - clearConfluenceEnv(t) - path := writeEnvFile(t, "CONFLUENCE_URL=https://from-file\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n") - c, err := Resolve(ResolveOptions{URL: "https://from-flag", EnvFile: path}) +func TestResolveMissingExplicitEnvFileErrors(t *testing.T) { + missing := filepath.Join(t.TempDir(), "nope.env") + if _, err := Resolve(missing); err == nil { + t.Fatal("Resolve: want error for a missing --env-file path") + } +} + +func TestResolveUsesTheCredentialsFile(t *testing.T) { + withCredentialsFile(t, full) + c, err := Resolve("") if err != nil { t.Fatalf("Resolve: %v", err) } - if c.BaseURL() != "https://from-flag" { - t.Errorf("baseURL = %q, want the flag value", c.BaseURL()) + if c.SiteURL() != "https://wiki" { + t.Errorf("SiteURL = %q, want https://wiki from the credentials file", c.SiteURL()) } } -func TestResolveMissingExplicitEnvFileErrors(t *testing.T) { - clearConfluenceEnv(t) - missing := filepath.Join(t.TempDir(), "nope.env") - if _, err := Resolve(ResolveOptions{EnvFile: missing}); err == nil { - t.Fatal("Resolve: want error for a missing --env-file path") +// TestResolvePrecedence: --env-file beats the environment, and the environment +// beats the credentials file, one key at a time. The username is the key under +// test, since it is the one that may legitimately come from anywhere. +func TestResolvePrecedence(t *testing.T) { + withCredentialsFile(t, "CONFLUENCE_URL=https://wiki\nCONFLUENCE_USERNAME=from-creds\nCONFLUENCE_TOKEN=secret\n") + if c, err := Resolve(""); err != nil || c.username != "from-creds" { + t.Fatalf("credentials file alone: username = %v, %v", c, err) + } + t.Setenv(usernameEnv, "from-env") + if c, err := Resolve(""); err != nil || c.username != "from-env" { + t.Errorf("environment should beat the credentials file: %v, %v", c, err) + } + path := writeEnvFile(t, "CONFLUENCE_USERNAME=from-file\n") + if c, err := Resolve(path); err != nil || c.username != "from-file" { + t.Errorf("--env-file should beat the environment: %v, %v", c, err) } } -func TestResolveDefaultEnvFileMissingIsFine(t *testing.T) { - clearConfluenceEnv(t) - // No ./.env in this temp cwd, and no explicit env file: the missing default - // is tolerated, so we fail only on missing config values (not a read error). - t.Chdir(t.TempDir()) - _, err := Resolve(ResolveOptions{}) - if err == nil { - t.Fatal("want a missing-config error") +func TestCredentialsPath(t *testing.T) { + home := t.TempDir() + tests := []struct { + name, xdg, home, want string + }{ + {"absolute XDG_CONFIG_HOME", "/cfg", home, "/cfg/markfluence/credentials"}, + {"relative XDG_CONFIG_HOME is ignored", "cfg", home, filepath.Join(home, ".config/markfluence/credentials")}, + {"unset XDG_CONFIG_HOME", "", home, filepath.Join(home, ".config/markfluence/credentials")}, + {"relative HOME means no file", "", "home", ""}, + {"empty HOME means no file", "", "", ""}, } - // It should be the missing-values error, not a file-read error. - if !strings.Contains(err.Error(), "missing Confluence") { - t.Errorf("error = %q, want a missing-Confluence-config error", err) + for _, tt := range tests { + 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) + } + }) } } -func TestResolveDefaultEnvFileFoundInCwdWithNoProjectFile(t *testing.T) { - clearConfluenceEnv(t) - dir := t.TempDir() - if err := os.WriteFile(filepath.Join(dir, ".env"), - []byte("CONFLUENCE_URL=https://wiki\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n"), 0o644); err != nil { - t.Fatal(err) +func TestResolveWithNoHomeIsNotAnError(t *testing.T) { + t.Setenv("XDG_CONFIG_HOME", "") + t.Setenv("HOME", "") + if _, err := Resolve(writeEnvFile(t, full)); err != nil { + t.Fatalf("Resolve: %v", err) } - // No markfluence.yaml anywhere above dir, so discovery falls back to dir - // itself -- today's behavior, preserved. - t.Chdir(dir) +} - c, err := Resolve(ResolveOptions{}) - if err != nil { - t.Fatalf("Resolve: %v", err) +// TestResolveMissingCredentialsFileIsFine: no file, and a path that runs +// through a regular file, both mean the user has not made one. +func TestResolveMissingCredentialsFileIsFine(t *testing.T) { + // No username anywhere, so the credentials file is consulted. + t.Setenv(urlEnv, "https://wiki") + t.Setenv(tokenEnv, "secret") + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) // empty: no markfluence/ in it + if _, err := Resolve(""); err == nil || !strings.Contains(err.Error(), "missing Confluence username") { + t.Errorf("no credentials file: err = %v, want only the missing-username error", err) } - if c.BaseURL() != "https://wiki" { - t.Errorf("baseURL = %q, want https://wiki", c.BaseURL()) + + notADir := filepath.Join(t.TempDir(), "file") + if err := os.WriteFile(notADir, nil, 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("XDG_CONFIG_HOME", notADir) + if _, err := Resolve(""); err == nil || !strings.Contains(err.Error(), "missing Confluence username") { + t.Errorf("ENOTDIR: err = %v, want only the missing-username error", err) } } -func TestResolveDefaultEnvFileFoundAtDiscoveredProjectRoot(t *testing.T) { - clearConfluenceEnv(t) - root := t.TempDir() - if err := os.WriteFile(filepath.Join(root, "markfluence.yaml"), []byte("# marker\n"), 0o644); err != nil { +func TestResolveUnreadableCredentialsFileFails(t *testing.T) { + cfg := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", cfg) + path := filepath.Join(cfg, "markfluence", "credentials") + if err := os.MkdirAll(path, 0o700); err != nil { // a directory where the file goes t.Fatal(err) } - if err := os.WriteFile(filepath.Join(root, ".env"), - []byte("CONFLUENCE_URL=https://wiki\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n"), 0o644); err != nil { - t.Fatal(err) + _, err := Resolve("") + if err == nil || !strings.Contains(err.Error(), path) { + t.Errorf("err = %v, want an error naming %s", err, path) } - sub := filepath.Join(root, "docs", "team") - if err := os.MkdirAll(sub, 0o755); err != nil { +} + +// TestResolveSkipsTheCredentialsFileWhenComplete: a broken or loose file must +// not fail or warn on a run that never uses it. +func TestResolveSkipsTheCredentialsFileWhenComplete(t *testing.T) { + got := captureSecurityWarnings(t) + path := withCredentialsFile(t, "CONFLUENCE_TOKEN=secret\n") + if err := os.Chmod(path, 0o644); err != nil { t.Fatal(err) } - // No .env in the working directory itself -- only at the project root - // discovery finds by walking up. - t.Chdir(sub) - - c, err := Resolve(ResolveOptions{}) - if err != nil { + t.Setenv(urlEnv, "https://wiki") + t.Setenv(usernameEnv, "bot") + t.Setenv(tokenEnv, "secret") + if _, err := Resolve(""); err != nil { t.Fatalf("Resolve: %v", err) } - if c.BaseURL() != "https://wiki" { - t.Errorf("baseURL = %q, want https://wiki from the project root's .env", c.BaseURL()) + if len(*got) != 0 { + t.Errorf("warnings = %v, want none: the file was never needed", *got) } } -// TestResolveRootsOverridesEnvDiscovery covers ResolveOptions.Roots: when the caller -// passes its own --root-backed project.Cache, .env is read from that root, not -// from a plain upward walk from the working directory -- so a --root pointed -// at a different project also redirects which .env create/update/ -// attachment-upload read, matching the flag's stated meaning of overriding -// discovery for the whole invocation. -func TestResolveRootsOverridesEnvDiscovery(t *testing.T) { +func TestResolveURLAndTokenMustShareASource(t *testing.T) { + credPath := withCredentialsFile(t, full) + + t.Setenv(urlEnv, "https://elsewhere") + _, err := Resolve("") + if err == nil { + t.Fatal("URL from the environment, token from the credentials file: want an error") + } + for _, want := range []string{"CONFLUENCE_URL comes from the environment", + "CONFLUENCE_TOKEN comes from " + displayPath(credPath), "Set both in the same place"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q missing %q", err, want) + } + } + clearConfluenceEnv(t) - cwd := t.TempDir() // no .env here - override := t.TempDir() - if err := os.WriteFile(filepath.Join(override, ".env"), - []byte("CONFLUENCE_URL=https://from-root\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n"), 0o644); err != nil { - t.Fatal(err) + t.Setenv(tokenEnv, "other") + envFile := writeEnvFile(t, "CONFLUENCE_URL=https://b\nCONFLUENCE_USERNAME=bot\n") + if _, err := Resolve(envFile); err == nil || + !strings.Contains(err.Error(), "CONFLUENCE_URL comes from --env-file "+envFile) { + t.Errorf("URL from --env-file, token from the environment: err = %v", err) } - t.Chdir(cwd) - roots := project.NewCache(override) - defer roots.Close() - c, err := Resolve(ResolveOptions{Roots: roots}) - if err != nil { - t.Fatalf("Resolve: %v", err) + // An empty key in a higher source is unset, and is not blamed. + envFile = writeEnvFile(t, "CONFLUENCE_URL=https://b\nCONFLUENCE_TOKEN=\n") + if _, err := Resolve(envFile); err == nil || + !strings.Contains(err.Error(), "CONFLUENCE_TOKEN comes from the environment") { + t.Errorf("empty token in --env-file: err = %v, want the environment named", err) } - if c.BaseURL() != "https://from-root" { - t.Errorf("baseURL = %q, want https://from-root from --root's .env", c.BaseURL()) + + // Both from one place, with the username from another, is fine. + clearConfluenceEnv(t) + t.Setenv(usernameEnv, "someone") + if _, err := Resolve(writeEnvFile(t, "CONFLUENCE_URL=https://b\nCONFLUENCE_TOKEN=t\n")); err != nil { + t.Errorf("URL and token from --env-file, username from the environment: %v", err) } } -func TestResolveCloudIDPrecedence(t *testing.T) { - clearConfluenceEnv(t) - path := writeEnvFile(t, - "CONFLUENCE_URL=https://wiki\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n"+ - "CONFLUENCE_CLOUD_ID=from-file\n") +// TestResolveCloudIDFollowsTheURL: the cloud ID names one site, so it is read +// only from the source that supplied the URL. +func TestResolveCloudIDFollowsTheURL(t *testing.T) { + withCredentialsFile(t, full+"CONFLUENCE_CLOUD_ID=instance-a\n") - // From .env: requests move to the gateway, the site is untouched. - c, err := Resolve(ResolveOptions{EnvFile: path}) + c, err := Resolve("") if err != nil { t.Fatalf("Resolve: %v", err) } - if want := gatewayPrefix + "from-file"; c.BaseURL() != want { - t.Errorf("BaseURL = %q, want %q", c.BaseURL(), want) + if want := gatewayPrefix + "instance-a"; c.BaseURL() != want { + t.Errorf("URL and cloud ID from the credentials file: BaseURL = %q, want %q", c.BaseURL(), want) } - if c.SiteURL() != "https://wiki" { - t.Errorf("SiteURL = %q, want https://wiki", c.SiteURL()) + + c, err = Resolve(writeEnvFile(t, "CONFLUENCE_URL=https://b\nCONFLUENCE_TOKEN=t\n")) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if c.BaseURL() != "https://b" { + t.Errorf("URL from --env-file: BaseURL = %q, want https://b and no cloud ID from the credentials file", c.BaseURL()) } - // Env beats .env; flag beats env. t.Setenv(cloudIDEnv, "from-env") - if c, _ := Resolve(ResolveOptions{EnvFile: path}); c.BaseURL() != gatewayPrefix+"from-env" { - t.Errorf("env should beat .env, got %q", c.BaseURL()) + c, err = Resolve("") + if err != nil { + t.Fatalf("Resolve: %v", err) } - if c, _ := Resolve(ResolveOptions{CloudID: "from-flag", EnvFile: path}); c.BaseURL() != gatewayPrefix+"from-flag" { - t.Errorf("flag should win, got %q", c.BaseURL()) + if want := gatewayPrefix + "instance-a"; c.BaseURL() != want { + t.Errorf("cloud ID in the environment, URL in the credentials file: BaseURL = %q, want %q", c.BaseURL(), want) } } func TestResolveRejectsURLishCloudID(t *testing.T) { - clearConfluenceEnv(t) - path := writeEnvFile(t, "CONFLUENCE_URL=https://wiki\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n") - // Pasting a whole gateway URL (or any path fragment) is the likely mistake; // it must fail with a usable message rather than a 404 at request time. for _, bad := range []string{ @@ -187,7 +250,7 @@ func TestResolveRejectsURLishCloudID(t *testing.T) { "ex/confluence/abc", "abc/wiki", } { - _, err := Resolve(ResolveOptions{CloudID: bad, EnvFile: path}) + _, err := Resolve(writeEnvFile(t, full+"CONFLUENCE_CLOUD_ID="+bad+"\n")) if err == nil { t.Errorf("Resolve(cloud ID %q): want an error", bad) continue @@ -199,9 +262,7 @@ func TestResolveRejectsURLishCloudID(t *testing.T) { } func TestResolveWithoutCloudIDKeepsSiteURL(t *testing.T) { - clearConfluenceEnv(t) - path := writeEnvFile(t, "CONFLUENCE_URL=https://wiki/\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n") - c, err := Resolve(ResolveOptions{EnvFile: path}) + c, err := Resolve(writeEnvFile(t, "CONFLUENCE_URL=https://wiki/\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n")) if err != nil { t.Fatalf("Resolve: %v", err) } @@ -211,6 +272,64 @@ func TestResolveWithoutCloudIDKeepsSiteURL(t *testing.T) { } } +// TestResolveMissingNamesEverySource: someone whose settings are not found +// learns from the error where to put them, and that they go in one place. +func TestResolveMissingNamesEverySource(t *testing.T) { + cfg := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", cfg) + _, err := Resolve("") + if err == nil { + 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"), + "--env-file"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q missing %q", err, want) + } + } +} + +// TestResolveReadsNoDotenv: a .env in the working directory, or at the root +// of the project the working directory is in, is never read (#188). A checkout +// must not be able to supply credentials. +func TestResolveReadsNoDotenv(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "markfluence.yaml"), []byte("# marker\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, ".env"), []byte(full), 0o600); err != nil { + t.Fatal(err) + } + sub := filepath.Join(root, "docs") + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(sub, ".env"), []byte(full), 0o600); err != nil { + t.Fatal(err) + } + for _, dir := range []string{root, sub} { + t.Chdir(dir) + if _, err := Resolve(""); err == nil || !strings.Contains(err.Error(), "missing Confluence") { + t.Errorf("in %s: err = %v, want the missing-settings error", dir, err) + } + } +} + +// TestResolveIgnoresAMalformedProjectFile: credential resolution no longer +// discovers a root, so a project file it cannot parse is not its business. +// Commands that read the project file report it themselves. +func TestResolveIgnoresAMalformedProjectFile(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "markfluence.yaml"), []byte("spce: ENG\n"), 0o644); err != nil { + t.Fatal(err) + } + t.Chdir(root) + if _, err := Resolve(writeEnvFile(t, full)); err != nil { + t.Errorf("Resolve: %v", err) + } +} + func TestSpaceKeyFromWebUI(t *testing.T) { if got := SpaceKeyFromWebUI("/spaces/ENG/pages/123/Title"); got != "ENG" { t.Errorf("space = %q, want ENG", got) @@ -395,76 +514,19 @@ func TestWarnLoosePermissionsFollowsASymlink(t *testing.T) { } } -// TestResolveWarnsThroughTheDiscoveredEnvFile exercises the real path a command +// TestResolveWarnsThroughTheCredentialsFile exercises the real path a command // takes -- Resolve, not loadDotenv -- so the check cannot be wired only to the // explicit --env-file branch. -func TestResolveWarnsThroughTheDiscoveredEnvFile(t *testing.T) { - clearConfluenceEnv(t) +func TestResolveWarnsThroughTheCredentialsFile(t *testing.T) { got := captureSecurityWarnings(t) - dir := t.TempDir() - path := filepath.Join(dir, ".env") - body := "CONFLUENCE_URL=https://wiki\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n" - if err := os.WriteFile(path, []byte(body), 0o644); err != nil { - t.Fatal(err) - } + path := withCredentialsFile(t, full) if err := os.Chmod(path, 0o644); err != nil { t.Fatal(err) } - t.Chdir(dir) - - if _, err := Resolve(ResolveOptions{}); err != nil { + if _, err := Resolve(""); err != nil { t.Fatalf("Resolve: %v", err) } if len(*got) != 1 { t.Errorf("warnings = %v, want exactly one", *got) } } - -// A malformed markfluence.yaml used to be swallowed here, silently reading -// .env from the working directory instead of the project root. It matters most -// for a command with no per-file root of its own -- read, search, info -- which -// would otherwise never report the malformed file at all. -func TestResolveFailsOnAMalformedProjectFile(t *testing.T) { - clearConfluenceEnv(t) - root := t.TempDir() - if err := os.WriteFile(filepath.Join(root, "markfluence.yaml"), - []byte("spce: ENG\n"), 0o644); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(root, ".env"), - []byte("CONFLUENCE_URL=https://wiki\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n"), 0o644); err != nil { - t.Fatal(err) - } - t.Chdir(root) - - if _, err := Resolve(ResolveOptions{}); err == nil { - t.Fatal("Resolve succeeded with a malformed project file, want an error") - } else if !strings.Contains(err.Error(), "unknown setting") { - t.Errorf("error = %q, want it to name the unknown setting", err) - } -} - -// --env-file overrides discovery absolutely, which has to keep holding: an -// explicit path is how someone works around a project file they cannot fix. -func TestResolveEnvFileOverridesAMalformedProjectFile(t *testing.T) { - clearConfluenceEnv(t) - root := t.TempDir() - if err := os.WriteFile(filepath.Join(root, "markfluence.yaml"), - []byte("spce: ENG\n"), 0o644); err != nil { - t.Fatal(err) - } - explicit := filepath.Join(root, "creds.env") - if err := os.WriteFile(explicit, - []byte("CONFLUENCE_URL=https://wiki\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n"), 0o600); err != nil { - t.Fatal(err) - } - t.Chdir(root) - - c, err := Resolve(ResolveOptions{EnvFile: explicit}) - if err != nil { - t.Fatalf("Resolve: %v", err) - } - if c.SiteURL() != "https://wiki" { - t.Errorf("SiteURL = %q, want https://wiki", c.SiteURL()) - } -} From ce4be3ab0109ba401c7e827f9f7e99b0ca8eaf5e Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Wed, 23 Sep 2026 12:39:19 -0400 Subject: [PATCH 4/7] docs: document the credentials file The README's Configure section, CONTRIBUTING, SECURITY, .env.example, docs/root-model.md, docs/json-output.md, the schema's warnings descriptions, docs/confluence/, the internal/project and internal/jsonout comments, and CLAUDE.md now describe #188's credential sources: an --env-file, the environment, and ~/.config/markfluence/credentials, with no credential flags and nothing discovered from a project. --- .env.example | 7 ++- CLAUDE.md | 19 ++----- CONTRIBUTING.md | 7 ++- README.md | 97 ++++++++++++++++++++++----------- SECURITY.md | 11 ++-- docs/confluence/README.md | 2 +- docs/confluence/api.md | 2 +- docs/json-output.md | 5 +- docs/root-model.md | 35 +++--------- internal/jsonout/jsonout.go | 11 ++-- internal/project/config.go | 16 +++--- internal/project/config_test.go | 9 ++- internal/project/project.go | 13 ++--- schema/json-output/v1.json | 4 +- 14 files changed, 123 insertions(+), 115 deletions(-) diff --git a/.env.example b/.env.example index da1cf73..1f46bf8 100644 --- a/.env.example +++ b/.env.example @@ -1,6 +1,7 @@ -# Copy to .env and fill in. Required by markfluence. -# Then: chmod 600 .env -- it holds your API token, and markfluence warns if -# anyone else can read or write it. +# A template for your markfluence credentials file. Copy it to +# ~/.config/markfluence/credentials (or to a file you name with --env-file) +# and fill it in. Then chmod 600 the copy: it holds your API token, and +# markfluence warns if anyone else can read or write it. CONFLUENCE_URL=https://your-org.atlassian.net CONFLUENCE_USERNAME=you@example.com CONFLUENCE_TOKEN=your-api-token diff --git a/CLAUDE.md b/CLAUDE.md index 1e7284a..22acd30 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -31,18 +31,11 @@ A change that adds a package, exports a function, adds a make target, or introdu ## Configuration -The CLI needs a site URL, a username, and an API token, plus an optional cloud ID. Each resolves with the precedence **flag > environment variable > `.env` file**: - -| Setting | Flag | Env / `.env` | -|---|---|---| -| site URL | `--url` | `CONFLUENCE_URL` | -| username | `--username` | `CONFLUENCE_USERNAME` | -| API token | *(none)* | `CONFLUENCE_TOKEN` | -| cloud ID (optional) | `--cloud-id` | `CONFLUENCE_CLOUD_ID` | +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, which is why it may be a flag while the token may not. Gateway details, the scope list, and what is verified vs. assumed: [docs/confluence/api.md](docs/confluence/api.md). -markfluence reads a `.env` from the working directory itself (a minimal built-in parser — no shell expansion), or an explicit path via the persistent `--env-file` flag (a missing explicit path is an error; a missing default `./.env` is not); `.env.example` is the template. The API token is deliberately never a command-line flag. `internal/client.Resolve` is the single place this is read and validated. +**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); 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 tests reach `Resolve` has a `TestMain` calling `clienttest.RunIsolated` (or, in `internal/client` itself, which `clienttest` imports, its own `runIsolated`). ## Commit conventions @@ -60,7 +53,7 @@ Module `github.com/mozilla/markfluence` (`go 1.25`). `main.go` is a shim to `cmd ### Layout -- `cmd/root.go` — the cobra root: `--url`/`--username`/`--debug`/`--no-color` 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/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/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. @@ -74,7 +67,7 @@ Module `github.com/mozilla/markfluence` (`go 1.25`). `main.go` is a shim to `cmd - `cmd/attachment{list,upload,download}/` — the flat `attachment-list`/`attachment-upload`/`attachment-download` commands (noun-first so cobra's alphabetized help keeps them together and `attachment-` completes as a group). `upload` reuses the checksum skip/update logic, with `--force` (`client.ForceUploadAttachments`) and `--dry-run` (`PlanAttachments`); its `--name` takes a *path* whose base name becomes the stored name, and the recorded `path=` is the path as given, so a later publish can't create a duplicate under a different name; a batch whose base names collide is refused, since `planAttachments` reads the page's attachments once before its loop and would otherwise plan two creates for one name. `download` restores an attachment to its recorded `path=`, using the stored name verbatim when there is none (never interpreting it — a file really called `a%2Fb.png` must not be scattered into `a/b.png`, and `convert.sourceFor` answers the same way on the Markdown side, which is what keeps a downloaded file where the Markdown says it is), with `--flat` to opt out; `destPath` is the only place server data becomes a filesystem path and clamps to `--dest`, refusing rather than clipping an escape, since `..` is legitimate in a source path. - `cmd/schema/` — `schema`: print the embedded `--json` schema to stdout verbatim (no args, no credentials, no Confluence call). `--json` is deliberately a no-op — the output is already the schema document, not an envelope — which is also why `schema` is absent from the schema's own `command` enum. - `schema/` — the published JSON Schema (`json-output/v1.json`) *and* the `schema` Go package that embeds it (`V1`). The Go file lives beside the schema because `go:embed` cannot reach outside its own directory, and the schema stays at a top-level path a non-Go consumer can browse, mirroring its own `$id`. `internal/schematest` validates against the embed rather than reading the file, which is what makes "what ships" and "what the tests checked" the same bytes — do not reintroduce a disk read or a second copy. The version number is **not** restated here: `jsonout.SchemaVersion` and the document's own `schema_version` const are the two copies, tied together by a test in `cmd/schema`. The envelope's and the error object's top-level **`warnings`** are the one field no command fills: `jsonout.NewEnvelope`/`EmitError` drain a package-level collector (`AddWarning`), because the only thing in it is raised during credential resolution — below any command, before either document exists. -- `internal/project` — the documentation root: `Discover` (walk up from a directory looking for `markfluence.yaml`), `FromPath` (`--root`), `Resolve`, and `Cache`, which consults itself at every level of the walk so a batch spanning a subtree pays for the walk — and `os.OpenRoot` — once rather than per directory (the quadratic cost `_plans/025` measured). A `Root` carries `Dir`, `File`, `Config` and an `os.Root` that refuses an escape even through a symlink partway down. `Discover` is called from **two starting points for two reasons** — once per invocation from the working directory to locate `.env`, and once per Markdown file from its own directory to bound its reads and name its attachments — which is why it returns a type rather than a string; the two diverge legitimately, so a multi-root batch is allowed and nothing refuses it ([docs/root-model.md](docs/root-model.md)). `config.go` reads the project file's **settings** (#100): `space` and `page_width`, resolving **flag > frontmatter > project file**, which is *not* the credentials chain and must never be conflated with it. Three things about it are load-bearing. It is read through `frontmatter.Dialect.ReadMapping` rather than a second parser, since every rule there was found by probing goccy and a second copy would be a second set of the same bugs. An **unknown top-level key is fatal**, and that is the point rather than a cost — a silently ignored `spce: ENG` is wrong for every file at once, and a file written for a newer markfluence holds keys this binary would ignore, so there is no schema version and this must not be loosened; an empty or comment-only file stays valid, being what ships and what `export` plants. And loading happens in `open()`, the single place a `Root` is built from a marker hit, so `Discover`/`Cache`/`FromPath` cannot disagree that a file which cannot be understood **is not a valid marker**: the walk does not continue upward and does not fall back to the starting directory, because the root decides every attachment name and guessing at it is worse than stopping. `ConfigError`/`IsConfigError`/`RootError` exist so a caller reports that as a local defect (`VALIDATION`) rather than under `resolving the documentation root` as I/O. `pages.go` holds the **`pages:`** key (#139) and `SetPageEntry`, which records one file's metadata there: read-modify-write **once per page, not once per run**, because `create` writes each file's frontmatter as that page is published and a run that dies partway has to leave every already-created page recorded (D10). It **retries once** on a concurrent write, because the read-modify-write is not serialized and this file is shared by every page in the project: before the manifest each page's metadata went into its own file, so two concurrent `create`s could not collide, and they now can — A reads, B reads, A writes, B writes, and A's entry is gone while A's page exists. Optimistic rather than locked, matching `client.SetContentProperty`'s retry-once, since a lock file brings stale-lock handling for a verb a person invokes by hand. `beforeReplace` is a test hook for the give-up path, the `SetRetryLogger` arrangement. It also verifies twice — `frontmatter.SetNested` re-reads its own output, then `parseConfig` (split out of `loadConfig` for this) re-runs the *loader's* rules — since a tool that corrupts the file it is recording success in is the worst version of the feature; a test pins that a file which would not load afterwards is left byte-identical. A root with no project file refuses rather than creating one (#5's question, not a `create`'s to answer silently). An `Entry` is `{Fields, Lists}`, the same two maps `frontmatter.MarkdownFile` carries, which is the design rather than a convenience — `pagewidth` and `labels` both reach `client`, which holds a `*Cache`, so a typed validated entry would need a broken cycle or a second copy of every field's rules, and with two maps `labels.Declared(e.Lists, e.Fields)` works unchanged. `entryFields` is the manifest's schema and the only place it is written down; it mirrors frontmatter's fields deliberately, since adding one there and not here would make a field expressible in a file and not in an entry. Load checks **structure** — a mapping of mappings, legal paths, known field *names*, right shapes — and never a field's **value**, because #139 requires a semantically bad entry to be reported only when its file is one of the arguments, and this package has no idea which files the command was given. An unknown field *name* is the exception and is fatal at load, being the same typo class as an unknown setting. `NormalizePageKey` is lexical (L2 forbids a key whose meaning depends on the checkout's layout), and an escaping key or two keys normalizing to one are load-time errors naming both spellings. `Config.Pages` is **nil when there is no `pages:` key and empty-non-nil for `pages: {}`**, which is how a command tells "has not chosen the manifest" from "has, and has registered nothing". What it validates is **structure only**: `internal/pagewidth` cannot be imported here (`pagewidth` → `client` → `project`), so a width's vocabulary is checked by `pagewidth.Declared` where it already runs and by `check`'s offline lint. `Config` deliberately holds **no `url` or token**, and the reason is sharper than "those are credentials": basic auth goes to whatever host the resolved URL names, so a committed, walked-up file naming one would decide where `CONFLUENCE_TOKEN` is sent — a worse version of the `.env` hole #136 records. +- `internal/project` — the documentation root: `Discover` (walk up from a directory looking for `markfluence.yaml`), `FromPath` (`--root`), `Resolve`, and `Cache`, which consults itself at every level of the walk so a batch spanning a subtree pays for the walk — and `os.OpenRoot` — once rather than per directory (the quadratic cost `_plans/025` measured). A `Root` carries `Dir`, `File`, `Config` and an `os.Root` that refuses an escape even through a symlink partway down. `Discover` starts from a Markdown file's own directory, to bound its reads and name its attachments, which is why it returns a type rather than a string; nothing discovers a root from the working directory any more (that walk located `.env` until #188). Files in different projects have different roots, so a multi-root batch is allowed and nothing refuses it ([docs/root-model.md](docs/root-model.md)). `config.go` reads the project file's **settings** (#100): `space` and `page_width`, resolving **flag > frontmatter > project file**, which is *not* the credentials chain and must never be conflated with it. Three things about it are load-bearing. It is read through `frontmatter.Dialect.ReadMapping` rather than a second parser, since every rule there was found by probing goccy and a second copy would be a second set of the same bugs. An **unknown top-level key is fatal**, and that is the point rather than a cost — a silently ignored `spce: ENG` is wrong for every file at once, and a file written for a newer markfluence holds keys this binary would ignore, so there is no schema version and this must not be loosened; an empty or comment-only file stays valid, being what ships and what `export` plants. And loading happens in `open()`, the single place a `Root` is built from a marker hit, so `Discover`/`Cache`/`FromPath` cannot disagree that a file which cannot be understood **is not a valid marker**: the walk does not continue upward and does not fall back to the starting directory, because the root decides every attachment name and guessing at it is worse than stopping. `ConfigError`/`IsConfigError`/`RootError` exist so a caller reports that as a local defect (`VALIDATION`) rather than under `resolving the documentation root` as I/O. `pages.go` holds the **`pages:`** key (#139) and `SetPageEntry`, which records one file's metadata there: read-modify-write **once per page, not once per run**, because `create` writes each file's frontmatter as that page is published and a run that dies partway has to leave every already-created page recorded (D10). It **retries once** on a concurrent write, because the read-modify-write is not serialized and this file is shared by every page in the project: before the manifest each page's metadata went into its own file, so two concurrent `create`s could not collide, and they now can — A reads, B reads, A writes, B writes, and A's entry is gone while A's page exists. Optimistic rather than locked, matching `client.SetContentProperty`'s retry-once, since a lock file brings stale-lock handling for a verb a person invokes by hand. `beforeReplace` is a test hook for the give-up path, the `SetRetryLogger` arrangement. It also verifies twice — `frontmatter.SetNested` re-reads its own output, then `parseConfig` (split out of `loadConfig` for this) re-runs the *loader's* rules — since a tool that corrupts the file it is recording success in is the worst version of the feature; a test pins that a file which would not load afterwards is left byte-identical. A root with no project file refuses rather than creating one (#5's question, not a `create`'s to answer silently). An `Entry` is `{Fields, Lists}`, the same two maps `frontmatter.MarkdownFile` carries, which is the design rather than a convenience — `pagewidth` and `labels` both reach `client`, which holds a `*Cache`, so a typed validated entry would need a broken cycle or a second copy of every field's rules, and with two maps `labels.Declared(e.Lists, e.Fields)` works unchanged. `entryFields` is the manifest's schema and the only place it is written down; it mirrors frontmatter's fields deliberately, since adding one there and not here would make a field expressible in a file and not in an entry. Load checks **structure** — a mapping of mappings, legal paths, known field *names*, right shapes — and never a field's **value**, because #139 requires a semantically bad entry to be reported only when its file is one of the arguments, and this package has no idea which files the command was given. An unknown field *name* is the exception and is fatal at load, being the same typo class as an unknown setting. `NormalizePageKey` is lexical (L2 forbids a key whose meaning depends on the checkout's layout), and an escaping key or two keys normalizing to one are load-time errors naming both spellings. `Config.Pages` is **nil when there is no `pages:` key and empty-non-nil for `pages: {}`**, which is how a command tells "has not chosen the manifest" from "has, and has registered nothing". What it validates is **structure only**: `internal/pagewidth` cannot be imported here (`pagewidth` → `client` → `project`), so a width's vocabulary is checked by `pagewidth.Declared` where it already runs and by `check`'s offline lint. `Config` deliberately holds **no `url` or token**, and the reason is sharper than "those are credentials": basic auth goes to whatever host the resolved URL names, so a committed, walked-up file naming one would decide where `CONFLUENCE_TOKEN` is sent — a worse version of the walked-up `.env` hole #188 closed. - `internal/pagemeta` — `Resolve`, the one merge of a file's page metadata from the two places it may live: its own frontmatter and a `pages:` entry in the project file (#139). A package because `update`, `create`, `check` **and `internal/linkindex`** all need it, and a per-command copy is how two commands come to publish one file to two different pages. It imports `frontmatter` and `project` and nothing else, which is also why it validates no *value*: `pagewidth` and `labels` are unreachable from here, and the commands that need them already call them on the maps it returns. **Frontmatter and an entry are two spellings of one level, not two levels of a precedence chain** — when both speak the rule is not "the higher wins" but a grading: `page_id`/`space`/`parent` are coordinates and a disagreement fails the file (for `create`, the batch, since it preflights everything), while `title`/`page_width`/`labels` are visible and recoverable, so they warn and frontmatter wins. Agreement is silent, which is what makes migration incremental. Three things took a second pass and should not be flattened: `Source` (what `--json` reports as `metadata_source`) and `Managed` are computed from **different predicates** — the first answers "who contributed metadata", the second "should `update` act on this file", which is true when an entry exists or a `page_id` is named, so `a.md: {}` is a claim that must fail for want of an id rather than be skipped; contributed metadata is counted only over fields `project.IsPageField` knows, or a file carrying only keys markfluence *preserves but does not understand* (`reviewers:`, pinned by a frontmatter test) reads as claimed and a whole tree of them fails; and `labels` is compared as a **set**, since Confluence has no label order and a reordering cannot reach the page. A blank value is not a disagreement — every null spelling already reads as `""`. `KeyFor` is the one place a file's path becomes a manifest key, used on both sides because a mismatch is a silent skip rather than an error. `Origin` is the same question per field — which location supplied *this* value — recorded as `Resolve` grades them, because `diff` reports it beside every frontmatter difference and "the title differs" is otherwise ambiguous about which file to edit; unlike `MetadataSource()` it does **not** collapse `FromBoth`, which is the most useful of the three there (correcting a field two locations supply means editing two files). Computed here rather than by the caller for the reason the package exists: a second copy of the precedence rules is a second copy whatever it is used for. - `internal/actionlog` — the append-only record of what markfluence published, one log per project root at `/.markfluence/log.jsonl` (#149). `create`, `update` and `export` each append a line as a page completes, carrying the page version they left behind and `Sum`'s hash of what the body `PUT` sent; the last successful line for a file is that copy's **merge base**. It exists because nothing else can tell "the page differs because I have edits" from "the page differs because somebody published first" — that needs what *this copy* was derived from, which is a per-copy fact no page-side state can hold. Three things about it are load-bearing. **It is not committed**, and the reason is structural: a shared repository is itself a declaration that the repository is the source of truth, which is the arrangement where `update --force` is the answer and no base is consulted — so the log serves a local copy with the source of truth in Confluence, where per-checkout state is the right shape. The directory ignores itself (a planted `.gitignore` holding `*`, never overwritten) rather than editing a `.gitignore` markfluence does not own. **Nothing in it may fail a command**: a missing, unreadable, corrupt or half-written log degrades the check that reads it and never the run, which is why `read` skips a line it cannot parse instead of erroring and why a failed `Append` is a warning at the call site — the page is published by then, so failing would report that it was not. And **`Sum` covers exactly what the body `PUT` sends**, the resolved title and the rendered body: the title because `convert.ConfluencePage` carries none (a render-only hash would skip the publish of a file whose only change was its title), and *not* page width, labels or attachments, each of which has its own pass that runs whether or not the body is republished — folding them in would bump the page version for a change that never touched the body. Its parts are length-prefixed, which is part of the persisted format: changing the framing invalidates every recorded base. A root with `File == ""` gets no log at all (`For` returns nil, and every method is nil-safe), matching #139's rule that a root with no project file refuses rather than creating one. `Cache` hands out one `Log` per root so a batch reads each log once, and a batch spanning roots writes to several. `update` reads it for **two orthogonal checks, both only when `--force` is absent** (`--force` means always PUT, and no logic may suppress the request): the logged **`page_version`** against the live one decides **divergence** and *refuses* the file with `CodeConflict` — deliberately not qualified by the sha, since the case where you have no local edits is the worse one, publishing their work away with the bytes they started from — and the logged **`publish_sha256`** against this run's decides **idempotence** and skips the *body* `PUT` alone, leaving the attachment, width and label passes to run. An entry naming a different `page_id` is discarded rather than compared (a retarget would otherwise read as "the page moved 40 versions"), and the two fields degrade independently, so an `export` line written before its sha pass still refuses a moved page. A body-unchanged skip **records a line too**, which is load-bearing: once the sha does the skipping most runs skip, and a publish-only log would never keep a base current in a tree that is already published. Principles: **S8** (`no-overwrite-of-a-moved-page`), decided by the logged page version, and **L4** (`publish-is-idempotent`), decided by the publish sha rather than by mtime; both accept that a file with no base, or a root with no project file, gets neither. - `internal/pageslug` — `Slug`/`For`/`Filename`: a title to a filename-safe slug. A package rather than a helper because `export`, `read` and `attachment-download` all place attachments under a page's own directory and must agree. It lowercases (so case-variant titles collide and can be caught) and drops `/` (so no title can inject a path separator); it is lossy, and no readable slug can avoid being, so the caller decides what a collision means. Known limit: NFD and NFC spellings of one title are different Go strings but one filename on APFS, so that pair is not disambiguated. @@ -82,7 +75,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` reader, plus the **`.env` permission warning** (#136): a `.env` reachable by anyone but its owner (`mode.Perm()&0o077`) *and* containing `CONFLUENCE_TOKEN` earns a warning naming the file, its mode, and the `chmod`. Both halves matter — a `.env` holding only the URL and username leaks nothing, and a warning that fires on a file with no secret in it is how one becomes something people scroll past. It stats rather than lstats (a link's own `0777` would cry wolf over a `0600` target), lives in `loadDotenv` because that is the one function both the discovered `.env` and `--env-file` pass through, and reaches the reader through `SetSecurityWarner` for the same reason `SetRetryLogger` exists — 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* `.env` with no token in it is knowingly **not** covered, though `CONFLUENCE_URL` resolves from there too and rewriting it would redirect the token: see #136. Why each of these is shaped this way, with the evidence: [docs/confluence/api.md](docs/confluence/api.md) and [attachments.md](docs/confluence/attachments.md). +- `internal/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 credentials file or `--env-file` reachable by anyone but its owner (`mode.Perm()&0o077`) *and* containing `CONFLUENCE_TOKEN` earns a warning naming the file, its mode, and the `chmod`. Both halves matter — a 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 `