Reproduction steps (widens finding #1's blast-radius, same module/flow):
- Read the REAL
_handle_credential_save(workbench/routes/dispatch_integration.py:105-167): a bootstrap save never suppliescred_id
(_save_to_named_vault in handler.py doesn't pass one), so each call
MINTS a new cred_id and ADDS it to slot["credentials"] — it never
deletes an existing entry. But set_default: true (which
_save_to_named_vault always sends) hits:
if body.get("set_default") or not slot.get("default") or slot.get("default") not in creds:
slot["default"] = cred_id
— an unconditional flip of which cred_id is authoritative.
- Reproduce the exact shape
_handle_credential_savewrites for
credentials.local.json: an operator's pre-existing legitimate
cred_legit_abc entry (default), then the attacker's forged-code
credential landing as a second entry cred_attacker_x7f with
default flipped to it — exactly what finding #1's CSRF produces.
- Call the REAL, unmodified
credential_resolver.resolve(ws, "google-sheets")
— the ONE function every google.sheets_* command reads its credential
from — before and after.
- Run repro_google_sheets_cred_default_flip_persistence_v131.py against a
clean v1.3.1 extraction. Output:
BEFORE: resolve() -> refresh_token '1//LEGIT_OPERATOR_REFRESH_TOKEN'
AFTER: resolve() -> refresh_token '1//ATTACKER_ACCOUNT_REFRESH_TOKEN'
both credentials still stored side by side: True
resolve() now silently returns the ATTACKER's credential: True
legitimate credential entry left untouched (not deleted, just demoted): True
Expected:
Hijacking which identity a governed integration acts as should require
either destroying evidence (which an audit could catch) or should be
reversible/detectable — an operator who "notices something's wrong" should
be able to trust that fixing the default restores exactly the prior state,
and any credential that arrived via an unreviewed race should be
distinguishable from one the operator actually saved.
Actual:
The attack in finding #1 doesn't need to overwrite or destroy anything: it
only needs to win the default flip, which _handle_credential_save
performs unconditionally whenever set_default is true — true on every
bootstrap save, with no comparison against what was previously the default
and no confirmation step. credential_resolver.resolve() has no way to
express "this default changed without operator review" — it just returns
whatever slot["default"] currently points to. Correcting the situation
(operator re-authenticates) mints yet ANOTHER cred_id and flips default
again, but the attacker's credential entry — like the operator's original
one before it — is never removed; it sits in credentials.local.json
indefinitely. Nothing in the save path, the resolve path, or (per this
repro's grep of the module tree) anywhere else in this codebase
distinguishes "a credential entry that arrived via a race" from an ordinary
saved one, and nothing prunes a demoted entry automatically. Practical
severity note (corrects an overstatement risk in finding #1): becausegoogle.sheets_append_row/get_metadata take spreadsheet_id as a
caller-supplied input rather than something derived from the credential,
the most likely real-world outcome of a successful hijack against the
operator's OWN, non-publicly-shared sheet is that subsequent calls start
failing with a Google 403 (the attacker's account has no access to that
sheet) — a silent, hard-to-diagnose breakage of the integration — rather
than automatic exfiltration to an attacker-controlled sheet. The
higher-severity "writes/reads actually succeed under the attacker's
identity" case requires the target spreadsheet to already be broadly
shared (e.g. "anyone with the link can edit"), which is a real but
narrower precondition.
Suggested fix:
In _handle_credential_save, require an explicit prior-default confirmation
(or at minimum: don't let a bootstrap flow silently flip an EXISTING default
— only auto-set default when none was previously set, and surface any
default CHANGE as its own reviewable event, not folded into an ordinary
save receipt). Separately, give the save/resolve layer a way to tag and
prune orphaned/demoted credential entries so a fixed default doesn't leave a
live, indefinitely-stored secret behind with no operator-visible signal of
why it's there.