diff --git a/config.yaml b/config.yaml index 3d589682..d85bf9e1 100644 --- a/config.yaml +++ b/config.yaml @@ -35,12 +35,10 @@ market_data: auto_trade: mode: paper + # NOTE: currently UNUSED by any code path -- it is NOT a kill-switch and setting it + # true or false changes nothing. Use `keel kill` to halt trading. enabled: false interval_sec: 900 - # how long a `keel arm-bypass` token stays valid (seconds) before bypass mode requires - # re-arming (Issue #60) -- `keel agent --bypass` still needs a fresh armed token even with - # the CLI passphrase gate satisfied. - bypass_arm_ttl_sec: 3600 promotion: min_trades: 100 diff --git a/docs/go-live-runbook.md b/docs/go-live-runbook.md new file mode 100644 index 00000000..354787c7 --- /dev/null +++ b/docs/go-live-runbook.md @@ -0,0 +1,174 @@ +# Go-live runbook — the first supervised live order + +The purpose of this run is **not** profit. It is to prove the live execution path works, because +`place_order` has never been executed against the real Coinbase API. Everything up to now — +683 tests, a positive backtest, a passing verdict — says the *logic* is right. None of it says the +*plumbing* is. Treat the first order as an experiment whose result is "the order appeared on the +exchange, correctly, and we have a record of it". + +**Budget the whole thing as money you are willing to lose outright.** + +--- + +## 0. Before you start + +| | check | why | +|---|---|---| +| ☐ | `keel --version` reports `[release]`, no `DIRTY`, no `[checkout]` | a build that matches no commit is not reproducible; do not run it against funds | +| ☐ | you have a **trade-enabled** CDP key (the read-only one cannot place orders) | | +| ☐ | `.env` holds `CDP_API_KEY` / `CDP_API_SECRET`, and `.env` is git-ignored | credentials live only here — there is no vault | +| ☐ | you are at a real terminal | confirmation and every halt-releasing command fail closed off a TTY | +| ☐ | funds are in **USDC**, not USD | rail 13 vetoes a BUY that is not funded from `quote_currency`, and never draws from a bank/ACH | + +## 1. Bring the deployment up + +```bash +keel migrate # existing database: apply outstanding schema migrations +# or, on a brand-new deployment: +keel init # writes config.yaml and seeds the rule library as `candidate` +keel init-config --live # optional: the production config (mode: confirm) +``` + +Then check the config you are actually about to run: + +```bash +keel config show 2>/dev/null || grep -A3 '^auto_trade:' config.yaml +``` + +- `auto_trade.mode: confirm` — live, and asks before each order. +- `caps.max_exposure_usd` — set this to something you can lose. For a first run, small. +- `allowlist` — only assets you have screened and attested. + +## 2. Attest what the rails need + +Several rails **fail closed** until a human has attested something. This is deliberate: an +unattested venue or asset is treated as unknown, not as fine. + +```bash +keel assets list # what is attested, and what is not +keel subscription show # rail 14's spend allowance comes from this record +keel withdrawals show # rail 17: withdrawal capability is a compliance precondition +``` + +Attest anything missing (`keel assets attest`, `keel subscription attest`, +`keel withdrawals attest`) before continuing. If a rail vetoes later, its message names the +command that fixes it. + +## 3. Promote exactly one rule + +Seeded rules are `candidate` and trade nothing. Promote **one**, on **one** product: + +```bash +keel rules list +keel rules promote # candidate -> paper -> live +``` + +Promoting to `live` is what the promotion floor exists to gate. If you are deliberately +short-circuiting it for this test, know that you are — and promote a single rule, not the library. + +## 4. Run one cycle, in confirm mode, and watch + +```bash +keel agent +``` + +Autonomy is **off** by default, so this is what you should see: + +``` +Rails PASSED. Coinbase order preview: + order_total: 5.00 + ... +Place this order? [y/N]: +``` + +**Read the preview before answering.** It is the broker's own numbers, not keel's estimate. Check +the product, the side, and the total. If anything surprises you, answer `N` — declining places +nothing and costs nothing. + +The rails have already passed at this point. The prompt is an **additional** human gate, never a +replacement for them. + +## 5. Verify against reality + +Do not trust the tool's own success message alone. Check the exchange: + +```bash +keel pnl +keel rules list +``` + +- The order appears in the Coinbase UI/app with matching product, side and size. +- `orders` has a row with the real broker order id. +- The fill price is sane against the market at that moment. + +If the order did **not** appear but keel thinks it placed one, stop and investigate before +running anything else. That is the exact failure this run exists to catch. + +## 6. Halting + +```bash +keel kill # engage the kill-switch: halts all trading immediately +keel resume # release it -- asks for a typed "yes" at a terminal +``` + +The kill-switch is checked first on every cycle and **defaults to engaged**, so a damaged or +unreadable state halts trading rather than permitting it. + +**Five** commands re-permit trading after a halt. Each demands a typed `yes` from a terminal and +**cannot be run from a script or cron job** — a breaker that can reset itself is not a breaker. + +| command | releases | +|---|---| +| `keel resume` | the kill-switch | +| `keel resume-entries` | a consecutive-loss halt (rail 16) | +| `keel record-flow --amount ±N` | rebases rail 11's high-water mark | +| `keel reset-hwm` | rail 11's high-water mark, clearing a stuck drawdown halt | +| `keel withdrawals attest --enabled` | rail 17's entry halt | + +The de-risking direction of each is always allowed and needs no terminal: `keel kill`, +`keel withdrawals attest --suspended`, `keel autonomy off`. + +## 7. Only afterwards: autonomy + +Do **not** turn this on for the first run. + +```bash +keel autonomy show +keel autonomy on # asks for a typed "yes"; requires a terminal +keel autonomy off # always allowed, works anywhere, needs no terminal +``` + +Autonomy stops keel asking before each order. It changes **who is asked, never what is allowed** — +every hard rail still runs first, and it does **not** let the agent clear a safety halt. + +The setting lives in your profile in the database and is re-read **at the start of every cycle**. +So `keel autonomy off` takes effect on the **next cycle**, not the next restart — but note that +with `interval_sec: 900` a cycle already in flight can still place orders for up to 15 minutes. +**If you need trading to stop immediately, use `keel kill`, not `autonomy off`.** + +**Prefer a time-bounded session:** + +```bash +keel autonomy on --for-hours 4 # lapses on its own; nothing to remember +``` + +Without `--for-hours` autonomy has **no expiry** and stays on until you turn it off — a forgotten +`autonomy on` will still be trading unattended weeks later. `keel autonomy show` tells you which +you have and how long is left. + +Turn it on only once you have watched several supervised cycles behave correctly. + +--- + +## What can still go wrong + +- **`place_order` has never run against the real API.** This runbook is that test. Expect the + unexpected on the first attempt, and keep the size trivial. +- **A rail vetoes and you disagree.** Read the veto message; it names the rail and the command + that clears it. Do not work around a rail — the rails are un-overridable by design, including + in autonomous mode. +- **Confirm mode places nothing when run headless.** That is not a bug: with no TTY the + confirmation declines. Use a terminal, or turn autonomy on deliberately. +- **Equity moved because you deposited or withdrew.** Tell the tool (`keel record-flow --amount + ±N`), or rail 11 will read the movement as drawdown and veto entries on an account that lost + nothing. diff --git a/docs/superpowers/plans/2026-07-21-security-simplification.md b/docs/superpowers/plans/2026-07-21-security-simplification.md new file mode 100644 index 00000000..fce0f20e --- /dev/null +++ b/docs/superpowers/plans/2026-07-21-security-simplification.md @@ -0,0 +1,78 @@ +# Security simplification — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: superpowers:executing-plans. Steps use `- [ ]` checkboxes. + +**Goal:** Delete the vault and the passphrase gate; make autonomy a live-read profile choice. + +**Architecture:** A new single-row `profile` table (schema 6→7) holds `autonomous`, read live on every order decision. `auto_trade.mode` collapses to `paper|confirm`. The four halt-releasing commands swap the passphrase for an interactive typed confirmation. Rails are untouched throughout. + +**Tech Stack:** Python 3.12, click, sqlite3, pytest. + +## Global Constraints + +- **The 17 rails do not change.** This changes *who is asked*, never *what is allowed*. +- Every safety invariant in the spec's "Core safety invariants" gets a test. +- Autonomy fails closed (absent row ⇒ `False`); `autonomy on` needs a TTY; `autonomy off` never does. +- Autonomy **never** clears a safety halt. +- Full suite + `uv run ruff check keel tests packages scripts` green before each commit. + +--- + +### Task 1: `profile` table + Repository API (schema 6→7) + +**Files:** `keel/data/db.py`, `keel/data/repository.py`, `keel/types.py`, `tests/data/test_repository.py` + +**Produces:** `Profile(autonomous: bool, updated_ts: int)`; `Repository.get_profile() -> Profile`; `Repository.set_autonomous(value: bool, now_ts: int) -> None`; `db.SCHEMA_VERSION == 7`. + +- [ ] Failing tests: absent row ⇒ `autonomous False`; set/get round-trips; `set_autonomous` upserts (never a 2nd row); a v6 DB migrates to v7 and gains the table; `SCHEMA_VERSION == 7` literal tripwire. +- [ ] Run → FAIL. Implement table + `_migrate_v7_profile` + repo methods + `Profile`. Run → PASS. ruff. Commit. + +### Task 2: Delete the vault and the passphrase gate + +**Files:** delete `keel/security/`, `tests/security/`; `pyproject.toml`; `keel/cli.py` + +**Produces:** `_require_interactive_confirmation(action: str, detail: str) -> None` in `cli.py` (typed `yes`, fails closed off-TTY). + +- [ ] Failing tests: each of `resume`/`resume-entries`/`record-flow`/`reset-hwm` proceeds on typed `yes`, aborts on anything else, aborts with no TTY. +- [ ] Run → FAIL. Delete both modules + their tests + the `cryptography` dep; remove `_require_authz`, `--passphrase`, `--authz-path`, `DEFAULT_AUTHZ_PATH`; add `_require_interactive_confirmation` and wire the four commands. Run → PASS. ruff. Commit. + +### Task 3: Autonomy in the agent and executor + +**Files:** `keel/agent.py`, `keel/execution/executor.py`, `tests/test_agent.py`, `tests/execution/test_executor.py` + +**Consumes:** `Repository.get_profile()`. +**Produces:** `agent._effective_mode(config, repo) -> Literal["confirm","autonomous"]`; executor `mode: Literal["confirm","autonomous"]`. + +- [ ] Failing tests: `autonomous=True` places with no `confirm_fn`; `autonomous=False` still fails closed without one; **a rail veto never reaches placement in autonomous mode**; profile re-read per cycle (flip between two `run_once` calls); `mode: paper` places nothing even when autonomous; kill-switch short-circuits first. +- [ ] Run → FAIL. Replace `_confirm_or_bypass` with `_effective_mode`; retire `arm_bypass`/`is_bypass_armed`/`disarm_bypass` and `bypass_refused_reason`; rename the executor literal. Run → PASS. ruff. Commit. + +### Task 4: `keel autonomy on|off|show` + +**Files:** `keel/cli.py`, `tests/test_cli.py` + +- [ ] Failing tests: `show` prints off by default; `on` with typed `yes` enables and persists; `on` aborts off-TTY and on a non-`yes` answer; `off` disables **without** a TTY; **`on` does not let the four halt commands skip their confirmation**. +- [ ] Run → FAIL. Add the `autonomy` group; remove `arm-bypass`/`disarm-bypass`/`--bypass`. Run → PASS. ruff. Commit. + +### Task 5: Config — validate `mode`, drop `bypass_arm_ttl_sec` + +**Files:** `packages/keel-core/keel_core/config.py`, `config.yaml`, `keel/templates/config.yaml`, `keel/templates/config.live.yaml`, `tests/fixtures/config_golden_*`, `tests/test_config.py` + +- [ ] Failing tests: `mode: bypass` raises `ConfigError` naming the key; `paper`/`confirm` load; `bypass_arm_ttl_sec` is gone from the parsed config. +- [ ] Run → FAIL. Validate mode, drop the field, update all three configs (**root and dev template must stay byte-identical**) and the golden fixtures. Run → PASS. ruff. Commit. + +### Task 6: Docs — go-live runbook + spec §14 + +**Files:** `docs/go-live-runbook.md`, the main design spec's §14 + +- [ ] Write the runbook against the real flow: `.env` → `keel migrate`/`init` → promote a rule → `keel agent` (confirm) → one tiny supervised order → optional `keel autonomy on`. Amend §14 to record the removal and why. Commit. + +### Task 7: Verification + +- [ ] `uv run pytest -q` green; `uv run ruff check keel tests packages scripts` clean. +- [ ] Grep-prove the surface is gone: no `authz`, `save_vault`, `arm_bypass`, `bypass` outside history/docs. +- [ ] Smoke: `keel migrate` on a v6 DB; `keel autonomy show/on/off`; `keel autonomy on` piped (must refuse). + +## Self-Review + +- **Spec coverage:** §3.1→T2, §3.2→T2, §3.3→T1+T3+T5, §3.4→T4, §3.5→T6. Invariants 1,3,6→T3; 2→T4; 4→T4; 5→T3. +- **Placeholders:** none. **Type consistency:** `Profile`, `get_profile`, `set_autonomous`, `_effective_mode`, `_require_interactive_confirmation` used identically across tasks. diff --git a/docs/superpowers/specs/2026-07-15-keel-autotrade-design.md b/docs/superpowers/specs/2026-07-15-keel-autotrade-design.md index f519cf21..8c3b6cf8 100644 --- a/docs/superpowers/specs/2026-07-15-keel-autotrade-design.md +++ b/docs/superpowers/specs/2026-07-15-keel-autotrade-design.md @@ -272,16 +272,27 @@ Enforced in `guards.py` **before every order**, **un-overridable in any mode (in losable slice (§22.3). - **No classic user authentication.** For a local single-user CLI/daemon, logins/accounts/sessions are YAGNI (OS access already opens that door). We invest instead in the two controls below. -- **Portable encrypted secrets vault** (`security/secrets.py`): the CDP key/secret (and any remote-control token) - live in an **AES-GCM-encrypted `secrets.enc`** unlocked by a **master passphrase** (passphrase → scrypt KDF → - key). One file, **copyable between machines** (explicitly *not* the machine-bound OS Keychain, so it's - transportable). `chmod 600`; secrets never logged; `.enc`/`.env` git-ignored. An optional per-machine keychain - *cache* of the derived key may be added for convenience, but the portable `.enc` is the source of truth. -- **Dangerous-action authorization gate** (`security/authz.py`): a passphrase (stored as a scrypt hash, - rate-limited) required **only** to **arm bypass/autonomous mode**, **raise caps above config maxima**, or - **disable the kill-switch / resume**. Read-only commands and confirm-mode trades need none. This is local - *authorization*, not a login — it forces intentionality and blocks casual/accidental/shoulder-surf misuse of - the autonomy capability. +- ⛔ **SUPERSEDED 2026-07-21 — the vault and the passphrase gate were BUILT and then REMOVED.** + See `2026-07-21-security-simplification-design.md`. Both are gone; do not re-propose them without + reading that spec first. What replaced them: + - **Credentials come from a git-ignored `.env`** (`config.load_secrets`). The encrypted + `secrets.enc` vault (`security/secrets.py`) was deleted. It had in fact never been on the live + path: `cb_client`/`cli` always loaded credentials from `.env`, and the vault was reachable only + via `migrate_from_env` — so it was a *competing* credential path, not the real one. + - **The scrypt passphrase gate (`security/authz.py`) was deleted.** Of its four declared + actions, `raise_caps` and `unlock_vault` were never used at all. Once placing a real, + money-spending order needs only a typed confirmation, requiring a remembered secret to reset a + high-water mark is ceremony without a matching threat model. One rule replaced it: **dangerous + actions need a human at a terminal; nothing needs a stored secret.** The four halt-releasing + commands (`resume`, `resume-entries`, `record-flow`, `reset-hwm`) demand a typed `yes` and fail + closed off a TTY, with deliberately no env/flag override (any such seam would be settable from + cron and would defeat the fail-closed). + - **Autonomy is a profile choice, not a config mode.** `auto_trade.mode` is now `paper|confirm`; + the `profile` table (schema v7) holds the user's `autonomous` flag, re-read live on every order + so `keel autonomy off` binds on the *next* order. It fails closed on an absent row, cannot be + enabled without a terminal, and **never clears a safety halt**. + - Real user authentication arrives with the future **server-hosted** deployment; a local + single-user vault + passphrase was ceremony in the meantime. - **Honest boundary:** on a single-user machine the real security boundary is the **OS account + full-disk encryption**. These controls are defense-in-depth (reduce plaintext exposure, prevent casual misuse); they do **not** stop an attacker who already holds the OS account. Stated so they aren't mistaken for more. diff --git a/docs/superpowers/specs/2026-07-21-security-simplification-design.md b/docs/superpowers/specs/2026-07-21-security-simplification-design.md new file mode 100644 index 00000000..2a692440 --- /dev/null +++ b/docs/superpowers/specs/2026-07-21-security-simplification-design.md @@ -0,0 +1,234 @@ +# Security simplification — design + +**Date:** 2026-07-21 +**Status:** Approved design (pending user review) +**Workstream:** A of three. Items 1 and 2 of the 2026-07-21 requirements. + +## Context + +`keel` grew two local security mechanisms from the original spec §14: + +- an **encrypted secrets vault** (`keel/security/secrets.py`) — AES-GCM over a scrypt-derived key, + holding CDP credentials in `secrets.enc`; +- a **dangerous-action passphrase gate** (`keel/security/authz.py`) — scrypt-hashed passphrase with + rate limiting, gating `{arm_bypass, raise_caps, disable_killswitch, unlock_vault}`. + +Both are being removed. The app will be **server-hosted with real user authentication** in the +future; between here and there, a local single-user vault and passphrase buy little. The honest +assessment was already in `authz.py`'s own docstring: it "does not stop an attacker who already +holds the OS account." + +**Two findings materially shrink this work:** + +1. **The vault is already dead code.** `data/cb_client.py` and `cli.py` both load credentials via + `config.load_secrets()` from a git-ignored `.env`. The vault is reachable only through + `migrate_from_env`. Removing it deletes a *competing* credential path; it does not change how + credentials are actually loaded today. (PR #115, which would have wired it up, was closed.) +2. **PR #117 (merged) already did half of item 2.** Confirm mode now places orders via an + interactive prompt with no passphrase and no bypass token, rails running first, failing closed + on a non-TTY. What remains is the *autonomous* half. + +## Goals + +1. **Item 1 — `.env` only.** Delete the vault. Credentials come from a git-ignored `.env`. +2. **Item 2 — autonomy is a profile choice.** Confirmation before every order is the default. When + the user turns **autonomous mode** on, there is no passphrase and no per-order confirmation. The + choice is **tracked in the user's profile**, persisted in the database and read live. +3. **Delete the passphrase gate entirely**, re-gating the four halt-releasing commands with the + same interactive confirmation #117 established for orders. + +## Non-goals + +- No user authentication, accounts, or sessions — those arrive with the hosted deployment. +- **No change to the 17 hard rails.** They remain un-overridable in every mode. This work changes + *who is asked*, never *what is allowed*. +- No change to the kill-switch's fail-closed default. + +## Core safety invariants (must hold after this work) + +These are the properties the removed ceremony was standing in for. Each gets a test. + +1. **Rails run first, always.** Guards veto before any preview or placement, in every mode. +2. **Autonomy never clears a safety halt.** `resume`, `resume-entries`, `record-flow` and + `reset-hwm` require an interactive confirmation **even when autonomous mode is on**. "Trade + without asking me" and "un-stick your own drawdown breaker" are different powers. A rail that + fired because something went wrong still needs a human to clear it — otherwise a breaker can + silently reset itself and the rail stops meaning anything. +3. **Autonomy fails closed.** An absent/unreadable/damaged profile row reads as + `autonomous = False` — `get_profile` catches `sqlite3.Error` rather than relying on the caller + crashing first. +3a. **Autonomy may be time-bounded.** The removed bypass-arm token was TTL-limited so a forgotten + arm could not grant unattended trading forever. `keel autonomy on --for-hours N` restores that + bound; the default is a durable choice (per the requirement that it be tracked as a user + preference), and `autonomy on` says plainly when no expiry is set. +4. **Autonomy cannot be enabled non-interactively.** `keel autonomy on` requires a TTY and an + explicit typed confirmation, so a script, cron job or piped command can never arm it. +5. **The kill-switch still short-circuits everything**, checked first, defaulting to engaged. +6. **Paper mode never places**, regardless of the profile flag. + +--- + +## Design + +### 3.1 Delete the vault (item 1) + +Remove `keel/security/secrets.py` and `tests/security/test_secrets.py`. Drop the now-unused +`cryptography>=49.0.0` dependency from `pyproject.toml` (verified: used nowhere else in `keel`, +`packages`, or `tests`). `config.load_secrets()` is unchanged and remains the single credential +path. + +### 3.2 Delete the passphrase gate, re-gate the four halt-releasing commands + +Remove `keel/security/authz.py` and `tests/security/test_authz.py`. With both files gone the +`keel/security/` package has no remaining contents and is deleted entirely. + +Of the four declared dangerous actions, `raise_caps` and `unlock_vault` were **never wired to any +command** — the gate was declared for them and never applied. ⚠️ Note the premise is narrower than +it first looks: `raise_caps` was never *enforced*, but capability-raising commands do exist and +remain ungated — `keel subscription set/attest` raises rail 14's spend allowance at runtime, and +`keel assets attest` admits an asset. Neither is a regression (both were ungated before this work), +but neither should be mistaken for "caps cannot be raised at runtime". `arm_bypass` disappears with bypass mode. That leaves +four *commands*, all one idea — re-permitting trading after a safety halt: + +| command | what it releases | +|---|---| +| `resume` | the kill-switch | +| `withdrawals attest --enabled` | rail 17's entry halt (⚠️ found in review — its old justification cited the confirm gate and the bypass-arm token, both of which this work removes) | +| `resume-entries` | an armed consecutive-loss halt (rail 16) | +| `record-flow` | declares an external deposit/withdrawal so rail 11 isn't fooled | +| `reset-hwm` | rail 11's equity high-water mark, clearing a stuck drawdown halt | + +Each gains `_require_interactive_confirmation(action, detail)`: prints what is about to be +released and demands an explicit typed `yes` (not bare `y` — these are rarer and heavier than an +order confirmation), and **fails closed on a non-TTY**. `--passphrase` options and `--authz-path` +are removed from the CLI. + +**Rationale for parity with orders:** once placing a real money-spending order needs only a typed +confirmation, requiring a remembered secret to reset a high-water mark is ceremony without a +matching threat model. One rule — *dangerous actions need a human at a terminal; nothing needs a +stored secret* — is easier to reason about, and to audit, than two. + +### 3.3 Autonomy as a live-read profile choice (item 2) + +**New `profile` table** (schema 6 → 7, via the existing incremental `db.migrate` chain): + +```sql +CREATE TABLE IF NOT EXISTS profile ( + id INTEGER PRIMARY KEY CHECK (id = 1), -- single row today + autonomous INTEGER NOT NULL DEFAULT 0, + updated_ts INTEGER NOT NULL +); +``` + +`CHECK (id = 1)` keeps it a single row deliberately rather than by accident; a `user_id` column is +the obvious seam when the hosted, multi-user deployment arrives. + +**Repository API:** + +- `get_profile() -> Profile` — returns `Profile(autonomous: bool, updated_ts: int)`. **An absent + row returns `autonomous=False`**, so a fresh or damaged database fails closed. +- `set_autonomous(value: bool, now_ts: int) -> None` — upserts the single row. + +**Read live, every cycle.** `agent` calls `repo.get_profile()` on each order decision and never +caches it, so `keel autonomy off` takes effect on the **next order**, not the next restart. This +mirrors rail 14's monthly allowance, which was deliberately moved from config to the database for +exactly this property. + +**Why not `config.yaml`:** it is now a *shipped release asset* in confirm mode. Arming unattended +trading should not be a YAML line that travels between machines or gets pasted from a gist. Why +not `agent_state`: that holds operational state (kill-switch, open positions); this is a durable +user preference. + +**Mode vocabulary.** `auto_trade.mode` collapses from `paper | confirm | bypass` to +**`paper | confirm`**, and is now **validated** (an unknown value raises `ConfigError` naming the +key — today it silently falls back). `bypass_arm_ttl_sec` is removed. Effective behaviour: + +| `mode` | `profile.autonomous` | result | +|---|---|---| +| `paper` | *(ignored)* | simulated; places nothing | +| `confirm` | `false` *(default)* | live; prompts before every order | +| `confirm` | `true` | live; places without prompting | + +One switch for *is this real money*, a separate one for *do you ask me* — rather than one enum +conflating both. The shipped `config.yaml` needs no change to stay safe: autonomy is off until +someone deliberately turns it on. + +⚠️ **Breaking:** a config carrying `mode: bypass` or `bypass_arm_ttl_sec` now fails to load. That +is deliberate — failing loudly beats silently reinterpreting a config that requested autonomy. + +**Removed machinery:** `Repository.arm_bypass` / `is_bypass_armed` / `disarm_bypass`, the +`agent_state` bypass token, `agent._confirm_or_bypass`, `LoopResult.bypass_refused_reason`, and +the `keel arm-bypass` / `disarm-bypass` commands and `agent --bypass` flag. + +**Executor:** `mode: Literal["confirm", "bypass"]` becomes `Literal["confirm", "autonomous"]`. +`"autonomous"` places without consulting `confirm_fn`; `"confirm"` still requires an approving +`confirm_fn` and fails closed without one. Guard ordering is untouched. + +**Agent:** `_confirm_or_bypass` is replaced by `_effective_mode(config, repo) -> str`, which +returns `"autonomous"` only when `config.auto_trade.mode == "confirm"` **and** +`repo.get_profile().autonomous` is true; anything else is `"confirm"`. Paper mode is handled +upstream as today. + +### 3.4 CLI: `keel autonomy` + +``` +keel autonomy show # prints on/off and when it was last changed +keel autonomy on # interactive: explains the consequences, demands a typed "yes" +keel autonomy off # ungated -- de-risking is always allowed (§5 asymmetry) +``` + +`on` is a dangerous action: it prints the current mode, caps and allowlist, states plainly that +orders will be placed without asking, and requires a typed `yes` on a TTY. `off` is deliberately +ungated and needs no TTY — the asymmetry principle that runs through this project is that +*reducing* risk should never be obstructed, while *increasing* it goes through a gate. + +### 3.5 Rewrite the go-live runbook + +`docs/go-live-runbook.md` (proposed in the unmerged PR #116) documents the old +`arm-bypass`/`--passphrase` dance, which #117 already invalidated and this work removes entirely. +It is rewritten here against the real flow: `.env` → `keel migrate`/`init` → promote a rule → +`keel agent` in confirm mode → approve one tiny supervised order → optionally `keel autonomy on`. +**PR #116 is superseded and should be closed** rather than merged. + +--- + +## Components & interfaces + +| File | Change | +|---|---| +| `keel/security/` | **deleted** (both `secrets.py` and `authz.py`; package removed) | +| `tests/security/` | **deleted** | +| `pyproject.toml` | drop `cryptography` | +| `keel/data/db.py` | `SCHEMA_VERSION = 7`; `profile` table + `_migrate_v7_profile` | +| `keel/data/repository.py` | `get_profile()`, `set_autonomous()`; remove the three bypass methods | +| `keel/types.py` | `Profile` dataclass | +| `keel/agent.py` | `_effective_mode` replaces `_confirm_or_bypass`; drop `bypass_refused_reason` | +| `keel/execution/executor.py` | mode literal `confirm`/`autonomous` | +| `keel/cli.py` | `autonomy` group; `_require_interactive_confirmation`; remove authz/bypass surface | +| `packages/keel-core/keel_core/config.py` | validate `mode ∈ {paper, confirm}`; drop `bypass_arm_ttl_sec` | +| `config.yaml`, `keel/templates/config.yaml`, `keel/templates/config.live.yaml` | drop `bypass_arm_ttl_sec` + its comment (root and dev template must stay byte-identical) | +| `tests/fixtures/config_golden_*.{json,yaml}` | drop `bypass_arm_ttl_sec` | +| `docs/go-live-runbook.md` | written fresh (supersedes #116) | +| `docs/superpowers/specs/...-halal-cb-autotrade-design.md` | §14 amended to record the removal and why | + +## Testing + +Beyond updating existing tests, each safety invariant in §"Core safety invariants" gets a test: + +- a rail-vetoed order never reaches placement in **autonomous** mode (rails first); +- each of the four halt-releasing commands still demands confirmation **with `autonomous=true`**; +- absent profile row ⇒ `autonomous False`; +- `autonomy on` refuses without a TTY; `autonomy off` works without one; +- profile is re-read per cycle (flip it between two `run_once` calls, observe the change); +- `mode: paper` places nothing with `autonomous=true`; +- kill-switch engaged short-circuits before any of it; +- `mode: bypass` in a config now raises `ConfigError` naming the key; +- a v6 database migrates to v7 and gains a `profile` row default-off. + +## Migration notes for the operator + +- Delete any `secrets.enc` and `authz.json` — both are now ignored. (Neither is read; no automatic + deletion is performed, since removing files the user owns is not this tool's business.) +- Remove `bypass_arm_ttl_sec` from `config.yaml`, and change `mode: bypass` to `mode: confirm` + plus `keel autonomy on`. +- Run `keel migrate` to add the `profile` table. diff --git a/keel/agent.py b/keel/agent.py index 261290b4..d1e78c47 100644 --- a/keel/agent.py +++ b/keel/agent.py @@ -3,7 +3,7 @@ `run_once()` is a single cycle: poll fresh candles (`data.market_feed.poll_once`), reconstruct the `live` rules (`repo.get_rules("live")` -> real `Rule` instances, `RULE_REGISTRY` below), `strategy.engine.evaluate` them for ENTER `Signal`s, drive EXIT `Signal`s off currently-held -positions, and run every signal through `execution.executor.execute` (confirm|bypass, from +positions, and run every signal through `execution.executor.execute` (confirm|autonomous, from `config.auto_trade.mode`) -- respecting the kill-switch (`repo.get_state("kill_switch")`) throughout. `loop()` is the scheduled wrapper: call `run_once` every `interval_sec` until `stop_flag()` returns `True`. @@ -34,22 +34,19 @@ only ever deals in real `Rule` instances, never in raw JSON dicts. **Confirm mode places via a caller-supplied `confirm_fn`.** `run_once`/`loop` take an optional -to `(broker, repo, config, now_ts)` -- there is no `confirm_fn` slot for a human-in-the-loop -approval callback. So in `mode="confirm"`, every `executor.execute` call is made with -`confirm_fn=None`, which -- per `executor.execute`'s own contract -- fails closed: the order is -previewed and logged but never placed. That is the intended behavior, not an oversight: wiring -an actual interactive confirm prompt is the CLI's job (Task 9's `agent --loop --confirm`), which -can drive `run_once`/`loop` with a real `confirm_fn` some other way, or review the previewed-but- -unplaced orders out of band. `mode="bypass"` places without a prompt, still subject to every -guard rail (un-overridable, per `guards.check`). - -**Bypass requires an armed token (Issue #60).** `mode="bypass"` additionally requires -`repo.is_bypass_armed(now_ts)` -- an explicit, authenticated, time-limited arm set by -`Repository.arm_bypass` (wired to the CLI's passphrase-gated `keel arm-bypass` command). This -check is enforced by `_confirm_or_bypass` inside `run_once` itself, not only at the CLI, so a -caller invoking `run_once`/`loop` directly with `config.auto_trade.mode == "bypass"` cannot skip -it -- an unarmed or expired request fails safe by falling back to `"confirm"` (which places -nothing without a `confirm_fn`) rather than trading autonomously. +`confirm_fn`, which `executor.execute` calls with the broker preview. With `confirm_fn=None` -- +the default, and what any caller that does not supply one gets -- confirm mode **fails closed**: +the order is previewed and logged but never placed. The CLI supplies a real interactive prompt. + +**Autonomy is a profile choice, not a config mode.** `_effective_mode` returns `"autonomous"` +only when `config.auto_trade.mode == "confirm"` **and** +`repo.get_profile().is_autonomous(now_ts)` is true. +The profile is read fresh every cycle and never cached, so `keel autonomy off` takes effect on +the next cycle rather than the next restart (a cycle in flight can still place; `keel kill` is +what stops trading immediately). The check lives inside `run_once`, not only at the +CLI, so an in-process caller cannot obtain autonomy the CLI would have refused; an absent or +unreadable profile row reads as not-autonomous. In every mode, `guards.check` runs FIRST and is +un-overridable -- autonomy changes who is asked, never what is allowed. """ from __future__ import annotations @@ -554,42 +551,33 @@ class LoopResult: enter_signals: list[Signal] = field(default_factory=list) enter_results: list[ExecutionResult] = field(default_factory=list) exit_results: list[ExecutionResult] = field(default_factory=list) - # Issue #60 (bypass-arm hardening): non-`None` iff `config.auto_trade.mode == "bypass"` was - # requested but `repo.is_bypass_armed(now_ts)` was `False` -- `mode` above then reports the - # *effective* mode actually used (always `"confirm"` in that case), and this field explains - # why the fallback happened. `None` whenever bypass wasn't requested, or was armed and ran. - bypass_refused_reason: str | None = None - - -def _confirm_or_bypass(config: Config, repo: Repository, now_ts: int) -> tuple[str, str | None]: - """The effective executor mode for this cycle, plus an optional bypass-refusal reason. - - `AutoTradeConfig.mode` defaults to `"paper"` (a Phase-1 placeholder, spec §21) which isn't - an `executor.execute` mode at all -- per the Global Constraints, confirm mode is *always* - the default, so anything other than an explicit `"bypass"`/`"confirm"` falls back to - `"confirm"` rather than raising or silently bypassing rails. - - **Issue #60 (bypass-arm hardening).** A request for `"bypass"` is only honored if - `repo.is_bypass_armed(now_ts)` is `True` -- an explicit, authenticated, time-limited arm - (`keel arm-bypass`, passphrase-gated, or any other caller that has called - `repo.arm_bypass`) recently granted it. This check lives here, inside `run_once`'s own call - path, specifically so it cannot be bypassed by a caller that invokes `agent.run_once` - in-process with `config.auto_trade.mode == "bypass"` and skips the CLI's passphrase gate - entirely -- the CLI gate (`cli._require_authz`) is defense-in-depth, this is the - un-overridable second layer. An unarmed/expired request fails safe: it falls back to - `"confirm"` (which, with no `confirm_fn`, places nothing -- see `executor.execute`'s own - contract) rather than raising or silently proceeding, and the second return value carries a - human-readable reason a caller can log or surface. + + +def _effective_mode(config: Config, repo: Repository, now_ts: int) -> str: + """The executor mode for this cycle: `"autonomous"` or `"confirm"`. + + Two independent switches, deliberately not conflated into one enum: + + * `config.auto_trade.mode` says whether this is real money at all (`paper` never reaches an + executor mode -- it routes to the paper path upstream). + * `repo.get_profile().is_autonomous(now_ts)` says whether the user has opted out of being + asked -- honouring any expiry the user set. + + `"autonomous"` is returned ONLY when the config is live (`confirm`) **and** the profile says + so. Anything else -- an unknown mode, an absent profile row, a damaged database -- yields + `"confirm"`, which with no `confirm_fn` places nothing at all. The failure direction is + always toward asking a human. + + **The profile is read here, fresh, once per cycle and never cached**, so `keel autonomy off` + takes effect on the NEXT order rather than the next restart. This mirrors rail 14's + allowance, which is re-read live for exactly the same reason. + + This lives inside `run_once`'s own call path, not only at the CLI, so a caller driving + `run_once` in-process cannot obtain autonomy the CLI would have refused. """ - mode = config.auto_trade.mode - if mode not in ("confirm", "bypass"): - return "confirm", None - if mode == "bypass" and not repo.is_bypass_armed(now_ts): - return "confirm", ( - "bypass mode requested but not armed (no active `keel arm-bypass` token, or it " - "expired) -- falling back to confirm mode; no order will be placed" - ) - return mode, None + if config.auto_trade.mode != "confirm": + return "confirm" + return "autonomous" if repo.get_profile().is_autonomous(now_ts) else "confirm" def run_once( @@ -641,15 +629,11 @@ def run_once( # for recording that the check happened, independent of whether it found anything new. repo.set_state("last_feed_ts", now_ts) - mode, bypass_refused_reason = _confirm_or_bypass(config, repo, now_ts) - # `mode: paper` is not an executor mode (see `_confirm_or_bypass`); it routes to the + mode = _effective_mode(config, repo, now_ts) + # `mode: paper` is not an executor mode (see `_effective_mode`); it routes to the # PAPER path instead, which never touches the broker. Constructed once per cycle and # rehydrated from the orders table, so a per-cycle agent resumes its open positions. paper_trader = PaperTrader(repo) if config.auto_trade.mode == "paper" else None - if bypass_refused_reason is not None: - log_event( - logger, logging.WARNING, "agent.bypass_refused", reason=bypass_refused_reason - ) log_event(logger, logging.INFO, "agent.mode_resolved", mode=mode) max_age_sec = config.auto_trade.interval_sec * FEED_STALENESS_CYCLES finest = _finest_granularity(granularities) @@ -791,7 +775,6 @@ def run_once( enter_signals=enter_signals, enter_results=enter_results, exit_results=exit_results, - bypass_refused_reason=bypass_refused_reason, ) finally: unbind_cycle(cycle_token) diff --git a/keel/cli.py b/keel/cli.py index 03c2dfca..365f0ccf 100644 --- a/keel/cli.py +++ b/keel/cli.py @@ -1,8 +1,8 @@ """keel command-line interface (P3 Task 9). Wires the merged Phase 1-3 modules into a `click` CLI: `db import` (`data.csv_import.import_dir`), -`monitor` (`data.market_feed`), `agent` (`agent.run_once`/`agent.loop`), `arm-bypass`/ -`disarm-bypass` (the `agent_state` bypass-arm token `agent.run_once` itself enforces, Issue #60), +`monitor` (`data.market_feed`), `agent` (`agent.run_once`/`agent.loop`), `autonomy on|off|show` +(the `profile` row `agent.run_once` itself re-reads each cycle), `rules list|backtest|promote|demote|disable|seed` (`data.repository` + `strategy.backtest`/ `promotion`; `seed` populates the otherwise-empty `rules` table from `agent.RULE_REGISTRY`, Issue #81), `pnl` (`analysis.pnl`), `kill`/`resume` (the `agent_state` kill-switch), @@ -12,39 +12,32 @@ `subscription attest|set|show` (the per-venue, user-attested allowance rail 14 reads live), and a Phase-4 `insights` stub. -**Dangerous commands are gated.** Per the main spec §14 and `security.authz`, only -`{arm_bypass, raise_caps, disable_killswitch, unlock_vault}` require the passphrase gate. In this -CLI's surface that's `agent --bypass` and `arm-bypass` (both map to the `arm_bypass` action) and -`resume`, `resume-entries`, `record-flow` and `reset-hwm` (all `disable_killswitch` -- -each either releases a tripped breaker or rebases the state one reads) -- `kill` -(engaging the kill-switch) and `disarm-bypass` are -*safe* actions (they only ever reduce capability) and are always allowed; every other command -here is read-only or a local rules-table/DB mutation with no live-trading blast radius, so per -the plan ("read-only commands require no passphrase") they are not gated. There is no CLI surface -for `raise_caps`/`unlock_vault` in this task -- caps are config-file-only (no runtime override -exists in the merged `execution.guards`/`config` modules) and the vault (`security.secrets`) has -no CLI command in this task's scope. - -**`agent --bypass`'s passphrase gate is not sufficient on its own (Issue #60).** It only guards -this CLI entry point -- any other caller invoking `agent.run_once`/`agent.loop` in-process with -`config.auto_trade.mode == "bypass"` would bypass it entirely. `agent.run_once` therefore also -requires a separately armed, time-limited token (`Repository.is_bypass_armed`, set by -`arm-bypass`) before it will actually run in bypass mode -- an unarmed or expired request fails -safe to confirm-mode behavior (nothing placed) regardless of how `run_once` was invoked. Both -gates apply to the CLI path (defense in depth); only the arm-token check is un-bypassable -in-process. - -**No interactive hangs in tests.** `--passphrase` may be passed explicitly; if omitted, it is only -read via an interactive `click.prompt` when stdin is a real TTY (`_resolve_passphrase`) -- -non-interactive invocations (like `CliRunner`) get an empty passphrase, which fails the gate -closed rather than blocking on stdin. +**Dangerous commands ask a human; nothing needs a stored secret.** The former scrypt passphrase +gate is gone (see `2026-07-21-security-simplification-design.md`). Five commands re-permit trading +after a safety halt -- `resume`, `resume-entries`, `record-flow`, `reset-hwm` and +`withdrawals attest --enabled` (rail 17) -- and each +demands a typed `yes` via `_require_interactive_confirmation`, **failing closed off a TTY** so a +script or cron job can never release a halt. `kill` (engaging the kill-switch) and `autonomy off` +are *safe* actions -- they only ever reduce capability -- and are always allowed, from anywhere. +Every other command here is read-only or a local rules-table/DB mutation with no live-trading +blast radius. + +**Autonomy is a profile choice, checked in-process.** `keel autonomy on` (typed `yes`, TTY +required) sets `profile.autonomous`; `agent.run_once` re-reads it every cycle via +`_effective_mode`, so the check cannot be skipped by a caller driving `run_once` in-process, and +turning autonomy off binds on the next cycle. It changes who is asked, never what is allowed: +`guards.check` runs first in every mode, and autonomy never releases a halt. + +**No interactive hangs in tests.** `_is_interactive()` is the single TTY predicate, with +deliberately no env-var or flag override -- any such seam would be settable from cron and would +defeat every fail-closed built on it. Tests patch the predicate. **Disclaimer.** Every command prints the halal + not-financial/religious-advice disclaimer footer (`with_disclaimer`, a decorator applied to every command's callback) -- always, even when -the command errors out or is refused by the authz gate. +the command errors out or is refused at a confirmation prompt. **No live network in tests.** `_build_broker` is the one seam that would construct a real -`CoinbaseClient` (from `.env`/vault secrets via `coinbase.rest.RESTClient`); tests monkeypatch it +`CoinbaseClient` (from `.env` secrets via `coinbase.rest.RESTClient`); tests monkeypatch it to inject a fake broker instead, exactly like `tests/test_agent.py`'s `FakeBroker`. """ @@ -84,7 +77,6 @@ from keel.research import deflate as deflate_mod from keel.research import ledger as trials_ledger from keel.research import matrix as matrix_mod -from keel.security import authz from keel.sim import artifact as artifact_mod from keel.sim import benchmark as benchmark_mod from keel.sim import metrics as metrics_mod @@ -102,13 +94,17 @@ "You are solely responsible for your own trading decisions." ) +#: Upper bound on `keel autonomy on --for-hours` (1 year). Guards against inf/nan/overflow +#: and against a "window" so long it is indistinguishable from no expiry at all. +_MAX_AUTONOMY_HOURS = 8760.0 +#: One second. Below this the window rounds to zero and we would write an already-lapsed row +#: while printing "autonomy ON until ..." -- fails safe, but the message would be a lie. +_MIN_AUTONOMY_HOURS = 1.0 / 3600.0 + DEFAULT_DB_PATH = "keel.db" DEFAULT_CONFIG_PATH = "config.yaml" -DEFAULT_AUTHZ_PATH = "authz.json" # The dangerous-action names this CLI's gated commands map to (see module docstring). -_ARM_BYPASS = "arm_bypass" -_DISABLE_KILLSWITCH = "disable_killswitch" # `rules demote` steps a rule back one lifecycle stage; `disabled` is terminal (see # `strategy.promotion`'s own `_PROMOTE_NEXT` docstring) and so is not a demote target. @@ -136,30 +132,39 @@ def wrapper(*args: Any, **kwargs: Any) -> Any: return wrapper -# -- passphrase resolution (no interactive hangs under CliRunner) ----------------------------- +# -- interactive confirmation (no interactive hangs under CliRunner) ----------------------------- -def _resolve_passphrase(passphrase: str | None) -> str: - """`passphrase` if given; else an interactive prompt on a real TTY; else `""` (fails closed). +def _is_interactive() -> bool: + """True when a human is at a terminal. - A non-interactive invocation (tests, scripts, CI) never blocks on stdin -- it simply gets an - empty passphrase, which `authz.verify`/`authz.require` correctly reject. + Deliberately has NO env-var or flag override: any such seam would be settable from cron and + would defeat the fail-closed behaviour of every gate built on it. Tests patch this predicate. """ - if passphrase: - return passphrase - if sys.stdin is not None and sys.stdin.isatty(): - return click.prompt("Passphrase", hide_input=True) - return "" + return sys.stdin is not None and sys.stdin.isatty() -def _require_authz(ctx: click.Context, action: str, passphrase: str | None) -> None: - """Enforce the authz gate for `action`, aborting the command (exit 1) on denial.""" - resolved = _resolve_passphrase(passphrase) - try: - authz.require(action, resolved, path=ctx.obj["authz_path"]) - except authz.AuthzError as exc: - click.echo(f"Error: {exc}", err=True) - ctx.exit(1) +def _require_interactive_confirmation(action: str, detail: str) -> None: + """Demand an explicit typed `yes` from a human at a terminal before a dangerous action. + + This replaces the former scrypt passphrase gate. Once placing a real, money-spending order + needs only a typed confirmation, requiring a remembered secret to release a safety halt is + ceremony without a matching threat model -- on a single-user machine the honest boundary is + the OS account, as the old gate's own docstring conceded. One rule ("dangerous actions need a + human at a terminal; nothing needs a stored secret") is easier to reason about and to audit. + + Demands the full word `yes` rather than a bare `y`: these actions are rarer and heavier than + an order confirmation. **Fails closed off a TTY**, so cron jobs, pipes and scripts can never + release a halt. + """ + if not _is_interactive(): + raise click.ClickException( + f"refusing to {action}: this needs confirmation from an interactive terminal." + ) + click.echo(f"About to {action}.") + click.echo(f" {detail}") + if click.prompt('Type "yes" to confirm', default="", show_default=False).strip() != "yes": + raise click.ClickException("aborted (confirmation not given).") # -- DB / config / broker construction --------------------------------------------------------- @@ -247,12 +252,6 @@ def _print_version(ctx: click.Context, param: object, value: bool) -> None: show_default=True, help="config.yaml path.", ) -@click.option( - "--authz-path", - default=DEFAULT_AUTHZ_PATH, - show_default=True, - help="Passphrase-gate state file (see `keel.security.authz`).", -) @click.option( "--verbose", "-v", @@ -263,13 +262,12 @@ def _print_version(ctx: click.Context, param: object, value: bool) -> None: ) @click.pass_context def cli( - ctx: click.Context, db_path: str, config_path: str, authz_path: str, verbose: bool + ctx: click.Context, db_path: str, config_path: str, verbose: bool ) -> None: """keel: an offline-first, halal, guard-railed Coinbase auto-trading agent.""" ctx.ensure_object(dict) ctx.obj["db_path"] = db_path ctx.obj["config_path"] = config_path - ctx.obj["authz_path"] = authz_path ctx.obj["verbose"] = verbose @@ -370,7 +368,8 @@ def migrate_cmd(ctx: click.Context, db_override: str | None) -> None: on migrate would resurrect rules that were deliberately deleted or refuted. Runs `keel.data.db.migrate`, which steps the stored `schema_version` up incrementally and is - safe to call repeatedly. No network, no authz gate, no orders -- safe against a live database. + safe to call repeatedly. No network, no confirmation gate, no orders -- safe against a + live database. """ path = db_override or ctx.obj["db_path"] conn = connect(path) @@ -823,11 +822,22 @@ def withdrawals_attest(ctx: click.Context, enabled: bool) -> None: taking physical possession whenever he desires". An asset we cannot withdraw is an asset we may not validly possess -- so rail 17 halts new ENTRIES when this is suspended or unknown. - Not passphrase-gated in either direction. `--suspended` only ever REDUCES capability, and - `--enabled` cannot itself place an order: it restores a precondition that every other rail, - the confirm gate and the bypass-arm token still sit in front of. + **Asymmetric, like `autonomy`.** `--suspended` only ever REDUCES capability and is ungated, + usable from anywhere. `--enabled` RELEASES a rail-17 entry halt, so it demands a typed `yes` + at a terminal exactly like `resume`/`resume-entries`/`record-flow`/`reset-hwm`. + + That gate used to be unnecessary for a reason that no longer holds: this command was + justified by "the confirm gate and the bypass-arm token still sit in front of it". The + bypass-arm token no longer exists, and with `keel autonomy on` the confirm gate is not there + either -- so without this, a cron line could clear a rail-17 halt and the next cycle would + place live orders with no human anywhere in the loop. """ repo = _open_repo(ctx) + if enabled: + _require_interactive_confirmation( + "attest withdrawals as ENABLED", + "This RELEASES rail 17's entry halt; the agent may place orders on its next cycle.", + ) now_ts = int(time.time()) repo.set_state("withdrawals_enabled", bool(enabled)) repo.set_state("withdrawals_attested_at", now_ts) @@ -1174,7 +1184,7 @@ def _interactive_confirm(preview: dict) -> bool: click.echo(f" {key}: {value}") else: click.echo(f" {preview!r}") - if not (sys.stdin is not None and sys.stdin.isatty()): + if not _is_interactive(): click.echo("no TTY -- declining (confirm mode fails closed).", err=True) return False return click.confirm("Place this order?", default=False) @@ -1191,22 +1201,12 @@ def _print_loop_result(result: agent.LoopResult) -> None: f"products={result.products} stale={result.stale_products} " f"signals={len(result.enter_signals)} entered={entered} exited={exited}" ) - if result.bypass_refused_reason is not None: - # Issue #60: `run_once` itself refused bypass (unarmed/expired token) and fell back to - # confirm mode -- surface *why*, not just the silently-different `mode=confirm` above. - click.echo(f"[{result.ts}] bypass refused: {result.bypass_refused_reason}") @cli.command() @click.option( "--loop", is_flag=True, default=False, help="Run the scheduled loop, not one cycle." ) -@click.option( - "--confirm", "mode", flag_value="confirm", default=True, help="Confirm mode (default, safe)." -) -@click.option( - "--bypass", "mode", flag_value="bypass", help="Bypass mode (dangerous; requires --passphrase)." -) @click.option( "--interval", "interval_sec", @@ -1220,28 +1220,26 @@ def _print_loop_result(result: agent.LoopResult) -> None: default=None, help="Stop --loop after N cycles (default: run until interrupted).", ) -@click.option("--passphrase", default=None, help="Required to arm --bypass.") @click.pass_context @with_disclaimer def agent_cmd( ctx: click.Context, loop: bool, - mode: str, interval_sec: float | None, max_cycles: int | None, - passphrase: str | None, ) -> None: - """Run the agent loop (confirm mode by default; every order is still hard-rail-guarded).""" - if mode == "bypass": - _require_authz(ctx, _ARM_BYPASS, passphrase) + """Run the agent loop. Every order is hard-rail-guarded, in every mode. + Whether orders are placed at all comes from `config.auto_trade.mode` (`paper` simulates, + `confirm` is live). Whether you are ASKED comes from your profile: with autonomy off (the + default) each order needs your approval at the terminal; with `keel autonomy on` it does not. + """ config = _load_cfg(ctx) - config = replace(config, auto_trade=replace(config.auto_trade, mode=mode)) repo = _open_repo(ctx) broker = _build_broker(config) if not loop: - confirm_fn = _interactive_confirm if mode == "confirm" else None + confirm_fn = _interactive_confirm _print_loop_result( agent.run_once(broker, repo, config, now_ts=int(time.time()), confirm_fn=confirm_fn) ) @@ -1255,46 +1253,111 @@ def stop_flag(_count: list[int] = [0]) -> bool: # noqa: B006 - intentional muta _count[0] += 1 return False - confirm_fn = _interactive_confirm if mode == "confirm" else None + confirm_fn = _interactive_confirm for result in agent.loop(broker, repo, config, interval, stop_flag, confirm_fn=confirm_fn): _print_loop_result(result) -# -- arm-bypass / disarm-bypass (Issue #60, bypass-arm hardening) --------------------------- +# -- autonomy (the user's own choice, stored in their profile) ------------------------------ + +@cli.group("autonomy") +def autonomy_group() -> None: + """Whether the agent places orders without asking you first.""" -@cli.command("arm-bypass") -@click.option("--passphrase", default=None, help="Required to arm bypass mode.") + +@autonomy_group.command("show") @click.pass_context @with_disclaimer -def arm_bypass_cmd(ctx: click.Context, passphrase: str | None) -> None: - """Arm autonomous bypass mode for a limited window (dangerous: requires the passphrase). - - This is the second, in-process enforcement layer `agent.run_once` itself checks - (`repo.is_bypass_armed`) before ever honoring `config.auto_trade.mode == "bypass"` -- the - existing `agent --bypass --passphrase` gate alone is not enough, since that only guards the - CLI entry point, not a caller that invokes `agent.run_once`/`agent.loop` directly. The - window lasts `config.auto_trade.bypass_arm_ttl_sec` seconds (default 3600 = 1h) from now; - call this again to reset it, or `keel disarm-bypass` to clear it early. +def autonomy_show(ctx: click.Context) -> None: + """Print the current autonomy setting.""" + repo = _open_repo(ctx) + profile = repo.get_profile() + # `_open_repo` runs `migrate()`, which recreates a merely MISSING table -- so this covers + # damage migrate cannot heal (a corrupt page, a table of the wrong shape), not a fresh DB. + if not repo.profile_readable(): + click.echo( + "WARNING: the profile row could not be read (see the log). Reporting autonomy as " + "OFF, which is the safe reading -- but the stored setting is UNKNOWN.", + err=True, + ) + now_ts = int(time.time()) + live = profile.is_autonomous(now_ts) + state = ( + "ON -- orders are placed WITHOUT asking" + if live + else "off -- every order asks first" + ) + click.echo(f"autonomy: {state}") + if profile.autonomous and not live: + click.echo(f" (was ON but LAPSED at {profile.autonomous_until})") + elif live and profile.autonomous_until is not None: + left = profile.autonomous_until - now_ts + click.echo(f" lapses at {profile.autonomous_until} ({left}s left)") + elif live: + click.echo(" no expiry set -- stays on until `keel autonomy off`") + if profile.updated_ts: + click.echo(f" last changed: {profile.updated_ts}") + + +@autonomy_group.command("on") +@click.option( + "--for-hours", + "for_hours", + type=float, + default=None, + help="Let autonomy LAPSE automatically after this many hours (default: never lapses).", +) +@click.pass_context +@with_disclaimer +def autonomy_on(ctx: click.Context, for_hours: float | None) -> None: + """Let the agent place orders without asking (dangerous: asks for confirmation). + + Every order is still subject to all hard rails -- autonomy changes who is asked, never what + is allowed. It does NOT let the agent clear a safety halt: releasing the kill-switch or a + drawdown breaker always needs a human, whatever this is set to. """ - _require_authz(ctx, _ARM_BYPASS, passphrase) config = _load_cfg(ctx) repo = _open_repo(ctx) now_ts = int(time.time()) - ttl_sec = config.auto_trade.bypass_arm_ttl_sec - repo.arm_bypass(now_ts, ttl_sec) - click.echo(f"bypass ARMED: expires at {now_ts + ttl_sec} (ttl={ttl_sec}s)") + if for_hours is not None and not (_MIN_AUTONOMY_HOURS <= for_hours <= _MAX_AUTONOMY_HOURS): + # 0/negative would write an already-lapsed row while printing "autonomy ON until ...", + # and inf/nan/1e18 would overflow int() after the operator had already typed `yes`. + raise click.BadParameter( + f"--for-hours must be at least {_MIN_AUTONOMY_HOURS} (one second) and at most " + f"{_MAX_AUTONOMY_HOURS} ({int(_MAX_AUTONOMY_HOURS) // 24} days); got {for_hours!r}." + ) + expires_ts = None if for_hours is None else now_ts + int(for_hours * 3600) + window = ( + "until you turn it off" if expires_ts is None else f"for {for_hours}h (until {expires_ts})" + ) + _require_interactive_confirmation( + "turn autonomy ON", + f"Orders will be placed with NO further prompt, {window} " + f"(mode={config.auto_trade.mode}, allowlist={config.allowlist}).", + ) + repo.set_autonomous(True, now_ts, expires_ts=expires_ts) + if expires_ts is None: + click.echo( + "autonomy ON, with NO expiry -- it stays on until you run `keel autonomy off`.\n" + " Consider `--for-hours N` for a supervised session, so a forgotten `on` cannot " + "grant unattended trading indefinitely." + ) + else: + click.echo(f"autonomy ON until {expires_ts}. It lapses on its own after that.") -@cli.command("disarm-bypass") +@autonomy_group.command("off") @click.pass_context @with_disclaimer -def disarm_bypass_cmd(ctx: click.Context) -> None: - """Clear the bypass-arm token immediately. Always allowed (safe action; only reduces - capability -- fail-safe direction, unlike arming, needs no passphrase).""" - repo = _open_repo(ctx) - repo.disarm_bypass() - click.echo("bypass DISARMED.") +def autonomy_off(ctx: click.Context) -> None: + """Require confirmation before every order again. + + Deliberately ungated and usable without a terminal: reducing risk must never be obstructed, + so this works from a script, a cron job or a pipe. Arming is what needs a human. + """ + _open_repo(ctx).set_autonomous(False, int(time.time())) + click.echo("autonomy off: every order will ask for confirmation.") # -- rules ------------------------------------------------------------------------------ @@ -1496,7 +1559,8 @@ def rules_seed( already has a rule row of any status, so it's safe to call repeatedly (e.g. from a setup script) without piling up duplicate candidates. `--force` inserts a fresh candidate anyway. - Read-only w.r.t. the exchange: no network call, no authz gate -- it only ever writes local + Read-only w.r.t. the exchange: no network call, no confirmation gate -- it only ever + writes local `rules` rows, exactly like `rules promote`/`demote`/`disable`. """ repo = _open_repo(ctx) @@ -1887,7 +1951,7 @@ def simulate( no_trial_record: bool, skip_within_cap: bool, ) -> None: - """Simulate the deterministic engine over historical candles (read-only; no authz gate). + """Simulate the deterministic engine over historical candles (read-only; no confirmation gate). Pulls (unless `--no-fetch`) and caches ~`--years` years of candle history in the persistent DB (`--db`, never in-memory), replays the real rule set through the engine + a dollar @@ -2251,22 +2315,23 @@ def kill(ctx: click.Context) -> None: @cli.command() -@click.option("--passphrase", default=None, help="Required to disengage the kill-switch.") @click.pass_context @with_disclaimer -def resume(ctx: click.Context, passphrase: str | None) -> None: - """Disengage the kill-switch (dangerous: requires the authz passphrase).""" - _require_authz(ctx, _DISABLE_KILLSWITCH, passphrase) +def resume(ctx: click.Context) -> None: + """Disengage the kill-switch (dangerous: asks for confirmation).""" + _require_interactive_confirmation( + "disengage the kill-switch", + "Trading resumes immediately: the agent may place orders on its next cycle.", + ) repo = _open_repo(ctx) repo.set_state("kill_switch", False) click.echo("kill-switch disengaged: trading resumed.") @cli.command(name="resume-entries") -@click.option("--passphrase", default=None, help="Required to clear the breaker.") @click.pass_context @with_disclaimer -def resume_entries(ctx: click.Context, passphrase: str | None) -> None: +def resume_entries(ctx: click.Context) -> None: """Clear an armed consecutive-loss halt (rail 16), re-permitting new entries. This is the ONLY way to release the halt early: rail 16 reads `streak_halt_until` and never @@ -2277,7 +2342,10 @@ def resume_entries(ctx: click.Context, passphrase: str | None) -> None: re-arm the breaker on the very next loss, which is not what an operator clearing a halt means. Exits, sells and DCA are never affected by rail 16 and are unaffected here. """ - _require_authz(ctx, _DISABLE_KILLSWITCH, passphrase) + _require_interactive_confirmation( + "clear the consecutive-loss halt (rail 16)", + "New entries are re-permitted; the loss counter is reset with it.", + ) repo = _open_repo(ctx) repo.set_state("streak_halt_until", 0) repo.set_state("consecutive_losses", 0) @@ -2290,10 +2358,9 @@ def resume_entries(ctx: click.Context, passphrase: str | None) -> None: required=True, help="Signed flow in quote currency: positive for a deposit, negative for a withdrawal.", ) -@click.option("--passphrase", default=None, help="Required to rebase the high-water mark.") @click.pass_context @with_disclaimer -def record_flow(ctx: click.Context, amount: str, passphrase: str | None) -> None: +def record_flow(ctx: click.Context, amount: str) -> None: """Declare an external deposit or withdrawal so rail 11 does not mistake it for P&L. Equity is `cash + positions`, so money moving in or out shifts it -- but neither is a @@ -2312,7 +2379,11 @@ def record_flow(ctx: click.Context, amount: str, passphrase: str | None) -> None will never infer a flow on its own: guessing a withdrawal would lower the HWM and silently mask a real trading drawdown, which is the one direction a circuit breaker must not fail in. """ - _require_authz(ctx, _DISABLE_KILLSWITCH, passphrase) + _require_interactive_confirmation( + f"rebase rail 11's high-water mark by {amount}", + "A wrong amount or sign here silently mis-states drawdown from now on " + "(positive = deposit, negative = withdrawal).", + ) try: parsed = Decimal(amount) except InvalidOperation: @@ -2337,10 +2408,9 @@ def record_flow(ctx: click.Context, amount: str, passphrase: str | None) -> None @cli.command(name="reset-hwm") -@click.option("--passphrase", default=None, help="Required to reset the high-water mark.") @click.pass_context @with_disclaimer -def reset_hwm(ctx: click.Context, passphrase: str | None) -> None: +def reset_hwm(ctx: click.Context) -> None: """Reset rail 11's equity high-water mark, clearing a stuck drawdown halt. The HWM is MONOTONIC by design -- it never falls -- so any equity reading that was wrong or @@ -2352,7 +2422,10 @@ def reset_hwm(ctx: click.Context, passphrase: str | None) -> None: equity, which is the same path a fresh install takes. `drawdown_total_pct` is zeroed so the rail is not left vetoing on a stale scalar in the window before that next cycle runs. """ - _require_authz(ctx, _DISABLE_KILLSWITCH, passphrase) + _require_interactive_confirmation( + "reset rail 11's high-water mark", + "Any real, unrecovered drawdown stops being visible to the rail.", + ) repo = _open_repo(ctx) repo.set_state("equity_high_water_mark", None) repo.set_state("drawdown_total_pct", Decimal("0")) diff --git a/keel/data/db.py b/keel/data/db.py index e9263318..6d174236 100644 --- a/keel/data/db.py +++ b/keel/data/db.py @@ -19,7 +19,7 @@ from pathlib import Path from typing import Any -SCHEMA_VERSION = 6 +SCHEMA_VERSION = 8 # Creation order matters for readability (and for backends that validate FK targets eagerly); # SQLite itself only checks FK targets at DML time, but we still declare referenced tables first. @@ -238,6 +238,16 @@ attested_at INTEGER NOT NULL ) """, + """ + CREATE TABLE IF NOT EXISTS profile ( + id INTEGER PRIMARY KEY CHECK (id = 1), + autonomous INTEGER NOT NULL DEFAULT 0, + -- NULL = no expiry (a durable choice). A timestamp makes autonomy LAPSE on its own, + -- restoring the time bound the removed bypass-arm token used to provide. + autonomous_until INTEGER, + updated_ts INTEGER NOT NULL + ) + """, ) @@ -357,12 +367,42 @@ def _migrate_v6_asset_attestations(conn: sqlite3.Connection) -> None: """ +def _migrate_v7_profile(conn: sqlite3.Connection) -> None: + """v7 adds `profile`, which carries the user's autonomy choice. Table creation is handled by + `_SCHEMA_STATEMENTS`; there is deliberately NO backfill. + + No row means `get_profile()` reports `autonomous=False`, which is the correct and safe + reading of an upgraded database: the user has never opted into unattended trading, so we must + not infer that they did. Seeding a row here -- even an explicitly `autonomous=0` one -- would + only manufacture a consent record that no human gave. + """ + + +def _migrate_v8_autonomy_expiry(conn: sqlite3.Connection) -> None: + """v8 adds `profile.autonomous_until` (NULL = the choice never lapses). + + This exists because the column was first added to v7's `CREATE TABLE IF NOT EXISTS`, which + silently does nothing for a database already stamped at 7 -- it kept the old three-column + table forever, so a recorded choice read back as "off" and `keel autonomy off` died on a + missing column. Only ever reachable on a developer database built from the unmerged branch, + but the de-risking command must never be the one that crashes. + + Idempotent: a database that got the column from the DDL (fresh, or upgraded from <=v6) is + left alone rather than hitting "duplicate column name". + """ + columns = {row["name"] for row in conn.execute("PRAGMA table_info(profile)")} + if "autonomous_until" not in columns: + conn.execute("ALTER TABLE profile ADD COLUMN autonomous_until INTEGER") + + _MIGRATIONS: dict[int, Callable[[sqlite3.Connection], None]] = { 2: _migrate_v2_broker_subscriptions, 3: _migrate_v3_trade_outcomes, 4: _migrate_v4_positions, 5: _migrate_v5_candle_gap_probes, 6: _migrate_v6_asset_attestations, + 7: _migrate_v7_profile, + 8: _migrate_v8_autonomy_expiry, } diff --git a/keel/data/repository.py b/keel/data/repository.py index 0f6666d1..26d9b669 100644 --- a/keel/data/repository.py +++ b/keel/data/repository.py @@ -8,6 +8,7 @@ from __future__ import annotations import json +import logging import sqlite3 import time from decimal import Decimal @@ -15,7 +16,7 @@ from keel_core.subscription import BrokerSubscription, SubscriptionStatus -from keel.types import Candle, Granularity +from keel.types import Candle, Granularity, Profile _TRANSACTION_COLUMNS = ( "coinbase_id", @@ -580,42 +581,70 @@ def _position_row_to_dict(self, row: sqlite3.Row) -> dict[str, Any]: d[field] = _text_to_dec(d[field]) return d - # -- bypass arm token (Issue #60, in-process bypass hardening) --------- + # -- profile (the user's own settings) ------------------------------------ - def arm_bypass(self, now_ts: int, ttl_sec: int) -> None: - """Arm autonomous bypass mode for `ttl_sec` seconds starting at `now_ts`. + def get_profile(self) -> Profile: + """The user's profile, defaulting to NOT autonomous. - Overwrites any previous arm token outright -- there is only ever one live token, and - arming again (e.g. re-running `keel arm-bypass`) always resets the window from `now_ts` - rather than extending the old one. `agent.run_once`'s own `is_bypass_armed` check reads - this token fresh on every cycle, so it is the one place bypass mode can be authorized - from -- CLI (`keel arm-bypass`, passphrase-gated) or any other authenticated caller. + **Fails closed.** An absent row -- a fresh database, or one upgraded from before the + `profile` table existed -- reports `autonomous=False`. The safe reading of "no record" + is that the user never opted into unattended trading, never that they did. + + Callers must re-read this each cycle rather than caching it, so that + `keel autonomy off` takes effect on the next cycle instead of the next restart. """ - self.set_state( - "bypass_arm", - {"armed_at": now_ts, "armed_until": now_ts + ttl_sec}, + try: + row = self._conn.execute( + "SELECT autonomous, autonomous_until, updated_ts FROM profile WHERE id = 1" + ).fetchone() + except sqlite3.Error as exc: + # A missing or damaged `profile` table must read as "no consent recorded", not + # propagate. But it must not be SILENT either: swallowing this indistinguishably + # from "the user never opted in" hides a broken migration or a corrupt database + # behind a reassuring `autonomy: off`. Fail closed AND say so. + logging.getLogger(__name__).error( + "profile unreadable (%s: %s) -- treating autonomy as OFF", type(exc).__name__, exc + ) + return Profile() + if row is None: + return Profile() + until = row["autonomous_until"] + return Profile( + autonomous=bool(row["autonomous"]), + autonomous_until=None if until is None else int(until), + updated_ts=int(row["updated_ts"]), ) - def is_bypass_armed(self, now_ts: int) -> bool: - """True iff a bypass-arm token exists and `now_ts` is still inside its window. + def profile_readable(self) -> bool: + """Whether the `profile` row could actually be read. - Freshness is a strict `now_ts < armed_until` (matching `market_feed.is_fresh`'s own - convention elsewhere in this codebase) -- the instant `armed_until` is reached, the - token is treated as expired, not one tick still-good. + `get_profile()` fails closed on a damaged table, which is right for the trading path but + indistinguishable from "the user never opted in". This lets a caller tell a human that + the stored setting is UNKNOWN rather than confidently reporting it as off. """ - token = self.get_state("bypass_arm") - if token is None: + try: + self._conn.execute("SELECT autonomous FROM profile WHERE id = 1").fetchone() + except sqlite3.Error: return False - return now_ts < token["armed_until"] + return True - def disarm_bypass(self) -> None: - """Clear the bypass-arm token immediately. + def set_autonomous(self, value: bool, now_ts: int, expires_ts: int | None = None) -> None: + """Record the user's autonomy choice, upserting the single profile row. - Fail-safe direction: disarming only ever *reduces* capability, so unlike `arm_bypass` - this needs no passphrase gate at the CLI layer -- it is always safe to call, including - when nothing is currently armed (a no-op). + `expires_ts=None` means the choice never lapses. Passing a timestamp makes autonomy + expire on its own -- the time bound the removed bypass-arm token used to enforce. """ - self.set_state("bypass_arm", None) + self._conn.execute( + """ + INSERT INTO profile (id, autonomous, autonomous_until, updated_ts) + VALUES (1, ?, ?, ?) + ON CONFLICT(id) DO UPDATE SET autonomous = excluded.autonomous, + autonomous_until = excluded.autonomous_until, + updated_ts = excluded.updated_ts + """, + (1 if value else 0, expires_ts, now_ts), + ) + self._conn.commit() # -- candle gap probes ---------------------------------------------------- # A row asserts: "we asked the venue for this exact window and it returned nothing new." diff --git a/keel/execution/executor.py b/keel/execution/executor.py index 0d9f8966..94fb4884 100644 --- a/keel/execution/executor.py +++ b/keel/execution/executor.py @@ -2,7 +2,7 @@ `execute()` is the only path from a strategy `Signal` to a real order: it sizes the candidate (`execution.sizing`), runs the twelve un-overridable §14 hard rails (`execution.guards.check`) -**before** anything reaches the broker, previews the order, honors the confirm/bypass mode gate, +**before** anything reaches the broker, previews the order, honors the confirm/autonomous mode gate, places it, and writes a full audit trail to the `orders` table both before and after the broker call (so a crash mid-placement, or a broker-side rejection, still leaves a record). No path in this module calls `broker.place_order` (or even `broker.preview_order`) without `guards.check` @@ -109,15 +109,15 @@ def execute( broker: Any, repo: Repository, config: Config, - mode: Literal["confirm", "bypass"], + mode: Literal["confirm", "autonomous"], confirm_fn: ConfirmFn | None = None, now_ts: int | None = None, ) -> ExecutionResult: """Turn `signal` into a guarded order: size -> guards (veto on any violation) -> preview -> - confirm|bypass -> place -> log (before and after). + confirm|autonomous -> place -> log (before and after). `mode="confirm"` requires `confirm_fn(preview) -> bool`; a missing or rejecting `confirm_fn` - means the order is not placed (fails closed, never silently proceeds). `mode="bypass"` + means the order is not placed (fails closed, never silently proceeds). `mode="autonomous"` places without a prompt but is *not* exempt from `guards.check` -- rails run before every order in every mode, un-overridable, per the main spec §14. """ @@ -364,7 +364,7 @@ def _held_position(repo: Repository, product_id: str) -> tuple[Decimal, Decimal] return (net_qty if net_qty > 0 else Decimal("0")), avg_cost -# -- shared guard -> preview -> confirm/bypass -> place -> log pipeline ------------------------ +# -- shared guard -> preview -> confirm/autonomous -> place -> log pipeline ------------------------ def _run_order( @@ -433,8 +433,8 @@ def _run_order( preview=preview, reason="rejected at confirm gate", ) - elif mode != "bypass": - raise ValueError(f"execute: unknown mode {mode!r} -- must be 'confirm' or 'bypass'") + elif mode != "autonomous": + raise ValueError(f"execute: unknown mode {mode!r} -- must be 'confirm' or 'autonomous'") order_id = repo.insert_order(_order_row(intent, mode, now_ts)) @@ -561,6 +561,9 @@ def _order_row(intent: OrderIntent, mode: str, now_ts: int) -> dict[str, Any]: expected_fill=intent.entry, actual_fill=None, raw_response=None, + # NOTE: rows written before 2026-07-21 carry `confirmation='bypass'` for what is now + # called `'autonomous'`. Nothing reads this column back -- it is an audit trail only -- + # so the rows are deliberately left as-written rather than rewritten by a migration. confirmation=mode, rule_id=None, created_at=now_ts, @@ -717,7 +720,7 @@ def place_bracket( broker, repo, config, - "bypass", + "autonomous", None, now_ts, order_configuration=_bracket_order_configuration(qty, target, stop), @@ -762,7 +765,7 @@ def scale_out( ) -> ExecutionResult: """Partially close `qty` of an open position at `exit_price` (a rule-driven profit-take, e.g. "sell half at the first target") -- runs the same guard+preview+place+log pipeline as - `execute()` for a plain SELL leg, system-initiated so it proceeds in bypass mode (still + `execute()` for a plain SELL leg, system-initiated so it proceeds in autonomous mode (still subject to every guard rail, still fully logged). """ intent = OrderIntent( @@ -784,7 +787,7 @@ def scale_out( exit_price=exit_price, rule=rule_name, ) - return _run_order(intent, broker, repo, config, "bypass", None, now_ts) + return _run_order(intent, broker, repo, config, "autonomous", None, now_ts) # -- stop management: break-even roll + ATR trailing ------------------------------------------- @@ -858,7 +861,7 @@ def _roll_stop( broker, repo, config, - "bypass", + "autonomous", None, now_ts, order_configuration=_bracket_order_configuration(qty, target, new_stop), diff --git a/keel/execution/guards.py b/keel/execution/guards.py index bf613bb4..2db981fd 100644 --- a/keel/execution/guards.py +++ b/keel/execution/guards.py @@ -3,7 +3,8 @@ `check()` runs the twelve safety rails from the main spec's §14, plus three later, equally un-overridable safety-critical rails: 13/14 added by Issue #59 (USDC-funding + monthly-allowance), and 16, the consecutive-loss circuit breaker (Task 4), before any order is placed, in every -`auto_trade` mode (confirm *and* bypass) and for both rule-trading and DCA order classes. It never +`auto_trade` mode (confirm *and* autonomous) and for both rule-trading and DCA order +classes. It never short-circuits: every violated rail is collected and reported so an operator (or the executor, Task 4) sees the full picture, not just the first trip-wire. @@ -242,7 +243,8 @@ def check( ) -> GuardResult: """Run all fifteen §14 (+ Issue #59, Task 4) hard rails against `intent`. Never short-circuits. - Called before every order in every `auto_trade` mode (confirm *and* bypass) — un-overridable. + Called before every order in every `auto_trade` mode (confirm *and* autonomous) -- + un-overridable. `offline=True` (paper trading only) skips `LIVE_STATE_RAILS` — the two rails whose inputs describe the real account, which a paper rehearsal has no access to. **Every other rail still diff --git a/keel/security/__init__.py b/keel/security/__init__.py deleted file mode 100644 index 2075fe8b..00000000 --- a/keel/security/__init__.py +++ /dev/null @@ -1,3 +0,0 @@ -"""Operational security: portable encrypted secrets vault (`secrets.py`) and the dangerous-action -passphrase gate (`authz.py`, added in a later task). See main spec §14 Part A. -""" diff --git a/keel/security/authz.py b/keel/security/authz.py deleted file mode 100644 index 70c34534..00000000 --- a/keel/security/authz.py +++ /dev/null @@ -1,161 +0,0 @@ -"""Dangerous-action authorization gate (main spec §14). - -A local passphrase — stored as a **stdlib `hashlib.scrypt` hash + random salt, never -plaintext** — gates only the small set of *dangerous* actions: arming bypass/autonomous -mode, raising caps above config maxima, disabling the kill-switch/resume, and unlocking -the secrets vault. Read-only commands and confirm-mode trades need no passphrase at all. - -This is local *authorization*, not a login: on a single-user machine the real security -boundary is the OS account + full-disk encryption. The gate is defense-in-depth — it -forces intentionality and blocks casual/accidental/shoulder-surf misuse of the autonomy -capability. It does not stop an attacker who already holds the OS account. - -Failed attempts are tracked and rate-limited with an exponential backoff lockout, -persisted alongside the hash so restarts don't reset the counter. -""" - -from __future__ import annotations - -import hashlib -import hmac -import json -import os -import time -from pathlib import Path - -DANGEROUS_ACTIONS = frozenset( - {"arm_bypass", "raise_caps", "disable_killswitch", "unlock_vault"} -) - -# scrypt cost parameters (interactive-login-grade; OWASP minimum: N=2**14, r=8, p=1). -_SCRYPT_N = 2**14 -_SCRYPT_R = 8 -_SCRYPT_P = 1 -_SCRYPT_DKLEN = 32 -_SALT_BYTES = 16 - -# Rate limiting: lock out after MAX_ATTEMPTS consecutive failures, with the lockout -# window growing exponentially for each additional failure beyond the threshold. -MAX_ATTEMPTS = 5 -_LOCKOUT_BASE_SECONDS = 30.0 - -_DEFAULT_PATH = "authz.json" - - -class AuthzError(Exception): - """Raised by `require()` when an action is dangerous and authorization fails.""" - - -def _scrypt_hash( - passphrase: str, - salt: bytes, - n: int = _SCRYPT_N, - r: int = _SCRYPT_R, - p: int = _SCRYPT_P, - dklen: int = _SCRYPT_DKLEN, -) -> bytes: - return hashlib.scrypt( - passphrase.encode("utf-8"), - salt=salt, - n=n, - r=r, - p=p, - dklen=dklen, - ) - - -def _load_state(path: Path) -> dict | None: - if not path.exists(): - return None - with path.open() as f: - return json.load(f) - - -def _save_state(path: Path, state: dict) -> None: - path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(json.dumps(state)) - if os.name != "nt": - os.chmod(path, 0o600) - - -def set_passphrase(passphrase: str, path: str | Path = _DEFAULT_PATH) -> None: - """Hash `passphrase` with a fresh random salt and persist it at `path`. - - Overwrites any existing state (including the rate-limit counters) — setting a new - passphrase starts a clean slate. The passphrase itself is never written to disk. - """ - path = Path(path) - salt = os.urandom(_SALT_BYTES) - digest = _scrypt_hash(passphrase, salt) - state = { - "salt": salt.hex(), - "hash": digest.hex(), - "n": _SCRYPT_N, - "r": _SCRYPT_R, - "p": _SCRYPT_P, - "dklen": _SCRYPT_DKLEN, - "failed_attempts": 0, - "locked_until": 0.0, - } - _save_state(path, state) - - -def verify(passphrase: str, path: str | Path = _DEFAULT_PATH, now: float | None = None) -> bool: - """Check `passphrase` against the hash stored at `path`. - - Returns `False` (never raises) when: no passphrase has been set yet, the passphrase - is wrong, or the gate is currently locked out from too many recent failures. `now` - defaults to the wall clock; callers (and tests) may pass an explicit epoch timestamp. - """ - path = Path(path) - now = time.time() if now is None else now - - state = _load_state(path) - if state is None: - return False - - if now < state.get("locked_until", 0.0): - return False # rate limited — still inside the lockout window - - computed = _scrypt_hash( - passphrase, - bytes.fromhex(state["salt"]), - n=state["n"], - r=state["r"], - p=state["p"], - dklen=state["dklen"], - ) - stored = bytes.fromhex(state["hash"]) - ok = hmac.compare_digest(computed, stored) - - if ok: - state["failed_attempts"] = 0 - state["locked_until"] = 0.0 - _save_state(path, state) - return True - - state["failed_attempts"] = state.get("failed_attempts", 0) + 1 - if state["failed_attempts"] >= MAX_ATTEMPTS: - excess = state["failed_attempts"] - MAX_ATTEMPTS - lockout_seconds = _LOCKOUT_BASE_SECONDS * (2**excess) - state["locked_until"] = now + lockout_seconds - _save_state(path, state) - return False - - -def require( - action: str, - passphrase: str, - path: str | Path = _DEFAULT_PATH, - now: float | None = None, -) -> None: - """Enforce the authorization gate for `action`. - - No-ops for anything outside `DANGEROUS_ACTIONS` (read-only/confirm-mode actions need - no passphrase). For a dangerous action, raises `AuthzError` unless `passphrase` - verifies — including while rate-limit locked out, even with the correct passphrase. - """ - if action not in DANGEROUS_ACTIONS: - return - if not verify(passphrase, path=path, now=now): - raise AuthzError(f"authorization denied for dangerous action {action!r}") diff --git a/keel/security/secrets.py b/keel/security/secrets.py deleted file mode 100644 index aae4da33..00000000 --- a/keel/security/secrets.py +++ /dev/null @@ -1,114 +0,0 @@ -"""Portable AES-GCM encrypted secrets vault (main spec §14 Part A). - -The CDP API key/secret (and any future remote-control token) live in a single file, -`secrets.enc`, unlocked by a master passphrase: passphrase -> **scrypt KDF** -> symmetric key -> -**AES-GCM** encrypt/decrypt of a JSON secrets blob. Deliberately *not* the OS keychain — this file -is copyable between machines, which is the portability the design calls for. - -On-disk layout (all bytes, no plaintext): - MAGIC (7 bytes) | salt (16 bytes) | nonce (12 bytes) | AES-GCM ciphertext+tag - -`MAGIC` is also passed as AES-GCM associated data, so tampering with it (or any other byte in the -file) fails authentication rather than silently decrypting garbage. - -Secrets are never logged: no function in this module writes secret values to a log, exception -message, or `repr()`. -""" - -from __future__ import annotations - -import json -import os -from pathlib import Path - -from cryptography.exceptions import InvalidTag -from cryptography.hazmat.primitives.ciphers.aead import AESGCM -from cryptography.hazmat.primitives.kdf.scrypt import Scrypt - -_MAGIC = b"HCBVLT1" -_SALT_LEN = 16 -_NONCE_LEN = 12 -_KEY_LEN = 32 - -# scrypt cost parameters for an interactive unlock (local CLI, not a server). ~16MB of memory. -_SCRYPT_N = 2**14 -_SCRYPT_R = 8 -_SCRYPT_P = 1 - -DEFAULT_VAULT_PATH = "secrets.enc" - - -class VaultError(Exception): - """Raised when a vault cannot be sealed/unlocked: wrong passphrase, tampering, or a missing - or corrupt vault file. Never includes secret values in its message. - """ - - -def _derive_key(passphrase: str, salt: bytes) -> bytes: - kdf = Scrypt(salt=salt, length=_KEY_LEN, n=_SCRYPT_N, r=_SCRYPT_R, p=_SCRYPT_P) - return kdf.derive(passphrase.encode("utf-8")) - - -def save_vault(secrets: dict, passphrase: str, path: str | Path = DEFAULT_VAULT_PATH) -> None: - """Encrypt `secrets` (a JSON-serializable dict) with a passphrase-derived key and write the - result to `path`. A fresh random salt + nonce are generated on every call, so encrypting the - same secrets twice yields different ciphertext. The file is `chmod 600`'d. - """ - path = Path(path) - salt = os.urandom(_SALT_LEN) - nonce = os.urandom(_NONCE_LEN) - key = _derive_key(passphrase, salt) - - plaintext = json.dumps(secrets).encode("utf-8") - ciphertext = AESGCM(key).encrypt(nonce, plaintext, associated_data=_MAGIC) - - path.write_bytes(_MAGIC + salt + nonce + ciphertext) - path.chmod(0o600) - - -def load_vault(passphrase: str, path: str | Path = DEFAULT_VAULT_PATH) -> dict: - """Decrypt the vault at `path` with `passphrase` and return the secrets dict. - - Raises `VaultError` for a missing file, a wrong passphrase, or a tampered/corrupt file. - """ - path = Path(path) - if not path.exists(): - raise VaultError(f"vault not found at {path}") - - blob = path.read_bytes() - if not blob.startswith(_MAGIC): - raise VaultError("not a valid secrets vault file") - - rest = blob[len(_MAGIC) :] - if len(rest) < _SALT_LEN + _NONCE_LEN: - raise VaultError("vault file is truncated or corrupt") - - salt = rest[:_SALT_LEN] - nonce = rest[_SALT_LEN : _SALT_LEN + _NONCE_LEN] - ciphertext = rest[_SALT_LEN + _NONCE_LEN :] - - key = _derive_key(passphrase, salt) - try: - plaintext = AESGCM(key).decrypt(nonce, ciphertext, associated_data=_MAGIC) - except InvalidTag as exc: - raise VaultError("wrong passphrase or tampered vault file") from exc - - return json.loads(plaintext) - - -def migrate_from_env( - env_path: str | Path = ".env", - *, - passphrase: str, - path: str | Path = DEFAULT_VAULT_PATH, -) -> None: - """Read CDP secrets out of a git-ignored `.env` file and seal them into the vault at `path`. - - Delegates to `config.load_secrets`, so it inherits the same absent/empty-file handling - (returns `{}` rather than raising, so a fresh vault with no secrets configured is still - valid). Does not delete or modify `env_path` — the caller decides when it's safe to remove it. - """ - from keel.config import load_secrets - - secrets = load_secrets(env_path) - save_vault(secrets, passphrase, path=path) diff --git a/keel/templates/config.live.yaml b/keel/templates/config.live.yaml index a1840e98..05bb0c83 100644 --- a/keel/templates/config.live.yaml +++ b/keel/templates/config.live.yaml @@ -10,7 +10,6 @@ # 3. `caps` -- max_exposure_usd / max_per_asset_pct are your risk ceilings. # 4. `subscription` -- attest your venue tier (`keel subscription attest`); rail 14 caps spend. # 5. rules -- `keel init` seeds them as `candidate`; nothing trades until you promote. -# 6. `auto_trade.enabled` -- still false; set it true to start the scheduled loop. # # `allowlist` and `caps` are required and validated by keel.config.load_config; missing or # invalid values raise ConfigError naming the offending key rather than silently defaulting. @@ -47,12 +46,10 @@ market_data: auto_trade: # confirm = preview + explicit approval for every order (the safe live default). mode: confirm + # NOTE: currently UNUSED by any code path -- it is NOT a kill-switch and setting it + # true or false changes nothing. Use `keel kill` to halt trading. enabled: false interval_sec: 900 - # how long a `keel arm-bypass` token stays valid (seconds) before bypass mode requires - # re-arming (Issue #60) -- `keel agent --bypass` still needs a fresh armed token even with - # the CLI passphrase gate satisfied. - bypass_arm_ttl_sec: 3600 promotion: min_trades: 100 diff --git a/keel/templates/config.yaml b/keel/templates/config.yaml index 3d589682..d85bf9e1 100644 --- a/keel/templates/config.yaml +++ b/keel/templates/config.yaml @@ -35,12 +35,10 @@ market_data: auto_trade: mode: paper + # NOTE: currently UNUSED by any code path -- it is NOT a kill-switch and setting it + # true or false changes nothing. Use `keel kill` to halt trading. enabled: false interval_sec: 900 - # how long a `keel arm-bypass` token stays valid (seconds) before bypass mode requires - # re-arming (Issue #60) -- `keel agent --bypass` still needs a fresh armed token even with - # the CLI passphrase gate satisfied. - bypass_arm_ttl_sec: 3600 promotion: min_trades: 100 diff --git a/packages/keel-core/keel_core/config.py b/packages/keel-core/keel_core/config.py index b514c2fe..45fd486e 100644 --- a/packages/keel-core/keel_core/config.py +++ b/packages/keel-core/keel_core/config.py @@ -96,11 +96,6 @@ class AutoTradeConfig: mode: str = "paper" enabled: bool = False interval_sec: int = 900 - # Issue #60: how long a `keel arm-bypass` token stays valid (`Repository.arm_bypass`'s - # `ttl_sec`) before `agent.run_once` refuses `mode="bypass"` again and falls back to - # `"confirm"`. 3600s (1h) is a sensible default -- long enough for one supervised session, - # short enough that a forgotten arm doesn't grant unattended autonomy indefinitely. - bypass_arm_ttl_sec: int = 3600 @dataclass(frozen=True) @@ -135,6 +130,10 @@ class DcaConfig: _VALID_PACING_MODES = ("opportunistic", "even_daily") +#: `paper` simulates and places nothing; `confirm` is live. Whether you are ASKED is a +#: PROFILE choice (`keel autonomy on|off`), deliberately not a config mode -- config.yaml +#: ships as a release asset, and arming unattended trading should not be a YAML edit. +_VALID_AUTO_TRADE_MODES = ("paper", "confirm") @dataclass(frozen=True) @@ -278,6 +277,23 @@ class Config: ) +def _parse_auto_trade_mode(raw: dict[str, Any]) -> str: + """`auto_trade.mode`, validated against `_VALID_AUTO_TRADE_MODES`. + + Previously any unknown value silently degraded to confirm. It now raises: a config saying + `mode: bypass` was explicitly asking for autonomy, and autonomy has since become a profile + choice. Reinterpreting that request quietly -- in either direction -- is worse than stopping. + """ + mode = raw.get("mode", "paper") + if mode not in _VALID_AUTO_TRADE_MODES: + raise ConfigError( + f"auto_trade.mode: invalid value {mode!r}; must be one of " + f"{_VALID_AUTO_TRADE_MODES!r}. Autonomy is no longer a mode -- it is a profile " + f"choice: use `mode: confirm` plus `keel autonomy on`." + ) + return str(mode) + + def _parse_allowlist(raw: dict[str, Any]) -> list[str]: allowlist = raw.get("allowlist") if not allowlist or not isinstance(allowlist, list): @@ -523,10 +539,9 @@ def load_config(path: str | Path) -> Config: caps=caps, market_data=market_data, auto_trade=AutoTradeConfig( - mode=auto_trade_raw.get("mode", "paper"), + mode=_parse_auto_trade_mode(auto_trade_raw), enabled=bool(auto_trade_raw.get("enabled", False)), interval_sec=int(auto_trade_raw.get("interval_sec", 900)), - bypass_arm_ttl_sec=int(auto_trade_raw.get("bypass_arm_ttl_sec", 3600)), ), promotion=PromotionConfig( min_trades=int(promotion_raw.get("min_trades", 100)), diff --git a/packages/keel-core/keel_core/types.py b/packages/keel-core/keel_core/types.py index 5b0b3afd..fb5e95a7 100644 --- a/packages/keel-core/keel_core/types.py +++ b/packages/keel-core/keel_core/types.py @@ -41,4 +41,33 @@ class Candle: volume: Decimal -__all__ = ["Granularity", "Side", "Candle"] +@dataclass(frozen=True) +class Profile: + """The user's own settings, as opposed to operational state or file configuration. + + `autonomous` is the single choice today: when true, the agent places rule-generated orders + without asking. It is stored in the database (not `config.yaml`) and re-read once per + cycle, so turning it off takes effect on the NEXT cycle rather than the next restart. + """ + + autonomous: bool = False + #: `None` = the choice never lapses. Otherwise autonomy stops applying at this timestamp, + #: which is how the time bound of the removed bypass-arm token is preserved for anyone who + #: wants it -- a forgotten `autonomy on` need not grant unattended trading forever. + autonomous_until: int | None = None + updated_ts: int = 0 + + def is_autonomous(self, now_ts: int) -> bool: + """Whether autonomy actually applies at `now_ts`, honouring any expiry. + + Strict `now_ts < autonomous_until`, matching the freshness convention used throughout + this codebase: the instant the expiry is reached, autonomy is over. + """ + if not self.autonomous: + return False + if self.autonomous_until is None: + return True + return now_ts < self.autonomous_until + + +__all__ = ["Granularity", "Side", "Candle", "Profile"] diff --git a/pyproject.toml b/pyproject.toml index d873c90d..78328467 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,7 +9,6 @@ authors = [ requires-python = ">=3.12" dependencies = [ "click>=8.4.2", - "cryptography>=49.0.0", # pyyaml/python-dotenv are deliberately NOT listed here: nothing under `keel/` imports # them any more (config parsing and secret loading both moved into keel-core), so they # arrive transitively via the `keel-core` dependency below. Re-add them here only if diff --git a/tests/conftest.py b/tests/conftest.py index 82f43087..1450f90a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -83,7 +83,6 @@ def attest_subscription( mode: paper enabled: false interval_sec: 900 - bypass_arm_ttl_sec: 3600 promotion: min_trades: 100 diff --git a/tests/data/test_db.py b/tests/data/test_db.py index c472e9f1..690c30d5 100644 --- a/tests/data/test_db.py +++ b/tests/data/test_db.py @@ -101,3 +101,81 @@ def test_agent_state_table_has_key_primary_key(): columns = conn.execute("PRAGMA table_info(agent_state)").fetchall() pk_columns = {row["name"] for row in columns if row["pk"] > 0} assert pk_columns == {"key"} + + +def test_schema_version_is_8(): + """Deliberate tripwire: bump this literal consciously on every schema change.""" + from keel.data.db import SCHEMA_VERSION + + assert SCHEMA_VERSION == 8 + + +def test_a_v6_database_migrates_up_and_gains_the_profile_table(tmp_path): + from keel.data.db import SCHEMA_VERSION, connect, migrate + + conn = connect(str(tmp_path / "old.db")) + migrate(conn) + conn.execute("UPDATE schema_version SET version = 6") + conn.commit() + + migrate(conn) + assert int(conn.execute("SELECT version FROM schema_version").fetchone()["version"]) == ( + SCHEMA_VERSION + ) + named = conn.execute( + "SELECT name FROM sqlite_master WHERE type='table' AND name='profile'" + ).fetchone() + assert named is not None, "v7 must add the profile table" + + +def test_a_v7_database_without_the_expiry_column_gains_it(tmp_path): + """Regression: `autonomous_until` was added to v7's CREATE TABLE, but a database already + stamped 7 runs no migration step and IF NOT EXISTS is a no-op -- so it kept the old table + forever, and `keel autonomy off` (the DE-RISKING command) died on a missing column.""" + import sqlite3 + + from keel.data.db import SCHEMA_VERSION, connect, migrate + + path = str(tmp_path / "v7.db") + conn = connect(path) + migrate(conn) + # Recreate the pre-fix v7 shape: profile without the expiry column, stamped at 7. + conn.execute("DROP TABLE profile") + conn.execute( + "CREATE TABLE profile (id INTEGER PRIMARY KEY CHECK (id = 1), " + "autonomous INTEGER NOT NULL DEFAULT 0, updated_ts INTEGER NOT NULL)" + ) + conn.execute("INSERT INTO profile (id, autonomous, updated_ts) VALUES (1, 1, 5)") + conn.execute("UPDATE schema_version SET version = 7") + conn.commit() + + migrate(conn) + + cols = [r["name"] for r in conn.execute("PRAGMA table_info(profile)")] + assert "autonomous_until" in cols, "v8 must add the expiry column to an existing v7 table" + assert int(conn.execute("SELECT version FROM schema_version").fetchone()["version"]) == ( + SCHEMA_VERSION + ) + # the recorded choice survives, and writing to it no longer raises + from keel.data.repository import Repository + + repo = Repository(conn) + assert repo.get_profile().autonomous is True + repo.set_autonomous(False, now_ts=9) # must not raise sqlite3.OperationalError + assert repo.get_profile().autonomous is False + assert isinstance(conn, sqlite3.Connection) + + +def test_migrating_a_v6_database_twice_is_idempotent(tmp_path): + """v8's ALTER must not fire twice on a DB that already got the column from the DDL.""" + from keel.data.db import SCHEMA_VERSION, connect, migrate + + conn = connect(str(tmp_path / "v6.db")) + migrate(conn) + conn.execute("UPDATE schema_version SET version = 6") + conn.commit() + migrate(conn) + migrate(conn) # must not raise "duplicate column name" + assert int(conn.execute("SELECT version FROM schema_version").fetchone()["version"]) == ( + SCHEMA_VERSION + ) diff --git a/tests/data/test_migrations.py b/tests/data/test_migrations.py index bb2bd7e6..e5d8e29d 100644 --- a/tests/data/test_migrations.py +++ b/tests/data/test_migrations.py @@ -46,7 +46,7 @@ def test_fresh_database_is_stamped_at_the_current_version() -> None: conn = db.connect(":memory:") db.migrate(conn) version = conn.execute("SELECT version FROM schema_version").fetchone()["version"] - assert version == db.SCHEMA_VERSION == 6 + assert version == db.SCHEMA_VERSION == 8 def test_fresh_database_gets_no_subscription_row() -> None: diff --git a/tests/data/test_repository.py b/tests/data/test_repository.py index ffb87c6c..1cd7d963 100644 --- a/tests/data/test_repository.py +++ b/tests/data/test_repository.py @@ -218,53 +218,6 @@ def test_set_state_overwrites_existing_key(repo): assert repo.get_state("kill_switch") is True -# -- bypass arm token (Issue #60, in-process bypass hardening) --------------- - - -def test_is_bypass_armed_false_when_never_armed(repo): - assert repo.is_bypass_armed(now_ts=1_700_000_000) is False - - -def test_is_bypass_armed_true_right_after_arm_within_ttl(repo): - repo.arm_bypass(now_ts=1_700_000_000, ttl_sec=3600) - - assert repo.is_bypass_armed(now_ts=1_700_000_001) is True - assert repo.is_bypass_armed(now_ts=1_700_003_599) is True - - -def test_is_bypass_armed_false_once_now_ts_reaches_armed_until(repo): - repo.arm_bypass(now_ts=1_700_000_000, ttl_sec=3600) - - # armed_until = 1_700_000_000 + 3600 = 1_700_003_600 -- freshness is a strict `<`. - assert repo.is_bypass_armed(now_ts=1_700_003_600) is False - assert repo.is_bypass_armed(now_ts=1_700_004_000) is False - - -def test_is_bypass_armed_false_after_disarm(repo): - repo.arm_bypass(now_ts=1_700_000_000, ttl_sec=3600) - assert repo.is_bypass_armed(now_ts=1_700_000_001) is True - - repo.disarm_bypass() - - assert repo.is_bypass_armed(now_ts=1_700_000_001) is False - - -def test_arm_bypass_overwrites_a_previous_arm(repo): - repo.arm_bypass(now_ts=1_700_000_000, ttl_sec=10) - repo.arm_bypass(now_ts=1_700_000_100, ttl_sec=3600) - - # the earlier, shorter-lived arm is gone; the new one governs. - assert repo.is_bypass_armed(now_ts=1_700_000_015) is True - assert repo.is_bypass_armed(now_ts=1_700_003_699) is True - assert repo.is_bypass_armed(now_ts=1_700_003_700) is False - - -def test_disarm_bypass_is_a_no_op_when_never_armed(repo): - repo.disarm_bypass() # must not raise - - assert repo.is_bypass_armed(now_ts=1_700_000_000) is False - - # -- rules ------------------------------------------------------------------- @@ -431,3 +384,71 @@ def test_held_products_lists_products_with_filled_live_orders(repo: Repository) ) assert repo.held_products() == ["BTC-USD", "ETH-USD"] + + +# -- profile (autonomy is a durable USER CHOICE, read live) --------------------- + + +def test_an_absent_profile_row_reads_as_NOT_autonomous(repo): + """Fails closed: a fresh or damaged database must never imply unattended trading.""" + assert repo.get_profile().autonomous is False + + +def test_set_autonomous_round_trips(repo): + repo.set_autonomous(True, now_ts=1000) + p = repo.get_profile() + assert p.autonomous is True + assert p.updated_ts == 1000 + + repo.set_autonomous(False, now_ts=2000) + p = repo.get_profile() + assert p.autonomous is False + assert p.updated_ts == 2000 + + +def test_set_autonomous_upserts_and_never_creates_a_second_row(repo): + """The table is deliberately single-row; two rows would make 'the' profile ambiguous.""" + for ts in range(1, 6): + repo.set_autonomous(ts % 2 == 0, now_ts=ts) + rows = repo._conn.execute("SELECT COUNT(*) AS n FROM profile").fetchone()["n"] + assert rows == 1 + + +def test_autonomy_can_carry_an_expiry_and_lapses_on_its_own(repo): + """The removed bypass-arm token was TIME-LIMITED so a forgotten arm could not grant + unattended trading forever. An unbounded profile flag loses that; an optional expiry + restores it without forcing it on a user who wants a durable choice.""" + repo.set_autonomous(True, now_ts=1000, expires_ts=2000) + p = repo.get_profile() + assert p.autonomous is True + assert p.autonomous_until == 2000 + + assert p.is_autonomous(now_ts=1999) is True + # strict now < expiry, like every other freshness check in this codebase + assert p.is_autonomous(now_ts=2000) is False + assert p.is_autonomous(now_ts=5000) is False + + +def test_autonomy_without_an_expiry_never_lapses(repo): + repo.set_autonomous(True, now_ts=1000) + p = repo.get_profile() + assert p.autonomous_until is None + assert p.is_autonomous(now_ts=10**12) is True + + +def test_autonomy_off_is_never_autonomous_whatever_the_expiry(repo): + repo.set_autonomous(False, now_ts=1000, expires_ts=10**12) + assert repo.get_profile().is_autonomous(now_ts=1001) is False + + +def test_profile_readable_reports_damage_that_get_profile_hides(repo): + """`get_profile` fails closed on a damaged table, which is right for the trading path but + indistinguishable from "never opted in". `profile_readable` is how a caller tells a human + the stored setting is UNKNOWN rather than confidently reporting it off.""" + assert repo.profile_readable() is True + + repo._conn.execute("DROP TABLE profile") + repo._conn.commit() + + assert repo.profile_readable() is False + assert repo.get_profile().autonomous is False # still fails closed, still no exception diff --git a/tests/data/test_trade_outcomes.py b/tests/data/test_trade_outcomes.py index dcf1ccb8..22075335 100644 --- a/tests/data/test_trade_outcomes.py +++ b/tests/data/test_trade_outcomes.py @@ -35,11 +35,11 @@ def _outcome(**overrides: object) -> dict: return base -def test_schema_is_at_version_6() -> None: +def test_schema_is_at_version_8() -> None: conn = db.connect(":memory:") db.migrate(conn) version = conn.execute("SELECT version FROM schema_version").fetchone()["version"] - assert version == db.SCHEMA_VERSION == 6 + assert version == db.SCHEMA_VERSION == 8 def test_fresh_database_has_no_outcomes() -> None: diff --git a/tests/execution/test_executor.py b/tests/execution/test_executor.py index 2247e207..293c00f0 100644 --- a/tests/execution/test_executor.py +++ b/tests/execution/test_executor.py @@ -2,7 +2,7 @@ `execute()` turns a `Signal` into a guarded live order: build an `OrderIntent` (sized via `execution.sizing`), run `guards.check` FIRST (un-overridable -- a violation must never reach -`preview_order`/`place_order`), preview it, honor confirm/bypass mode, place it, and log it to +`preview_order`/`place_order`), preview it, honor confirm/autonomous mode, place it, and log it to the `orders` table both before and after placement (a full audit trail even if the broker call fails). Every test here injects a **fake broker** (no network) -- `FakeBroker` below duck-types `CoinbaseClient.preview_order`/`.place_order` (+ an optional `cancel_order` for OCO) against @@ -341,7 +341,7 @@ def test_rail_violating_signal_is_vetoed_before_preview_or_place(repo): broker = NoNetworkBroker() signal = _enter_signal(product_id="DOGE-USD", setup=_setup(product_id="DOGE-USD")) - result = execute(signal, broker, repo, _config(), mode="bypass", now_ts=NOW_TS) + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) assert result.placed is False assert result.order_id is None @@ -355,7 +355,7 @@ def test_kill_switch_vetoes_even_in_bypass_mode(repo): broker = NoNetworkBroker() signal = _enter_signal() - result = execute(signal, broker, repo, _config(), mode="bypass", now_ts=NOW_TS) + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) assert result.placed is False assert any(v.startswith("kill_switch") for v in result.vetoed_by) @@ -385,7 +385,7 @@ def test_execute_fetches_the_available_quote_balance_for_a_buy_and_places(repo): broker = FakeBroker(usdc_balance=Decimal("100000")) signal = _enter_signal() - result = execute(signal, broker, repo, _config(), mode="bypass", now_ts=NOW_TS) + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) assert result.placed is True assert broker.get_accounts_calls == 1 @@ -395,7 +395,7 @@ def test_broker_balance_fetch_error_vetoes_the_buy_before_preview_or_place(repo) broker = _BrokerAccountsError() signal = _enter_signal() - result = execute(signal, broker, repo, _config(), mode="bypass", now_ts=NOW_TS) + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) assert result.placed is False assert result.order_id is None @@ -408,7 +408,7 @@ def test_insufficient_usdc_balance_vetoes_the_buy_before_preview_or_place(repo): broker = FakeBroker(usdc_balance=Decimal("10")) # entry 50000, qty ~1 -> notional ~50000 signal = _enter_signal() - result = execute(signal, broker, repo, _config(), mode="bypass", now_ts=NOW_TS) + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) assert result.placed is False assert result.preview is None @@ -432,20 +432,21 @@ def test_exit_signal_never_fetches_a_balance(repo): ts=NOW_TS, ) - result = execute(signal, broker, repo, _config(), mode="bypass", now_ts=NOW_TS) + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) assert result.placed is True assert broker.get_accounts_calls == 0 -# -- bypass mode: compliant -> placed without a prompt -------------------------------------------- +# -- autonomous mode: compliant -> placed without a prompt +# -------------------------------------------- def test_bypass_mode_compliant_signal_places_without_confirm_fn(repo): broker = FakeBroker() signal = _enter_signal() - result = execute(signal, broker, repo, _config(), mode="bypass", now_ts=NOW_TS) + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) assert result.placed is True assert result.order_id is not None @@ -458,10 +459,10 @@ def test_bypass_mode_ignores_confirm_fn_if_provided(repo): signal = _enter_signal() result = execute( - signal, broker, repo, _config(), mode="bypass", confirm_fn=_reject, now_ts=NOW_TS + signal, broker, repo, _config(), mode="autonomous", confirm_fn=_reject, now_ts=NOW_TS ) - # bypass mode never consults confirm_fn -- a reject-everything fn must not block it. + # autonomous mode never consults confirm_fn -- a reject-everything fn must not block it. assert result.placed is True @@ -473,7 +474,8 @@ def test_unknown_mode_raises_value_error(repo): signal = _enter_signal() with pytest.raises(ValueError, match="mode"): - execute(signal, broker, repo, _config(), mode="yolo", now_ts=NOW_TS) # type: ignore[arg-type] + # type: ignore[arg-type] + execute(signal, broker, repo, _config(), mode="yolo", now_ts=NOW_TS) # -- broker place failure -------------------------------------------------------------------- @@ -483,7 +485,7 @@ def test_broker_place_failure_is_logged_but_not_placed(repo): broker = FakeBroker(place_success=False) signal = _enter_signal() - result = execute(signal, broker, repo, _config(), mode="bypass", now_ts=NOW_TS) + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) assert result.placed is False assert result.order_id is not None # still logged (audit trail even on rejection) @@ -499,7 +501,7 @@ def test_monthly_allowance_vetoes_a_buy_over_the_live_subscription_cap(repo): broker = FakeBroker() signal = _enter_signal() - result = execute(signal, broker, repo, _config(), mode="bypass", now_ts=NOW_TS) + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) assert result.placed is False assert any(v.startswith("monthly_subscription_allowance") for v in result.vetoed_by) @@ -514,11 +516,11 @@ def test_monthly_allowance_updated_subscription_takes_effect_on_the_next_order(r broker = FakeBroker() signal = _enter_signal() - first = execute(signal, broker, repo, _config(), mode="bypass", now_ts=NOW_TS) + first = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) assert first.placed is False _attest(repo, free_volume_usd=Decimal("10000000")) - second = execute(signal, broker, repo, _config(), mode="bypass", now_ts=NOW_TS) + second = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) assert second.placed is True @@ -529,7 +531,7 @@ def test_dca_signal_sizes_via_dca_size_and_places(repo): broker = FakeBroker() signal = _dca_signal() - result = execute(signal, broker, repo, _config(), mode="bypass", now_ts=NOW_TS) + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) assert result.placed is True order = repo.get_order(result.order_id) @@ -549,7 +551,7 @@ def test_dca_signal_exempt_from_averaging_into_losers_but_bound_by_allowlist(rep ), ) - result = execute(signal, broker, repo, _config(), mode="bypass", now_ts=NOW_TS) + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) assert result.placed is False assert any(v.startswith("halal_allowlist") for v in result.vetoed_by) @@ -572,7 +574,7 @@ def _seed_open_position(repo: Repository, product_id: str, qty: Decimal, price: expected_fill=price, actual_fill=price, raw_response=None, - confirmation="bypass", + confirmation="autonomous", rule_id=None, created_at=NOW_TS - 1000, updated_at=NOW_TS - 1000, @@ -594,7 +596,7 @@ def test_exit_signal_sells_the_held_position(repo): ts=NOW_TS, ) - result = execute(signal, broker, repo, _config(), mode="bypass", now_ts=NOW_TS) + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) assert result.placed is True order = repo.get_order(result.order_id) @@ -615,7 +617,7 @@ def test_exit_signal_with_no_open_position_is_not_placed(repo): ts=NOW_TS, ) - result = execute(signal, broker, repo, _config(), mode="bypass", now_ts=NOW_TS) + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) assert result.placed is False assert result.order_id is None @@ -628,7 +630,7 @@ def test_execute_attaches_oco_bracket_after_a_stop_target_entry_fills(repo): broker = FakeBroker() signal = _enter_signal() - result = execute(signal, broker, repo, _config(), mode="bypass", now_ts=NOW_TS) + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) assert result.placed is True # entry + ONE native bracket = 2 place_order calls (was 3: entry + stop leg + target leg) @@ -844,7 +846,7 @@ def test_a_filled_order_records_the_previewed_commission_as_its_fee(repo): ) signal = _enter_signal() - result = execute(signal, broker, repo, _config(), "bypass", confirm_fn=None, now_ts=NOW_TS) + result = execute(signal, broker, repo, _config(), "autonomous", confirm_fn=None, now_ts=NOW_TS) assert result.placed is True order = repo.get_order(result.order_id) @@ -1006,7 +1008,7 @@ def test_execute_surfaces_the_bracket_order_id_it_placed(repo): """ broker = FakeBroker() - result = execute(_enter_signal(), broker, repo, _config(), mode="bypass", now_ts=NOW_TS) + result = execute(_enter_signal(), broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) assert result.bracket_order_id is not None, "execute discarded the bracket's order id again" bracket = repo.get_order(result.bracket_order_id) @@ -1024,7 +1026,7 @@ def test_a_vetoed_bracket_leaves_no_bracket_order_id(repo): # DCA carries no stop, so no bracket is ever placed for it. result = execute( _enter_signal(setup=_setup(context={"order_class": "dca"})), - broker, repo, _config(), mode="bypass", now_ts=NOW_TS, + broker, repo, _config(), mode="autonomous", now_ts=NOW_TS, ) assert result.placed is True @@ -1188,7 +1190,9 @@ def test_an_exit_cancels_the_resting_bracket_before_selling(repo): rule_name="pullback_continuation", now_ts=NOW_TS, ) - result = execute(_exit_signal(), broker, repo, _config(), "bypass", None, now_ts=NOW_TS + 10) + result = execute( + _exit_signal(), broker, repo, _config(), "autonomous", None, now_ts=NOW_TS + 10 + ) assert result.placed is True assert repo.get_order(bracket_id)["status"] == "canceled" @@ -1215,7 +1219,9 @@ def cancel_order(self, order_id: str) -> bool: ) placed_before = len(broker.place_calls) - result = execute(_exit_signal(), broker, repo, _config(), "bypass", None, now_ts=NOW_TS + 10) + result = execute( + _exit_signal(), broker, repo, _config(), "autonomous", None, now_ts=NOW_TS + 10 + ) assert result.placed is False assert "bracket" in (result.reason or "").lower() @@ -1226,7 +1232,7 @@ def test_an_entry_does_not_try_to_cancel_anything(repo): """Negative control: the bracket-clearing step is EXIT-only. An entry must not touch it.""" broker = FakeBroker() - execute(_enter_signal(), broker, repo, _config(), "bypass", None, now_ts=NOW_TS) + execute(_enter_signal(), broker, repo, _config(), "autonomous", None, now_ts=NOW_TS) assert broker.cancel_calls == [] @@ -1251,7 +1257,7 @@ def get_order(self, order_id: str) -> dict: broker = _ObservingBroker() - result = execute(_enter_signal(), broker, repo, _config(), "bypass", None, now_ts=NOW_TS) + result = execute(_enter_signal(), broker, repo, _config(), "autonomous", None, now_ts=NOW_TS) order = repo.get_order(result.order_id) assert order["actual_fill"] == Decimal("50123.45") @@ -1269,7 +1275,7 @@ def get_order(self, order_id: str) -> dict: broker = _BlindBroker() - result = execute(_enter_signal(), broker, repo, _config(), "bypass", None, now_ts=NOW_TS) + result = execute(_enter_signal(), broker, repo, _config(), "autonomous", None, now_ts=NOW_TS) assert result.placed is True order = repo.get_order(result.order_id) diff --git a/tests/execution/test_withdrawal_attestation.py b/tests/execution/test_withdrawal_attestation.py index 4e5e7cec..081738e9 100644 --- a/tests/execution/test_withdrawal_attestation.py +++ b/tests/execution/test_withdrawal_attestation.py @@ -65,15 +65,20 @@ def get_state(self, *a, **k): # -- CLI ----------------------------------------------------------------------- -def test_cli_roundtrip_and_suspension_message(tmp_path): +def test_cli_roundtrip_and_suspension_message(tmp_path, monkeypatch): db_path = tmp_path / "t.db" runner = CliRunner() + # `--enabled` RELEASES rail 17's entry halt, so it now demands a typed `yes` at a terminal + # (`--suspended` stays ungated -- it only ever reduces capability). + import keel.cli as cli_module + + monkeypatch.setattr(cli_module, "_is_interactive", lambda: True) unknown = runner.invoke(cli, ["--db", str(db_path), "withdrawals", "show"]) assert "UNKNOWN (never attested)" in unknown.output assert runner.invoke( - cli, ["--db", str(db_path), "withdrawals", "attest", "--enabled"] + cli, ["--db", str(db_path), "withdrawals", "attest", "--enabled"], input="yes\n" ).exit_code == 0 assert "ENABLED" in runner.invoke(cli, ["--db", str(db_path), "withdrawals", "show"]).output diff --git a/tests/fixtures/config_golden_defaults.json b/tests/fixtures/config_golden_defaults.json index 40055e73..6e229176 100644 --- a/tests/fixtures/config_golden_defaults.json +++ b/tests/fixtures/config_golden_defaults.json @@ -3,7 +3,6 @@ "BTC" ], "auto_trade": { - "bypass_arm_ttl_sec": 3600, "enabled": false, "interval_sec": 900, "mode": "paper" diff --git a/tests/fixtures/config_golden_full.json b/tests/fixtures/config_golden_full.json index e89d7124..e3943abb 100644 --- a/tests/fixtures/config_golden_full.json +++ b/tests/fixtures/config_golden_full.json @@ -5,7 +5,6 @@ "PAXG" ], "auto_trade": { - "bypass_arm_ttl_sec": 7200, "enabled": true, "interval_sec": 1800, "mode": "confirm" diff --git a/tests/fixtures/config_golden_full.yaml b/tests/fixtures/config_golden_full.yaml index 95d8b602..b768bdd0 100644 --- a/tests/fixtures/config_golden_full.yaml +++ b/tests/fixtures/config_golden_full.yaml @@ -35,7 +35,6 @@ auto_trade: mode: confirm enabled: true interval_sec: 1800 - bypass_arm_ttl_sec: 7200 promotion: min_trades: 42 diff --git a/tests/security/__init__.py b/tests/security/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/tests/security/test_authz.py b/tests/security/test_authz.py deleted file mode 100644 index 1ad2a623..00000000 --- a/tests/security/test_authz.py +++ /dev/null @@ -1,177 +0,0 @@ -"""Tests for keel.security.authz: set_passphrase, verify, require, AuthzError. - -Main spec §14 "Dangerous-action authorization gate": a passphrase (stored as a scrypt hash, -rate-limited) is required only for {arm_bypass, raise_caps, disable_killswitch, unlock_vault}. -Read-only/confirm-mode actions require nothing. -""" - -from __future__ import annotations - -import json -import os - -import pytest - -from keel.security.authz import ( - DANGEROUS_ACTIONS, - MAX_ATTEMPTS, - AuthzError, - require, - set_passphrase, - verify, -) - - -@pytest.fixture -def authz_path(tmp_path): - return tmp_path / "authz.json" - - -def test_set_passphrase_persists_hash_not_plaintext(authz_path): - set_passphrase("correct horse battery staple", path=authz_path) - - raw = authz_path.read_text() - assert "correct horse battery staple" not in raw - - state = json.loads(raw) - assert "hash" in state - assert "salt" in state - assert state["hash"] != "correct horse battery staple" - - -def test_set_passphrase_uses_a_random_salt_per_call(authz_path, tmp_path): - other_path = tmp_path / "authz2.json" - - set_passphrase("same passphrase", path=authz_path) - set_passphrase("same passphrase", path=other_path) - - state_a = json.loads(authz_path.read_text()) - state_b = json.loads(other_path.read_text()) - assert state_a["salt"] != state_b["salt"] - assert state_a["hash"] != state_b["hash"] - - -def test_verify_correct_passphrase_returns_true(authz_path): - set_passphrase("swordfish", path=authz_path) - - assert verify("swordfish", path=authz_path) is True - - -def test_verify_wrong_passphrase_returns_false(authz_path): - set_passphrase("swordfish", path=authz_path) - - assert verify("wrong-guess", path=authz_path) is False - - -def test_verify_with_no_passphrase_configured_returns_false(authz_path): - assert not authz_path.exists() - - assert verify("anything", path=authz_path) is False - - -def test_verify_resets_failed_attempts_after_success(authz_path): - set_passphrase("swordfish", path=authz_path) - - verify("wrong-1", path=authz_path) - verify("wrong-2", path=authz_path) - assert verify("swordfish", path=authz_path) is True - - state = json.loads(authz_path.read_text()) - assert state["failed_attempts"] == 0 - - -def test_require_correct_passphrase_passes_for_dangerous_action(authz_path): - set_passphrase("swordfish", path=authz_path) - - require("arm_bypass", "swordfish", path=authz_path) # must not raise - - -def test_require_wrong_passphrase_raises_authzerror(authz_path): - set_passphrase("swordfish", path=authz_path) - - with pytest.raises(AuthzError, match="arm_bypass"): - require("arm_bypass", "wrong-guess", path=authz_path) - - -@pytest.mark.parametrize("action", sorted(DANGEROUS_ACTIONS)) -def test_require_gates_every_dangerous_action(authz_path, action): - set_passphrase("swordfish", path=authz_path) - - require(action, "swordfish", path=authz_path) # correct passphrase passes - with pytest.raises(AuthzError): - require(action, "wrong-guess", path=authz_path) - - -def test_require_readonly_action_needs_no_passphrase_even_when_unset(authz_path): - assert not authz_path.exists() - - require("view_status", "", path=authz_path) # must not raise - - -def test_require_readonly_action_needs_no_passphrase_even_when_wrong(authz_path): - set_passphrase("swordfish", path=authz_path) - - require("confirm_trade", "totally-wrong", path=authz_path) # must not raise - - -def test_dangerous_actions_constant_matches_spec(): - assert DANGEROUS_ACTIONS == frozenset( - {"arm_bypass", "raise_caps", "disable_killswitch", "unlock_vault"} - ) - - -def test_require_with_no_passphrase_configured_raises_for_dangerous_action(authz_path): - assert not authz_path.exists() - - with pytest.raises(AuthzError): - require("unlock_vault", "whatever", path=authz_path) - - -def test_n_wrong_attempts_locks_out_further_attempts(authz_path): - set_passphrase("swordfish", path=authz_path) - now = 1_000_000.0 - - for i in range(MAX_ATTEMPTS): - assert verify(f"wrong-{i}", path=authz_path, now=now) is False - - # Locked out now: even the *correct* passphrase is rejected while locked. - assert verify("swordfish", path=authz_path, now=now) is False - - state = json.loads(authz_path.read_text()) - assert state["locked_until"] > now - - -def test_lockout_expires_after_backoff_window(authz_path): - set_passphrase("swordfish", path=authz_path) - now = 2_000_000.0 - - for i in range(MAX_ATTEMPTS): - verify(f"wrong-{i}", path=authz_path, now=now) - assert verify("swordfish", path=authz_path, now=now) is False - - state = json.loads(authz_path.read_text()) - locked_until = state["locked_until"] - - # Still locked just before the window elapses. - assert verify("swordfish", path=authz_path, now=locked_until - 1) is False - # Free again once the backoff window has passed. - assert verify("swordfish", path=authz_path, now=locked_until + 1) is True - - -def test_require_raises_during_lockout_even_with_correct_passphrase(authz_path): - set_passphrase("swordfish", path=authz_path) - now = 3_000_000.0 - - for i in range(MAX_ATTEMPTS): - verify(f"wrong-{i}", path=authz_path, now=now) - - with pytest.raises(AuthzError): - require("raise_caps", "swordfish", path=authz_path, now=now) - - -@pytest.mark.skipif(os.name == "nt", reason="POSIX file permissions only") -def test_authz_file_is_not_world_readable(authz_path): - set_passphrase("swordfish", path=authz_path) - - mode = authz_path.stat().st_mode & 0o777 - assert mode & 0o077 == 0 diff --git a/tests/security/test_secrets.py b/tests/security/test_secrets.py deleted file mode 100644 index f5339cda..00000000 --- a/tests/security/test_secrets.py +++ /dev/null @@ -1,108 +0,0 @@ -"""Tests for keel.security.secrets: the portable AES-GCM encrypted secrets vault. - -Main spec §14 Part A: master passphrase -> scrypt KDF -> key -> AES-GCM encrypt/decrypt a JSON -secrets blob in `secrets.enc`, copyable between machines. -""" - -from __future__ import annotations - -import stat - -import pytest - -from keel.security.secrets import VaultError, load_vault, migrate_from_env, save_vault - -SECRETS = { - "api_key": "organizations/abc/apiKeys/def", - "api_secret": "-----BEGIN EC PRIVATE KEY-----\nverysecret\n-----END EC PRIVATE KEY-----", -} - - -def test_save_then_load_vault_round_trips_secrets(tmp_path): - vault_path = tmp_path / "secrets.enc" - - save_vault(SECRETS, "correct horse battery staple", path=vault_path) - loaded = load_vault("correct horse battery staple", path=vault_path) - - assert loaded == SECRETS - - -def test_save_vault_chmods_file_600(tmp_path): - vault_path = tmp_path / "secrets.enc" - - save_vault(SECRETS, "correct horse battery staple", path=vault_path) - - mode = stat.S_IMODE(vault_path.stat().st_mode) - assert mode == 0o600 - - -def test_load_vault_with_wrong_passphrase_raises_vaulterror(tmp_path): - vault_path = tmp_path / "secrets.enc" - save_vault(SECRETS, "correct horse battery staple", path=vault_path) - - with pytest.raises(VaultError): - load_vault("wrong passphrase", path=vault_path) - - -def test_load_vault_with_tampered_file_raises_vaulterror(tmp_path): - vault_path = tmp_path / "secrets.enc" - save_vault(SECRETS, "correct horse battery staple", path=vault_path) - - raw = bytearray(vault_path.read_bytes()) - raw[-1] ^= 0xFF # flip the last byte of the ciphertext/tag - vault_path.write_bytes(bytes(raw)) - - with pytest.raises(VaultError): - load_vault("correct horse battery staple", path=vault_path) - - -def test_load_vault_missing_file_raises_vaulterror(tmp_path): - vault_path = tmp_path / "does-not-exist.enc" - - with pytest.raises(VaultError): - load_vault("any passphrase", path=vault_path) - - -def test_on_disk_blob_is_ciphertext_not_plaintext_values(tmp_path): - vault_path = tmp_path / "secrets.enc" - - save_vault(SECRETS, "correct horse battery staple", path=vault_path) - - raw = vault_path.read_bytes() - assert b"organizations/abc/apiKeys/def" not in raw - assert b"BEGIN EC PRIVATE KEY" not in raw - assert b"verysecret" not in raw - - -def test_two_saves_of_the_same_secrets_produce_different_ciphertext(tmp_path): - """Fresh salt+nonce per save (no key/nonce reuse) even for identical plaintext.""" - path_a = tmp_path / "a.enc" - path_b = tmp_path / "b.enc" - - save_vault(SECRETS, "correct horse battery staple", path=path_a) - save_vault(SECRETS, "correct horse battery staple", path=path_b) - - assert path_a.read_bytes() != path_b.read_bytes() - - -def test_migrate_from_env_seals_env_secrets_into_vault(tmp_path): - env_path = tmp_path / ".env" - env_path.write_text('CDP_API_KEY="organizations/abc/apiKeys/def"\nCDP_API_SECRET="shh-dont-log-me"\n') - vault_path = tmp_path / "secrets.enc" - - migrate_from_env(env_path=env_path, passphrase="correct horse battery staple", path=vault_path) - - loaded = load_vault("correct horse battery staple", path=vault_path) - assert loaded == { - "api_key": "organizations/abc/apiKeys/def", - "api_secret": "shh-dont-log-me", - } - - -def test_migrate_from_env_missing_file_yields_empty_vault(tmp_path): - env_path = tmp_path / ".env" - vault_path = tmp_path / "secrets.enc" - - migrate_from_env(env_path=env_path, passphrase="correct horse battery staple", path=vault_path) - - assert load_vault("correct horse battery staple", path=vault_path) == {} diff --git a/tests/test_agent.py b/tests/test_agent.py index 0a0507de..6d8c517f 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -152,13 +152,10 @@ def repo() -> Repository: # of each test's `now_ts`. r.set_state("withdrawals_enabled", True) r.set_state("withdrawals_attested_at", 10**12) - # `_config()` below defaults `auto_trade.mode` to "bypass" -- Issue #60 (bypass-arm - # hardening) means `run_once` now refuses that mode unarmed. Every pre-existing test in - # this module was written against "bypass just works"; arming here (a huge ttl so no test's - # `now_ts` can ever run past `armed_until`) keeps that behavior for tests that aren't - # specifically exercising the arm/disarm/expiry gate itself (those call - # `repo.disarm_bypass()` or a short ttl explicitly). - r.arm_bypass(now_ts=0, ttl_sec=10**12) + # Autonomy is a PROFILE choice now. Most tests in this module were written against "it just + # places"; opting the profile in here preserves that, and the tests that are specifically + # about the confirm/autonomous decision set the profile themselves. + r.set_autonomous(True, now_ts=0) # rail 14 now derives its cap from the attested subscription record rather than a config # default; attest a very large allowance so pre-existing tests here (none of which exercise # rail 14) aren't incidentally tripped by it. @@ -178,7 +175,7 @@ def _config(**overrides: Any) -> Config: max_per_asset_pct=Decimal("1"), ), market_data=MarketDataConfig(granularities=[Granularity.ONE_DAY], history_days=365), - auto_trade=AutoTradeConfig(mode="bypass", interval_sec=50_000), + auto_trade=AutoTradeConfig(mode="confirm", interval_sec=50_000), money_mgmt=MoneyMgmtConfig(), dca=DcaConfig(budget_usd=Decimal("50"), cadence_days=7), ) @@ -222,7 +219,7 @@ def _seed_open_position( expected_fill=price, actual_fill=price, raw_response=None, - confirmation="bypass", + confirmation="autonomous", rule_id=None, created_at=ts, updated_at=ts, @@ -315,73 +312,75 @@ def test_run_once_polls_evaluates_and_executes_a_real_dca_rule(repo): assert repo.get_state("last_feed_ts") == 90_000 -# -- run_once: bypass-arm hardening (Issue #60) -------------------------------------------------- +# -- run_once: autonomy is a live-read profile choice -------------------------------------------- -def test_bypass_without_armed_token_places_nothing_and_reports_refusal(repo): - """The core fix: `config.auto_trade.mode == "bypass"` with no armed token must not place - any order, even though a real, merged `Dca` rule would otherwise fire -- it fails safe by - falling back to confirm behavior (`confirm_fn=None` -> never placed) and surfaces why. - """ - repo.disarm_bypass() +def test_autonomy_off_places_nothing_even_though_the_rule_fires(repo): + """The default. The rule still fires and is logged, but with no `confirm_fn` the order is + previewed and never placed -- confirm mode fails closed.""" + repo.set_autonomous(False, now_ts=0) repo.insert_rule("dca", {"product_id": PRODUCT}, status="live") broker = FakeBroker(series={(PRODUCT, Granularity.ONE_DAY): [_candle(0, "100")]}) result = run_once(broker, repo, _config(), now_ts=90_000) - assert result.skipped is False assert len(result.enter_signals) == 1 # the rule still fires... assert result.enter_results[0].placed is False # ...but nothing is placed. assert broker.place_calls == [] - assert result.mode == "confirm" # fell back, fail-safe - assert result.bypass_refused_reason is not None - assert "armed" in result.bypass_refused_reason.lower() + assert result.mode == "confirm" assert repo.get_orders() == [] -def test_bypass_with_expired_token_places_nothing(repo): - repo.arm_bypass(now_ts=1_000, ttl_sec=10) # armed_until = 1_010 +def test_autonomy_on_places_without_a_confirm_fn(repo): + repo.set_autonomous(True, now_ts=0) repo.insert_rule("dca", {"product_id": PRODUCT}, status="live") broker = FakeBroker(series={(PRODUCT, Granularity.ONE_DAY): [_candle(0, "100")]}) - result = run_once(broker, repo, _config(), now_ts=90_000) # long past armed_until + result = run_once(broker, repo, _config(), now_ts=90_000) + + assert result.mode == "autonomous" + assert result.enter_results[0].placed is True + assert len(broker.place_calls) == 1 + assert repo.get_orders(mode="live", product_id=PRODUCT)[0]["side"] == "BUY" + + +def test_an_absent_profile_row_is_treated_as_NOT_autonomous(repo): + """Fails closed: a database that never recorded a choice must not imply consent.""" + repo._conn.execute("DELETE FROM profile") + repo._conn.commit() + repo.insert_rule("dca", {"product_id": PRODUCT}, status="live") + broker = FakeBroker(series={(PRODUCT, Granularity.ONE_DAY): [_candle(0, "100")]}) + + result = run_once(broker, repo, _config(), now_ts=90_000) - assert result.enter_results[0].placed is False - assert broker.place_calls == [] assert result.mode == "confirm" - assert result.bypass_refused_reason is not None + assert broker.place_calls == [] -def test_bypass_with_fresh_armed_token_places_normally(repo): - """A freshly armed token (well within ttl) lets bypass through -- still subject to every - guard, but with no confirm prompt required, exactly like bypass behaved before Issue #60.""" - repo.disarm_bypass() - repo.arm_bypass(now_ts=1_000, ttl_sec=100_000) # armed_until = 101_000 +def test_the_profile_is_re_read_every_cycle_not_cached(repo): + """`keel autonomy off` must take effect on the NEXT order, not the next restart.""" + repo.set_autonomous(True, now_ts=0) repo.insert_rule("dca", {"product_id": PRODUCT}, status="live") broker = FakeBroker(series={(PRODUCT, Granularity.ONE_DAY): [_candle(0, "100")]}) - result = run_once(broker, repo, _config(), now_ts=90_000) # inside the armed window + first = run_once(broker, repo, _config(), now_ts=90_000) + assert first.mode == "autonomous" - assert result.mode == "bypass" - assert result.bypass_refused_reason is None - assert result.enter_results[0].placed is True - assert len(broker.place_calls) == 1 - assert repo.get_orders(mode="live", product_id=PRODUCT)[0]["side"] == "BUY" + repo.set_autonomous(False, now_ts=1) + second = run_once(broker, repo, _config(), now_ts=180_000) + assert second.mode == "confirm", "the profile was cached; turning autonomy off did nothing" -def test_confirm_mode_unaffected_by_arming_state(repo): - """`mode="confirm"` never even looks at the arm token -- arm-check only gates bypass.""" - repo.disarm_bypass() +def test_paper_mode_places_nothing_even_when_autonomous(repo): + """The two switches are independent: autonomy never turns a simulation into real money.""" + repo.set_autonomous(True, now_ts=0) repo.insert_rule("dca", {"product_id": PRODUCT}, status="live") broker = FakeBroker(series={(PRODUCT, Granularity.ONE_DAY): [_candle(0, "100")]}) - config = _config(auto_trade=AutoTradeConfig(mode="confirm", interval_sec=50_000)) + config = _config(auto_trade=AutoTradeConfig(mode="paper", interval_sec=50_000)) - result = run_once(broker, repo, config, now_ts=90_000) + run_once(broker, repo, config, now_ts=90_000) - assert result.mode == "confirm" - assert result.bypass_refused_reason is None # bypass was never requested - assert result.enter_results[0].placed is False # confirm_fn=None -> fails closed, as always - assert broker.place_calls == [] + assert broker.place_calls == [], "paper mode must never reach the broker" # -- run_once: EXIT wiring on a held position --------------------------------------------------- @@ -1091,6 +1090,7 @@ def _live_ready_repo(repo): def test_confirm_APPROVED_places_the_order(repo, monkeypatch): """The change: an approved confirm-mode order is actually placed (was: never).""" + repo.set_autonomous(False, now_ts=0) # these tests are ABOUT the confirm gate _live_ready_repo(repo) _seed_rule(repo, monkeypatch, _AlwaysEnterRule(PRODUCT), status="live") broker = FakeBroker(series={(PRODUCT, Granularity.ONE_DAY): [_candle(0, "100")]}) @@ -1103,6 +1103,7 @@ def test_confirm_APPROVED_places_the_order(repo, monkeypatch): def test_confirm_DECLINED_places_nothing(repo, monkeypatch): + repo.set_autonomous(False, now_ts=0) # these tests are ABOUT the confirm gate _live_ready_repo(repo) _seed_rule(repo, monkeypatch, _AlwaysEnterRule(PRODUCT), status="live") broker = FakeBroker(series={(PRODUCT, Granularity.ONE_DAY): [_candle(0, "100")]}) @@ -1114,6 +1115,7 @@ def test_confirm_DECLINED_places_nothing(repo, monkeypatch): def test_confirm_fn_defaulting_to_None_still_fails_closed(repo, monkeypatch): """No confirm_fn (the old default) must still place nothing -- backward compatible.""" + repo.set_autonomous(False, now_ts=0) # these tests are ABOUT the confirm gate _live_ready_repo(repo) _seed_rule(repo, monkeypatch, _AlwaysEnterRule(PRODUCT), status="live") broker = FakeBroker(series={(PRODUCT, Granularity.ONE_DAY): [_candle(0, "100")]}) @@ -1124,6 +1126,7 @@ def test_confirm_fn_defaulting_to_None_still_fails_closed(repo, monkeypatch): def test_confirm_fn_sees_the_broker_preview(repo, monkeypatch): + repo.set_autonomous(False, now_ts=0) # these tests are ABOUT the confirm gate _live_ready_repo(repo) _seed_rule(repo, monkeypatch, _AlwaysEnterRule(PRODUCT), status="live") broker = FakeBroker(series={(PRODUCT, Granularity.ONE_DAY): [_candle(0, "100")]}) @@ -1140,6 +1143,7 @@ def _capture(preview): def test_a_rail_veto_means_the_confirm_prompt_is_never_reached(repo, monkeypatch): """The rails run FIRST. A vetoed order never asks the human -- confirmation is not a substitute for the hard limits.""" + repo.set_autonomous(False, now_ts=0) # these tests are ABOUT the confirm gate _live_ready_repo(repo) _seed_rule(repo, monkeypatch, _AlwaysEnterRule("DOGE-USD"), status="live") # off allowlist broker = FakeBroker(series={("DOGE-USD", Granularity.ONE_DAY): [_candle(0, "100")]}) @@ -1183,6 +1187,7 @@ def test_interactive_confirm_fails_closed_without_a_tty(monkeypatch): def test_agent_command_passes_interactive_confirm_in_CONFIRM_mode(repo, monkeypatch): """The wiring: `keel agent` (confirm) hands run_once the interactive confirm_fn; bypass hands it None.""" + repo.set_autonomous(False, now_ts=0) # these tests are ABOUT the confirm gate from click.testing import CliRunner import keel.cli as cli_module @@ -1206,3 +1211,27 @@ def _fake_run_once(broker, repo_arg, config, now_ts, confirm_fn=None): assert result.exit_code == 0, result.output assert captured["mode"] == "confirm" assert captured["confirm_fn"] is cli_module._interactive_confirm + + +def test_an_EXPIRED_autonomy_falls_back_to_confirm_and_places_nothing(repo): + """End-to-end guard on the enforcement chain run_once -> _effective_mode -> is_autonomous. + Without this, dropping the now_ts argument would silently un-bound autonomy again.""" + repo.set_autonomous(True, now_ts=1_000, expires_ts=50_000) + repo.insert_rule("dca", {"product_id": PRODUCT}, status="live") + broker = FakeBroker(series={(PRODUCT, Granularity.ONE_DAY): [_candle(0, "100")]}) + + result = run_once(broker, repo, _config(), now_ts=90_000) # well past the expiry + + assert result.mode == "confirm", "expired autonomy must fall back to asking a human" + assert broker.place_calls == [] + + +def test_autonomy_still_applies_strictly_before_its_expiry(repo): + repo.set_autonomous(True, now_ts=1_000, expires_ts=90_001) + repo.insert_rule("dca", {"product_id": PRODUCT}, status="live") + broker = FakeBroker(series={(PRODUCT, Granularity.ONE_DAY): [_candle(0, "100")]}) + + result = run_once(broker, repo, _config(), now_ts=90_000) + + assert result.mode == "autonomous" + assert len(broker.place_calls) == 1 diff --git a/tests/test_cli.py b/tests/test_cli.py index 98fa2245..cca1b64a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -6,9 +6,8 @@ `keel.cli._build_broker` (the one seam that would otherwise construct a real, network-talking `CoinbaseClient`) to return it instead. -Dangerous commands (`agent --bypass`, `resume`) are gated by `keel.security.authz`; read-only -commands (`db import`, `monitor`, `rules list`, `pnl`) are not and work with no `authz.json` on -disk at all. +Halt-releasing commands (`resume`, `reset-hwm`, ...) demand a typed `yes` from a terminal; +read-only commands (`db import`, `monitor`, `rules list`, `pnl`) need no confirmation at all. """ from __future__ import annotations @@ -25,11 +24,9 @@ from keel.cli import DISCLAIMER, cli from keel.data.db import connect, migrate from keel.data.repository import Repository -from keel.security import authz from keel.types import Candle, Granularity FIXTURES_DIR = Path(__file__).parent / "fixtures" / "transactions_dir" -PASSPHRASE = "correct-horse-battery-staple" def _repo_at(db_path: Path) -> Repository: @@ -86,20 +83,6 @@ def test_db_import_runs_importer_against_temp_db(tmp_path): assert len(repo.get_transactions()) > 0 -def test_db_import_needs_no_passphrase(tmp_path): - """Read-only command: works even though no --authz-path file exists at all.""" - db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" - runner = CliRunner() - - result = runner.invoke( - cli, - ["--db", str(db_path), "--authz-path", str(authz_path), "db", "import", str(FIXTURES_DIR)], - ) - - assert result.exit_code == 0, result.output - assert not authz_path.exists() - # -- disclaimer ----------------------------------------------------------------------------- @@ -115,10 +98,9 @@ def test_disclaimer_shown_on_every_command(tmp_path): def test_disclaimer_shown_even_when_refused(tmp_path): db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" runner = CliRunner() - result = runner.invoke(cli, ["--db", str(db_path), "--authz-path", str(authz_path), "resume"]) + result = runner.invoke(cli, ["--db", str(db_path), "resume"]) assert result.exit_code != 0 assert DISCLAIMER in result.output @@ -127,74 +109,12 @@ def test_disclaimer_shown_even_when_refused(tmp_path): # -- agent --bypass gating -------------------------------------------------------------------- -def test_agent_bypass_without_passphrase_is_refused(tmp_path, valid_config_path, monkeypatch): - monkeypatch.setattr(cli_module, "_build_broker", lambda config: FakeBroker()) - db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" - runner = CliRunner() - - result = runner.invoke( - cli, - [ - "--db", str(db_path), - "--config", str(valid_config_path), - "--authz-path", str(authz_path), - "agent", "--bypass", - ], - ) - - assert result.exit_code != 0 - assert "denied" in result.output.lower() -def test_agent_bypass_with_wrong_passphrase_is_refused(tmp_path, valid_config_path, monkeypatch): - monkeypatch.setattr(cli_module, "_build_broker", lambda config: FakeBroker()) - db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" - authz.set_passphrase(PASSPHRASE, path=str(authz_path)) - runner = CliRunner() - - result = runner.invoke( - cli, - [ - "--db", str(db_path), - "--config", str(valid_config_path), - "--authz-path", str(authz_path), - "agent", "--bypass", "--passphrase", "wrong-passphrase", - ], - ) - - assert result.exit_code != 0 - assert "denied" in result.output.lower() - - -def test_agent_bypass_with_correct_passphrase_proceeds(tmp_path, valid_config_path, monkeypatch): - monkeypatch.setattr(cli_module, "_build_broker", lambda config: FakeBroker()) - db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" - authz.set_passphrase(PASSPHRASE, path=str(authz_path)) - runner = CliRunner() - - result = runner.invoke( - cli, - [ - "--db", str(db_path), - "--config", str(valid_config_path), - "--authz-path", str(authz_path), - "agent", "--bypass", "--passphrase", PASSPHRASE, - ], - ) - - # No `live` rules are configured and the kill-switch defaults to engaged, so `run_once` - # fails closed immediately -- but crucially the authz gate let it get that far. - assert result.exit_code == 0, result.output - assert "skipped: kill_switch" in result.output - def test_agent_confirm_mode_needs_no_passphrase(tmp_path, valid_config_path, monkeypatch): monkeypatch.setattr(cli_module, "_build_broker", lambda config: FakeBroker()) db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" runner = CliRunner() result = runner.invoke( @@ -202,13 +122,11 @@ def test_agent_confirm_mode_needs_no_passphrase(tmp_path, valid_config_path, mon [ "--db", str(db_path), "--config", str(valid_config_path), - "--authz-path", str(authz_path), - "agent", + "agent", ], ) assert result.exit_code == 0, result.output - assert not authz_path.exists() def test_agent_loop_bounded_by_max_cycles(tmp_path, valid_config_path, monkeypatch): @@ -229,139 +147,12 @@ def test_agent_loop_bounded_by_max_cycles(tmp_path, valid_config_path, monkeypat assert result.output.count("skipped: kill_switch") == 3 -# -- arm-bypass / disarm-bypass (Issue #60, bypass-arm hardening) ------------------------------ -def test_arm_bypass_without_passphrase_is_refused(tmp_path, valid_config_path): - db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" - runner = CliRunner() - result = runner.invoke( - cli, - [ - "--db", str(db_path), - "--config", str(valid_config_path), - "--authz-path", str(authz_path), - "arm-bypass", - ], - ) - - assert result.exit_code != 0 - assert "denied" in result.output.lower() - repo = _repo_at(db_path) - assert repo.is_bypass_armed(now_ts=0) is False - - -def test_arm_bypass_with_wrong_passphrase_is_refused(tmp_path, valid_config_path): - db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" - authz.set_passphrase(PASSPHRASE, path=str(authz_path)) - runner = CliRunner() - - result = runner.invoke( - cli, - [ - "--db", str(db_path), - "--config", str(valid_config_path), - "--authz-path", str(authz_path), - "arm-bypass", "--passphrase", "wrong-passphrase", - ], - ) - - assert result.exit_code != 0 - assert "denied" in result.output.lower() - repo = _repo_at(db_path) - assert repo.is_bypass_armed(now_ts=0) is False - - -def test_arm_bypass_with_correct_passphrase_arms(tmp_path, valid_config_path): - db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" - authz.set_passphrase(PASSPHRASE, path=str(authz_path)) - runner = CliRunner() - - result = runner.invoke( - cli, - [ - "--db", str(db_path), - "--config", str(valid_config_path), - "--authz-path", str(authz_path), - "arm-bypass", "--passphrase", PASSPHRASE, - ], - ) - assert result.exit_code == 0, result.output - assert "armed" in result.output.lower() - repo = _repo_at(db_path) - # `valid_config_path`'s auto_trade.bypass_arm_ttl_sec is 3600 -- armed "now" is well inside. - assert repo.is_bypass_armed(now_ts=int(time.time())) is True -def test_disarm_bypass_clears_the_token_no_passphrase_needed(tmp_path, valid_config_path): - db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" - authz.set_passphrase(PASSPHRASE, path=str(authz_path)) - runner = CliRunner() - runner.invoke( - cli, - [ - "--db", str(db_path), - "--config", str(valid_config_path), - "--authz-path", str(authz_path), - "arm-bypass", "--passphrase", PASSPHRASE, - ], - ) - repo = _repo_at(db_path) - assert repo.is_bypass_armed(now_ts=int(time.time())) is True - - result = runner.invoke( - cli, - [ - "--db", str(db_path), - "--config", str(valid_config_path), - "--authz-path", str(authz_path), - "disarm-bypass", - ], - ) - - assert result.exit_code == 0, result.output - assert "disarmed" in result.output.lower() - repo = _repo_at(db_path) - assert repo.is_bypass_armed(now_ts=int(time.time())) is False - - -def test_agent_bypass_without_arm_bypass_places_nothing_even_with_passphrase( - tmp_path, valid_config_path, monkeypatch -): - """The Issue #60 gap being closed: the CLI passphrase gate on `agent --bypass` alone is not - enough -- without a separate `arm-bypass` call, `run_once` itself refuses to trade - autonomously and the CLI surfaces that refusal.""" - monkeypatch.setattr(cli_module, "_build_broker", lambda config: FakeBroker()) - db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" - authz.set_passphrase(PASSPHRASE, path=str(authz_path)) - repo = _repo_at(db_path) - repo.set_state("kill_switch", False) - repo.insert_rule("dca", {"product_id": "BTC-USD"}, status="live") - runner = CliRunner() - - result = runner.invoke( - cli, - [ - "--db", str(db_path), - "--config", str(valid_config_path), - "--authz-path", str(authz_path), - "agent", "--bypass", "--passphrase", PASSPHRASE, - ], - ) - - assert result.exit_code == 0, result.output - assert "bypass" in result.output.lower() - assert "not armed" in result.output.lower() or "refused" in result.output.lower() - repo = _repo_at(db_path) - assert repo.get_orders() == [] - # -- kill / resume ---------------------------------------------------------------------------- @@ -379,38 +170,20 @@ def test_kill_engages_kill_switch_no_passphrase_needed(tmp_path): def test_resume_without_passphrase_is_refused(tmp_path): db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" runner = CliRunner() runner.invoke(cli, ["--db", str(db_path), "kill"]) - result = runner.invoke(cli, ["--db", str(db_path), "--authz-path", str(authz_path), "resume"]) + result = runner.invoke(cli, ["--db", str(db_path), "resume"]) assert result.exit_code != 0 repo = _repo_at(db_path) assert repo.get_state("kill_switch", default=True) is True -def test_resume_with_wrong_passphrase_is_refused(tmp_path): - db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" - authz.set_passphrase(PASSPHRASE, path=str(authz_path)) - runner = CliRunner() - runner.invoke(cli, ["--db", str(db_path), "kill"]) - - result = runner.invoke( - cli, - ["--db", str(db_path), "--authz-path", str(authz_path), "resume", "--passphrase", "nope"], - ) - - assert result.exit_code != 0 - repo = _repo_at(db_path) - assert repo.get_state("kill_switch") is True - -def test_resume_with_correct_passphrase_disengages(tmp_path): +def test_resume_disengages_when_confirmed(tmp_path, monkeypatch): + _at_a_terminal(monkeypatch) db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" - authz.set_passphrase(PASSPHRASE, path=str(authz_path)) runner = CliRunner() runner.invoke(cli, ["--db", str(db_path), "kill"]) @@ -418,9 +191,9 @@ def test_resume_with_correct_passphrase_disengages(tmp_path): cli, [ "--db", str(db_path), - "--authz-path", str(authz_path), - "resume", "--passphrase", PASSPHRASE, + "resume", ], + input="yes\n", ) assert result.exit_code == 0, result.output @@ -434,7 +207,6 @@ def test_resume_with_correct_passphrase_disengages(tmp_path): def test_monitor_single_poll_needs_no_passphrase(tmp_path, valid_config_path, monkeypatch): monkeypatch.setattr(cli_module, "_build_broker", lambda config: FakeBroker()) db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" runner = CliRunner() result = runner.invoke( @@ -442,14 +214,12 @@ def test_monitor_single_poll_needs_no_passphrase(tmp_path, valid_config_path, mo [ "--db", str(db_path), "--config", str(valid_config_path), - "--authz-path", str(authz_path), - "monitor", + "monitor", ], ) assert result.exit_code == 0, result.output assert result.output.count("polled") == 1 - assert not authz_path.exists() def test_monitor_loop_bounded_by_max_cycles(tmp_path, valid_config_path, monkeypatch): @@ -674,22 +444,19 @@ def test_rules_seed_rows_round_trip_through_build_rule(tmp_path, valid_config_pa def test_rules_seed_needs_no_passphrase(tmp_path): db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" runner = CliRunner() result = runner.invoke( cli, [ "--db", str(db_path), - "--authz-path", str(authz_path), - "rules", "seed", + "rules", "seed", "--products", "BTC-USD", "--kinds", "dca", ], ) assert result.exit_code == 0, result.output - assert not authz_path.exists() # -- pnl -------------------------------------------------------------------------------------- @@ -1019,7 +786,7 @@ def _repo_at(db_path): return Repository(conn) -def test_resume_entries_clears_an_armed_streak_halt(tmp_path): +def test_resume_entries_clears_an_armed_streak_halt(tmp_path, monkeypatch): """Rail 16's violation message tells the operator to run `keel resume-entries`. Until now that command did not exist, and a test merely asserted the message MENTIONED it -- pinning a promise nothing implemented. @@ -1028,50 +795,29 @@ def test_resume_entries_clears_an_armed_streak_halt(tmp_path): setting `max_consecutive_losses: 0` does NOT release an armed halt. Without this, an operator who mis-set the cooloff waits it out or edits sqlite by hand. """ + _at_a_terminal(monkeypatch) db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" - authz.set_passphrase(PASSPHRASE, path=str(authz_path)) repo = _repo_at(db_path) repo.set_state("streak_halt_until", 2_000_000_000) runner = CliRunner() result = runner.invoke( cli, - ["--db", str(db_path), "--authz-path", str(authz_path), - "resume-entries", "--passphrase", PASSPHRASE], + ["--db", str(db_path), "resume-entries"], + input="yes\n", ) assert result.exit_code == 0, result.output assert _repo_at(db_path).get_state("streak_halt_until") == 0 -def test_resume_entries_is_passphrase_gated(tmp_path): - """Negative control: clearing a live-money breaker is a dangerous action, gated like - `resume` -- not something a stray shell command can do.""" - db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" - authz.set_passphrase(PASSPHRASE, path=str(authz_path)) - repo = _repo_at(db_path) - repo.set_state("streak_halt_until", 2_000_000_000) - runner = CliRunner() - - result = runner.invoke( - cli, - ["--db", str(db_path), "--authz-path", str(authz_path), - "resume-entries", "--passphrase", "wrong-passphrase"], - ) - - assert result.exit_code != 0 - assert _repo_at(db_path).get_state("streak_halt_until") == 2_000_000_000 - -def test_reset_hwm_clears_the_equity_high_water_mark(tmp_path): +def test_reset_hwm_clears_the_equity_high_water_mark(tmp_path, monkeypatch): """Rail 11's high-water mark is MONOTONIC, so any bad equity write is permanent: a deposit ratchets it up and a later withdrawal then reads as a drawdown that never recovers. Without this command the only remedy is hand-editing sqlite.""" + _at_a_terminal(monkeypatch) db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" - authz.set_passphrase(PASSPHRASE, path=str(authz_path)) repo = _repo_at(db_path) repo.set_state("equity_high_water_mark", Decimal("15000")) repo.set_state("drawdown_total_pct", Decimal("0.33")) @@ -1079,8 +825,8 @@ def test_reset_hwm_clears_the_equity_high_water_mark(tmp_path): result = runner.invoke( cli, - ["--db", str(db_path), "--authz-path", str(authz_path), - "reset-hwm", "--passphrase", PASSPHRASE], + ["--db", str(db_path), "reset-hwm"], + input="yes\n", ) assert result.exit_code == 0, result.output @@ -1089,78 +835,56 @@ def test_reset_hwm_clears_the_equity_high_water_mark(tmp_path): assert after.get_state("drawdown_total_pct") == Decimal("0") -def test_record_flow_rebases_the_high_water_mark(tmp_path): +def test_record_flow_rebases_the_high_water_mark(tmp_path, monkeypatch): """A deposit is not profit and a withdrawal is not a loss, but equity is cash + positions so both move it. Declaring the flow keeps rail 11's drawdown measuring TRADING performance.""" + _at_a_terminal(monkeypatch) db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" - authz.set_passphrase(PASSPHRASE, path=str(authz_path)) repo = _repo_at(db_path) repo.set_state("equity_high_water_mark", Decimal("10000")) runner = CliRunner() result = runner.invoke( cli, - ["--db", str(db_path), "--authz-path", str(authz_path), - "record-flow", "--amount", "5000", "--passphrase", PASSPHRASE], + ["--db", str(db_path), "record-flow", "--amount", "5000"], + input="yes\n", ) assert result.exit_code == 0, result.output assert _repo_at(db_path).get_state("equity_high_water_mark") == Decimal("15000") -def test_record_flow_accepts_a_negative_amount_for_a_withdrawal(tmp_path): +def test_record_flow_accepts_a_negative_amount_for_a_withdrawal(tmp_path, monkeypatch): + _at_a_terminal(monkeypatch) db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" - authz.set_passphrase(PASSPHRASE, path=str(authz_path)) repo = _repo_at(db_path) repo.set_state("equity_high_water_mark", Decimal("15000")) runner = CliRunner() result = runner.invoke( cli, - ["--db", str(db_path), "--authz-path", str(authz_path), - "record-flow", "--amount", "-5000", "--passphrase", PASSPHRASE], + ["--db", str(db_path), "record-flow", "--amount", "-5000"], + input="yes\n", ) assert result.exit_code == 0, result.output assert _repo_at(db_path).get_state("equity_high_water_mark") == Decimal("10000") -def test_record_flow_is_passphrase_gated(tmp_path): - """Lowering the HWM relaxes a live-money breaker, so this is a dangerous action: an - unauthenticated caller must not be able to shrink a measured drawdown.""" - db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" - authz.set_passphrase(PASSPHRASE, path=str(authz_path)) - repo = _repo_at(db_path) - repo.set_state("equity_high_water_mark", Decimal("15000")) - runner = CliRunner() - result = runner.invoke( - cli, - ["--db", str(db_path), "--authz-path", str(authz_path), - "record-flow", "--amount", "-5000", "--passphrase", "wrong-passphrase"], - ) - - assert result.exit_code != 0 - assert _repo_at(db_path).get_state("equity_high_water_mark") == Decimal("15000") - - -def test_record_flow_rejects_a_non_finite_amount(tmp_path): +def test_record_flow_rejects_a_non_finite_amount(tmp_path, monkeypatch): """`Decimal("nan")` parses without raising. Written into the high-water mark it poisons it permanently: every later `equity > hwm` is False, so the HWM can never re-seed.""" + _at_a_terminal(monkeypatch) db_path = tmp_path / "test.db" - authz_path = tmp_path / "authz.json" - authz.set_passphrase(PASSPHRASE, path=str(authz_path)) repo = _repo_at(db_path) repo.set_state("equity_high_water_mark", Decimal("10000")) runner = CliRunner() result = runner.invoke( cli, - ["--db", str(db_path), "--authz-path", str(authz_path), - "record-flow", "--amount", "nan", "--passphrase", PASSPHRASE], + ["--db", str(db_path), "record-flow", "--amount", "nan"], + input="yes\n", ) assert result.exit_code != 0 @@ -1223,3 +947,233 @@ def test_migrate_honours_an_explicit_db_option(tmp_path): assert int(conn.execute("SELECT version FROM schema_version").fetchone()["version"]) == ( SCHEMA_VERSION ) + + +# -- halt-releasing commands: interactive confirmation, no passphrase ---------- +# +# These four RE-PERMIT trading after a safety halt. They keep a human gate even when autonomous +# mode is on -- "trade without asking me" and "un-stick your own drawdown breaker" are different +# powers, and a breaker that can reset itself is not a breaker. +# +# There is deliberately NO env-var/flag override for the TTY check: any such seam would be +# settable from cron and would defeat the fail-closed. Tests patch the predicate instead. + +_HALT_COMMANDS = ( + ["resume"], + ["resume-entries"], + ["record-flow", "--amount", "500"], + ["reset-hwm"], +) + + +def _at_a_terminal(monkeypatch, yes: bool = True) -> None: + monkeypatch.setattr(cli_module, "_is_interactive", lambda: yes) + + +def test_halt_commands_proceed_on_a_typed_yes(tmp_path, monkeypatch): + _at_a_terminal(monkeypatch) + for args in _HALT_COMMANDS: + db = tmp_path / f"{args[0]}-yes.db" + _repo_at(db) + result = CliRunner().invoke(cli, ["--db", str(db), *args], input="yes\n") + assert result.exit_code == 0, f"{args}: {result.output}" + + +def test_halt_commands_abort_on_anything_other_than_yes(tmp_path, monkeypatch): + """A bare 'y' is not enough -- these are rarer and heavier than an order confirmation.""" + _at_a_terminal(monkeypatch) + for args in _HALT_COMMANDS: + db = tmp_path / f"{args[0]}-no.db" + _repo_at(db) + result = CliRunner().invoke(cli, ["--db", str(db), *args], input="y\n") + assert result.exit_code != 0, f"{args} should have aborted: {result.output}" + assert "aborted" in result.output.lower() + + +def test_halt_commands_fail_closed_without_a_tty(tmp_path, monkeypatch): + """A cron job or piped script must never be able to release a safety halt.""" + _at_a_terminal(monkeypatch, yes=False) + for args in _HALT_COMMANDS: + db = tmp_path / f"{args[0]}-notty.db" + _repo_at(db) + result = CliRunner().invoke(cli, ["--db", str(db), *args], input="yes\n") + assert result.exit_code != 0, f"{args} should have refused off-TTY: {result.output}" + assert "terminal" in result.output.lower() + + +def test_resume_actually_disengages_the_kill_switch_when_confirmed(tmp_path, monkeypatch): + _at_a_terminal(monkeypatch) + db = tmp_path / "resume.db" + repo = _repo_at(db) + repo.set_state("kill_switch", True) + result = CliRunner().invoke(cli, ["--db", str(db), "resume"], input="yes\n") + assert result.exit_code == 0, result.output + assert _repo_at(db).get_state("kill_switch") is False + + +# -- keel autonomy ------------------------------------------------------------ + + +def test_autonomy_is_off_by_default(tmp_path): + db = tmp_path / "a.db" + _repo_at(db) + result = CliRunner().invoke(cli, ["--db", str(db), "autonomy", "show"]) + assert result.exit_code == 0, result.output + assert "off" in result.output + + +def test_autonomy_on_requires_a_typed_yes_and_persists(tmp_path, monkeypatch, valid_config_path): + _at_a_terminal(monkeypatch) + db = tmp_path / "a.db" + _repo_at(db) + result = CliRunner().invoke( + cli, ["--db", str(db), "--config", str(valid_config_path), "autonomy", "on"], input="yes\n" + ) + assert result.exit_code == 0, result.output + assert _repo_at(db).get_profile().autonomous is True + + +def test_autonomy_on_aborts_on_a_bare_y(tmp_path, monkeypatch, valid_config_path): + _at_a_terminal(monkeypatch) + db = tmp_path / "a.db" + _repo_at(db) + result = CliRunner().invoke( + cli, ["--db", str(db), "--config", str(valid_config_path), "autonomy", "on"], input="y\n" + ) + assert result.exit_code != 0 + assert _repo_at(db).get_profile().autonomous is False + + +def test_autonomy_on_refuses_without_a_terminal(tmp_path, monkeypatch, valid_config_path): + """Arming unattended trading must not be scriptable.""" + _at_a_terminal(monkeypatch, yes=False) + db = tmp_path / "a.db" + _repo_at(db) + result = CliRunner().invoke( + cli, ["--db", str(db), "--config", str(valid_config_path), "autonomy", "on"], input="yes\n" + ) + assert result.exit_code != 0 + assert "terminal" in result.output.lower() + assert _repo_at(db).get_profile().autonomous is False + + +def test_autonomy_off_works_without_a_terminal(tmp_path, monkeypatch): + """De-risking is never obstructed: this must work from cron, a pipe, anywhere.""" + _at_a_terminal(monkeypatch, yes=False) + db = tmp_path / "a.db" + repo = _repo_at(db) + repo.set_autonomous(True, now_ts=1) + + result = CliRunner().invoke(cli, ["--db", str(db), "autonomy", "off"]) + + assert result.exit_code == 0, result.output + assert _repo_at(db).get_profile().autonomous is False + + +def test_autonomy_ON_does_not_let_halt_commands_skip_confirmation(tmp_path, monkeypatch): + """THE invariant. 'Trade without asking me' and 'un-stick your own drawdown breaker' are + different powers; a breaker that can reset itself is not a breaker.""" + _at_a_terminal(monkeypatch, yes=False) # no terminal available + for args in _HALT_COMMANDS: + db = tmp_path / f"auto-{args[0]}.db" + repo = _repo_at(db) + repo.set_autonomous(True, now_ts=1) # fully autonomous... + + result = CliRunner().invoke(cli, ["--db", str(db), *args], input="yes\n") + + assert result.exit_code != 0, f"{args} was released without a human: {result.output}" + assert "terminal" in result.output.lower() + + +def test_withdrawals_attest_ENABLED_needs_a_terminal(tmp_path, monkeypatch): + """Rail 17 halts ENTRIES when withdrawals are suspended/stale; `--enabled` releases that + halt, so it is a halt-releasing command like the other four. Its old justification was that + the confirm gate and the bypass-arm token sat in front of it -- with autonomy on, neither + does, so a cron line could re-permit live entries with no human anywhere.""" + _at_a_terminal(monkeypatch, yes=False) + db = tmp_path / "w.db" + repo = _repo_at(db) + repo.set_autonomous(True, now_ts=1) + + result = CliRunner().invoke( + cli, ["--db", str(db), "withdrawals", "attest", "--enabled"], input="yes\n" + ) + + assert result.exit_code != 0, result.output + assert "terminal" in result.output.lower() + assert _repo_at(db).get_state("withdrawals_enabled") is None + + +def test_withdrawals_attest_SUSPENDED_is_ungated_and_needs_no_terminal(tmp_path, monkeypatch): + """De-risking is never obstructed: suspending only ever halts entries.""" + _at_a_terminal(monkeypatch, yes=False) + db = tmp_path / "w2.db" + _repo_at(db) + + result = CliRunner().invoke(cli, ["--db", str(db), "withdrawals", "attest", "--suspended"]) + + assert result.exit_code == 0, result.output + assert _repo_at(db).get_state("withdrawals_enabled") is False + + +def test_withdrawals_attest_ENABLED_proceeds_on_a_typed_yes(tmp_path, monkeypatch): + _at_a_terminal(monkeypatch) + db = tmp_path / "w3.db" + _repo_at(db) + + result = CliRunner().invoke( + cli, ["--db", str(db), "withdrawals", "attest", "--enabled"], input="yes\n" + ) + + assert result.exit_code == 0, result.output + assert _repo_at(db).get_state("withdrawals_enabled") is True + + +def test_autonomy_on_for_hours_sets_an_expiry_that_lapses(tmp_path, monkeypatch, valid_config_path): + """A forgotten `autonomy on` should not be able to grant unattended trading forever.""" + _at_a_terminal(monkeypatch) + db = tmp_path / "exp.db" + _repo_at(db) + result = CliRunner().invoke( + cli, + ["--db", str(db), "--config", str(valid_config_path), "autonomy", "on", "--for-hours", "1"], + input="yes\n", + ) + assert result.exit_code == 0, result.output + profile = _repo_at(db).get_profile() + assert profile.autonomous_until is not None + assert profile.is_autonomous(profile.autonomous_until - 1) is True + assert profile.is_autonomous(profile.autonomous_until) is False + + +def test_autonomy_on_without_for_hours_warns_that_it_never_lapses( + tmp_path, monkeypatch, valid_config_path +): + _at_a_terminal(monkeypatch) + db = tmp_path / "noexp.db" + _repo_at(db) + result = CliRunner().invoke( + cli, ["--db", str(db), "--config", str(valid_config_path), "autonomy", "on"], input="yes\n" + ) + assert result.exit_code == 0, result.output + assert "NO expiry" in result.output + assert _repo_at(db).get_profile().autonomous_until is None + + +def test_autonomy_on_rejects_a_nonsensical_for_hours(tmp_path, monkeypatch, valid_config_path): + """0/negative would write an already-lapsed row while claiming 'autonomy ON until ...'; + inf/nan/huge would overflow int() AFTER the operator had already typed yes.""" + _at_a_terminal(monkeypatch) + for bad in ("0", "-5", "inf", "nan", "1e18"): + db = tmp_path / f"bad-{bad}.db" + _repo_at(db) + result = CliRunner().invoke( + cli, + ["--db", str(db), "--config", str(valid_config_path), + "autonomy", "on", "--for-hours", bad], + input="yes\n", + ) + assert result.exit_code != 0, f"--for-hours {bad} should be rejected: {result.output}" + assert _repo_at(db).get_profile().autonomous is False + + diff --git a/tests/test_config.py b/tests/test_config.py index 454a2486..ff911a4d 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -277,31 +277,9 @@ def test_load_config_tier_missing_name_raises_configerror(write_config): load_config(path) -# -- auto_trade.bypass_arm_ttl_sec (Issue #60, bypass-arm hardening) --------------------------- -def test_load_config_bypass_arm_ttl_sec_default_is_one_hour(valid_config_path): - config = load_config(valid_config_path) - - assert config.auto_trade.bypass_arm_ttl_sec == 3600 - - -def test_load_config_bypass_arm_ttl_sec_overridable(write_config): - text = VALID_CONFIG_YAML.replace("bypass_arm_ttl_sec: 3600", "bypass_arm_ttl_sec: 120") - path = write_config(text) - - config = load_config(path) - - assert config.auto_trade.bypass_arm_ttl_sec == 120 - - -def test_load_config_bypass_arm_ttl_sec_absent_falls_back_to_default(write_config): - text = VALID_CONFIG_YAML.replace(" bypass_arm_ttl_sec: 3600\n", "") - path = write_config(text) - - config = load_config(path) - assert config.auto_trade.bypass_arm_ttl_sec == 3600 # -- logging (engine-activity logging feature) -------------------------------------------------- @@ -477,3 +455,40 @@ def test_non_negative_int_rejects_a_negative_threshold(write_config): ) with pytest.raises(ConfigError, match="max_consecutive_losses"): load_config(write_config(text)) + + +# -- auto_trade.mode is validated (it used to fall back silently) -------------- + + +def test_mode_bypass_is_now_a_hard_error(tmp_path): + """`bypass` no longer exists. Failing loudly beats silently reinterpreting a config that + explicitly asked for autonomy -- autonomy is now a profile choice, not a config mode.""" + p = tmp_path / "c.yaml" + p.write_text( + "allowlist: [BTC]\ncaps: {max_exposure_usd: 100, max_per_asset_pct: 0.5}\n" + "auto_trade: {mode: bypass}\n" + ) + with pytest.raises(ConfigError) as exc: + load_config(str(p)) + assert "auto_trade.mode" in str(exc.value) + + +def test_paper_and_confirm_both_load(tmp_path): + for mode in ("paper", "confirm"): + p = tmp_path / f"{mode}.yaml" + p.write_text( + "allowlist: [BTC]\ncaps: {max_exposure_usd: 100, max_per_asset_pct: 0.5}\n" + f"auto_trade: {{mode: {mode}}}\n" + ) + assert load_config(str(p)).auto_trade.mode == mode + + +def test_an_unknown_mode_names_the_offending_key(tmp_path): + p = tmp_path / "c.yaml" + p.write_text( + "allowlist: [BTC]\ncaps: {max_exposure_usd: 100, max_per_asset_pct: 0.5}\n" + "auto_trade: {mode: wibble}\n" + ) + with pytest.raises(ConfigError) as exc: + load_config(str(p)) + assert "wibble" in str(exc.value) diff --git a/tests/test_subscription_record.py b/tests/test_subscription_record.py index 5a708ccf..86b1656a 100644 --- a/tests/test_subscription_record.py +++ b/tests/test_subscription_record.py @@ -66,7 +66,7 @@ def test_overdue_attestation_overrides_a_stored_active_status() -> None: def test_due_exactly_now_is_already_overdue() -> None: """Boundary: due-at is the moment it expires, not one tick still-good. - Matches `is_bypass_armed`'s strict `now_ts < armed_until` convention elsewhere. + Strict `now_ts < expiry`, the freshness convention used throughout this codebase. """ assert _record(attest_due_ts=NOW).effective_status(NOW) is SubscriptionStatus.SUSPECT diff --git a/uv.lock b/uv.lock index 04e393d0..688dcf2d 100644 --- a/uv.lock +++ b/uv.lock @@ -398,7 +398,6 @@ version = "0.1.0" source = { editable = "." } dependencies = [ { name = "click" }, - { name = "cryptography" }, { name = "keel-broker-api" }, { name = "keel-broker-coinbase" }, { name = "keel-core" }, @@ -415,7 +414,6 @@ dev = [ [package.metadata] requires-dist = [ { name = "click", specifier = ">=8.4.2" }, - { name = "cryptography", specifier = ">=49.0.0" }, { name = "keel-broker-api", editable = "packages/keel-broker-api" }, { name = "keel-broker-coinbase", editable = "packages/keel-broker-coinbase" }, { name = "keel-core", editable = "packages/keel-core" },