Skip to content

Record a denial, instead of only printing one - #4

Closed
grantcox wants to merge 6 commits into
fix/rake-arguments-allowlistfrom
fix/console-guard-denial-record
Closed

Record a denial, instead of only printing one#4
grantcox wants to merge 6 commits into
fix/rake-arguments-allowlistfrom
fix/console-guard-denial-record

Conversation

@grantcox

@grantcox grantcox commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

The gap

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 zero.

So the only durable trace of a blocked command was Heroku's own api:dyno record, which shows that something was attempted but cannot distinguish a guard denial from an application error. The audit cross-check therefore had to read "no console record for this dyno" as "blocked, or lost" — which is not an audit trail, and is the one question an auditor is most likely to ask.

What this does

Each denial POSTs one record to the same endpoint and credential as the console_audit gem (CONSOLE_LOGGING_DATADOG_PROXY_URL), distinguished by event: "command_denied":

{"event":"command_denied","enforced":true,"rule":"compound_statement",
 "command":"psql \"a\\\"b\\\\c\"","operator":"becky","reason":"testing",
 "dyno_id":"dyno-uuid-1","app":"my-app","service":"my-service",
 "guard_version":"7f1e0d8","session_id":"1a7e…","timestamp":""}

The cross-check then reads a missing console record as lost, full stop. That is the disambiguation it was missing, and it comes from adding a record rather than from teaching the query to model the buildpack's policy — which would drift out of sync with the buildpack.

Reusing the gem's endpoint means the join keys the proxy already derives (dyno_id, and console_identity from operator) apply unchanged, and there is one credential to issue and rotate, one thing for an app to set, and one Datadog source to route to an index.

Both halves report. Roughly half the guard's denials happen in the command wrapper rather than the profile script, and a trail carrying only the profile script's denials would show every argument-policy block as a clean session.

Attribution: app and service, not env

The gem sends service/env/app/version but stamps them on its worker, to ensure heroku run -e cannot rewrite them. This runs inside the one-off dyno, where that defence is not available.

It sends app and service regardless, because for these logs the tampering argument does not transfer. An operator who wants their denial record gone can unset the endpoint and delete it outright, so forging tags is weaker than what they can already do. And our monitor is stricter on dyno webhooks without matching denials, so a missing or malformed denial causes more visibility rather than less.

Fail-open, and not sufficient on its own

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. curl's stderr is discarded because curl reports failures by quoting the URL, which carries the Basic credential — a status code is reported instead.

The URL variable is inherited by the one-off dyno, so an operator who knows about this can suppress their own denial record with -e CONSOLE_LOGGING_DATADOG_PROXY_URL=. That is inherent to anything POSTing from inside the dyno — a separate variable would not help — and it is why the gem put delivery on the worker. Suppression still leaves the api:dyno webhook and the exit status. Documented under Limitations as "evidence of what was blocked, not proof that nothing was".

Notes for review

  • rule is a short stable identifier for the check that refused (15 of them). Group monitors on it rather than on the denial text, which gets reworded.
  • enforced is false in permit mode, and records are sent in both modes: phase 1 exists to measure what enforcement would block, which is only measurable if the would-be denials are recorded.
  • dyno_id comes from the dyno metadata file via a newly exported CONSOLE_GUARD_DYNO_ID, not from HEROKU_DYNO_ID, which -e can set to anything.
  • The command extraction moved above the identity gate, so every record carries the command — including the identity denial, which is the one CI hits. Only the extraction moved; the two "cannot read the argv" refusals stayed where they were, so denial precedence is unchanged.
  • Every string in the record is escaped to pure ASCII, with any byte >= 0x80 replaced by U+FFFD. command and reason are operator-controlled bytes, and a JSON string has to be valid UTF-8, so one stray byte would otherwise cost the whole record — rule, operator and dyno_id with it — and cost it invisibly, since the only warning goes to the terminal of the operator who was just blocked. The price is fidelity: puts 'héllo' records a replacement character where the é was.

Verification

200 assertions, 0 failed on heroku/heroku:24; shellcheck clean.

New coverage: a record is POSTed from both halves; rule / command / operator / reason / trusted dyno_id / enforced are present; app and service are sent, and service is omitted when the app sets no DD_SERVICE; env and version are never sent even when set on the dyno; a permitted command posts nothing; no URL configured means no POST; the --exit-code marker is not in the record; the body survives a real JSON parser given a command containing a quote, a backslash, a tab and a newline; a failed report warns without leaking the credential; permit mode records enforced:false and still permits.

@grantcox
grantcox force-pushed the fix/console-guard-denial-record branch from 7a3de13 to 6db9f74 Compare September 1, 2026 00:03
Comment thread profile/console_guard.sh
fi
done

# ---------- extract the dyno command ----------

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two just moved up, so we can include the command in the recorded denial if there is one.

@grantcox
grantcox marked this pull request as ready for review September 1, 2026 06:03
@grantcox
grantcox requested a review from becky-ynab September 1, 2026 06:04
Base automatically changed from fix/console-guard-hardening to main September 2, 2026 02:58
@grantcox
grantcox force-pushed the fix/console-guard-denial-record branch from dc772e4 to 8a22a41 Compare September 2, 2026 02:58
grantcox and others added 6 commits September 2, 2026 13:04
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>
@grantcox
grantcox force-pushed the fix/console-guard-denial-record branch from 8a22a41 to 28d2961 Compare September 2, 2026 03:16
@grantcox
grantcox changed the base branch from main to fix/rake-arguments-allowlist September 2, 2026 03:16
@grantcox
grantcox marked this pull request as draft September 2, 2026 19:43
@grantcox
grantcox removed the request for review from becky-ynab September 2, 2026 19:43
@grantcox

grantcox commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

Actually I'm going to work on a completely different direction - moving all this buildpack logic into Ruby instead of shell scripts. So let's not bother with these PRs right now, as they'll likely be replaced entirely soon.

@grantcox

grantcox commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author

Closing in favor of #8

@grantcox grantcox closed this Sep 4, 2026
@grantcox
grantcox deleted the fix/console-guard-denial-record branch September 4, 2026 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant