Add diagnostic logs, shown through mapbox history show - #58
Conversation
42c5bf3 to
b0fc212
Compare
For each run command history records, `mapbox config set log on` (or MAPBOX_LOG=1) adds a line of detail to ~/.mapbox/logs/<UTC date>.jsonl, linked to the history record by its id: the command line with tokens redacted, which token was used, each request (method, redacted URL, status, request id, timing) and the error message. Off by default. Logging needs history. With history off it never runs, and `config set log on` refuses with history_required rather than store a setting that does nothing. A run history doesn't record gets no log either, since nothing could lead back to it. `mapbox history show` includes the log and says what became of it: diagnostics.status is captured, not_captured (logging was off) or unavailable (captured, since expired or evicted). The history record carries diagnosticsCaptured so the last two can be told apart. There is no separate logs command. Logs are kept up to 30 days and 100 MB in total. Past the limit the oldest go first, down to the line, and their history records stay. On every run, a day of logs whose day of history has expired or gone is deleted, logging on or off.
b0fc212 to
d695a08
Compare
- History and the log each read the clock, so a run finishing across UTC midnight could put its log a day after its history, and the same run's cleanup then deleted it. Both lines now share one time and one day's file, which also drops next_date. - The log no longer repeats what the history record has (command, invocation, exitCode, durationMs, version). - scrub_tokens now finds a token that starts inside a word, as after a percent-encoded `=` (`%3Dpk.`) or in a short-flag cluster (`-ytpk.`). - tests/history.rs clears MAPBOX_LOG like the other suites. - README: a day of logs goes with its day of history, and the refusal follows the `history` setting, not MAPBOX_HISTORY.
mattpodwysocki
left a comment
There was a problem hiding this comment.
This is the one I pushed hardest on, since it's the security-sensitive piece. Built it, ran the suite, then actually did the thing the PR body admits wasn't fully checked: passed a real token-shaped value (pk.., 40+ chars) on the command line to a request that hit a real 401, with logging on, and read the raw jsonl line back off disk.
Confirmed: --token's value is in argv, access_token= is in the request URL, and the account claim it did keep (from the token's own payload, not the token itself) is exactly what the doc says it keeps. Also confirmed history_required fires correctly when history is off and you try to turn logging on, and the not_captured status shows up correctly in history show when logging was off for that run.
Good call reusing tilesets_cli's looks_like_a_token and redacted_argv rather than writing new redaction logic for this, those are already the audited mechanism this codebase trusts for the same job in --debug output.
One read worth double-checking on your end rather than mine: scrub_tokens finds the leftmost token-shaped suffix of a token-char run and redacts from there to the end of that run. That matches every test case here, but it does mean if a URL ever legitimately has two token-shaped-looking segments in the same unbroken run of token characters, only the first one triggers redaction and the rest of that run goes with it anyway (redacted as one unit) rather than each being found independently. Given how token-shaped detection works (40+ chars starting with pk./sk./tk.) I don't see a real string that would exercise this differently, just noting I didn't construct an adversarial case beyond the ones already in the test suite.
Approving.
Stacked on #56 (command history), which is stacked on #57.
Adds optional diagnostic logs for troubleshooting. History keeps only what is safe without anyone having asked; this keeps what answers "why did that command fail". It is off by default and reached only through
mapbox history show— there is no separate logs command.mapbox config set log on(orMAPBOX_LOG=1for a session) writes, for each run history records, one line to~/.mapbox/logs/<UTC date>.jsonl, linked to the history record by its id: the command line, which token was used (source, type, account — never the token), each request (method, URL, status, request id, timing) and the error message. Tokens are redacted in argv and URLs, and every string is also scrubbed of token-shaped words.historysetting offconfig set log onfails withhistory_requiredinstead of storing a setting that does nothing. A run history doesn't record (--help,sudo, ...) gets no log either.history showreportsdiagnostics.status:captured(the log is included),not_captured(logging was off for that run) orunavailable(captured, since expired or evicted). The history record carriesdiagnosticsCapturedso the last two can be told apart.history listmarks runs with a log.The three
statusvalues are new machine-readable values, documented indocs/commands.md.Not checked: the 100 MB eviction is tested with a sparse file, not 100 MB of real lines. A run that appends while another trims can lose its line; the logs are best-effort and a lock didn't seem worth it. The
history showsample indocs/commands.mdwas edited by hand after the review fixes, not re-captured. Nothing tests that a run's history and log land in the same day's file across midnight.