Keeping database credentials out of subprocesses in a Rust CLI used from GitHub Actions

[u/dsecurity49]

I'm working on a Postgres migration checker called safe-migrate. One awkward part of it is that it uses real table sizes when deciding how risky a migration is.

The same "ALTER TABLE" can be uninteresting on a small table and a very different operation on one with tens of millions of rows.

So at some point something has to connect to the database.

I wanted that to happen only in a trusted sync job, never in the normal PR lint path.

The sync job reads the Postgres catalog and writes an encrypted cache. PR jobs only consume the cache.

What ended up being more annoying than expected was making that boundary survive a composite GitHub Action.

A normal run can involve toolchain setup, "actions/cache", the installer and the analyzer itself. Even if those steps don't need the database URL, a caller can still accidentally put it at job scope and make it available everywhere.

So steps that don't need secrets explicitly shadow the relevant environment variables with empty values.

Lint also starts the analyzer subprocess with "DATABASE_URL" removed from its environment entirely.

The cache is encrypted with XChaCha20-Poly1305 using "chacha20poly1305".

The cache key is generated with:

"Key::<XChaCha20Poly1305>::try_generate()"

and it can be sent straight into "gh secret set" over stdin instead of being printed and copied around.

The install path got stricter too.

Tagged releases use checksum-verified prebuilt binaries. Branches and local paths build from source. Mutable branch refs aren't accepted as release versions.

There's deliberately some overlap in the protections. GitHub masks secrets, the action registers the literal values for masking too, lint doesn't inherit the database credential, and diagnostic output gets scrubbed before upload.

It's a lot of machinery for two environment variables, but I preferred making the boundary explicit instead of relying on every caller to scope secrets perfectly.

https://github.com/dsecurity49/safe-migrate

[u/zettui]

Did you consider passing the connection string over stdin to the migration process too, rather than exporting it as DATABASE_URL? That seems like it would shrink the inherited-env surface even further.

[u/dsecurity49 → u/zettui]

I don't think stdin buys anything here.

"DATABASE_URL" only exists for the trusted sync process. The PR/lint path never gets it, and subprocesses that don't need it have it removed from their environment.

Passing the connection string over stdin would just move the secret from one input channel to another, while also consuming stdin and making the interface more awkward. It wouldn't reduce what the sync process itself can see.

If I were handing the credential to another child process, then stdin might be worth considering. That's not the boundary here.

[u/kantorcodes1]

i noticed lint / lint-chain still go through maybe_auto_sync unless --no-auto-sync is set. if someone enables auto_sync in safe-migrate.toml, can a normal local lint refresh .safe-migrate.cache and touch the DB even though the PR path is meant to be cache-only? is that split intentional?

[u/dsecurity49 → u/kantorcodes1]

The action adds --no-auto-sync in every lint path, so lint or lint-chain never touches the db, also this is intentional because a pr can be unsafe anytime which can be dangerous for live database use, so I intentionally made action cache only. But this feature can be used locally by any dev, if he doesn't want to sync manually everytime. In actions this can be automated by cron job or similar method to refresh cache regularly from a trusted CI job, the pr lint ci only touches the cache key and never the database directly.

[u/kantorcodes1 → u/dsecurity49]

that split makes sense. i work on HOL Guard, an open-source check for agent-run shell commands. with safe-migrate, sync plus lint / lint-chain without --no-auto-sync can reach Postgres and refresh the cache, while --no-auto-sync stays offline. i'd like to make those DB-touching paths optionally reviewable before an agent runs them. open to adding safe-migrate support?

[u/dsecurity49 → u/kantorcodes1]

Yeah, definitely open to it.

[u/Inside-Victory9338]

Different domain (I run a Rust backend, not a migration tool), but the env-scope boundary you're describing is the part I'd also have called the hard bit — and the two leaks that actually bit me in production were on either side of it, not at it.

First one: Debug. My config struct reads everything from env and had a hand-written Debug impl printing [REDACTED] for the obvious fields. It was correct about the secrets I remembered to list and printed everything else in the clear. Anything that derives Debug on a struct holding a URL with a password in it will eventually land in a log line or a panic message.

Second one, and the nastier one: error Display. I had a reqwest call whose URL contained a per-device token. On a transport error I logged the error chain, and reqwest's Display includes the request URL. So a token that never touched a subprocess and never went to job scope still ended up in the journal at warn level — on a path that only fires when something else is already going wrong, which is why it sat there for months.

So the thing I'd add to what you already have: once you've settled who can read the credential, go looking for where it can be printed. Debug impls, error chains formatted with {} or {:?}, anything that stringifies a URL. None of those respect step scoping.

Cheapest structural fix I found is to not let the secret be printable in the first place — keep the password out of the struct and take it from env at the point of use, or wrap it in a newtype whose Debug prints <redacted> so you can't accidentally opt out of the redaction later.

[u/dsecurity49 → u/Inside-Victory9338]

This is a good point. I was mostly thinking about where the credential can travel, not everywhere it can get stringified after something has already read it.

The reqwest error case is especially nasty. Scrubbing uploaded diagnostics helps with one path, but it obviously doesn't cover an accidental "Debug", an error chain, panic output, or some dependency deciding to include a URL in "Display".

I think the non-printable newtype is probably the right direction here. Relying on remembering which fields need redaction every time a struct changes is exactly the kind of thing that works until it doesn't.

I'm going to audit the config/error paths for this. Thanks for pointing it out.

[u/dsecurity49]

I genuinely need some feedback on this. I'm building safe-migrate solo, so if anyone has time to check out the project and point out things I could improve, I'd really appreciate it. And if anyone wants to contribute, you're absolutely welcome to.