fix(scripts): make the robinhood credential guard catch the error it claimed to

scripts/robinhood_smoke.py

@@ -83,6 +83,7 @@
 from __future__ import annotations
 
 import argparse
+import base64
 import json
 import sys
 from collections import Counter
@@ -118,11 +119,32 @@
 #: "that account cannot trade that asset".
 PROBE_SYMBOL = "BTC-USD"
 
-#: A raw Ed25519 seed is 32 bytes, which is 44 base64 characters with padding. Checking this
-#: before the first request turns the most likely operator error -- pasting the public key, or a
-#: PEM, or a hex string -- into a precise message instead of a 401.
+#: A raw Ed25519 seed is 32 bytes, which is 44 base64 characters with padding.
+#:
+#: ⚠️ **A raw Ed25519 PUBLIC key is ALSO 32 bytes, hence also 44 characters.** This constant used
+#: to be described as the check that catches "pasting the public key"; it cannot be, and never
+#: was. The two values are indistinguishable by length, by alphabet and by decoded size -- the
+#: only thing that separates them is that one is derived from the other, which is what
+#: `_public_key_b64` below actually tests. The length check still earns its place against a PEM,
+#: a hex string, or a truncated paste; it just never covered the case it claimed to.
+#:
+#: This is not hypothetical. On 2026-08-19 an operator pasted the public key into
+#: `ROBINHOOD_API_KEY`, this script's guards passed it, and every request came back 401 -- which
+#: is indistinguishable from a bad key, a stale clock or a signing bug, and cost an afternoon to
+#: tell apart. The guards below name it in one line instead.
 _SEED_B64_LEN = 44
 
+#: A raw Ed25519 key, seed or public, before base64.
+_KEY_RAW_LEN = 32
+
+#: The shape Robinhood issues for an API key, from the one credential observed to authenticate:
+#: `rh-api-` followed by a UUID, 43 characters. Quoted in the error messages as guidance, and
+#: deliberately NOT enforced -- it is an observation about one credential, not a documented
+#: contract, and a hard gate on it would reject a valid key the day Robinhood changes the format.
+#: The two checks that DO gate are the ones that cannot be wrong: an API key slot holding a
+#: 32-byte Ed25519 key is never right, whatever the identifier format turns out to be.
+_API_KEY_HINT = "rh-api-<uuid>"
+
 #: The keys `RobinhoodTransport._paginate` consumes and does not pass on.
 #:
 #: Every probe below is a paginated read, so every probe's response reaches this script already
@@ -374,12 +396,56 @@ def compare_shapes(live: Any, fixture: Any, path: str = "") -> list[str]:
     return diffs
 
 
+def _raw_key_bytes(value: str) -> bytes | None:
+    """The 32 raw bytes `value` encodes, or `None` if it is not base64 of an Ed25519-sized key.
+
+    Total by construction: this runs against operator-pasted text, so every way base64 decoding
+    can fail -- wrong alphabet, bad padding, plain prose -- has to mean "not a key" rather than a
+    traceback. `validate=True` matters: without it `b64decode` silently DISCARDS characters
+    outside the alphabet, so `rh-api-0f1e...` would decode to something rather than being
+    rejected, and a real API key could be mistaken for a malformed one.
+    """
+    try:
+        raw = base64.b64decode(value, validate=True)
+    except (ValueError, TypeError):
+        return None
+    return raw if len(raw) == _KEY_RAW_LEN else None
+
+
+def _public_key_b64(seed_b64: str) -> str | None:
+    """The base64 PUBLIC key derived from a base64 Ed25519 seed, or `None` if it cannot be.
+
+    `None` covers both "the seed is not a seed" and "pynacl is not installed", and both degrade
+    to skipping one check rather than failing the run: this is a diagnostic, and a diagnostic
+    that crashes on the malformed input it exists to describe is worse than one that stays quiet.
+    The neighbouring length check still fires on a malformed seed, and a missing pynacl surfaces
+    with its own clear ImportError at the first signature.
+    """
+    try:
+        import nacl.signing
+
+        raw = base64.b64decode(seed_b64, validate=True)
+        return base64.b64encode(bytes(nacl.signing.SigningKey(raw).verify_key)).decode()
+    except Exception:
+        return None
+
+
 def load_credentials(env_path: Path) -> tuple[str, str]:
     """Read the credential from `.env`, failing with instructions rather than a stack trace.
 
-    The private key's length is checked here because the overwhelmingly likely operator error --
-    pasting the base64 PUBLIC key that Robinhood's credential page asked for -- yields a 401 that
-    looks exactly like a signing bug, and chasing that costs far more than this check.
+    Three checks, and the ORDER is the design: each one only runs when the more specific check
+    above it did not fire, so the operator is told the most actionable thing true of their file
+    rather than the most generic.
+
+    1. Absent values, named individually.
+    2. A private key that is not seed-shaped -- a PEM, a hex string, a truncated paste.
+    3. An API key slot holding an Ed25519 key, with the case where it is THIS seed's own public
+       key called out by name, because that is the one an operator can act on without going
+       back to Robinhood at all: the value is already exactly what the credential page wants.
+
+    Everything here runs BEFORE the first request, because every one of these mistakes produces
+    the same 401 the venue returns for a revoked key, a stale clock or a genuine signing bug --
+    and a 401 is the least informative failure this integration can hand back.
     """
     values = dotenv_values(env_path)
     api_key = (values.get("ROBINHOOD_API_KEY") or "").strip()
@@ -403,9 +469,34 @@ def load_credentials(env_path: Path) -> tuple[str, str]:
         raise SystemExit(
             f"ROBINHOOD_PRIVATE_KEY is {len(private_key)} characters; a base64-encoded 32-byte "
             f"Ed25519 seed is {_SEED_B64_LEN}.\n"
-            "This is almost always the PUBLIC key, a PEM, or a hex string. Sending it would "
-            "produce a 401 indistinguishable from a signing bug."
+            "That usually means a PEM, a hex string, or a truncated paste. (It does NOT mean the "
+            "public key -- a public key is also 32 bytes and passes this check; see below.) "
+            "Sending it would produce a 401 indistinguishable from a signing bug."
         )
+
+    if _raw_key_bytes(api_key) is not None:
+        if api_key == _public_key_b64(private_key):
+            raise SystemExit(
+                "ROBINHOOD_API_KEY holds the base64 PUBLIC key of ROBINHOOD_PRIVATE_KEY, not an "
+                "API key.\n\n"
+                "The public key is what you paste INTO Robinhood's credential page. What belongs "
+                f"here is the identifier Robinhood issues back once the credential exists "
+                f"({_API_KEY_HINT}).\n"
+                "Every request would sign correctly and be rejected 401, because the venue has no "
+                "record of this key.\n\n"
+                "Fix: sign in to web classic, open https://robinhood.com/account/crypto, choose "
+                "Add key, paste the value currently in ROBINHOOD_API_KEY as the public key, tick "
+                "the API actions this credential needs, and put the identifier it returns here. "
+                "ROBINHOOD_PRIVATE_KEY stays as it is -- the keypair is already correct."
+            )
+        raise SystemExit(
+            "ROBINHOOD_API_KEY decodes as a 32-byte base64 value, which is an Ed25519 KEY -- an "
+            f"API key is an identifier issued by Robinhood ({_API_KEY_HINT}).\n"
+            "It does not match ROBINHOOD_PRIVATE_KEY's public key either, so it is most likely a "
+            "public key from a different credential.\n"
+            "See packages/keel-broker-robinhood/README.md, 'Credentials'."
+        )
+
     return api_key, private_key
 
 

tests/scripts/test_robinhood_smoke.py

@@ -7,11 +7,13 @@
 
 from __future__ import annotations
 
+import base64
 import json
 from decimal import Decimal
 from pathlib import Path
 from typing import Any
 
+import nacl.signing
 import pytest
 
 from scripts.robinhood_smoke import (
@@ -27,7 +29,23 @@
     shape_of,
 )
 
-_VALID_SEED_B64 = "A" * 44  # a base64 32-byte Ed25519 seed is 44 characters
+#: Raw bytes behind the test seed. Any 32 bytes is a valid Ed25519 seed.
+_SEED_RAW = bytes(range(32))
+
+#: A REAL base64 Ed25519 seed, not 44 arbitrary characters. It has to be real now: the guard
+#: derives a public key from it, and `"A" * 44` is not valid base64 for 32 bytes at all -- it
+#: decodes to 33, which is the sort of thing a constant named `_VALID_SEED_B64` should not be.
+_VALID_SEED_B64 = base64.b64encode(bytes(_SEED_RAW)).decode()
+
+#: The public key that seed derives to -- the exact value an operator pastes into Robinhood's
+#: credential page, and the exact value that must never appear in `ROBINHOOD_API_KEY`.
+_ITS_PUBLIC_KEY_B64 = base64.b64encode(
+    bytes(nacl.signing.SigningKey(bytes(_SEED_RAW)).verify_key)
+).decode()
+
+#: Stands in for a real `rh-api-<uuid>`: the shape observed on the one credential known to
+#: authenticate. Not base64, which is the property the guard keys off.
+_VALID_API_KEY = "rh-api-1e2d3c4b-5a69-4788-9f01-23456789abcd"
 
 _FIXTURES = Path(__file__).resolve().parents[1] / "fixtures"
 
@@ -303,19 +321,72 @@ def test_a_missing_api_key_is_named(tmp_path: Path) -> None:
         load_credentials(env)
 
 
-def test_a_public_key_pasted_as_the_private_key_is_caught_before_a_request(
-    tmp_path: Path,
-) -> None:
-    """Wrong-length seeds must fail here, not as a 401 that reads like a signing bug."""
-    env = _write_env(tmp_path, "ROBINHOOD_API_KEY=abc\nROBINHOOD_PRIVATE_KEY=tooshort\n")
+def test_a_misshapen_private_key_is_caught_before_a_request(tmp_path: Path) -> None:
+    """A PEM, a hex string or a truncated paste must fail here, not as a 401 that reads like a
+    signing bug."""
+    env = _write_env(
+        tmp_path, f"ROBINHOOD_API_KEY={_VALID_API_KEY}\nROBINHOOD_PRIVATE_KEY=tooshort\n"
+    )
     with pytest.raises(SystemExit) as excinfo:
         load_credentials(env)
-    assert "PUBLIC key" in str(excinfo.value)
+    message = str(excinfo.value)
+    assert "8 characters" in message
+    # ...and it no longer CLAIMS to have caught a public key, which it cannot do by length.
+    assert "does NOT mean the public key" in message
+
+
+def test_the_public_key_pasted_as_the_api_key_is_named_exactly(tmp_path: Path) -> None:
+    """The real 2026-08-19 incident, and the whole reason this guard was rewritten.
+
+    A seed and a public key are both 32 bytes and both 44 base64 characters, so no length check
+    can separate them -- the old guard's comment claimed it did, and the operator got a bare 401
+    instead. Deriving the public key from the seed is the only test that actually distinguishes
+    them, and it turns an afternoon of "is it the key, the clock, or the signature?" into one
+    line naming the file, the variable and the fix.
+    """
+    env = _write_env(
+        tmp_path,
+        f"ROBINHOOD_API_KEY={_ITS_PUBLIC_KEY_B64}\nROBINHOOD_PRIVATE_KEY={_VALID_SEED_B64}\n",
+    )
+    with pytest.raises(SystemExit) as excinfo:
+        load_credentials(env)
+    message = str(excinfo.value)
+    assert "PUBLIC key of ROBINHOOD_PRIVATE_KEY" in message
+    # The fix has to be actionable without a round trip to figure out what to do.
+    assert "robinhood.com/account/crypto" in message
+    assert "Add key" in message
+    assert "ROBINHOOD_PRIVATE_KEY stays as it is" in message
+
+
+def test_an_unrelated_ed25519_key_in_the_api_key_slot_is_still_refused(tmp_path: Path) -> None:
+    """The same mistake made with a DIFFERENT credential's public key. It cannot be named as
+    precisely -- the guard has nothing to match it against -- but a 32-byte base64 value is never
+    an API key identifier whatever the identifier format turns out to be, so it must not pass."""
+    other = base64.b64encode(bytes(range(100, 132))).decode()  # not _SEED_RAW, not its public key
+    env = _write_env(
+        tmp_path, f"ROBINHOOD_API_KEY={other}\nROBINHOOD_PRIVATE_KEY={_VALID_SEED_B64}\n"
+    )
+    with pytest.raises(SystemExit) as excinfo:
+        load_credentials(env)
+    assert "Ed25519 KEY" in str(excinfo.value)
+
+
+def test_a_real_api_key_is_not_mistaken_for_base64(tmp_path: Path) -> None:
+    """`rh-api-<uuid>` contains `-`, which is outside the base64 alphabet. Without
+    `validate=True`, `b64decode` DISCARDS such characters instead of refusing, and a genuine API
+    key could decode to 32 bytes by accident and be rejected as a pasted key. This is the test
+    that pins the strict decode."""
+    env = _write_env(
+        tmp_path, f"ROBINHOOD_API_KEY={_VALID_API_KEY}\nROBINHOOD_PRIVATE_KEY={_VALID_SEED_B64}\n"
+    )
+    assert load_credentials(env) == (_VALID_API_KEY, _VALID_SEED_B64)
 
 
 def test_a_wellformed_credential_is_returned(tmp_path: Path) -> None:
-    env = _write_env(tmp_path, f"ROBINHOOD_API_KEY=abc\nROBINHOOD_PRIVATE_KEY={_VALID_SEED_B64}\n")
-    assert load_credentials(env) == ("abc", _VALID_SEED_B64)
+    env = _write_env(
+        tmp_path, f"ROBINHOOD_API_KEY={_VALID_API_KEY}\nROBINHOOD_PRIVATE_KEY={_VALID_SEED_B64}\n"
+    )
+    assert load_credentials(env) == (_VALID_API_KEY, _VALID_SEED_B64)
 
 
 # --- probes and fixtures agree ---------------------------------------------------------------