Check origins and secret env names as master does

Allowed origins are trimmed the way JavaScript's trim() does, so a
leading byte-order mark is accepted and \x1c-\x1f or \x85 are not, and an
IPv6 zone is checked as Node's isIPv6 does, which Python 3.8 refused
outright. A `secrets` variable name longer than 256 characters is now
refused locally, as the server refuses it.

The README now says a frame fill skips the allowed-origin tab step, that
SECRET_ONEPASSWORD_AUTH_FAILED also covers a malformed token or one for
another sign-in host, and the 503 that CREDENTIAL_FILL_FAILED and
SECRET_ONEPASSWORD_FAILED can carry.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019dbwbLvQGyzwkxNetvqf3v

Author: AHarmlessPyro

README.md

@@ -225,8 +225,9 @@ Things to know:
   it.
   Without `page_id`, fill considers the tabs whose URL starts with `page_url`
   (all tabs when it's not given). If one is left, that's the target. If several
-  are left, it prefers the tabs on an origin every secret allows, then the
-  visible one among them; if more than one still qualifies, it fails with
+  are left, it prefers the tabs on an origin every secret allows (skipped with
+  `frame_url`, since a login frame's tab is usually on another origin), then
+  the visible one among them; if more than one still qualifies, it fails with
   `CREDENTIAL_PAGE_AMBIGUOUS`. The chosen tab's origin is then checked: on any
   other origin nothing is typed and the fill fails with
   `CREDENTIAL_ORIGIN_NOT_ALLOWED`. Under Playwright every tab reports as visible, so pass
@@ -266,15 +267,17 @@ Things to know:
     `SECRET_NOT_FOUND`, `SECRET_VERSION_NOT_FOUND`, `VAULT_NOT_FOUND`,
     `VAULT_ENTRY_NOT_FOUND` (404), `SESSION_NOT_ACTIVE` (409).
   - `CREDENTIAL_FILL_TIMEOUT` (503/504, retryable) and
-    `CREDENTIAL_FILL_FAILED` (500/502, retryable as flagged).
+    `CREDENTIAL_FILL_FAILED` (500/502/503, retryable as flagged).
   - `CREDENTIALS_UNSUPPORTED_RUNTIME` (400), `CREDENTIAL_FILL_UNSUPPORTED`
     (501) and `CREDENTIAL_VALUE_UNSUPPORTED` (501): the session's browser
     can't fill this, such as 1Password fields on an older host.
   - `SECRET_ONEPASSWORD_REFERENCE_FAILED` (422): a 1Password item or field
     couldn't be read, so nothing was filled. `details` has the failing
     `selectors` and `reasons` by selector. `SECRET_ONEPASSWORD_AUTH_FAILED`
-    (422): the token can't sign in. `SECRET_ONEPASSWORD_RATE_LIMITED` (429)
-    and `SECRET_ONEPASSWORD_FAILED` (502) are retryable. Each has the
+    (422): 1Password rejected the token, or the token is malformed or doesn't
+    sign in to `1password.com`, `.ca` or `.eu` (or a subdomain), so it wasn't
+    sent to 1Password. `SECRET_ONEPASSWORD_RATE_LIMITED` (429) and
+    `SECRET_ONEPASSWORD_FAILED` (502/503) are retryable. Each has the
     token's `tokenSecretId` and `tokenVersion` in `details`.
   - Locally, invalid fill requests raise `invalid_request` and invalid values
     raise `invalid_secret_value`, before any request.
@@ -816,8 +819,10 @@ Things to know:
     sandbox's host predates TOTP or 1Password, so nothing started. Retryable
     on sandbox create, which may land on a newer host; not on a process start.
   - `SECRET_ONEPASSWORD_REFERENCE_FAILED` (422, `details` names the
-    variables) and `SECRET_ONEPASSWORD_AUTH_FAILED` (422): fix the fields or
-    the token. `SECRET_ONEPASSWORD_RATE_LIMITED` (429) and
+    variables) and `SECRET_ONEPASSWORD_AUTH_FAILED` (422, also a malformed
+    token, or one that doesn't sign in to `1password.com`, `.ca` or `.eu`,
+    which is never sent to 1Password): fix the fields or the token.
+    `SECRET_ONEPASSWORD_RATE_LIMITED` (429) and
     `SECRET_ONEPASSWORD_FAILED` (502/503) are retryable.
   - `retryable` is the API's flag when it sends one.
 

hyperbrowser/models/secret.py

@@ -77,15 +77,7 @@ def normalize_secret_references(value):
             f"At most {MAX_SECRET_BINDINGS} secrets can be delivered at once"
         )
     for env in value:
-        if (
-            not isinstance(env, str)
-            or not SECRET_ENV_NAME_PATTERN.fullmatch(env)
-            or env == "__proto__"
-        ):
-            raise ValueError(
-                f"Invalid environment variable name {env!r} for a secret: "
-                "expected letters, digits and '_', not starting with a digit"
-            )
+        validate_secret_env_name(env)
     return {
         env: {"name": reference} if isinstance(reference, str) else reference
         for env, reference in value.items()
@@ -289,6 +281,30 @@ def onepassword_clashes(
 )
 _ORIGIN_HOST_LABEL = re.compile(r"(?!-)[a-z0-9-]{1,63}(?<!-)", re.ASCII)
 _LOCAL_ORIGIN_HOSTS = frozenset({"localhost", "127.0.0.1", "[::1]"})
+# What JavaScript's trim() removes, which the server trims origins with. Unlike
+# Python's strip(), it removes a byte-order mark and keeps \x1c-\x1f and \x85.
+_JS_WHITESPACE = "".join(
+    chr(code)
+    for code in [0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x20, 0xA0, 0x1680]
+    + list(range(0x2000, 0x200B))
+    + [0x2028, 0x2029, 0x202F, 0x205F, 0x3000, 0xFEFF]
+)
+# The zone ID Node's isIPv6 accepts after an address, as in fe80::1%eth0.
+_IPV6_ZONE = re.compile(r"[0-9a-zA-Z.:-]+", re.ASCII)
+
+
+def _is_ipv6(address: str) -> bool:
+    """Whether Node's isIPv6 accepts ``address``. Python 3.8 knows no zone
+    IDs, and later versions accept zones Node refuses, so the zone is
+    checked here."""
+    address, mark, zone = address.partition("%")
+    if mark and not _IPV6_ZONE.fullmatch(zone):
+        return False
+    try:
+        ipaddress.IPv6Address(address)
+    except ValueError:
+        return False
+    return True
 
 
 def validate_allowed_origin(entry) -> str:
@@ -309,7 +325,7 @@ def validate_allowed_origin(entry) -> str:
     def reject(reason: str) -> ValueError:
         return ValueError(f"Allowed origin {entry!r} {reason}")
 
-    match = _ALLOWED_ORIGIN_PATTERN.fullmatch(entry.strip())
+    match = _ALLOWED_ORIGIN_PATTERN.fullmatch(entry.strip(_JS_WHITESPACE))
     if not match:
         raise reject("must be scheme://host[:port] with no path")
     scheme, wildcard, host, port = match.groups()
@@ -333,11 +349,8 @@ def reject(reason: str) -> ValueError:
             raise reject("cannot use a wildcard with an IP address")
     elif wildcard:
         raise reject("cannot use a wildcard with an IP address")
-    else:
-        try:
-            ipaddress.IPv6Address(host[1:-1])
-        except ValueError:
-            raise reject("has an invalid host") from None
+    elif not _is_ipv6(host[1:-1]):
+        raise reject("has an invalid host")
     return entry
 
 

tests/test_credentials.py

@@ -405,6 +405,12 @@ def test_set_allowed_origins_replaces_or_clears_the_browser_metadata():
     "https://[::g]",
     "https://" + "a" * 505 + ".com",
     "",
+    # The server trims as JavaScript does, which keeps these.
+    "https://example.com\x1c",
+    "https://example.com\x85",
+    # Node's isIPv6 takes only these characters in a zone.
+    "https://[fe80::1%e_th]",
+    "https://[fe80::1%]",
 ]
 
 
@@ -451,6 +457,10 @@ def test_secrets_take_origins_the_server_accepts(private_key):
         "http://[::1]:8080",
         "https://[2001:db8::1]",
         "https://10.0.0.1:8443",
+        # JavaScript's trim() removes a byte-order mark; Python's strip() doesn't.
+        "\ufeffhttps://github.com",
+        # Zones Node's isIPv6 accepts, which Python 3.8's ipaddress refuses.
+        "https://[fe80::1%eth0]",
     ]
     http = ScriptedHTTPClient(_create_routes(private_key, _typed("secret")))
     SecretManager(FakeClient(http)).create(

tests/test_secrets.py

@@ -1532,11 +1532,12 @@ def test_secret_names_env_names_and_versions_are_checked_locally():
             CreateSecretParams(name=name)
     assert CreateSecretParams(name="a" * 128).name == "a" * 128
 
-    for env in ["1ABC", "API-KEY", "", "__proto__", "KEY\n"]:
+    for env in ["1ABC", "API-KEY", "", "__proto__", "KEY\n", "A" * 257]:
         with pytest.raises(ValueError, match="environment variable name"):
             SandboxExecParams(command="x", secrets={env: "provider-key"})
         with pytest.raises(ValueError, match="environment variable name"):
             CreateSandboxParams(image_name="node", secrets={env: "provider-key"})
+    assert SandboxExecParams(command="x", secrets={"A" * 256: "provider-key"}).secrets
     with pytest.raises(ValueError, match="Invalid secret name"):
         SandboxExecParams(command="x", secrets={"API_KEY": "bad name"})