Warn when the .env holding the API token is readable by others

[willkg]

Fixes #136.

The problem

internal/client.loadDotenv read .env and handed back whatever it found without looking at the file's mode. A .env created by a shell redirect or an editor default lands at 0644 on most systems, so if it holds CONFLUENCE_TOKEN then every account on the machine can read the API token, and markfluence said nothing. The token is deliberately never a command-line flag precisely because it is the one value that must not be casually visible — which makes the file it lives in the thing worth checking.

What it does

Warns — never fails — when the .env just read is reachable by anyone but its owner (mode.Perm()&0o077 != 0) and actually contains CONFLUENCE_TOKEN. The message names the file, what is wrong with its mode, and the remedy:

  ! /home/you/docs/.env is readable by others (mode 0644) and holds your API token; run: chmod 600 /home/you/docs/.env

The token gate is the interesting half. A .env holding only CONFLUENCE_URL and CONFLUENCE_USERNAME at 0644 leaks nothing — neither is a secret, and the cloud ID is documented as not being 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. loadDotenv has already parsed the file when the check runs, so the gate costs nothing. It also makes the message's "holds your API token" a fact rather than a guess. A useful side effect: a CI job that passes credentials through the environment has no token in .env and never warns, so there is no need for a suppression flag.

It stats rather than lstats — a .env symlinked to a 0600 file is perfectly safe, and the link's own 0777 would cry wolf on every run. The user execute bit is ignored for the same reason: 0700 is odd, not a leak. The check lives in loadDotenv because that is the one function both the discovered project-root .env and an explicit --env-file pass through, and it reaches the reader through SetSecurityWarner, a package-level hook wired in PersistentPreRunE beside the existing SetRetryLogger — whose doc comment already explains why: twelve commands build a client through Resolve with an identical literal, so anything passed per-call is something the thirteenth silently forgets, and internal/client produces no output of its own.

Where the warning goes, and the mistake on the way there

This started as a JSON-exempt ui.SecurityWarn: every internal/ui helper is a no-op under --json on the premise that "warnings are carried in the structured payload instead", and this warning has no payload to be carried in — it names no page and no markdown file, so no results[] entry fits it. Writing it to stderr regardless looked right, on the reasoning that stderr is not part of the JSON contract.

That reasoning was wrong, and review caught it. README.md publishes the stderr error object as #/$defs/errorObject, and cmd/children's own test runs schematest.ValidateError over all of stderr as a single document. So a run with both a bad .env mode and a stderr error object — a rejected credential, a missing username, a failed find/search/children --space — would have emitted one human line followed by JSON, breaking exactly the consumer the schema invites. The repo's tests missed it only because PersistentPreRunE never runs in subpackage tests, leaving the hook nil.

So the warning travels inside the documents instead. Both the envelope and the error object gain a top-level warnings array, and no command fills it: NewEnvelope and EmitError drain a package-level collector, because the only thing in it is raised during credential resolution — below any command, before either document exists. The error object needs it as much as the envelope does, since a fatal failure emits no envelope and a credential failure is the run where this warning matters most. In human mode the stderr line is unchanged. cmd/root.go's reportSecurityWarning feeds both paths at once, since at the time the warning is raised nothing yet knows which document this run will produce.

schema_version stays at 1: markfluence is unreleased, so there is no consumer to break.

Also from review

• The message named the wrong problem for two thirds of the modes it fires on. 0622 is a genuine finding and nobody can read it, so "readable by others" was a claim a reader could check and disbelieve. accessDescription now says readable / writable / accessible from the bits actually set.
• The remedy is the point of the message, so a path a shell would mangle is quoted: a .env under My Docs previously produced a chmod that would have silently acted on the wrong path.
• Nothing pinned the wiring — deleting the one SetSecurityWarner line left every test passing, because each half is tested against its own double. TestSecurityWarnerIsWired runs PersistentPreRunE and Resolve for real, and I confirmed by mutation that it fails without that line. A retry log going quiet is a debugging annoyance; a security warning going quiet is the feature not existing.
• retrylog.go still said "ten commands" next to the new hook's "twelve"; both now say twelve.

Verification

make check clean. Tests cover the mode × token table (including 0400 and 0700 staying silent, and a token-less 0644 staying silent), the message's wording per mode, the quoted remedy, the symlink case, delivery through Resolve itself so the check cannot end up wired only to the --env-file branch, and that stderr under --json remains a single valid document. Smoke-tested live in all three states: human warning, --json with the warning in the envelope and clean stdout, and a --json failure whose stderr still parses as one document with the warning inside it.

Deliberately out of scope

A group- or world-writable .env with no token in it. CONFLUENCE_URL resolves from .env too, so anyone who can write that file can point markfluence at a host they control and collect the token from $CONFLUENCE_TOKEN on the next run — arguably the more dangerous file, and the token gate skips it. Covering it means warning on &0o022 regardless of contents, which widens this past "should be rw-user-only" and warns everyone with a group-writable shared checkout. Recorded in #136 rather than fixed here.