feat(setup): a credential form, and the line a wizard must not cross to have one

keel/commands/setup.py

@@ -75,9 +75,21 @@
 
 
 class StepKind(str, Enum):
-    """Who can perform a step -- which is what decides whether a wizard may touch it."""
+    """Who can perform a step -- which is what decides whether a wizard may touch it.
+
+    `OPERATOR_INPUT` was split out of `JUDGEMENT` when the credential step arrived (#437), and the
+    distinction is worth the fourth member. A JUDGEMENT is a DECISION only a human may make -- a
+    Shariah classification, a promotion -- and a wizard that made one would be making a compliance
+    ruling on the operator's behalf. An OPERATOR_INPUT is a FACT only the operator possesses: an
+    API key. A wizard may record one and cannot possibly invent one, so it is safe to offer as a
+    form in a way a judgement is not.
+
+    Collapsing the two would have forced one of two bad outcomes: either the browser could record
+    an attestation (wrong), or it could never accept a credential (which leaves a desktop user
+    with no way to configure keel at all, since they have no terminal to type one in)."""
 
     MECHANICAL = "mechanical"
+    OPERATOR_INPUT = "operator_input"
     JUDGEMENT = "judgement"
     OFF_VENUE = "off_venue"
 
@@ -154,6 +166,21 @@ def blocking(self) -> bool:
         ),
         how="keel rules seed  (seeds candidates only; promotion is a separate, deliberate step)",
     ),
+    Step(
+        key="credentials",
+        title="A market-data credential",
+        kind=StepKind.OPERATOR_INPUT,
+        stage=Stage.PAPER,
+        why=(
+            "Candle history is fetched through an authenticated client, so `keel fetch` without a "
+            "key fails outright -- even in paper mode, where no order can be placed. Only you "
+            "have this key; keel can store it and cannot obtain or guess one."
+        ),
+        how=(
+            "keel credentials set CDP_API_KEY   (a free, read-only Coinbase Developer Platform "
+            "key is enough for market data)"
+        ),
+    ),
     Step(
         key="market_data",
         title="Market data",
@@ -342,6 +369,8 @@ def inspect(config_path: str | Path, db_path: str | Path) -> DeploymentState:
             f"{config_file} -- mode {mode}, {len(allowlist)} allowlisted asset(s)",
         )
 
+    observations["credentials"] = _credential_observation(config_file)
+
     # -- everything that needs the database --
     conn: sqlite3.Connection | None = None
     if db_file.exists():
@@ -444,6 +473,34 @@ def _database_observations(
     return out
 
 
+#: The pair `keel fetch` needs. Both must be present: a key with no secret authenticates nothing.
+MARKET_DATA_SECRETS: tuple[str, ...] = ("CDP_API_KEY", "CDP_API_SECRET")
+
+
+def _credential_observation(config_file: Path) -> tuple[bool | None, str]:
+    """Whether keel can see a market-data credential, and WHERE it is coming from.
+
+    The source is reported because it is the difference between the two support questions this
+    can produce -- "keel cannot see my key" and "keel is using a different key than the one I just
+    typed". The VALUE is never read into the message, and `ResolvedSecret` will not print one even
+    if a future edit tries."""
+    from keel_core.secrets import SecretSource, read_secret
+
+    env_path = config_file.parent / ".env"
+    resolved = [read_secret(name, env_path=env_path) for name in MARKET_DATA_SECRETS]
+    missing = [item.name for item in resolved if not item.found]
+    if missing:
+        return (False, f"missing: {', '.join(missing)}")
+    sources = {item.source for item in resolved}
+    if sources == {SecretSource.ENV_FILE}:
+        return (True, "found in your .env file")
+    if sources == {SecretSource.KEYCHAIN}:
+        return (True, "stored in the OS keychain")
+    if sources == {SecretSource.ENVIRONMENT}:
+        return (True, "set in this process's environment")
+    return (True, "found (" + ", ".join(sorted(s.value for s in sources)) + ")")
+
+
 def _asset_observation(conn: sqlite3.Connection, config: Any) -> tuple[bool | None, str]:
     """Attested against the ALLOWLIST, not against the count.
 
@@ -566,17 +623,27 @@ def _state_as_json(state: DeploymentState) -> dict[str, Any]:
 #
 # Everything below WRITES. Read the rules before adding to it:
 #
-# 1. Only steps declared `MECHANICAL` may appear here. `JUDGEMENT` steps are the operator's --
-#    a wizard may collect and record them but must never decide them -- and `OFF_VENUE` steps
-#    happen somewhere keel cannot reach. `tests/commands/test_setup_actions.py` enforces this
-#    against `STEPS`, so a new action for a judgement step fails rather than shipping.
+# 1. Only `MECHANICAL` and `OPERATOR_INPUT` steps may appear here, and the line between them and
+#    the rest is the point. A MECHANICAL step has no input: the machine simply does it. An
+#    OPERATOR_INPUT step is a FACT only the operator has -- an API key -- which a wizard can
+#    record and could not possibly invent.
+#
+#    `JUDGEMENT` steps are DECISIONS: a Shariah classification, a promotion. A form that recorded
+#    one would be making a compliance ruling on the operator's behalf, and no amount of "but they
+#    clicked it" makes that the same thing as their having decided it. `OFF_VENUE` steps happen
+#    somewhere keel cannot reach at all. Neither may ever be an action here, and the tests
+#    enforce it against `STEPS` rather than against a list kept in this file.
 #
-# 2. Every action is IDEMPOTENT and NEVER destructive. A setup flow is something a nervous user
+# 2. An action with `inputs` records ONLY what was submitted. It has no defaults, no fallbacks and
+#    no "sensible guess" -- an action that could fill in a field the operator left blank is one
+#    that could record something they never supplied.
+#
+# 3. Every action is IDEMPOTENT and NEVER destructive. A setup flow is something a nervous user
 #    clicks twice, and a browser reload re-submits. "The config already exists" is a successful
 #    outcome that changed nothing -- never an overwrite, and there is deliberately no `force`
 #    parameter for any web caller to pass.
 #
-# 3. Nothing here increases what keel can DO. Not one of the eleven capability-increasing actions
+# 4. Nothing here increases what keel can DO. Not one of the eleven capability-increasing actions
 #    in `keel/capabilities.py` is reachable from this module, and a test asserts the two sets are
 #    disjoint. Creating a config, a schema and a library of CANDIDATE rules leaves an engine that
 #    still places nothing: candidates trade nothing until a human promotes them, and promotion is
@@ -609,16 +676,36 @@ class ActionResult:
     message: str
 
 
+@dataclass(frozen=True)
+class ActionInput:
+    """One field an action needs from the operator.
+
+    `secret=True` means the value must never be rendered back into a page, echoed into a log, or
+    put in a URL. It is the difference between a `password` field and a `text` one, and between a
+    form that can be submitted safely and one that leaks its own contents into browser history."""
+
+    name: str
+    label: str
+    secret: bool = False
+
+
 @dataclass(frozen=True)
 class Action:
     key: str
     title: str
     #: What it will do, in the operator's words, shown before they choose it.
     detail: str
-    run: Callable[[Path, Path], ActionResult]
+    run: Callable[[Path, Path, dict[str, str]], ActionResult]
+    #: Empty for an action the machine performs unaided. Non-empty means it records what the
+    #: operator supplied and nothing else -- see rule 2 above.
+    inputs: tuple[ActionInput, ...] = ()
+
+    @property
+    def needs_input(self) -> bool:
+        return bool(self.inputs)
 
 
-def create_config(config_path: Path, db_path: Path) -> ActionResult:
+def create_config(config_path: Path, _db_path: Path, _values: dict[str, str]) -> ActionResult:
     """Write the PAPER template if there is no config. Never overwrites.
 
     Paper deliberately, and never the live template from here: paper places nothing at all, which
@@ -632,7 +719,7 @@ def create_config(config_path: Path, db_path: Path) -> ActionResult:
     return ActionResult("config", True, f"wrote {config_path} (paper -- places no orders)")
 
 
-def create_database(config_path: Path, db_path: Path) -> ActionResult:
+def create_database(_config_path: Path, db_path: Path, _values: dict[str, str]) -> ActionResult:
     """Create the database if absent and apply outstanding migrations.
 
     Migrations run every time, not only on first run, because that is what makes an upgrade
@@ -653,7 +740,7 @@ def create_database(config_path: Path, db_path: Path) -> ActionResult:
     return ActionResult("database", True, f"created {db_path} at the current schema")
 
 
-def seed_rule_library(config_path: Path, db_path: Path) -> ActionResult:
+def seed_rule_library(config_path: Path, db_path: Path, _values: dict[str, str]) -> ActionResult:
     """Seed one CANDIDATE rule per (kind, allowlisted product).
 
     Candidates trade nothing. Promoting one is a separate, deliberate, human step -- which is why
@@ -699,6 +786,51 @@ def seed_rule_library(config_path: Path, db_path: Path) -> ActionResult:
     )
 
 
+def store_market_data_credential(
+    config_path: Path, _db_path: Path, values: dict[str, str]
+) -> ActionResult:
+    """Record the CDP key and secret in the OS keychain.
+
+    Records EXACTLY what was submitted. Both fields are required and neither has a default: an
+    action that could fill in a field the operator left blank is one that could record something
+    they never supplied.
+
+    Nothing about the value reaches the result message, and `ResolvedSecret` refuses to print one
+    even if a future edit tries. The confirmation is read back through the same resolver a real
+    caller uses, so if a `.env` shadows what was just stored, the operator hears it now rather
+    than at the first fetch that used the other key.
+    """
+    from keel_core.secrets import SecretSource, read_secret, store_secret
+
+    missing = [name for name in MARKET_DATA_SECRETS if not values.get(name, "").strip()]
+    if missing:
+        return ActionResult(
+            "credentials", False, f"nothing saved -- {', '.join(missing)} was blank"
+        )
+
+    try:
+        for name in MARKET_DATA_SECRETS:
+            store_secret(name, values[name].strip())
+    except Exception as exc:
+        # The message names the `.env` alternative; see `keel_core.secrets.store_secret`.
+        return ActionResult("credentials", False, str(exc))
+
+    env_path = config_path.parent / ".env"
+    shadowed = [
+        name
+        for name in MARKET_DATA_SECRETS
+        if read_secret(name, env_path=env_path).source is not SecretSource.KEYCHAIN
+    ]
+    if shadowed:
+        return ActionResult(
+            "credentials",
+            True,
+            "saved to the OS keychain, but keel will still read "
+            f"{', '.join(shadowed)} from your .env or environment, which takes precedence",
+        )
+    return ActionResult("credentials", True, "saved to the OS keychain")
+
+
 #: THE closed set of steps a machine may perform on the operator's behalf.
 ACTIONS: tuple[Action, ...] = (
     Action(
@@ -725,6 +857,20 @@ def seed_rule_library(config_path: Path, db_path: Path) -> ActionResult:
         ),
         run=seed_rule_library,
     ),
+    Action(
+        key="credentials",
+        title="Save a market-data credential",
+        detail=(
+            "A free, read-only Coinbase Developer Platform key is enough for candle history. It "
+            "is stored in your operating system's keychain, not in a file, and keel never "
+            "displays it again."
+        ),
+        run=store_market_data_credential,
+        inputs=(
+            ActionInput("CDP_API_KEY", "CDP API key"),
+            ActionInput("CDP_API_SECRET", "CDP API secret", secret=True),
+        ),
+    ),
 )
 
 #: Mechanical steps that are deliberately NOT offered as one-click actions, and why. Recorded as

keel/web/render.py

@@ -90,6 +90,10 @@
   padding: 1rem 1.25rem; }
 pre { white-space: pre-wrap; word-break: break-word; margin: 0; font-size: 0.85rem; }
 form { margin: 0.5rem 0 0; }
+.field { display: flex; flex-direction: column; gap: 0.2rem; margin: 0.6rem 0; max-width: 26rem; }
+.field span { font-size: 0.8rem; color: var(--muted); }
+.field input { font: inherit; padding: 0.4rem 0.6rem; border-radius: 7px;
+  border: 1px solid var(--line); background: var(--bg); color: var(--fg); }
 button { font: inherit; font-weight: 550; padding: 0.35rem 0.9rem; border-radius: 7px;
   border: 1px solid var(--accent); background: var(--accent); color: var(--card);
   cursor: pointer; }
@@ -705,14 +709,31 @@ def render_setup(
 
 
 def _action_form(action: Any, csrf: str) -> str:
-    """One button, one action key, one write token. No hidden parameters: an action takes no
-    arguments at all, so there is nothing a crafted form could ask for that the registry does not
-    already fix."""
+    """One action key, one write token, and only the fields the action itself declares.
+
+    A field marked `secret` renders as `type="password"` and is NEVER given a `value` -- not even
+    on a re-render after a failure. Pre-filling a secret field puts the secret in the page source,
+    where it survives a screenshot, a "view source", and anything that saves the page. The cost is
+    that a failed submission must be retyped; that is the correct cost.
+
+    `autocomplete="off"` on the secret fields keeps a browser password manager from offering to
+    store an exchange API key as though it were a website login."""
+    fields = ""
+    for field in getattr(action, "inputs", ()):
+        kind = "password" if field.secret else "text"
+        extra = ' autocomplete="off" spellcheck="false"' if field.secret else ""
+        fields += (
+            '<label class="field">'
+            f"<span>{esc(field.label)}</span>"
+            f'<input type="{kind}" name="{esc(field.name)}" required{extra}>'
+            "</label>"
+        )
     return (
         f'<form method="post" action="/setup/{esc(action.key)}">'
         f'<input type="hidden" name="csrf" value="{esc(csrf)}">'
-        f'<button type="submit">{esc(action.title)}</button>'
         f'<div class="muted">{esc(action.detail)}</div>'
+        f"{fields}"
+        f'<button type="submit">{esc(action.title)}</button>'
         "</form>"
     )
 

keel/web/server.py

@@ -303,15 +303,20 @@ def page_glossary(_cfg: ServeConfig, _query: dict[str, list[str]]) -> tuple[str,
 SETUP_ACTION_PREFIX = "/setup/"
 
 
-def run_setup_action(cfg: ServeConfig, key: str) -> Any:
-    """Perform one declared mechanical action. Returns its `ActionResult`, or `None` for a key
-    that is not in the closed set -- never a lookup that falls through to something else."""
+def run_setup_action(cfg: ServeConfig, key: str, form: dict[str, str]) -> Any:
+    """Perform one declared setup action. Returns its `ActionResult`, or `None` for a key that is
+    not in the closed set -- never a lookup that falls through to something else.
+
+    Only the fields the action DECLARES are passed through. A submitted field the action did not
+    ask for is dropped rather than forwarded: the form is attacker-shaped input the moment anyone
+    can craft a POST, and an action should never receive a key it has no name for."""
     from keel.commands.setup import action_for
 
     action = action_for(key)
     if action is None:
         return None
-    return action.run(Path(cfg.config_path), Path(cfg.db_path))
+    values = {field.name: form.get(field.name, "") for field in action.inputs}
+    return action.run(Path(cfg.config_path), Path(cfg.db_path), values)
 
 
 def _close(repo: Any) -> None:
@@ -463,7 +468,7 @@ def do_POST(self) -> None:  # noqa: N802 - stdlib's naming, not ours
 
         key = parsed.path[len(SETUP_ACTION_PREFIX) :]
         try:
-            result = run_setup_action(self.cfg, key)
+            result = run_setup_action(self.cfg, key, body)
         except Exception as exc:
             self._refuse(500, "That step could not be completed", f"{type(exc).__name__}: {exc}")
             return
@@ -474,6 +479,11 @@ def do_POST(self) -> None:  # noqa: N802 - stdlib's naming, not ours
         # POST/redirect/GET: a browser reload must not re-submit. The actions are idempotent, so
         # a re-submission would be harmless -- but "harmless" is not a reason to leave a
         # re-submitting page in a setup flow someone is clicking nervously.
+        #
+        # The Location carries the step KEY and nothing else. A submitted value in a redirect URL
+        # is a secret in browser history, in the Referer header of anything the page later loads,
+        # and in any proxy log between here and nowhere -- which is the whole reason the form is a
+        # POST in the first place.
         self._send(303, "", extra=(("Location", f"/setup?ran={quote(result.step_key)}"),))
 
     def _read_form(self) -> dict[str, str]: