Move buildpack logic to Ruby, out of Bash - #8
Conversation
`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) <noreply@anthropic.com>
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 <noreply@anthropic.com>
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
Sending no attribution at all was over-cautious and had a concrete cost: a denial record arrived with no `@app`, and every cross-check query scopes on `@app` to reach both log sources at once. The records existed and nothing read them. The tampering argument that justified omitting it does not transfer from the gem. The gem stamps attribution on its worker because tampering is the only attack available there; in a one-off dyno an operator can unset the endpoint and delete the whole record, so forging the app name is strictly weaker than what they can already do. Attribution tampering needs closing where suppression is impossible, which is not this position. service/env/version stay unsent -- nothing needs them for scoping, and datadog-proxy derives the service from the credential, which cannot be forged.
datadog-proxy no longer derives a record's service from the credential label (ynab/datadog-proxy#100), and the comment here saying it does was the last thing keeping `service` off this record. The reason to send it now is parity, not richness. The gem stamps `service` from DD_SERVICE, and the proxy forwards it as `@app_service`. Leaving it off here made that attribute mean "this is a session record" rather than "the app's DD_SERVICE" -- a query filtering on it would return the sessions and silently drop the denials beside them, which is the same shape of gap `app` was added to close. Sent it means absent for one reason only: the app sets no DD_SERVICE, exactly as a gem record would be. Never absent because of which half of the audit trail produced the record. Sent under `service`, not the `app_service` it lands in Datadog as. The proxy does that rename, because Datadog's JSON preprocessing would otherwise promote a `service` key onto the reserved facet. One sender contract beats two -- and it means the proxy needs that rename deployed before this does. `env` stays unsent, and the asymmetry is the point: it is a reserved facet that scopes monitors, so forging it is the one case where tampering buys something suppression does not. The proxy infers it from the delivery topology, which nothing in the dyno can reach. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The two scripts had grown to ~1,150 lines of bash carrying the whole policy, and the parts that were hardest to get right were the parts bash is worst at: hand-rolled JSON escaping with a loop that needed a termination proof in a comment, an option allowlist that depended on deliberate word splitting, and array expansions written around the empty case. All of it is now Ruby under guard/lib/console_guard/, and behaviour is unchanged -- the end-to-end suite passes as it stood, on every supported stack image, from Ruby 3.0 to 3.3. What stays in bash is what only the login shell can do, because a child process cannot reach into its parent: read /proc/$$/cmdline, exit, and modify the shell's own environment. The gate reports back through its exit status, which is the only channel available for that. Running policy in Ruby adds one surface bash did not have: `heroku run -e` can choose what the interpreter loads before the guard's first line. All three vectors are closed -- an absolute interpreter path resolved at build time rather than a PATH lookup, `--disable=gems,rubyopt`, and RUBYLIB dropped from $LOAD_PATH before the first require. The denial reporter no longer shells out to curl, so the test harness records denials with a loopback HTTP recorder rather than a fake curl on PATH. CONSOLE_GUARD_DYNO_ID is gone with it: the wrapper reads the dyno metadata file itself now, which is what that variable existed to carry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The three vectors a Ruby guard has and a bash one does not, each with a
case that fails if the mitigation is removed:
PATH a hostile `ruby` earlier on PATH that exits 0. The gate reads
exit 0 as "not a dyno this applies to", so this would be a
complete bypass rather than a crash.
RUBYOPT a `-r` of a file that does not exist, which would abort the
interpreter if it were honoured.
RUBYLIB a directory holding a json.rb, which would both announce
itself and leave JSON undefined.
Each is checked against both halves, because the wrapper is a separate
process with its own interpreter to choose.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Discovered rather than listed, matching the shellcheck job, so adding a policy file cannot silently drop it from the lint. Parsed under `--disable=gems`, which is how the guard runs, so a file that reaches for a gem fails here rather than in a dyno. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds a "Where the code lives" section: which file is which, the exit status contract between the gate and the login shell, and the table of interpreter vectors with what closes each. Records CONSOLE_GUARD_RUBY as a build-time config var and drops CONSOLE_GUARD_DYNO_ID, which no longer exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The end-to-end suite was doing two jobs. Most of it was policy -- which commands, arguments and options are refused, and what each denial records -- expressed as shell payloads run through a simulated dyno, which is an expensive way to ask a question that has nothing to do with the shell. The rest was the chain itself, which nothing but a login shell can answer. Policy moves to test/run_ruby_tests.rb: 90 cases, 693 assertions, and it runs on a laptop in about ten seconds because it needs neither procfs nor a stack image. Both entry points are still driven as subprocesses rather than called in process -- each half refuses by calling `exit` and the wrapper ends in `exec`, so an in-process seam would exist only for the tests, and a seam is where a bypass hides. The slug under test is compiled by bin/compile, so these are the rendered files a dyno gets. test/run_tests.sh keeps 84 assertions, all of them about something the shell does: .profile.d being sourced, the login shell acting on the gate's exit status, the shell expanding the command before the wrapper sees it, a permitted command reaching the real binary, the interpreter hardening, and bin/compile's output. Running the policy suite in the stack images caught one thing worth naming: a dyno has no LANG, so Encoding.default_external is US-ASCII there and reading the guard's output with File.read hands back an invalidly-tagged string as soon as the exit-status sentinel appears. The tests compare bytes now. The guard itself was already correct -- it writes banner lines rather than interpolating them -- which is why the end-to-end suite never saw it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three pieces of belt-and-braces removed, and one bug they were hiding. The policy files are listed rather than discovered. `render` already refuses a source file that is not there, so a rename now fails the build instead of producing a slug with half a guard -- which is a better failure than the `find` was buying. CONSOLE_GUARD_RUBY is gone, both as a build-time override and as a runtime variable. The wrapper used to read it from the environment, which was only safe because the profile script overwrote it on every gated dyno; not reading the environment at all is one less thing to have got right. The profile script no longer re-searches for an interpreter either. bin/compile resolves one, or the build fails. Resolution itself is now `command -v ruby` and nothing else. heroku/ruby exports its PATH for the buildpacks that follow it, so that finds the app's own interpreter without a list of guessed locations -- none of which I could confirm against the platform. The bug: `command -v ruby` returns a path under BUILD_DIR when the interpreter is vendored into the slug, and the old code re-rooted to /app only for its hardcoded candidates. Anything reaching the `command -v` fallback got the build path baked in, which does not exist in the dyno -- and a missing interpreter is a refusal, so the next deploy would have taken out `heroku run` rather than failing the build. Re-rooting now applies to whatever is resolved, with a test that pins it using a layout none of the old candidates would have matched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The profile script claimed to read /proc/$$/cmdline, which it does not -- it names the file and the gate reads it. What is irreducibly the login shell's job is expanding `$$`, because this shell's argv is the dyno command and a child looking up its own PID finds the gate's argv instead. Says that instead. The status comments used "gated" and "audited" as though they were defined somewhere, and they were not. They are now, once each in the file that owns the question: ConsoleGuard::Gate for the exit-status contract, ConsoleGuard::Dyno for which families land where, and a short version in the profile script so it reads on its own. Status 10 needed more than a definition. "Audited but not gated" was three clauses that each stated a fact without saying what followed from it. The two reasons spelled out: gating asks who you are and why, and nobody is there to answer -- requiring CONSOLE_USER would refuse every scheduled job rather than protect anything; and the command is not an operator's to choose, being whatever the Scheduler entry or Procfile release line says, which is a deploy-time access path with its own controls. The record is what is left worth having, because a rake task run by Scheduler reaches the same data a console does. Same paragraph reworked in the README, where it read the same way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
6b18fb9 to
85b8b28
Compare
|
@becky-ynab this does have more commits than necessary, as it started from the shell-script branches that blocked sandbox mode, and added the denial logs. But the advantage of that is that those features also had shell-script test cases, and the first commit converting it all to Ruby kept that shell-script test suite. So it asserted parity nicely. Now that it's done, we could just squash the commits down to remove the shell-script versions from the history. But I also don't think it really matters! |
becky-ynab
left a comment
There was a problem hiding this comment.
This is great. So much more legible. I've left some suggestions but none are blocking.
| require "json" | ||
| require "net/http" | ||
| require "uri" | ||
|
|
There was a problem hiding this comment.
suggestion: Claude noticed that ruby 3.4.5 will fail with the --disable-gems option.
net/http loads resolv 0.6.x, which references RbConfig without requiring it. RubyGems is what normally loads rbconfig, and it is disabled. Every gate and wrapper invocation dies with a NameError, exits 1, and the shell treats that as a denial. Every heroku run on a Ruby 3.4 app would be refused. Reproduced on 3.4.5 with the compiled gate. Ruby 4.0.2 and 4.0.5 are fine (resolv 0.7.0), and CI never exercised 3.4. The fix is one line in guard/lib/console_guard.rb:
require "rbconfig" # before require "net/http"
There was a problem hiding this comment.
I don't think we have any repos where we'd install the buildpack that run lower than 4.0 so it's not a big deal for us, but nice to have in a public repo.
| end | ||
|
|
||
| def file?(path) | ||
| File.file?(path) |
There was a problem hiding this comment.
suggestion: Use File.exist? instead
And add a spec to show we don't only read files from disk, not stdin or other character devices.
def test_a_device_or_fd_path_is_refused_too
# Rails checks File.exist? and then Kernel.load, and loading /dev/stdin
# reads the program from stdin -- the same gap the bare `-` rule closes.
assert_denied wrapper("rails", "runner", "/dev/stdin"), "exists on disk"
assert_denied wrapper("rails", "runner", "/dev/fd/0"), "exists on disk"
endThere was a problem hiding this comment.
Ooh, that's a good one!
| "Use `rails #{subcommand}` instead. It is audited.", | ||
| "`--no-sandbox` is permitted and means the same thing." | ||
| end | ||
| end |
There was a problem hiding this comment.
suggestion: Block compound options like rails c -es
That command lets the sandbox option (s) get through to the buildpack. It's still blocked by console1984, but we don't get the warning banner from the buildpack.
➜ datadog-proxy git:(main) ✗ heroku run -a datadog-proxy-staging -e "CONSOLE_USER=$(heroku whoami);CONSOLE_REASON=test" -- rails c -es
Running rails c -es on ⬢ datadog-proxy-staging... up, run.4134
2026-09-03 23:18:16 UTC | TRACE | ERROR | (pkg/trace/api/api.go:407 in Start) | Could not start UDS listener: socket directory does not exist: /var/run/datadog/apm.socket
2026-09-03 23:18:18 UTC | CORE | ERROR | (comp/dogstatsd/server/impl/server.go:418 in start) | Can't init UDS listener on path /var/run/datadog/dsd.socket: can't listen: listen unixgram
/var/run/datadog/dsd.socket: bind: no such file or directory
config.eager_load is set to nil. Please update your config/environments/*.rb files accordingly:
- development - set it to false
- test - set it to false (unless you use a tool that preloads your test environment)
- production - set it to true
W, [2026-09-03T23:18:19.120689 #2] WARN -- datadog: [datadog] Failed to convert tag: tag 'version:' ends with a colon
Error: Unable to start console in sandbox mode as sandbox mode is disabled (config.disable_sandbox is true).
There was a problem hiding this comment.
Fixed in 5a3b5f2 . A bundled s param (eg in -es) now gets a specific "sandbox not permitted" error, and bundled params are also generally rejected.
| # 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. | ||
| exit DENIED |
There was a problem hiding this comment.
suggestion: Use deny for consistency and so there aren't paths where before_exit gets skipped.
There was a problem hiding this comment.
Fixed in 9a66499 , now every Ruby exit goes through deny
On Ruby 3.4 net/http loads resolv 0.6.x, which references RbConfig without requiring it. RubyGems is what normally loads rbconfig and `--disable=gems` turns it off, so every gate and wrapper invocation died with a NameError and the shell read that as a denial -- refusing every `heroku run` on the app. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It hand-rolled the banner, the record and the exit, which meant it skipped before_exit -- so `heroku run --exit-code` on a spoofed dyno printed no exit-status marker and the CLI reported success for a command it never ran. deny grows `fatal:`, for a refusal that stands in dry-run mode too, and `command:`/`dyno_id:` overrides for a refusal about something other than the command this half judged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`File.exist?` is the test Rails itself makes before handing the path to Kernel.load, so `rails runner /dev/stdin` read the program from stdin -- the gap the bare `-` rule closes -- while File.file? said no. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Thor splits a run of short flags into one option per letter, so `rails c -es` is `-e -s`: the sandbox flag reached the console behind an allowlisted `-e`, and the operator got console1984's error instead of the guard's banner. The sandbox rule now sees into a bundle, and the option allowlist no longer reads an attached short value for the two commands Thor parses -- it has no such thing. Rake's parser does, so `-Tdb` and `-j4` are unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The stack images' distribution Ruby is 3.0, 3.2 and 3.3 today, which covered those three by accident rather than by choice -- and missed 3.4, the one version whose stdlib needed rbconfig required before net/http. It also missed 4.0, which is what a current app runs. The guard uses the app's interpreter, not the stack's, so the versions are now named: 3.0 (the heroku-22 floor, and .rubocop.yml's target) through 4.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
d0cc4d0 to
882d0e4
Compare
Rewrites the guard's policy in Ruby. The shell was ~2,100 lines that nobody here wants to own; what's left of it is only the parts that genuinely need a shell.
This also carries the whole of
fix/console-guard-denial-record— the argument allowlist and the denial record posted to datadog-proxy — so that branch is subsumed by this one and does not need merging separately.What stayed in shell, and why
Three things can't move: reading the login shell's own
/proc/$$/cmdline,exiting that shell, and mutating its environment (PATH,EDITOR/VISUAL,CONSOLE_AUDIT_ENABLED,CONSOLE_USER). Soprofile/console_guard.shis now a ~130-line stub, mostly comments, andguard/shim.shis a few lines. Everything they do is invoke Ruby and act on its exit status.Policy in the Rails app environment was considered and rejected: it puts Rails boot on the
heroku runhot path, loads operator-influenced code before the gate decides, and a DB-backed ActiveJob enqueue for the denial record is exactly what thesandbox_consolerule exists to distrust.Where the code lives
guard/lib/console_guard/gate.rb— pre-expansion policy (the string the operator typed)guard/lib/console_guard/command.rb— post-expansion policy (the argv the wrapper receives), thenexecs the realrails/rake/bundleguard/lib/console_guard/{dyno,reporter,banner,denials,config}.rbguard/libexec/run_{gate,command}.rb— the two entry pointsInterpreter hardening
An operator controls
PATHand the environment viaheroku run -e, so the guard is never allowed to pick its own interpreter.bin/compileresolves Ruby once at build time and bakes the absolute path; every invocation is<ruby> --disable=gems,rubyopt;RUBYLIBentries are removed from$LOAD_PATHbefore the firstrequire. Each of those has a test that fails if the mitigation is removed.bin/compilealso had a latent bug worth calling out:command -v rubyreturns a$BUILD_DIR/...path when Ruby is vendored, and the old resolver only re-rooted its own hardcoded candidates. That would have baked a path that doesn't exist in the dyno — and a missing interpreter is a refusal, so it would have taken outheroku runon the next deploy rather than failing the build. Fixed, with a regression test.Tests
Two suites, both run in CI against heroku-22/24/26:
ruby test/run_ruby_tests.rb— 91 runs, 698 assertions. Policy, driven as subprocesses with a fabricated login-shell argv, so it runs on macOS too../test/run_tests.sh— 89 assertions. Only what needs a real login shell:.profile.dsourcing, shell expansion,PATHprepending, acting on the gate's exit status.The pre-existing 289-assertion behavioural suite was carried over near-unchanged first, to prove the port faithful, and then split.
Plus, I have run through this very extensive test plan, and confirmed all the results are as expected, with:
console-guard-acceptance-test-plan.html

Also
standard's ruleset driven through rubocop, matching Evergreen.TargetRubyVersion: 3.0rather than Evergreen's 4.0, since heroku-22 is the floor the guard has to run on. Most of that commit's diff is single→double quotes.