Strongbox opt-in migration cannot see targets that had it enabled
Nessuno ha ancora preso questa issue.
Valutazione
- Difficoltà
- 4/5
- Tempo stimato
- 3-5 giorni
- Idoneità per principianti
- 50/100
Direzione di ricerca
Parti dalla struct Go Vault e dal suo metodo UnmarshalYAML, quindi segui la gestione delle versioni e il percorso di scrittura della configurazione utilizzato da safe target. Riproduci entrambi gli stati YAML legacy con safe target --json; il lavoro è completato quando gli aggiornamenti preservano i target Strongbox abilitati dal legacy e le scritture successive rendono l'impostazione non ambigua.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
Upgrading to v1.20.0 silently turns Strongbox off for every target that had it on, because the legacy-key migration in cd043be keys off a field that f6509b8 made structurally impossible to write seven years ago.
The commit that broke it
f6509b8c34786964023b73df5fd512ce3c60fe34 — "don't write nostrongbox as strongbox in config", 2019-11-04, first released in v1.4.0.
type Vault struct {
- URL string `yaml:"url"`
- Token string `yaml:"token"`
- CACerts []string `yaml:"ca_certs,omitempty"`
- SkipVerify bool `yaml:"skip_verify"`
- //FIXME: NoStrongbox should not go into the config as "strongbox"
- NoStrongbox bool `yaml:"strongbox"`
+ URL string `yaml:"url"`
+ Token string `yaml:"token"`
+ CACerts []string `yaml:"ca_certs,omitempty"`
+ SkipVerify bool `yaml:"skip_verify,omitempty"`
+ NoStrongbox bool `yaml:"no_strongbox,omitempty"`
Renaming the key was correct — the old one was inverted, as the FIXME said. Adding omitempty is the problem. Strongbox-enabled means NoStrongbox == false, and omitempty drops false, so from v1.4.0 onward safe writes nothing at all for a Strongbox-enabled target. The enabled state has had no on-disk representation for seven years; it was encoded purely as the absence of a key.
That was self-consistent while absence meant "on". It stopped being self-consistent when the default flipped.
What surfaced it
c62a87e— "Make Strongbox opt-in per target" (2026-07-31, first in v1.20.0) changed the field toStrongbox bool yaml:"strongbox,omitempty"andHasStrongbox()to returnv.Strongboxdirectly. Absence now means off.cd043be— "Honor legacy no_strongbox key when present" (2026-08-01, also v1.20.0) added theUnmarshalYAMLshim intended to make that upgrade lossless.
The shim's own comment states the goal:
// UnmarshalYAML translates the legacy no_strongbox key into Strongbox when
// the key is present in the document [...] so an upgrade does not silently
// disarm Strongbox for every pre-existing target
But it is gated on presence:
if legacy.NoStrongbox != nil {
v.Strongbox = !*legacy.NoStrongbox
}
Because of f6509b8, no_strongbox is present only when Strongbox was disabled. So the shim fires exactly for targets that were already off — the state the new default produces anyway — and never for the targets it was written to protect. It is a no-op with respect to its stated purpose.
Reproduction
Two rc files, one per legacy state, read by safe vdev/develop/0f50bec:
# Strongbox was DISABLED under <= v1.10.0
vaults:
t: {url: https://vault.example.com, token: "", no_strongbox: true}
# Strongbox was ENABLED under <= v1.10.0 (the only on-disk form that exists)
vaults:
t: {url: https://vault.example.com, token: ""}
$ safe target --json # legacy-disabled
{ "name": "t", ..., "strongbox": false } # correct
$ safe target --json # legacy-enabled
{ "name": "t", ..., "strongbox": false } # WRONG — was on, now off, no warning
On a real ~/.saferc with 12 targets, none of which had ever been given --no-strongbox, all 12 now report strongbox: false.
Write behaviour, both eras
Each era uses omitempty and so can persist only its own non-default case. They are exact mirrors:
safe target … |
v1.4.0 – v1.10.0 | v1.20.0+ |
|---|---|---|
| (no flag) | writes nothing | writes nothing |
--strongbox |
writes nothing | strongbox: true |
--no-strongbox |
no_strongbox: true |
writes nothing |
Secondary problem: older safe strips the new key
strongbox is unknown to the v1.10.0-and-earlier struct, and gopkg.in/yaml.v2 is non-strict, so it is dropped on read and omitted on write. Any rc write by an older binary — including targeting an unrelated alias — silently deletes it:
$ safe-dev target --strongbox https://vault.example.com A # strongbox: true
$ safe-1.9.0 target https://vault.example.com B # unrelated alias
$ grep -A3 ' A:' ~/.saferc # strongbox: true is gone
$ safe-dev target --json # "strongbox": false
So on any machine where both versions are reachable, --strongbox cannot be made to stick. This makes "just re-run safe target --strongbox" an unreliable workaround as well as a manual one.
Downstream impact
Genesis reads strongbox from safe targets --json and compares it against the value recorded in a deployment repository's .genesis/config when it was first configured. Repos set up under <= v1.10.0 recorded true; the same target now reports false, and Genesis refuses to resolve the vault at all:
[FATAL] Could not find matching safe target, but the following are similar:
Alias: 'ops'
Strongbox: '0' (expected '1')
Ten of eleven deployment repos pointing at one vault became unusable on upgrade. (Genesis is over-strict here — a seal-status sidecar should not gate vault resolution — and that is being fixed separately. But the trigger is the flip.)
Suggested fix
The shim cannot work as written, because on-disk absence is ambiguous between "pre-flip default: on" and "post-flip default: off" and nothing in the file distinguishes them. The rc already carries the discriminator:
version: 1
current: ...
Proposal: on reading version: 1, treat an absent key as on (the pre-flip default) and honour no_strongbox when present; then write version: 2 with strongbox always explicit — no omitempty — so the state is unambiguous from then on. A version: 2 file is read literally.
That also fixes the stripping problem for anyone who upgrades once and stays upgraded, since the key is then always present.
Whatever the mechanism, it seems worth (a) not changing a security-relevant default silently on upgrade, and (b) emitting a one-time notice when a target's effective Strongbox setting changes as a result of the migration.
Versions
- Broken by:
f6509b8(v1.4.0, 2019-11-04) — made the enabled state unrepresentable - Surfaced by:
c62a87e+cd043be(both v1.20.0, 2026-07-31/08-01) - Observed on:
safe vdev/develop/0f50bec, compared against homebrewsafe v1.9.0 safe targets --jsonfirst reportedstrongboxin4c68409(v1.5.8)
- Lingua principale
- Go
- Stelle
- 5
- Fork
- 2
- Metriche di merge delle PR
- Nessuna PR unita negli ultimi 30g
Preparare l'ambiente
Questo progetto non fornisce container di sviluppo, Dockerfile né guida per i contributori, quindi l'ambiente è a tuo carico: parti dal suo README e consulta la nostra guida al primo contributo per i passaggi generali.
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Issue simili
-
bug frontend grammar sev-3
Difficoltà 2/5 1-3 ore Idoneità per principianti 63/100
-
connector docs good first issue
Difficoltà 2/5 1-3 ore Idoneità per principianti 72/100
soma-intelligence/open-health-sync#134 ·
I maintainer di solito rispondono entro 1 giorno
-
bug frontend
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100
I maintainer di solito rispondono entro 3 giorni
-
Difficoltà 1/5 Meno di un'ora Idoneità per principianti 72/100
I maintainer di solito rispondono entro 1 giorno
-
oss/0 pending team/container-integrations
Difficoltà 2/5 1-3 ore Idoneità per principianti 68/100
DataDog/datadog-agent#58323 ·
I maintainer di solito rispondono entro 1 giorno