Bug: fnox sync breaks secrets that use json_path
#368
rpendleton
started this conversation in
General
Replies: 1 comment 1 reply
|
C seems like the best to me |
1 reply
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Problem
When using
fnox syncto cache a secret that usesjson_pathto extract a key from a JSON value, the cached secret fails on subsequent reads. I ran into this with an AWS Secrets Manager secret whose value is a JSON object, but it's easy to reproduce locally with theplainprovider.For example, given the following plain-provider config:
The behavior looks like this:
fnox get USERworks — fetches the JSON, extracts"admin"viajson_pathfnox sync -p age --force— resolves the secret (applyingjson_path), then encrypts the resultfnox get USER— decrypts the cached value ("admin"), then tries to applyjson_pathagain, failing with:Root cause
src/commands/sync.rscallsresolve_secrets_batchwithout strippingjson_pathfrom the secret configs first. As a result,apply_post_processingruns during sync resolution, and the synced value becomes the extracted field ("admin") instead of the full JSON blob. On read-back,json_pathis applied again to that already-extracted string, which is not valid JSON.Compare with
src/commands/reencrypt.rs, which already correctly stripsjson_path(andsyncanddefault) before resolving.Fix options
I've implemented three approaches on separate branches so you can review the trade-offs. I'm happy to open a PR for whichever direction you prefer, but I wanted to align on the shape of the fix before I spent more time writing tests for all three.
Let me know which option you'd prefer (or if you have a different idea) and I'll put up a PR with tests.
Option A: Strip
json_pathinline in sync.rs (diff)The minimal fix. It adds
secret_config.json_path = Nonenext to the existingsecret_config.sync = Nonein the resolve loop, matching the pattern already used byreencrypt.rs.sync.rssyncandreencryptstill need to be kept in sync manually, which is the kind of drift that caused this bugOption B: Add
resolve_secrets_batch_rawwrapper (diff)Adds a
resolve_secrets_batch_rawfunction that wraps the existing resolver but skips all post-processing internally. Threads askip_post_processingflag through the internal functions (resolve_level,resolve_provider_batch,process_batch_results, etc.).Option C: Add
SecretConfig::for_raw_resolve()(diff)Adds a method on
SecretConfigthat returns a copy with all post-processing fields stripped (json_path,sync,default). Bothsync.rsandreencrypt.rsuse it, replacing the manual field-by-field stripping inreencrypt.rs.reencrypt.rsAll reactions