fix(vault): bind manager logins to every saved web origin
1Password/Bitwarden items can carry several websites, but both backends collapsed the item's urls[]/uris[] to the first origin that normalizes, so browser_vault_fill refused every other explicitly saved origin with origin_mismatch. Reordering the URLs in the manager just moved which single origin worked. VaultItemMeta now carries allowed_origins (every normalized, deduped web origin; origin stays the first/primary one). Fill matching stays exact-origin against that list — no wildcard, parent-domain or subdomain inference — and the in-page synchronous check pins the origin actually matched via build_fill_js(expected_origin=page_origin). App URIs such as androidapp:// never widen the fill set.
This commit is contained in:
@@ -94,20 +94,25 @@ class BitwardenLoginBackend(LoginBackend):
|
||||
if item.get("type") != 1 or not isinstance(item.get("login"), dict):
|
||||
continue
|
||||
login = item["login"]
|
||||
origin = None
|
||||
origins: List[str] = []
|
||||
for uri in login.get("uris") or []:
|
||||
try:
|
||||
origin = normalize_origin(str(uri.get("uri") or ""))
|
||||
break
|
||||
except Exception:
|
||||
continue
|
||||
if not origin:
|
||||
if origin and origin not in origins:
|
||||
origins.append(origin)
|
||||
if not origins:
|
||||
continue
|
||||
username = str(login.get("username") or "").strip() or None
|
||||
# Fill targets are browser pages, so app URIs (androidapp:// etc.) never widen
|
||||
# the fill set; an app-URI-only item keeps its single origin exactly as before.
|
||||
web_origins = tuple(o for o in origins if o.startswith(("http://", "https://"))) or (origins[0],)
|
||||
out.append(VaultItemMeta(
|
||||
id=f"{self.prefix}{item.get('id')}", kind="login", label=str(item.get("name") or origin),
|
||||
origin=origin, created_at=str(item.get("creationDate") or ""),
|
||||
identifier_type="username" if username else None, identifier=username))
|
||||
id=f"{self.prefix}{item.get('id')}", kind="login", label=str(item.get("name") or origins[0]),
|
||||
origin=origins[0], created_at=str(item.get("creationDate") or ""),
|
||||
identifier_type="username" if username else None, identifier=username,
|
||||
allowed_origins=web_origins))
|
||||
return out
|
||||
|
||||
def get_meta(self, handle: str) -> Optional[VaultItemMeta]:
|
||||
|
||||
@@ -105,14 +105,15 @@ class OnePasswordLoginBackend(LoginBackend):
|
||||
out: List[VaultItemMeta] = []
|
||||
for item in raw if isinstance(raw, list) else []:
|
||||
urls = [str(u["href"]) for u in item.get("urls") or [] if isinstance(u, dict) and u.get("href")]
|
||||
origin = _first_origin(urls)
|
||||
if not origin:
|
||||
origins = _all_origins(urls)
|
||||
if not origins:
|
||||
continue
|
||||
username = str(item.get("additional_information") or "").strip() or None
|
||||
out.append(VaultItemMeta(
|
||||
id=f"{self.prefix}{item.get('id')}", kind="login", label=str(item.get("title") or origin),
|
||||
origin=origin, created_at=str(item.get("created_at") or ""),
|
||||
identifier_type="username" if username else None, identifier=username))
|
||||
id=f"{self.prefix}{item.get('id')}", kind="login", label=str(item.get("title") or origins[0]),
|
||||
origin=origins[0], created_at=str(item.get("created_at") or ""),
|
||||
identifier_type="username" if username else None, identifier=username,
|
||||
allowed_origins=_web_origins(origins)))
|
||||
return out
|
||||
|
||||
def get_meta(self, handle: str) -> Optional[VaultItemMeta]:
|
||||
@@ -131,10 +132,26 @@ class OnePasswordLoginBackend(LoginBackend):
|
||||
return code if code.isdigit() else None
|
||||
|
||||
|
||||
def _first_origin(urls: List[str]) -> Optional[str]:
|
||||
def _web_origins(origins: List[str]) -> tuple:
|
||||
"""Fill targets are browser pages, so app URIs (``androidapp://`` etc.) never
|
||||
widen the fill set; an item whose only URI is an app URI keeps its single
|
||||
(unfillable-from-a-page) origin exactly as before."""
|
||||
web = tuple(o for o in origins if o.startswith(("http://", "https://")))
|
||||
return web or (origins[0],)
|
||||
|
||||
|
||||
def _all_origins(urls: List[str]) -> List[str]:
|
||||
"""Every normalized origin saved on the item, deduped, order preserved.
|
||||
|
||||
A 1Password Login item can carry several websites; each of them is a place the
|
||||
user told 1Password the credential belongs, so all of them are valid fill targets.
|
||||
"""
|
||||
out: List[str] = []
|
||||
for u in urls:
|
||||
try:
|
||||
return normalize_origin(u)
|
||||
origin = normalize_origin(u)
|
||||
except Exception:
|
||||
continue
|
||||
return None
|
||||
if origin not in out:
|
||||
out.append(origin)
|
||||
return out
|
||||
|
||||
@@ -167,6 +167,10 @@ class VaultItemMeta:
|
||||
identifier_type: Optional[str] = None
|
||||
identifier: Optional[str] = None
|
||||
has_otp: bool = False # a TOTP seed is stored: 2FA codes can be minted without asking the user
|
||||
# Every origin the password manager bound to this item (manager backends only;
|
||||
# ``origin`` is the first/primary one). Fill matching stays exact-origin against
|
||||
# this list — no wildcard or subdomain inference is ever derived from it.
|
||||
allowed_origins: tuple = ()
|
||||
|
||||
def to_dict(self) -> Dict[str, Any]:
|
||||
out = {
|
||||
@@ -181,6 +185,8 @@ class VaultItemMeta:
|
||||
out["identifier_type"] = self.identifier_type
|
||||
if self.has_otp:
|
||||
out["has_otp"] = True
|
||||
if len(self.allowed_origins) > 1:
|
||||
out["allowed_origins"] = list(self.allowed_origins)
|
||||
return out
|
||||
|
||||
|
||||
|
||||
@@ -168,3 +168,55 @@ def test_lock_during_unlock_wins_and_only_the_owning_session_release_drops_a_tok
|
||||
unlock_mod.release_session("sess-A")
|
||||
assert not backend.is_unlocked()
|
||||
unlock_mod.set_current_session_id(None)
|
||||
|
||||
|
||||
def test_bitwarden_multi_uri_item_binds_every_saved_web_origin():
|
||||
"""A Bitwarden login with several URIs binds all of them (deduped, first stays
|
||||
primary); non-web URIs never widen the fill set."""
|
||||
backend = BitwardenLoginBackend({"enabled": True})
|
||||
items_json = json.dumps([{
|
||||
"id": "multi", "type": 1, "name": "Amazon", "creationDate": "2026-01-01T00:00:00Z",
|
||||
"login": {"username": "jane@example.com", "uris": [
|
||||
{"uri": "https://amazon.co.uk/signin"},
|
||||
{"uri": "https://www.amazon.co.uk"},
|
||||
{"uri": "https://eu.account.amazon.com"},
|
||||
{"uri": "androidapp://com.amazon.shopping"},
|
||||
{"uri": "not a url"},
|
||||
]},
|
||||
}])
|
||||
with patch.object(BitwardenLoginBackend, "is_unlocked", return_value=True), \
|
||||
patch.object(backend, "_run", return_value=items_json):
|
||||
metas = backend.list_items()
|
||||
assert len(metas) == 1
|
||||
assert metas[0].origin == "https://amazon.co.uk"
|
||||
assert list(metas[0].allowed_origins) == ["https://amazon.co.uk", "https://www.amazon.co.uk",
|
||||
"https://eu.account.amazon.com"]
|
||||
|
||||
|
||||
def test_onepassword_multi_url_item_binds_every_saved_web_origin():
|
||||
"""A 1Password login with several websites binds all of them; the app URI is kept
|
||||
out of the fill set and a single-URL item is unchanged."""
|
||||
from agent.vault_backends.onepassword import OnePasswordLoginBackend, _all_origins, _web_origins
|
||||
|
||||
backend = OnePasswordLoginBackend({"enabled": True})
|
||||
items_json = json.dumps([{
|
||||
"id": "multi", "title": "Amazon", "created_at": "2026-01-01T00:00:00Z",
|
||||
"additional_information": "jane@example.com",
|
||||
"urls": [{"href": "https://amazon.co.uk"}, {"href": "https://www.amazon.co.uk"},
|
||||
{"href": "https://eu.account.amazon.com"}, {"href": "androidapp://com.amazon.shopping"},
|
||||
{"href": "::not parseable::"}],
|
||||
}, {
|
||||
"id": "single", "title": "Shop", "created_at": "2026-01-01T00:00:00Z",
|
||||
"urls": [{"href": "https://shop.example.com"}],
|
||||
}])
|
||||
with patch.object(OnePasswordLoginBackend, "is_unlocked", return_value=True), \
|
||||
patch.object(backend, "_run", return_value=items_json):
|
||||
metas = {m.id: m for m in backend.list_items()}
|
||||
assert metas["op:multi"].origin == "https://amazon.co.uk"
|
||||
assert list(metas["op:multi"].allowed_origins) == ["https://amazon.co.uk", "https://www.amazon.co.uk",
|
||||
"https://eu.account.amazon.com"]
|
||||
assert metas["op:single"].origin == "https://shop.example.com"
|
||||
assert list(metas["op:single"].allowed_origins) == ["https://shop.example.com"]
|
||||
# helpers: dedupe keeps first occurrence; app-only items keep their single origin
|
||||
assert _all_origins(["https://a.com/x", "https://a.com/y"]) == ["https://a.com"]
|
||||
assert _web_origins(["androidapp://com.x"]) == ("androidapp://com.x",)
|
||||
|
||||
@@ -302,6 +302,88 @@ class TestBrowserVaultTools:
|
||||
assert "Refused" in out["error"]
|
||||
assert "s3cret-pw" not in json.dumps(out)
|
||||
|
||||
@staticmethod
|
||||
def _manager_meta():
|
||||
from agent.vault_store import VaultItemMeta
|
||||
|
||||
return VaultItemMeta(
|
||||
id="op:multi", kind="login", label="Amazon", origin="https://amazon.co.uk",
|
||||
created_at="2026-01-01T00:00:00Z", identifier_type="username", identifier="jane@example.com",
|
||||
allowed_origins=("https://amazon.co.uk", "https://www.amazon.co.uk",
|
||||
"https://eu.account.amazon.com"))
|
||||
|
||||
def test_fill_allowed_on_every_saved_origin_of_a_multi_website_item(self):
|
||||
"""A manager item with several saved websites fills on each of them
|
||||
(exact match only), and the in-page origin assert pins the origin
|
||||
actually being filled — not just the first saved one."""
|
||||
from tools import browser_vault_tool
|
||||
|
||||
meta = self._manager_meta()
|
||||
|
||||
class _ManagerBackend:
|
||||
name, display_name, needs_unlock = "onepassword", "1Password", False
|
||||
|
||||
def is_unlocked(self):
|
||||
return True
|
||||
|
||||
def get_meta(self, handle):
|
||||
return meta if handle == meta.id else None
|
||||
|
||||
def resolve_password(self, handle):
|
||||
return "s3cret-pw"
|
||||
|
||||
controls = [
|
||||
{"autocomplete": "email", "formIndex": 0, "index": 0, "label": "", "name": "email", "type": "email"},
|
||||
{"autocomplete": "current-password", "formIndex": 0, "index": 1, "label": "", "name": "pw", "type": "password"},
|
||||
]
|
||||
secret_exprs = []
|
||||
|
||||
def fake_eval(task_id, expression):
|
||||
return {"success": True, "result": json.dumps(controls)}
|
||||
|
||||
def fake_eval_secret(task_id, expression):
|
||||
secret_exprs.append(expression)
|
||||
return {"success": True, "result": json.dumps({"filled": 1})}
|
||||
|
||||
with patch("agent.vault_backends.backend_for_handle", return_value=_ManagerBackend()), \
|
||||
patch.object(browser_vault_tool, "_current_page_origin", return_value="https://www.amazon.co.uk"), \
|
||||
patch.object(browser_vault_tool, "_eval_js", side_effect=fake_eval), \
|
||||
patch.object(browser_vault_tool, "_eval_js_secret", side_effect=fake_eval_secret):
|
||||
raw = browser_vault_tool.browser_vault_fill("op:multi")
|
||||
out = json.loads(raw)
|
||||
assert out["success"] is True
|
||||
assert out["origin"] == "https://www.amazon.co.uk"
|
||||
# the synchronous in-page check is bound to the origin actually matched
|
||||
assert "https://www.amazon.co.uk" in secret_exprs[0]
|
||||
assert "s3cret-pw" not in raw
|
||||
assert out["filled_fields"] == 1
|
||||
|
||||
def test_fill_still_refused_on_origin_not_saved_on_the_item(self):
|
||||
"""Multi-website items widen nothing: an unsaved origin — even a sibling
|
||||
subdomain — is still refused (fail-closed regression)."""
|
||||
from tools import browser_vault_tool
|
||||
|
||||
meta = self._manager_meta()
|
||||
|
||||
class _ManagerBackend:
|
||||
name, display_name, needs_unlock = "onepassword", "1Password", False
|
||||
|
||||
def is_unlocked(self):
|
||||
return True
|
||||
|
||||
def get_meta(self, handle):
|
||||
return meta if handle == meta.id else None
|
||||
|
||||
def resolve_password(self, handle):
|
||||
return "s3cret-pw"
|
||||
|
||||
with patch("agent.vault_backends.backend_for_handle", return_value=_ManagerBackend()), \
|
||||
patch.object(browser_vault_tool, "_current_page_origin", return_value="https://payments.amazon.co.uk"):
|
||||
out = json.loads(browser_vault_tool.browser_vault_fill("op:multi"))
|
||||
assert out["success"] is False
|
||||
assert out["error_type"] == "origin_mismatch"
|
||||
assert "s3cret-pw" not in json.dumps(out)
|
||||
|
||||
def test_fill_unknown_handle(self, store):
|
||||
from tools import browser_vault_tool
|
||||
|
||||
|
||||
@@ -229,6 +229,8 @@ def browser_vault_list() -> str:
|
||||
for meta in metas:
|
||||
entry = {"handle": meta.id, "backend": backend.name, "label": meta.label, "kind": meta.kind,
|
||||
"origin": meta.origin, "available": meta.kind == "login" or bool(meta.origin)}
|
||||
if len(meta.allowed_origins) > 1:
|
||||
entry["allowed_origins"] = list(meta.allowed_origins)
|
||||
if meta.has_otp or backend.needs_unlock:
|
||||
entry["two_factor"] = "automatic" if meta.has_otp else "automatic if the manager stores a TOTP seed, else the user is asked"
|
||||
if meta.identifier:
|
||||
@@ -438,20 +440,29 @@ def browser_vault_fill(handle: str, task_id: Optional[str] = None) -> str:
|
||||
|
||||
# ── Origin binding pre-check (cheap early exit; the authoritative check
|
||||
# runs synchronously inside the fill script itself) ──────────────────────
|
||||
page_origin = _focus_bound_origin(effective_task_id, str(meta.origin), meta.kind) or _current_page_origin(effective_task_id)
|
||||
# Manager items can bind several websites (e.g. amazon.co.uk + www.amazon.co.uk);
|
||||
# every saved origin is a valid fill target. Matching stays exact-origin —
|
||||
# nothing wildcard/parent-domain is ever inferred.
|
||||
allowed = list(meta.allowed_origins) or ([str(meta.origin)] if meta.origin else [])
|
||||
page_origin = None
|
||||
for candidate in allowed:
|
||||
page_origin = _focus_bound_origin(effective_task_id, candidate, meta.kind)
|
||||
if page_origin:
|
||||
break
|
||||
page_origin = page_origin or _current_page_origin(effective_task_id)
|
||||
if not page_origin:
|
||||
return json.dumps(
|
||||
{"success": False, "error": "Could not determine the current page origin. Navigate to the login page first."}
|
||||
)
|
||||
if page_origin != meta.origin:
|
||||
if page_origin not in allowed:
|
||||
return json.dumps(
|
||||
{
|
||||
"success": False,
|
||||
"error_type": "origin_mismatch",
|
||||
"error": (
|
||||
f"Refused: current page origin ({page_origin}) does not match "
|
||||
f"the vault item's bound origin ({meta.origin}). Vault fills "
|
||||
"only run on the exact origin the credential was saved for."
|
||||
f"the vault item's bound origin(s) ({', '.join(allowed)}). Vault fills "
|
||||
"only run on the exact origin(s) the credential was saved for."
|
||||
),
|
||||
}
|
||||
)
|
||||
@@ -505,7 +516,7 @@ def browser_vault_fill(handle: str, task_id: Optional[str] = None) -> str:
|
||||
|
||||
try:
|
||||
fill_result = _eval_js_secret(
|
||||
effective_task_id, build_fill_js(fills, expected_origin=str(meta.origin), nonce=nonce)
|
||||
effective_task_id, build_fill_js(fills, expected_origin=page_origin, nonce=nonce)
|
||||
)
|
||||
except Exception as exc:
|
||||
# Strip any secret material from exception text before surfacing.
|
||||
@@ -529,7 +540,7 @@ def browser_vault_fill(handle: str, task_id: Optional[str] = None) -> str:
|
||||
"error_type": "origin_changed",
|
||||
"error": (
|
||||
"Refused: the page navigated away from the bound origin "
|
||||
f"({meta.origin}) before the fill could run "
|
||||
f"({page_origin}) before the fill could run "
|
||||
f"(now on {parsed.get('found') or 'unknown'}). "
|
||||
"Nothing was written."
|
||||
),
|
||||
@@ -538,7 +549,7 @@ def browser_vault_fill(handle: str, task_id: Optional[str] = None) -> str:
|
||||
filled = parsed.get("filled", 0) if isinstance(parsed, dict) else 0
|
||||
|
||||
out = {"success": bool(filled), "filled_fields": int(filled), "backend": backend.name,
|
||||
"kind": meta.kind, "origin": meta.origin}
|
||||
"kind": meta.kind, "origin": page_origin}
|
||||
if meta.kind == "login":
|
||||
out["next"] = ("Submit. If the site then asks for a verification code, call browser_vault_enter_code with this handle"
|
||||
+ (" (a code will be generated automatically)." if meta.has_otp else "."))
|
||||
|
||||
Reference in New Issue
Block a user