Repository navigation
Pass CONSOLE_REASON through the query alias, and never send a blank CONSOLE_USER - #3263
Conversation
…ONSOLE_USER A console audit asks that sessions reading customer data carry the link that authorized them. rails query can't prompt for one, so the query alias now passes CONSOLE_REASON, which console1984 appends to the session's reason (basecamp/console1984#169; recorded once the gem is bumped, ignored before). It's free text bound for a single-quoted YAML scalar and then Kamal's Shellwords.split, so it's flattened to one line and escaped for both. Thor splits each -e pair at its first colon, so a URL arrives whole. A blank USER (a sandbox, cron, launchd, env -i) left a bare "CONSOLE_USER:" in the console and shell aliases, which breaks this file's YAML for every kamal command. Fall back to LOGNAME, then the login name.
Kamal leaves a ${...} in an env value for the remote shell to expand, so a
reason carrying one would run on the console host. Encode every $ as %24,
which a URL reads the same.
There was a problem hiding this comment.
🔵 Needs a closer look
Merging requires maintainer confirmation that the external bin/failed-jobs parser update lands first and coordination of the console1984 upgrade needed to audit query sessions and their reasons.
No open issues 🎉
What changed in this PR
Updates SaaS Kamal aliases to forward query authorization context and provide a nonblank console username when USER is missing. Recording query reasons still depends on the pending console1984 update.
Changes:
- Normalizes and escapes
CONSOLE_REASONfor thequeryalias. - Adds username fallbacks through
LOGNAME, the passwd entry, and finallyuid-<n>across console, query, and SSH aliases.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
saas/config/deploy.yml |
Prepares query reasons for forwarding and adds console username fallbacks. |
Review effort: Balanced
LOGNAME and the passwd name are interpolated into every alias, plain YAML scalars and the single-quoted query alias alike. A name with a space or an apostrophe would split into arguments or break this file's YAML, so take the first of USER, LOGNAME and the passwd name made only of word characters, dots, @ and hyphens.
Escaping free text for each layer it crosses (YAML, Shellwords, Thor,
String#dump, the remote shell) kept missing one: Unicode line separators
end the YAML scalar, a C locale splits multibyte characters, and dump
backslashes #@ and #{. Percent-encode every byte outside a URL's own
characters instead. A link arrives unchanged, and anything else arrives
as inert ASCII. Match login names on bytes, so an invalid USER falls
through rather than raising.
|
Review threads: none. |
basecamp/audits1984#108: the session header and the sensitive access card render the reason and justification escaped, with URLs still linked.
|
Since the last summary: audits1984 moves from 4271a8e to f99fc64 in |
0.2.5 records CONSOLE_REASON in rails query sessions (basecamp/console1984#169) and carries everything since this app's previous pin, including Prism parsing on Ruby 3.3+ (#144).
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Since the last summary: console1984 is pinned to the GitHub tag |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 similar comment
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
A console audit asks that sessions reading customer data carry the link that authorized them: the card, issue or Help Scout conversation.
rails querycan't prompt for a reason, so console1984 records every agent query asrails query (via Claude Code). basecamp/console1984#169 appendsCONSOLE_REASONto that reason. This passes it through thequeryalias:It also fixes a second finding from the same audit: query sessions recorded under user
unknown.CONSOLE_REASON through five parsers
On its way to
docker exec --env, the value crosses five layers:Shellwords.split, inKamal::Cli::Alias::Command-ehash, which splits each pair at its first colon, so a URL arrives wholeString#dump, in Kamal'sescape_shell_value${…}: Kamal escapes$(but leaves${…}in deliberatelyEscaping for each layer in turn kept missing one. A Unicode line separator ends the YAML scalar and swallows the command. A C locale splits multibyte characters.
dumpadds a backslash before#@and#{. And${…}runs on the console host. So the header percent-encodes every byte outside a URL's own characters (A-Za-z0-9, space,._~:/?#[]!&()*+,;=%-). A link arrives unchanged, and anything else arrives as inert ASCII:$becomes%24,ébecomes%C3%A9, a newline becomes%0A. When the variable is unset or blank, Kamal sends--env CONSOLE_REASONwith no value, the same way it already handles an emptyCODEX_THREAD_ID, and the container doesn't get the variable.CONSOLE_USER when USER is blank
In a sandbox, cron, launchd or
env -i,USERcan be unset. The aliases rendered a bareCONSOLE_USER:. In the unquotedconsole,bashandsshaliases, that turns intoKEY: "…", a YAML mapping, and the whole file fails to parse (mapping values are not allowed in this context). Every kamal command fails, not just the console ones. The header now takes the first ofUSER,LOGNAMEand the passwd name that is a plain login name (\A[\w.@-]+\z, matched on bytes), elseuid-<n>. A name with a space, an apostrophe or invalid bytes is skipped, not interpolated.So a blank
USERcan't be what recorded theunknownsessions through this alias. It breaks kamal before anything runs. Those sessions most likely came from abin/rails queryrun without-e CONSOLE_USER, for example a hand-builtbin/kamal app execthat doesn't go through the alias. console1984#169 proposes refusing a blank username, as an option rather than a default.Verified, without touching a host
A local harness renders this
deploy.ymlthe way Kamal 2.12 does: ERB withtrim_mode: "-", then YAML. It expandsquerythe wayKamal::Cli::Alias::Commanddoes and runs Kamal's own Thor CLI, withApp#execstubbed. It then passes each--envargument Kamal would send throughsh -c, standing in for the remote shell, and comparesCONSOLE_USERandCONSOLE_REASONwith literal expected values. 46 of 46 pass for each of bc3, haystack and Fizzy: 23 cases, each in the default locale and underLC_ALL=C.?,&,#__recording_…; Help Scout URLHS 12345: refund request,-p --hosts evil,:it's "quoted" \ backit%27s %22quoted%22 %5C back${X:=a[$(id)]}${!X} ${HOME} $HOMEand a backtick$, brace and backtick encoded; nothing expands#@ticket #{x} #$y#%40ticket #%7Bx%7D #%24y, with no backslash fromdump--env CONSOLE_REASONwith no value, as todayUSERunset, empty, whitespace,a b,\xFF,é;LOGNAME="Jane O'Doe"To check that the harness can fail, I mutated the header three ways: removing
.b, letting$through, and matching names on characters. Each mutation fails 1 to 4 cases.Landed dependencies
v0.2.5, soCONSOLE_REASONis recorded.bin/failed-jobsrenders the new alias.Fizzy doesn't record query sessions yet
Gemfile.saas.lockpins console1984 at 0.2.2 (23419ec).Console1984::QueryAuditorarrived in 0.2.3, so Fizzy'srails querysessions aren't recorded inconsole1984_sessionsat all, whatever the reason or user. The bump this PR waits on brings in the auditor along with the reason. It also brings the other changes since 0.2.2: the Prism parser on Ruby 3.3+, and theinsert_all/upsert_allfix to the auditable-tables shield.