From b3fc907905aafd5bd9eaff779d0788d3b45735b5 Mon Sep 17 00:00:00 2001 From: Grant Cox Date: Wed, 2 Sep 2026 09:19:42 +1000 Subject: [PATCH 01/25] Allowlist the options rails and rake may be given `rake -e/-p/-E CODE` evaluates CODE inside Rake's own option parser -- before the Rakefile is loaded, without booting Rails -- and then exits, so the console audit hook records nothing at all. That is weaker than the `rails runner 'system("bash")'` case the README accepts as best effort, where Rails at least boots and the invocation is recorded. Rails hands any command it does not recognise to that same parser with the whole argv, so `rails -e CODE` and `rails db:migrate -e CODE` reach it too. Allowlisted rather than screened. A deny list has to model which of Rake's short options take an argument, in order to know where a bundle such as `-Ne` or `-se` stops being flags. Wrong for one option, in this version of Rake or a later one, and the bundle hides an `-e`. An allowlist fails the other way: an unlisted option is refused, so being wrong costs a denial rather than an unlogged shell. It also refuses what nobody thought to exclude -- `-g`/`--system` loads tasks from $HOME/.rake, which is /app/.rake on a dyno, and dumping Rake's option table is the only reason it was noticed. Options are matched as whole tokens, so `-s` is permitted while `-se` is a different token and is refused. That keeps the bundling reasoning out of the wrapper entirely. Every command gets a list; none is exempt. `rails console` and `rails runner` parse their own options and never reach Rake, so `-e` there is the environment -- but they get a list of their own rather than an exemption, because a mistake in a list is a denial while a mistake in an exemption is a bypass, silently and with no failing test. `rails runner -s` was permitted and is now refused: `-s` is not a runner option in Rails 8.1. What that test pinned -- that the sandbox denial is scoped to the console -- is still pinned by `rake -s` and `rails c -s`. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 49 ++++++++++++++++- guard/shim.sh | 134 ++++++++++++++++++++++++++++++++++++++++++++++ test/run_tests.sh | 108 ++++++++++++++++++++++++++++++++++++- 3 files changed, 289 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 538bea8..351b198 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,7 @@ Together they: * require the `CONSOLE_USER` and `CONSOLE_REASON` environment variables * reject compound statements and redirections, while allowing the `--exit-code` marker the Heroku CLI appends * permit only unqualified `rails`, `rake` and `bundle exec rails|rake` invocations, minus an - explicit deny list + explicit deny list, and allowlist the options those may be given * warn if [dyno metadata](https://devcenter.heroku.com/articles/dyno-metadata) is not enabled * export `CONSOLE_AUDIT_ENABLED=true` @@ -116,6 +116,7 @@ duplicates every rule rather than delegating. | `rails credentials:*`, `rails encrypted:*` | Spawns `$EDITOR`, which the operator controls — a shell escape. `EDITOR` and `VISUAL` are also unset | | `rails runner -` (a bare `-` in any argument position) | Reads the program from **stdin**, so the executed code appears neither in the dyno command string nor in an `ARGV` capture inside the app. The session still produces a complete record with a correct user, reason and dyno UUID, while the code that ran is unrecorded | | `rails runner --file `, or any `runner` argument that exists on disk | Same shape — the command string names a file rather than the code that runs | +| Any argument beginning with `-` that is not on the option allowlist | See [Option allowlist](#option-allowlist) below | | `-c` in any argument position | Reaches a shell (`bash -c`). No legitimate `rails`/`rake` invocation uses it. `rails c` — the console shorthand — is unaffected, because that argument is `c`, not `-c` | | `rails console --sandbox` / `-s` (console only) | The sandbox transaction is rolled back on exit, and a database-backed ActiveJob queue on the primary database puts the audit enqueue inside it — so the rollback discards the audit trail and the session runs entirely unlogged ([console1984#91](https://github.com/basecamp/console1984/issues/91)). Scoped to `console`/`c`, because `-s` is `rake`'s silent flag; `--no-sandbox` is unaffected | @@ -139,6 +140,52 @@ The `runner` file check tests whether the argument **exists on disk**, which is Rails itself makes. There is no heuristic on how the argument looks, so `rails runner 'Model.where(x: 1).rb'` is permitted and `rails runner ~/script` is not. +### Option allowlist + +Arguments beginning with `-` are **allowlisted, not screened**. Anything not named below is +refused. + +The reason is `rake -e/-p/-E CODE` (`--execute`, `--execute-print`, `--execute-continue`). Rake +evaluates `CODE` inside its own option parser — before the Rakefile is loaded, without booting +Rails — and then exits. Nothing the code does reaches the console audit hook, which makes it +weaker than the `rails runner 'system("bash")'` case [below](#limitations), where Rails at least +boots and the invocation is recorded. `rails` is affected too: it hands any command it does not +recognise to that same parser with the whole argv, so `rails -e CODE` and `rails db:migrate -e +CODE` reach it. + +A deny list would have to model which of Rake's short options take an argument, in order to know +where a bundle such as `-Ne` or `-se` stops being flags. Get that wrong for one option — in this +version of Rake or a later one — and the bundle hides an `-e`. An allowlist fails the other way: +an unlisted option is refused, so being wrong costs a denial rather than an unlogged shell. It +also refuses things nobody had to think of, such as `-g`/`--system`, which loads tasks from +`$HOME/.rake` — `/app/.rake` on a dyno. + +Every command gets a list; none is exempt. `rails console` and `rails runner` parse their own +options and never reach Rake, so `-e` there is the *environment* — but they get a list of their +own rather than being waved through, because a mistake in a list is a denial while a mistake in +an exemption is a silent bypass. + +| After | Permitted | +|---|---| +| `rails console` / `c` | `-e`/`--environment`, `--no-sandbox`, `-h`/`--help` | +| `rails runner` / `r` | `-e`/`--environment`, `-w`/`--skip-executor`, `-h`/`--help` | +| everything else (Rake's parser) | `-T`/`--tasks`, `-D`/`--describe`, `-W`/`--where`, `-P`/`--prereqs`, `-A`/`--all`, `--comments`, `--rules`, `-t`/`--trace`, `--backtrace`, `--job-stats`, `-s`/`--silent`, `-q`/`--quiet`, `-n`/`--dry-run`, `-v`/`--verbose`, `-V`/`--version`, `-m`/`--multitask`, `-j`/`--jobs`, `-B`/`--build-all`, `-X`/`--no-deprecation-warnings`, `-h`/`-H`/`--help` | + +Task names, task arguments (`some:task[a,b]`) and `VAR=value` assignments are not options and are +not screened, so `rake db:rollback STEP=99` is unaffected. + +Two consequences worth knowing before you hit them: + +- **Short options are matched whole**, so `-sq` is refused where `-s -q` is permitted. This is + what makes `-se CODE` refusable without reasoning about bundling at all. +- **Abbreviated long forms are refused.** Rake accepts `--task` for `--tasks`; the allowlist does + not. Denials list the permitted set, so this is self-service. + +Deliberately absent from the Rake list: `-e`/`-E`/`-p` (evaluate code), `-f`/`-r`/`-I`/`-R`/`-C` +(name a path — the same shape as a bare `-`), and `-g`/`-G`/`-N` (change which Rakefile is found). +Exploiting the path-naming options needs a file already in the slug, i.e. the same deploy-access +trust boundary as the `BASH_ENV` limitation below. + ### Blocked outright (non-Rails commands) These all fall through the allowlist. Named here because they are the cases most likely to come up: diff --git a/guard/shim.sh b/guard/shim.sh index 7c91f0a..9d8482e 100755 --- a/guard/shim.sh +++ b/guard/shim.sh @@ -240,4 +240,138 @@ if [[ "$_cg_policy_prog" == "rails" && ( "$_cg_sub" == "runner" || "$_cg_sub" == done fi +# ---------- option allowlist ---------- +# `rake -e/-p/-E CODE` evaluates CODE inside Rake's own option parser -- before +# the Rakefile is loaded and without booting Rails -- and then exits. Nothing the +# code does reaches the console audit hook, so it is weaker even than the +# `rails runner 'system("bash")'` case the README accepts as best effort, where +# Rails at least boots and the invocation is recorded. `-f/-r/-I/-R/-C/-g` name a +# path rather than the code that runs, which is what blocks a bare `-` above. +# +# Rails hands any command it does not recognise to that same parser with the +# whole argv, so `rails -e CODE` and `rails db:migrate -e CODE` reach it too. +# +# Allowlisted rather than screened. A deny list has to model which of Rake's +# short options take an argument, in order to know where a bundle such as `-Ne` +# stops being flags -- get that wrong for one option, in this version of Rake or +# a later one, and the bundle hides an `-e`. An allowlist fails the other way: an +# option nobody listed is refused, so the cost of being wrong is a denial rather +# than an unlogged shell. +# +# Every command gets a list; none is exempt. The two Rails commands below parse +# their own options and never reach Rake, so `-e` there is the environment -- +# but they are given a list of their own rather than being waved through, +# because a mistake in a list is a denial while a mistake in an exemption is a +# bypass, silently and with no failing test. +# +# _cg_allow_exact options taking no value: the token must match exactly, so +# `-se` is refused rather than read as a bundle +# _cg_allow_value options taking one: exactly, or with the value attached +# (`-T db`, `-Tdb`, `--tasks=db`) +_cg_allow_why=( + "Options are allowlisted here. Rake evaluates \`-e/-p/-E CODE\` in" + "its own option parser, before the Rakefile is loaded and without" + "booting Rails, so nothing that code does reaches the console audit" + "hook -- and Rails hands any command it does not recognise to that" + "same parser. Options naming a path are excluded for the reason a" + "bare \`-\` is." +) +_cg_allow_extras="task names, VAR=value assignments, and:" + +case "${_cg_policy_prog}/${_cg_sub}" in + rails/console|rails/c) + _cg_allow_exact="--no-sandbox -h --help" + _cg_allow_value="-e --environment" + _cg_allow_why=( + "Options are allowlisted here, so one nobody vetted is refused" + "rather than passed through to Rails." + ) + _cg_allow_extras="" + ;; + rails/runner|rails/r) + _cg_allow_exact="-w --skip-executor -h --help" + _cg_allow_value="-e --environment" + _cg_allow_why=( + "Options are allowlisted here, so one nobody vetted is refused" + "rather than passed through to Rails. The code to run is not an" + "option and needs no entry." + ) + _cg_allow_extras="" + ;; + *) + # Rake's read-only and output-shaping options. Absent, deliberately: + # -e/-E/-p (evaluate code), -f/-r/-I/-R/-C (name a path), and -g/-G/-N, + # which change which Rakefile is found -- `--system` loads tasks from + # $HOME/.rake, and $HOME is /app on a dyno. + _cg_allow_exact="-A -B -m -n -P -q -s -t -v -V -X -h -H" + _cg_allow_exact+=" --all --build-all --multitask --dry-run --prereqs" + _cg_allow_exact+=" --quiet --silent --verbose --version --comments --rules" + _cg_allow_exact+=" --no-deprecation-warnings --help" + _cg_allow_value="-T -D -W -j" + _cg_allow_value+=" --tasks --describe --where --jobs --trace --backtrace" + _cg_allow_value+=" --job-stats" + ;; +esac + +_cg_arg_permitted() { + local _cg_tok="$1" _cg_name + # Unquoted on purpose: the lists are space-delimited and must word-split. + for _cg_name in $_cg_allow_exact; do + [[ "$_cg_tok" == "$_cg_name" ]] && return 0 + done + for _cg_name in $_cg_allow_value; do + [[ "$_cg_tok" == "$_cg_name" ]] && return 0 + case "$_cg_name" in + --*) [[ "$_cg_tok" == "$_cg_name="* ]] && return 0 ;; + *) [[ "$_cg_tok" == "$_cg_name"?* ]] && return 0 ;; + esac + done + return 1 +} + +# Wrap the permitted set for the denial banner. Derived from the lists above +# rather than written out again, so the two cannot drift apart. +_cg_wrap_allowed() { + local _cg_line="" _cg_word + # shellcheck disable=SC2086 # deliberate word splitting, as above + for _cg_word in $_cg_allow_value $_cg_allow_exact; do + if (( ${#_cg_line} + ${#_cg_word} + 1 > 58 )); then + echo " $_cg_line" + _cg_line="$_cg_word" + else + _cg_line="${_cg_line:+$_cg_line }$_cg_word" + fi + done + [[ -n "$_cg_line" ]] && echo " $_cg_line" +} + +for _cg_arg in ${_cg_policy_args[@]+"${_cg_policy_args[@]}"}; do + # A bare `-` is handled above, with a message about stdin that says more than + # this one would. + [[ "$_cg_arg" == "-" ]] && continue + [[ "$_cg_arg" == -* ]] || continue + _cg_arg_permitted "$_cg_arg" && continue + + # Only when it names a command; for `rake -e 1` the "subcommand" is the + # rejected option itself. + _cg_context="$_cg_policy_prog" + [[ -n "$_cg_sub" && "$_cg_sub" != -* ]] && _cg_context+=" $_cg_sub" + + _cg_allowed_lines=() + while IFS= read -r _cg_line; do + _cg_allowed_lines+=("$_cg_line") + done < <(_cg_wrap_allowed) + + _cg_deny "\`${_cg_policy_prog} ${_cg_arg}\` is not permitted on one-off dynos." \ + "" \ + "${_cg_allow_why[@]}" \ + "" \ + "Permitted after \`${_cg_context}\`:${_cg_allow_extras:+ $_cg_allow_extras}" \ + "${_cg_allowed_lines[@]}" \ + "" \ + "" \ + "Short options are matched whole, so pass them separately rather" \ + "than bundled into one argument." +done + exec "$_cg_real" "$@" diff --git a/test/run_tests.sh b/test/run_tests.sh index 7409dcf..9007a38 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -205,9 +205,115 @@ cg_env 'S=--sandbox' ; assert_blocked 'rails c $S' 'unlogge # direction. Neither is collateral damage. assert_ran 'rake -s some:task' assert_ran 'rails c --no-sandbox' -assert_ran 'rails runner -s' assert_ran 'rails c' +cg_section "rake evaluates code in its own option parser" +# -e/-p/-E eval and exit inside rake's option parser, before the Rakefile is +# loaded, so nothing boots and the audit hook records nothing at all. +assert_blocked 'rake -e 1' 'allowlisted' +assert_blocked 'rake --execute 1' 'allowlisted' +assert_blocked 'rake -p 1+1' 'allowlisted' +assert_blocked 'rake -E 1' 'allowlisted' +assert_blocked 'rake --execute-print 1' 'allowlisted' +assert_blocked 'rake --execute-continue 1' 'allowlisted' +assert_blocked 'rake "--execute=1"' 'allowlisted' +assert_blocked 'bundle exec rake -e 1' 'allowlisted' +cg_env 'P=-e' ; assert_blocked 'rake $P 1' 'allowlisted' +# Short options bundle in rake, so the allowlist matches whole tokens: `-s` is +# permitted but `-se` is a different token and is refused. +assert_blocked 'rake -Ne 1' 'allowlisted' +assert_blocked 'rake -se 1' 'allowlisted' +assert_blocked 'rake -qsNe 1' 'allowlisted' +# ...which also means a bundle of two permitted flags is refused. Cheap: -s -q. +assert_blocked 'rake -sq some:task' 'matched whole' +# Abbreviated long forms, which rake accepts and the allowlist does not. +assert_blocked 'rake --exec 1' 'allowlisted' +assert_blocked 'rake --ex 1' 'allowlisted' +assert_blocked 'rake --task' 'allowlisted' +# Options that name a path rather than the code that runs. +assert_blocked 'rake -f Rakefile some:task' 'allowlisted' +assert_blocked 'rake -r ./payload some:task' 'allowlisted' +assert_blocked 'rake -I /app some:task' 'allowlisted' +assert_blocked 'rake -R /app some:task' 'allowlisted' +assert_blocked 'rake -C /app some:task' 'allowlisted' +assert_blocked 'rake --require ./payload' 'allowlisted' +assert_blocked 'rake --rakefile Rakefile' 'allowlisted' +# `--system` loads tasks from $HOME/.rake, and $HOME is /app on a dyno. The +# allowlist refuses these without anyone having had to think of them. +assert_blocked 'rake -g some:task' 'allowlisted' +assert_blocked 'rake -G some:task' 'allowlisted' +assert_blocked 'rake -N some:task' 'allowlisted' +assert_blocked 'rake --system some:task' 'allowlisted' +assert_blocked 'rake --suppress-backtrace x' 'allowlisted' +assert_blocked 'rake --no-such-option' 'allowlisted' +# Rails hands a command it does not recognise to rake's option parser, argv and +# all, so the same options arrive by way of `rails`. +assert_blocked 'rails -e 1' 'allowlisted' +assert_blocked 'rails db:migrate -e 1' 'allowlisted' +assert_blocked 'bundle exec rails -e 1' 'allowlisted' +# The denial names what is permitted, so a false positive is self-service. +assert_output 'the denial lists the permitted options' \ + 'rake --no-such-option' '--tasks' +assert_output 'the denial names the command it applies to' \ + 'rails db:migrate --no-such-option' 'Permitted after `rails db:migrate`' + +cg_section "the permitted rake options still work" +assert_ran 'rake -T' 'rake -T' +assert_ran 'rake -T db' +assert_ran 'rake -Tdb' 'rake -Tdb' +assert_ran 'rake "--tasks=db"' +assert_ran 'rake -D db' +assert_ran 'rake -W some:task' +assert_ran 'rake -P' +assert_ran 'rake -s some:task' +assert_ran 'rake -q some:task' +assert_ran 'rake -n some:task' +assert_ran 'rake -t some:task' +assert_ran 'rake -v some:task' +assert_ran 'rake -V' +assert_ran 'rake -A -T' +assert_ran 'rake -B some:task' +assert_ran 'rake -m some:task' +assert_ran 'rake -j 4 some:task' +assert_ran 'rake -j4 some:task' +assert_ran 'rake -X some:task' +assert_ran 'rake --trace some:task' 'rake --trace some:task' +assert_ran 'rake "--trace=stderr" some:task' +assert_ran 'rake --backtrace some:task' +assert_ran 'rake --dry-run some:task' +assert_ran 'rake --all --tasks' +assert_ran 'rake --comments --tasks' +assert_ran 'rake --rules' +assert_ran 'rake --job-stats some:task' +assert_ran 'rake --silent some:task' +assert_ran 'rake --version' +# Task names, task arguments and VAR=value assignments are not options and are +# not screened. +assert_ran 'rake db:rollback STEP=99' 'rake db:rollback STEP=99' +assert_ran 'rake "some:task[a,b]"' +assert_ran 'rake -s db:migrate STEP=1' + +cg_section "the commands Rails parses itself get their own list" +# `-e` is the environment here, not rake's execute. These commands never reach +# rake's parser -- but they are allowlisted rather than exempted, so a mistake +# in the list is a denial rather than a silent bypass. +assert_ran 'rails runner -e production Model.foo' +assert_ran 'rails runner --environment production Model.foo' +assert_ran 'rails runner -w Model.foo' +assert_ran 'rails c -e production' +assert_ran 'rails console --environment production' +assert_ran 'rails c --no-sandbox' +assert_ran 'rails c' +assert_ran 'bundle exec rails c -e production' +# Options neither Rails command takes are refused rather than passed through. +assert_blocked 'rails c --no-such-option' 'allowlisted' +assert_blocked 'rails runner -f Model.foo' 'allowlisted' +assert_blocked 'rails c -w' 'allowlisted' +# regression: `-s` reaches neither parser as anything useful. The rule that +# matters -- the sandbox denial is scoped to the console -- is pinned by +# `rake -s` above and `rails c -s` below. +assert_blocked 'rails runner -s Model.foo' 'allowlisted' + cg_section "regression (finding 2): parameter expansion must not defeat policy" cg_env 'P=-' ; assert_blocked 'rails runner $P' 'stdin' cg_env 'P=-' ; assert_blocked 'rails runner "$P"' 'stdin' From dfd55d0855c2bad590f47b4554b6fb6d61552d58 Mon Sep 17 00:00:00 2001 From: Grant Cox Date: Wed, 2 Sep 2026 12:35:26 +1000 Subject: [PATCH 02/25] Deny the `=` spellings of --sandbox in the sandbox rule itself Thor parses `--sandbox=true` as well as the bare flag, and the rule matched `--sandbox|-s` exactly, so the `=` forms fell through to the option allowlist. They were refused there, but for the wrong reason and with a banner that says nothing about why a sandboxed console is dangerous -- and the coverage was incidental: adding a sandbox-ish entry to the console's allowlist would have reopened the bypass with the whole suite green. Denied whatever value they carry, `false` included. Deciding which values Thor reads as true means modelling the parser, which is the thing the option allowlist exists to avoid. `--no-sandbox` stays allowlisted and is named in the denial, so the operator who wanted the negative form is told the spelling that works. Co-Authored-By: Claude Opus 5 --- README.md | 2 +- guard/shim.sh | 10 ++++++++-- test/run_tests.sh | 15 +++++++++++++++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 351b198..62a0767 100644 --- a/README.md +++ b/README.md @@ -118,7 +118,7 @@ duplicates every rule rather than delegating. | `rails runner --file `, or any `runner` argument that exists on disk | Same shape — the command string names a file rather than the code that runs | | Any argument beginning with `-` that is not on the option allowlist | See [Option allowlist](#option-allowlist) below | | `-c` in any argument position | Reaches a shell (`bash -c`). No legitimate `rails`/`rake` invocation uses it. `rails c` — the console shorthand — is unaffected, because that argument is `c`, not `-c` | -| `rails console --sandbox` / `-s` (console only) | The sandbox transaction is rolled back on exit, and a database-backed ActiveJob queue on the primary database puts the audit enqueue inside it — so the rollback discards the audit trail and the session runs entirely unlogged ([console1984#91](https://github.com/basecamp/console1984/issues/91)). Scoped to `console`/`c`, because `-s` is `rake`'s silent flag; `--no-sandbox` is unaffected | +| `rails console --sandbox` / `-s` (console only) | The sandbox transaction is rolled back on exit, and a database-backed ActiveJob queue on the primary database puts the audit enqueue inside it — so the rollback discards the audit trail and the session runs entirely unlogged ([console1984#91](https://github.com/basecamp/console1984/issues/91)). Scoped to `console`/`c`, because `-s` is `rake`'s silent flag. Thor also takes `--sandbox=`, so the `=` forms are refused whatever they carry, `false` included — `--no-sandbox` is the spelling that opts out, and is unaffected | Because these are checked after expansion, the quoted, variable and glob spellings of each are blocked too: `rails "dbconsole"`, `rails "credentials:edit"`, `rails runner "-"` and diff --git a/guard/shim.sh b/guard/shim.sh index 9d8482e..b98b430 100755 --- a/guard/shim.sh +++ b/guard/shim.sh @@ -193,20 +193,26 @@ done # Scoped to `console`/`c` rather than applied to every argv, because `-s` is # `rake`'s silent flag and legitimate there. `--no-sandbox` must keep working. # +# Thor parses `--sandbox=true` as well as the bare flag, so the `=` forms are +# denied whatever value they carry. Enumerating Thor's boolean vocabulary would +# be modelling the parser, which is the thing the option allowlist below exists +# to avoid; `--no-sandbox` is the spelling that opts out. +# # The console_audit gem sets Rails' own `config.disable_sandbox = true` when # auditing is active, which is a second layer over the same dynos: it holds even # if the command never reaches this wrapper. if [[ "$_cg_policy_prog" == "rails" && ( "$_cg_sub" == "console" || "$_cg_sub" == "c" ) ]]; then for _cg_arg in "${_cg_policy_args[@]:1}"; do case "$_cg_arg" in - --sandbox|-s) + --sandbox|--sandbox=*|-s|-s=*) _cg_deny "\`rails ${_cg_sub} ${_cg_arg}\` is not permitted on one-off dynos." \ "" \ "A sandboxed console rolls back its transaction on exit, which" \ "discards the queued audit records with it -- the session would" \ "run entirely unlogged." \ "" \ - "Use \`rails ${_cg_sub}\` instead. It is audited." + "Use \`rails ${_cg_sub}\` instead. It is audited." \ + "\`--no-sandbox\` is permitted and means the same thing." ;; esac done diff --git a/test/run_tests.sh b/test/run_tests.sh index 9007a38..2c1ff8a 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -201,6 +201,21 @@ assert_blocked 'rails console -s' 'unlogged' assert_blocked 'bundle exec rails c --sandbox' 'unlogged' assert_blocked 'rails c "--sandbox"' 'unlogged' cg_env 'S=--sandbox' ; assert_blocked 'rails c $S' 'unlogged' +# Thor takes `--flag=value` for a boolean, so the `=` forms have to be denied by +# this rule and not merely by the option allowlist below -- otherwise adding a +# sandbox-ish entry to the console's allowlist reopens the bypass with the whole +# suite green. Denied whatever the value is, `false` included: deciding which +# values Thor reads as true is modelling the parser. +assert_blocked 'rails c "--sandbox=true"' 'unlogged' +assert_blocked 'rails console "--sandbox=true"' 'unlogged' +assert_blocked 'rails c "--sandbox=1"' 'unlogged' +assert_blocked 'rails c "--sandbox=false"' 'unlogged' +assert_blocked 'rails c "-s=true"' 'unlogged' +assert_blocked 'bundle exec rails c "--sandbox=true"' 'unlogged' +cg_env 'S=--sandbox=true' ; assert_blocked 'rails c $S' 'unlogged' +# The denial points at the spelling that works. +assert_output 'the sandbox denial names --no-sandbox' \ + 'rails c "--sandbox=true"' '`--no-sandbox` is permitted' # Scoped to the console: -s is rake's silent flag, and --no-sandbox is the safe # direction. Neither is collateral damage. assert_ran 'rake -s some:task' From fcf3ee49f4bb2b9ca831ee422d305979bafb7e71 Mon Sep 17 00:00:00 2001 From: Grant Cox Date: Fri, 28 Aug 2026 16:26:34 +1000 Subject: [PATCH 03/25] Record a denial, instead of only printing one The denial banner reaches only the operator's terminal, over the rendezvous connection. It is not in the app's log stream and it never reaches Datadog: a search for "not permitted on one-off dynos" returns nothing. So the only durable trace of a blocked command was Heroku's api:dyno record, which shows that something was attempted but cannot tell a guard denial from an application error -- and the audit cross-check had to read "no console record for this dyno" as "blocked, or lost", which is not an audit trail. Each denial now POSTs one record to the same endpoint and credential as the console_audit gem, distinguished by event=command_denied. Both halves of the guard report, because a trail with only the profile script's denials would show every argument-policy block as a clean session. No service/env/app attribution. The gem sends those but stamps them on its worker, because `heroku run -e` can rewrite them and a record tagged env:staging would keep flowing to Datadog while dropping out of every production-scoped monitor. This runs inside the one-off dyno, so it sends none of them and lets datadog-proxy attribute the record from the credential instead. Fail-open throughout: one attempt, four seconds, no retry, and a failure warns without holding up the denial. curl's stderr is discarded because it reports failures by quoting the URL, which carries the credential. The command extraction moves above the identity gate so every record carries the command -- including the identity denial, which is the one CI hits. Denial precedence is unchanged; only the extraction moved, not the refusals. Not sufficient on its own, and the README says so: the URL variable is inherited by the dyno, so an operator can suppress their own denial record with -e. # Conflicts: # guard/shim.sh --- README.md | 70 ++++++++++++++- bin/compile | 8 ++ guard/denial_report.sh | 152 ++++++++++++++++++++++++++++++++ guard/shim.sh | 55 ++++++++++-- profile/console_guard.sh | 183 ++++++++++++++++++++++++++------------- test/lib/harness.sh | 71 +++++++++++++++ test/run_tests.sh | 81 +++++++++++++++++ 7 files changed, 550 insertions(+), 70 deletions(-) create mode 100644 guard/denial_report.sh diff --git a/README.md b/README.md index 62a0767..6cb6241 100644 --- a/README.md +++ b/README.md @@ -274,6 +274,62 @@ names the first word it rejected. An operator's screenshot is then enough to tel objected to the command that was typed or to something else — a wrapper, a prefix, or a shape the parser does not handle. Long commands are truncated at 300 characters. +### Denials are recorded, not just printed + +That banner reaches only the operator's terminal, over the rendezvous connection. It is not in the +app's log stream and it never reaches Datadog. Left there, the only durable trace of a blocked +command is Heroku's own `api:dyno` record — which shows that *something* was attempted but cannot +distinguish a guard denial from an application error, and forces the audit cross-check to read "no +console record for this dyno" as "blocked, or lost". + +So each denial also POSTs one record, to the same endpoint and with the same credential as the +companion gem — `CONSOLE_LOGGING_DATADOG_PROXY_URL`. `event` is what tells the two apart: + +```json +{ + "event": "command_denied", + "enforced": true, + "rule": "command_not_allowed", + "command": "psql", + "operator": "becky@example.com", + "reason": "checking a migration", + "dyno_id": "b922dfe5-0ede-45c8-a267-78bff7a23481", + "guard_version": "7f1e0d8", + "timestamp": "2026-08-28T06:24:43.000Z" +} +``` + +- **`rule`** is a short stable identifier for the check that refused — group a monitor by this rather + than by the denial text, which gets reworded. Current values: `dyno_name_spoofed`, + `wrapper_missing`, `identity_missing`, `command_unreadable`, `command_not_bash_c`, + `compound_statement`, `command_not_allowed`, `bundle_not_exec`, `bundle_exec_not_allowed`, + `raw_database_session`, `editor_escape`, `stdin_program`, `dash_c_flag`, `sandbox_console`, + `runner_file`. +- **`enforced`** is `false` in [permit mode](#phased-rollout). Phase 1 exists to measure what + enforcement would block, and that is only measurable if the would-be denials are recorded, so they + are sent in both modes. +- **`command`** is what that half of the guard judged: the pre-expansion command string from the + profile script, the post-expansion argv from the command wrapper. The CLI's `--exit-code` marker + is stripped first, so a CI denial records the command the caller wrote. +- **`dyno_id`** comes from the dyno metadata file, not from `HEROKU_DYNO_ID`, which `-e` can set to + anything. It is the join key against the `api:dyno` webhook. +- No `service` / `env` / `app` fields. The gem sends those but stamps them on the **worker**, because + `heroku run -e` can rewrite every one of them and a record tagged `env:staging` would keep flowing + to Datadog while dropping quietly out of a production-scoped monitor. This runs inside the one-off + dyno, where that defence is not available, so it sends none of them and lets the proxy attribute + the record from the credential it was authenticated with. + +Reporting is **fail-open and best effort**: one attempt, a 4-second ceiling, no retry, and a failure +warns on stderr without holding up the denial. Refusing the command is the control; recording it must +not be able to block that. A failure is reported as a status code — never as the URL, which carries +the credential. + +It is also **not sufficient on its own.** The URL variable is inherited by the one-off dyno, so an +operator who knows about this can suppress their own denial record with +`heroku run -e CONSOLE_LOGGING_DATADOG_PROXY_URL=`. What survives that is the `api:dyno` webhook and +the exit status. Closing it properly needs the record to originate somewhere the operator cannot +reach, which a buildpack cannot be. + ## Setup Add the buildpack to a Heroku app alongside its existing buildpacks, **pinned to a commit SHA**: @@ -300,6 +356,7 @@ records the installed version: -----> Installing console guard 7f1e0d8 profile script: .profile.d/zzz_console_guard.sh command wrapper: .console-guard/bin/{rails,rake,bundle} + denial reporter: .console-guard/lib/denial_report.sh dyno metadata file: /etc/heroku/dyno enforcement: blocking unless CONSOLE_BLOCK_ENFORCE=false at run time ``` @@ -332,8 +389,8 @@ records the installed version: ## Companion gem -The buildpack blocks commands and exports `CONSOLE_AUDIT_ENABLED=true`; it does not record anything -itself. Recording console statements is done in-app by +The buildpack blocks commands and exports `CONSOLE_AUDIT_ENABLED=true`; the only thing it records +itself is [its own denials](#denials-are-recorded-not-just-printed). Recording console statements is done in-app by [console1984-datadog](https://github.com/ynab/console1984-datadog), which activates when `CONSOLE_AUDIT_ENABLED` is set. See that repository for what it records and how to configure it. @@ -408,6 +465,7 @@ Set as a config var on the app, and read at **run** time: | Variable | Required | Notes | |---|---|---| | `CONSOLE_BLOCK_ENFORCE` | No | `false` opts into phase 1 permit mode. Defaults to enforcing, and only the exact value `false` opts out. Temporary: removed at the end of phase 1, and until then not tamper-proof | +| `CONSOLE_LOGGING_DATADOG_PROXY_URL` | No | Where to POST a [denial record](#denials-are-recorded-not-just-printed). Same variable, endpoint and Basic credential as the companion gem. Unset means denials are not recorded. Read on the one-off dyno, so `-e` can suppress it | Set as a config var on the app, and read at **build** time: @@ -421,6 +479,7 @@ Set by the buildpack itself: | Variable | Value | Notes | |---|---|---| | `CONSOLE_AUDIT_ENABLED` | `true` | Exported on `run`, `scheduler` and `release` dynos, in both enforcement modes. Activates the audit hook in the companion gem. Because `.profile.d` scripts run *after* config vars and `-e` vars are applied, an operator cannot disable it via `-e`. In local and development environments, where this buildpack does not run, set it manually to opt in | +| `CONSOLE_GUARD_DYNO_ID` | dyno UUID | Exported on gated dynos only, from the dyno metadata file, so the command wrapper's denial records carry a join key `-e` cannot forge. Empty when metadata is disabled | | `PATH` | prepended | With `.console-guard/bin`, so `rails`, `rake` and `bundle` resolve to the command wrapper | | `EDITOR`, `VISUAL` | unset | They are a shell escape via `rails credentials:edit` | @@ -502,6 +561,13 @@ reach models or the database. **`CONSOLE_USER` is self-reported** and is not verified by the buildpack. Heroku's own audit trail (`heroku access -a app_name`) is the authoritative record of who started a session. +**A denial record can be suppressed by the operator it is about.** The endpoint is read from +`CONSOLE_LOGGING_DATADOG_PROXY_URL`, which a one-off dyno inherits, so `-e` on that variable stops +the POST. The gem does not have this problem because its *worker* reads the variable, out of the +operator's reach; nothing running inside the dyno can borrow that defence. Suppression leaves the +`api:dyno` webhook and the exit status, so the attempt is still visible — just not identifiable as a +guard denial. Treat the record as evidence of what was blocked, not as proof that nothing was. + **Statements executed after the audit path is disabled are not recorded.** A statement that disables auditing is itself recorded if the gem logs before execution, but statements after it are not. diff --git a/bin/compile b/bin/compile index 66af94a..6d26150 100755 --- a/bin/compile +++ b/bin/compile @@ -62,6 +62,13 @@ render() { echo "-----> Installing console guard ${CG_VERSION}" +# Sourced by both halves of the guard, so it belongs beside them rather than in +# either one. Installed first, before the two scripts that look for it. +LIB_DIR="$BUILD_DIR/.console-guard/lib" +mkdir -p "$LIB_DIR" +render "$BUILDPACK_DIR/guard/denial_report.sh" "$LIB_DIR/denial_report.sh" +chmod 644 "$LIB_DIR/denial_report.sh" + # Named zzz_ so it loads last, after every other buildpack's profile script. # CONSOLE_AUDIT_ENABLED must be the final exported value, and the PATH entry that # reaches the command wrapper must not be undone by a later script. @@ -89,6 +96,7 @@ chmod 755 "$SHIM_DIR/rails" "$SHIM_DIR/rake" "$SHIM_DIR/bundle" { echo "profile script: .profile.d/zzz_console_guard.sh" echo "command wrapper: .console-guard/bin/{rails,rake,bundle}" + echo "denial reporter: .console-guard/lib/denial_report.sh" echo "dyno metadata file: ${CG_DYNO_METADATA_FILE}" echo "enforcement: blocking unless CONSOLE_BLOCK_ENFORCE=false at run time" } | indent diff --git a/guard/denial_report.sh b/guard/denial_report.sh new file mode 100644 index 0000000..b209179 --- /dev/null +++ b/guard/denial_report.sh @@ -0,0 +1,152 @@ +# shellcheck shell=bash +# Durable record of a console-guard denial. Sourced by both halves of the guard +# (profile/console_guard.sh and guard/shim.sh); provides _cg_report_denial. +# +# WHY THIS FILE EXISTS +# +# The denial banner is written to the operator's terminal over the rendezvous +# connection. It is not in the app's log stream, and it never reaches Datadog. +# So a blocked command leaves behind only Heroku's own `api:dyno` record, which +# shows that a command was attempted but cannot distinguish a guard denial from +# an application error -- and the cross-check queries have to read "no console +# record for this dyno" as "blocked, or lost", which is not an audit trail. +# +# One record per denial closes that. The queries then read a missing console +# record as lost, full stop. +# +# WHERE IT GOES +# +# The same endpoint and the same credential as the console_audit gem: +# CONSOLE_LOGGING_DATADOG_PROXY_URL, carrying HTTP Basic userinfo. One endpoint, +# one credential to issue and rotate, one Datadog source, and the join keys the +# proxy already derives (`dyno_id`, and `console_identity` from `operator`) +# apply to these records unchanged. `event` is what tells them apart. +# +# NO ATTRIBUTION FIELDS +# +# The gem sends `service` / `env` / `app` / `version`, but stamps them on the +# *worker*, because `heroku run -e` can rewrite every one of them and a record +# tagged `env:staging` would keep flowing to Datadog while dropping quietly out +# of a production-scoped monitor. This runs inside the one-off dyno, where that +# defence is not available, so it sends none of them and lets datadog-proxy +# attribute the record from the credential it was authenticated with. +# +# LIMITATION +# +# Fail-open, and not sufficient on its own. The URL variable is inherited by the +# one-off dyno, so an operator who knows about this can suppress their own +# denial record with `heroku run -e CONSOLE_LOGGING_DATADOG_PROXY_URL=`. What +# survives that is the `api:dyno` webhook and the exit status. See the README. + +_CG_REPORT_VERSION="@@CG_VERSION@@" +_CG_REPORT_URL_VAR="CONSOLE_LOGGING_DATADOG_PROXY_URL" +_CG_REPORT_EVENT="command_denied" +_CG_REPORT_CONNECT_TIMEOUT=2 +_CG_REPORT_MAX_TIME=4 +# Matches the profile script's denial banner, so the record and the banner agree +# on what the guard was judging. +_CG_REPORT_CMD_MAX=300 + +# JSON-escape a string onto stdout, without the surrounding quotes. +# +# Byte-wise on purpose. A one-off dyno runs in the C locale, so `${s:i:1}` walks +# bytes; a multi-byte character's bytes are each >= 0x80, fall through to the +# default arm untouched, and are reassembled by concatenation. That keeps the +# U+FFFF the CLI's --exit-code marker carries intact instead of mangling it. +_cg_json_escape() { + local _s="$1" _out="" _i _c + for (( _i = 0; _i < ${#_s}; _i++ )); do + _c="${_s:_i:1}" + case "$_c" in + '"') _out+='\"' ;; + '\') _out+='\\' ;; + $'\n') _out+='\n' ;; + $'\r') _out+='\r' ;; + $'\t') _out+='\t' ;; + # The rest of the C0 controls have no short form and are illegal raw in a + # JSON string, so they would make the whole record unparseable. + [[:cntrl:]]) _out+="$(printf '\\u%04x' "'$_c")" ;; + *) _out+="$_c" ;; + esac + done + printf '%s' "$_out" +} + +# _cg_json_field -- emits `,"name":"value"`, or nothing when the +# value is empty. Omitted rather than null so that a missing operator reads as +# absent in Datadog rather than as the string "null". +_cg_json_field() { + [[ -n "${2:-}" ]] || return 0 + printf ',"%s":"%s"' "$1" "$(_cg_json_escape "$2")" +} + +_cg_report_truncate() { + if (( ${#1} > _CG_REPORT_CMD_MAX )); then + printf '%s [truncated]' "${1:0:_CG_REPORT_CMD_MAX}" + else + printf '%s' "$1" + fi +} + +# _cg_report_denial +# +# rule short stable identifier for the check that refused -- the field to +# group a monitor by, because denial *messages* get reworded +# command what the guard was judging, as the banner shows it +# enforced true|false. Sent in permit mode as well: phase 1 exists to measure +# what enforcement would block, which is only measurable if the +# would-be denials are recorded. +_cg_report_denial() { + local _cg_rule="${1:-unknown}" _cg_cmd="${2:-}" _cg_enforced="${3:-true}" + local _cg_url="${!_CG_REPORT_URL_VAR:-}" + + # Nothing configured: an app that has not been given the endpoint is not one + # this can report for. Silent, because it is also the state of every app + # before rollout reaches it. + [[ -n "$_cg_url" ]] || return 0 + + if ! command -v curl > /dev/null 2>&1; then + echo "console-guard: curl is unavailable, denial not recorded" >&2 + return 0 + fi + + local _cg_body + _cg_body="{\"event\":\"${_CG_REPORT_EVENT}\",\"enforced\":${_cg_enforced}" + _cg_body+="$(_cg_json_field rule "$_cg_rule")" + _cg_body+="$(_cg_json_field command "$(_cg_report_truncate "$_cg_cmd")")" + _cg_body+="$(_cg_json_field operator "${CONSOLE_USER:-}")" + _cg_body+="$(_cg_json_field reason "${CONSOLE_REASON:-}")" + # Resolved from the dyno metadata file by the profile script, which refuses a + # session whose $DYNO disagrees with it. HEROKU_DYNO_ID is the fallback and is + # `-e`-settable, so it is only as good as the app's metadata being enabled. + _cg_body+="$(_cg_json_field dyno_id "${CONSOLE_GUARD_DYNO_ID:-${HEROKU_DYNO_ID:-}}")" + _cg_body+="$(_cg_json_field guard_version "$_CG_REPORT_VERSION")" + # datadog-proxy claims `timestamp` as the log's official date, exactly as it + # does for the gem's records, so a denial is filed at the moment it happened. + _cg_body+="$(_cg_json_field timestamp "$(date -u '+%Y-%m-%dT%H:%M:%S.000Z')")" + _cg_body+="}" + + # One attempt, short timeouts, no retry: the dyno is about to exit, and the + # operator should not wait on the audit pipeline to be told they were denied. + # + # stderr is discarded because curl reports failures by quoting the URL, which + # carries the Basic credential. Report the status code instead, never the URL. + local _cg_code + _cg_code="$(curl --silent --output /dev/null --write-out '%{http_code}' \ + --connect-timeout "$_CG_REPORT_CONNECT_TIMEOUT" \ + --max-time "$_CG_REPORT_MAX_TIME" \ + --header 'Content-Type: application/json' \ + --data-binary "$_cg_body" \ + "$_cg_url" 2>/dev/null)" + + case "$_cg_code" in + 2*) return 0 ;; + *) + # Loud, because a denial that was not recorded is the gap this file + # exists to close. Never fatal: refusing the command is the control, and + # recording it must not be able to hold that up. + echo "console-guard: denial not recorded (${_CG_REPORT_URL_VAR} returned ${_cg_code:-no response})" >&2 + return 0 + ;; + esac +} diff --git a/guard/shim.sh b/guard/shim.sh index b98b430..41e6cea 100755 --- a/guard/shim.sh +++ b/guard/shim.sh @@ -42,7 +42,24 @@ fi _cg_prog="${0##*/}" +# ---------- denial reporting ---------- +# Half the guard's denials happen here rather than in the profile script, and an +# audit trail that records only the other half is worse than none. A missing +# library degrades to a no-op: the record is observability, and losing it must +# not change what the wrapper permits. +_cg_lib="${HOME:-/app}/.console-guard/lib/denial_report.sh" +if [[ -r "$_cg_lib" ]]; then + # shellcheck source=guard/denial_report.sh + . "$_cg_lib" +else + echo "console-guard: denial reporter is missing, denials will not be recorded" >&2 + _cg_report_denial() { :; } +fi + +# is a short stable identifier for the check that refused. It is what a +# monitor groups by, because denial messages get reworded and rule names do not. _cg_deny() { + local _cg_rule="$1"; shift local line { echo "" @@ -60,11 +77,22 @@ _cg_deny() { echo "" } >&2 + # The post-expansion argv, which is what this half actually judged -- the + # profile script's copy is the pre-expansion string, and the two differ in + # exactly the cases this wrapper exists for. + local _cg_enforced=false + [[ "$_cg_enforcing" == "true" ]] && _cg_enforced=true + _cg_report_denial "$_cg_rule" "$_cg_prog ${_cg_argv_seen[*]:-}" "$_cg_enforced" + if [[ "$_cg_enforcing" == "true" ]]; then exit 1 fi } +# Captured before any policy runs, and before `bundle exec` rewriting narrows +# what policy looks at, so a denial record shows what was invoked. +_cg_argv_seen=("$@") + # ---------- resolve the real rails/rake ---------- # PATH still contains this wrapper's directory, so a plain `exec rails` would # re-enter this script. Walk PATH and take the first match that is not this @@ -112,7 +140,8 @@ _cg_policy_args=("$@") if [[ "$_cg_prog" == "bundle" ]]; then if [[ "${1:-}" != "exec" ]]; then - _cg_deny "\`bundle ${1:-}\` is not permitted on one-off dynos." \ + _cg_deny bundle_not_exec \ + "\`bundle ${1:-}\` is not permitted on one-off dynos." \ "" \ "Only \`bundle exec rails\` and \`bundle exec rake\` are allowed," \ "because those are the forms Heroku's Ruby buildpack produces for" \ @@ -126,7 +155,8 @@ if [[ "$_cg_prog" == "bundle" ]]; then _cg_policy_prog="$2" ;; *) - _cg_deny "\`bundle exec ${2:-}\` is not permitted on one-off dynos." \ + _cg_deny bundle_exec_not_allowed \ + "\`bundle exec ${2:-}\` is not permitted on one-off dynos." \ "" \ "Only \`rails\` and \`rake\` may be run under \`bundle exec\`," \ "and the name must be unqualified." @@ -144,7 +174,8 @@ _cg_sub="${_cg_policy_args[0]:-}" # ever seen by the console audit hook. case "$_cg_sub" in dbconsole|db) - _cg_deny "\`${_cg_policy_prog} ${_cg_sub}\` is not permitted on one-off dynos." \ + _cg_deny raw_database_session \ + "\`${_cg_policy_prog} ${_cg_sub}\` is not permitted on one-off dynos." \ "" \ "It opens a raw database session, so no statement reaches the" \ "console audit hook." @@ -156,7 +187,8 @@ esac # profile script also unsets EDITOR and VISUAL; this is the second layer. case "$_cg_sub" in credentials:*|encrypted:*) - _cg_deny "\`${_cg_policy_prog} ${_cg_sub}\` is not permitted on one-off dynos." \ + _cg_deny editor_escape \ + "\`${_cg_policy_prog} ${_cg_sub}\` is not permitted on one-off dynos." \ "" \ "These commands spawn an editor, which is a shell escape." ;; @@ -167,7 +199,8 @@ for _cg_arg in ${_cg_policy_args[@]+"${_cg_policy_args[@]}"}; do # that runs appears in no log at all -- not the dyno command string, not the # api:dyno webhook, not an in-app ARGV capture. if [[ "$_cg_arg" == "-" ]]; then - _cg_deny "Reading the program from stdin is not permitted." \ + _cg_deny stdin_program \ + "Reading the program from stdin is not permitted." \ "" \ "A bare \`-\` argument means the executed code never appears in" \ "any audit record. Pass the code inline instead." @@ -177,7 +210,8 @@ for _cg_arg in ${_cg_policy_args[@]+"${_cg_policy_args[@]}"}; do # invocation uses it. `rails c` -- the console shorthand -- is unaffected, # because that argument is `c`, not `-c`. if [[ "$_cg_arg" == "-c" ]]; then - _cg_deny "The \`-c\` flag is not permitted on one-off dynos." \ + _cg_deny dash_c_flag \ + "The \`-c\` flag is not permitted on one-off dynos." \ "" \ "Use \`rails c\` for a console." fi @@ -205,7 +239,8 @@ if [[ "$_cg_policy_prog" == "rails" && ( "$_cg_sub" == "console" || "$_cg_sub" = for _cg_arg in "${_cg_policy_args[@]:1}"; do case "$_cg_arg" in --sandbox|--sandbox=*|-s|-s=*) - _cg_deny "\`rails ${_cg_sub} ${_cg_arg}\` is not permitted on one-off dynos." \ + _cg_deny sandbox_console \ + "\`rails ${_cg_sub} ${_cg_arg}\` is not permitted on one-off dynos." \ "" \ "A sandboxed console rolls back its transaction on exit, which" \ "discards the queued audit records with it -- the session would" \ @@ -228,13 +263,15 @@ if [[ "$_cg_policy_prog" == "rails" && ( "$_cg_sub" == "runner" || "$_cg_sub" == for _cg_arg in "${_cg_policy_args[@]:1}"; do case "$_cg_arg" in --file|--file=*) - _cg_deny "\`rails runner\` may not read its program from a file." \ + _cg_deny runner_file \ + "\`rails runner\` may not read its program from a file." \ "" \ "Pass the code inline instead." ;; esac if [[ -f "$_cg_arg" ]]; then - _cg_deny "\`rails runner\` may not read its program from a file." \ + _cg_deny runner_file \ + "\`rails runner\` may not read its program from a file." \ "" \ "\`${_cg_arg}\` exists on disk, so Rails would execute the" \ "file rather than the argument. The command string would then" \ diff --git a/profile/console_guard.sh b/profile/console_guard.sh index a38a8a7..d903c67 100644 --- a/profile/console_guard.sh +++ b/profile/console_guard.sh @@ -34,6 +34,19 @@ _CG_VERSION="@@CG_VERSION@@" _cg_metadata_file="@@CG_DYNO_METADATA_FILE@@" _cg_shim_dir="${HOME:-/app}/.console-guard/bin" +_cg_lib_dir="${HOME:-/app}/.console-guard/lib" + +# ---------- denial reporting ---------- +# Sourced before the first check, because the $DYNO-spoof refusal below is one of +# the denials worth recording. A missing library degrades to a no-op: the record +# is observability, and losing it must not change what the guard permits. +if [[ -r "$_cg_lib_dir/denial_report.sh" ]]; then + # shellcheck source=guard/denial_report.sh + . "$_cg_lib_dir/denial_report.sh" +else + echo "console-guard: denial reporter is missing, denials will not be recorded" >&2 + _cg_report_denial() { :; } +fi # ---------- enforcement mode (phase 1 rollout) ---------- # Phase 1 permits but does not block: every check still runs and reports, but a @@ -87,6 +100,11 @@ if [[ -r "$_cg_metadata_file" ]]; then echo "==========================================" echo "" } >&2 + # Recorded with the metadata's dyno id, not $DYNO's, so the record files + # under the dyno this actually is. + CONSOLE_GUARD_DYNO_ID="${_cg_meta_id:-}" \ + _cg_report_denial dyno_name_spoofed "\$DYNO=${DYNO}" true + # Fatal in both enforcement modes. This is not a command-policy decision an # operator can be warned about; it is an attempt to change which dyno the # guard believes it is running on. @@ -123,11 +141,23 @@ if [[ "$_cg_gated" != "true" ]]; then if [[ "$_cg_audited" == "true" ]]; then export CONSOLE_AUDIT_ENABLED=true fi - unset _cg_enforcing _CG_VERSION _cg_metadata_file _cg_shim_dir \ - _cg_dyno_name _cg_dyno_id _cg_metadata_seen _cg_gated _cg_audited + unset -f _cg_report_denial _cg_json_escape _cg_json_field \ + _cg_report_truncate 2>/dev/null + unset _cg_enforcing _CG_VERSION _cg_metadata_file _cg_shim_dir _cg_lib_dir \ + _cg_dyno_name _cg_dyno_id _cg_metadata_seen _cg_gated _cg_audited \ + _CG_REPORT_VERSION _CG_REPORT_URL_VAR _CG_REPORT_EVENT \ + _CG_REPORT_CONNECT_TIMEOUT _CG_REPORT_MAX_TIME _CG_REPORT_CMD_MAX return 0 fi +# The trusted dyno id, for the command wrapper's own denial records: it is +# resolved from the metadata file above, which `heroku run -e` cannot reach, +# whereas HEROKU_DYNO_ID can be set to anything. Exported here rather than +# earlier so it appears only on the dynos the wrapper is installed for. +if [[ -n "$_cg_dyno_id" ]]; then + export CONSOLE_GUARD_DYNO_ID="$_cg_dyno_id" +fi + # ---------- the CLI's exit-status marker ---------- # `heroku run --exit-code` appends # @@ -168,8 +198,66 @@ for _cg_arg in "${_cg_argv[@]}"; do fi done +# ---------- extract the dyno command ---------- +# Heroku executes the one-off command through a login shell, which is the process +# that sources this script, so its argv is `bash -c ` and we +# want the payload. +# +# Extracted here, before the first check, so that every denial record carries the +# command -- including the identity denial, which is the one CI hits and the one +# where "what did they try to run" matters most. The refusals for an argv this +# cannot read stay further down, where they were, so denial precedence is +# unchanged. +_CG_DYNO_CMD="" +_cg_cmd_read=false + +if (( ${#_cg_argv[@]} > 0 )); then + case "${_cg_argv[0]##*/}" in + bash|sh|zsh|dash) + _cg_i=1 + while (( _cg_i < ${#_cg_argv[@]} )); do + # Match `-c` and also combined short forms such as `-lc`, which mean the + # same thing to the shell. + case "${_cg_argv[_cg_i]}" in + -c|-[!-]*c) + _CG_DYNO_CMD="${_cg_argv[_cg_i + 1]:-}" + _cg_cmd_read=true + break + ;; + esac + (( _cg_i++ )) + done + ;; + esac +fi + +# ---------- strip the CLI's exit-status marker ---------- +# Removed before anything vets or reports the command, so `heroku run --exit-code +# rake foo` is judged -- and recorded -- as `rake foo` rather than as the compound +# the CLI made of it. See the marker definition above for why dropping +# --exit-code is not an option. +# +# Exact literal, anchored to the end, removed at most once. A looser pattern is a +# shell escape: `rails c ; bash # heroku-command-exit-status` would be stripped +# back to `rails c` and permitted. Two markers leave one behind, which the +# compound check then rejects. +# +# If Heroku changes the marker this stops matching and CI is denied again -- +# noisy, but the safe direction to fail in. +if [[ "$_cg_cmd_read" == "true" ]]; then + _cg_candidate="${_CG_DYNO_CMD%"${_CG_DYNO_CMD##*[![:space:]]}"}" + if [[ "$_cg_candidate" == *"$_CG_EXIT_MARKER" ]]; then + _cg_candidate="${_cg_candidate%"$_CG_EXIT_MARKER"}" + _CG_DYNO_CMD="${_cg_candidate%"${_cg_candidate##*[![:space:]]}"}" + fi +fi + # Print a denial. Exits the dyno when enforcing; warns and continues otherwise. +# +# is a short stable identifier for the check that refused. It is what a +# monitor groups by, because denial messages get reworded and rule names do not. _cg_deny() { + local _cg_rule="$1"; shift local _cg_line { echo "" @@ -187,6 +275,13 @@ _cg_deny() { echo "" } >&2 + # Before the exit, and in permit mode too: phase 1 exists to measure what + # enforcement would block, which is only measurable if the would-be denials + # are recorded. + local _cg_enforced=false + [[ "$_cg_enforcing" == "true" ]] && _cg_enforced=true + _cg_report_denial "$_cg_rule" "${_CG_DYNO_CMD:-${_cg_argv[*]:-}}" "$_cg_enforced" + if [[ "$_cg_enforcing" == "true" ]]; then # Stand in for the `echo` the CLI appended, which exiting here skips. On # stdout, because that is the stream the CLI parses -- the banner above goes @@ -206,7 +301,8 @@ _CG_USAGE='heroku run -e "CONSOLE_USER=$(heroku whoami);CONSOLE_REASON=test" rai # enforce anything meaningful, so refuse rather than run half a gate. if [[ ! -x "$_cg_shim_dir/rails" || ! -x "$_cg_shim_dir/rake" || ! -x "$_cg_shim_dir/bundle" ]]; then - _cg_deny "The console guard command wrapper is missing from this dyno." \ + _cg_deny wrapper_missing \ + "The console guard command wrapper is missing from this dyno." \ "" \ "Expected: ${_cg_shim_dir}/{rails,rake,bundle}" \ "" \ @@ -232,7 +328,8 @@ if (( ${#_cg_missing[@]} > 0 )); then else _cg_missing_desc="${_cg_missing[0]} is" fi - _cg_deny "${_cg_missing_desc} not set." \ + _cg_deny identity_missing \ + "${_cg_missing_desc} not set." \ "" \ "Both are required on one-off dynos. CONSOLE_USER must be your" \ "\`heroku whoami\` value, so that console records can be compared" \ @@ -261,11 +358,10 @@ if [[ "$_cg_enforcing" != "true" && -z "${_cg_user_check//[[:space:]]/}" ]]; the export CONSOLE_USER="[not provided]" fi -# ---------- determine the dyno command ---------- -# Heroku executes the one-off command through a login shell, which is the process -# that sources this script. Its argv is therefore `bash -c `; -# we want the payload, not the wrapper. `_cg_argv` was read above, where the -# exit-status marker check needed it. +# ---------- refuse an argv the gate cannot read ---------- +# The extraction itself happened above, before the first check. What is left here +# is the pair of refusals for the shapes it could not handle, kept in this +# position so that the identity gate above still takes precedence. # Denials echo the command back. Without it a denial cannot be diagnosed from the # operator's side -- "this command is not permitted" says nothing about which @@ -280,42 +376,23 @@ _cg_show() { fi } -_CG_DYNO_CMD="" -_cg_cmd_read=false - if (( ${#_cg_argv[@]} == 0 )); then # Fail closed: if we cannot read the command, we cannot vet it. - _cg_deny "Could not read the dyno command." \ + _cg_deny command_unreadable \ + "Could not read the dyno command." \ "" \ "/proc/\$\$/cmdline is empty or unreadable, and the console gate" \ "cannot vet a command it cannot see, so the session is refused." \ "" \ "This is a platform or build problem, not an operator mistake." else - case "${_cg_argv[0]##*/}" in - bash|sh|zsh|dash) - _cg_i=1 - while (( _cg_i < ${#_cg_argv[@]} )); do - # Match `-c` and also combined short forms such as `-lc`, which mean the - # same thing to the shell. - case "${_cg_argv[_cg_i]}" in - -c|-[!-]*c) - _CG_DYNO_CMD="${_cg_argv[_cg_i + 1]:-}" - _cg_cmd_read=true - break - ;; - esac - (( _cg_i++ )) - done - ;; - esac - # No `-c` payload means this is not the `bash -c ` shape the gate is # built on: the login shell was invoked some other way, or the command arrives # on stdin. There is no command string to vet, so refuse -- and say so. if [[ "$_cg_cmd_read" != "true" || -z "${_CG_DYNO_CMD//[[:space:]]/}" ]]; then _cg_cmd_read=false - _cg_deny "Could not determine the dyno command." \ + _cg_deny command_not_bash_c \ + "Could not determine the dyno command." \ "" \ "The gate expects this session's login shell to have been invoked" \ "as \`bash -c \`. It was not, so there is no command" \ @@ -328,26 +405,6 @@ else fi fi -# ---------- strip the CLI's exit-status marker ---------- -# Removed before vetting, so `heroku run --exit-code rake foo` is judged on -# `rake foo` rather than on the compound the CLI made of it. See the marker -# definition near the top for why dropping --exit-code is not an option. -# -# Exact literal, anchored to the end, removed at most once. A looser pattern is a -# shell escape: `rails c ; bash # heroku-command-exit-status` would be stripped -# back to `rails c` and permitted. Two markers leave one behind, which the -# compound check then rejects. -# -# If Heroku changes the marker this stops matching and CI is denied again -- -# noisy, but the safe direction to fail in. -if [[ "$_cg_cmd_read" == "true" ]]; then - _cg_candidate="${_CG_DYNO_CMD%"${_CG_DYNO_CMD##*[![:space:]]}"}" - if [[ "$_cg_candidate" == *"$_CG_EXIT_MARKER" ]]; then - _cg_candidate="${_cg_candidate%"$_CG_EXIT_MARKER"}" - _CG_DYNO_CMD="${_cg_candidate%"${_cg_candidate##*[![:space:]]}"}" - fi -fi - # ---------- block compound statements and redirections ---------- # The allowlist below matches argv[0] only, so without this an operator could # append a second command -- eg `rails runner "1"; bash` -- and reach a shell. @@ -367,7 +424,8 @@ if [[ "$_cg_cmd_read" == "true" ]] && "$_CG_DYNO_CMD" == *'<'* || "$_CG_DYNO_CMD" == *'>'* || "$_CG_DYNO_CMD" == *$'\n'* ]]; then - _cg_deny "Compound statements and redirections are not permitted on one-off" \ + _cg_deny compound_statement \ + "Compound statements and redirections are not permitted on one-off" \ "dynos." \ "" \ "The command may not contain any of: ; & | \` \$( < > newline" \ @@ -408,7 +466,8 @@ if [[ "$_cg_cmd_read" == "true" ]]; then case "$_cg_bin" in rails|rake|bundle) : ;; *) - _cg_deny "This command is not permitted on one-off dynos." \ + _cg_deny command_not_allowed \ + "This command is not permitted on one-off dynos." \ "" \ "Command:" \ " $(_cg_show "$_CG_DYNO_CMD")" \ @@ -466,8 +525,14 @@ export CONSOLE_AUDIT_ENABLED=true # This script is sourced, so clean up after ourselves rather than leaking state # into the console session. -unset -f _cg_deny _cg_show -unset _cg_enforcing _CG_VERSION _cg_metadata_file _cg_shim_dir _cg_dyno_name \ - _cg_dyno_id _cg_metadata_seen _cg_gated _cg_audited _cg_user_check \ - _cg_reason_check _cg_missing _cg_missing_desc _cg_argv _cg_arg _cg_i \ - _cg_tokens _cg_bin _cg_cmd_read _CG_DYNO_CMD _CG_USAGE +# +# The command wrapper sources the denial reporter itself, so nothing here needs +# to survive for it -- and CONSOLE_GUARD_DYNO_ID, which does, is exported. +unset -f _cg_deny _cg_show _cg_report_denial _cg_json_escape _cg_json_field \ + _cg_report_truncate +unset _cg_enforcing _CG_VERSION _cg_metadata_file _cg_shim_dir _cg_lib_dir \ + _cg_dyno_name _cg_dyno_id _cg_metadata_seen _cg_gated _cg_audited \ + _cg_user_check _cg_reason_check _cg_missing _cg_missing_desc _cg_argv \ + _cg_arg _cg_i _cg_tokens _cg_bin _cg_cmd_read _CG_DYNO_CMD _CG_USAGE \ + _CG_REPORT_VERSION _CG_REPORT_URL_VAR _CG_REPORT_EVENT \ + _CG_REPORT_CONNECT_TIMEOUT _CG_REPORT_MAX_TIME _CG_REPORT_CMD_MAX diff --git a/test/lib/harness.sh b/test/lib/harness.sh index a42b478..6a9cbc3 100644 --- a/test/lib/harness.sh +++ b/test/lib/harness.sh @@ -80,6 +80,29 @@ EOF chmod 755 "$base/fakebin/$fake" done + # A fake `curl`, so a test can see the denial record the guard POSTs without a + # network. Records the request body and the URL it was given. Answers 202 -- + # the real endpoint's success code -- unless the URL says to fail, which is how + # the "a failed report never leaks the credential" test is arranged. + CG_CURL_LOG="$base/curl.log" + : > "$CG_CURL_LOG" + cat > "$base/fakebin/curl" <> "$CG_CURL_LOG" ;; + esac + _prev="\$_a" +done +printf 'ARGV %s\n' "\$*" >> "$CG_CURL_LOG" +case "\$*" in + *fail-me*) exit 7 ;; +esac +printf '202' +EOF + chmod 755 "$base/fakebin/curl" + # Stand in for Heroku's own .profile. cat > "$CG_APP/.profile" < -- as cg_run, with the endpoint configured and the +# request log cleared first. +cg_run_reporting() { + : > "$CG_CURL_LOG" + # shellcheck disable=SC2086 # deliberate word splitting: keep any cg_env values + cg_env "CONSOLE_LOGGING_DATADOG_PROXY_URL=$CG_REPORT_URL" $CG_CURRENT_ENV + cg_run "$1" +} + +# assert_reported