From 49cb2051a544418df9d678fcdccee3ae3dac5c78 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Fri, 4 Sep 2026 15:23:16 -0400 Subject: [PATCH] feat(web): one chronology over four stores, with an injection-safe audit export (#703) Four stores record what keel has done and none of them knew about the others: the engine's JSONL log, the `orders` book, the `transactions` ledger, and the attestation tables. Merging them is only an improvement if the merge keeps them APART -- otherwise it is four tables with their labels removed. So provenance is a first-class field from a closed vocabulary, decided once in the service and never inferred from a row's shape by a renderer. A live fill is `venue-reported`. A paper fill is `simulated` -- the paper trader wrote it with no venue involved, and one word covering both would put synthetic and real evidence in one bucket, which is the one thing four separate tables at least never did. A `transactions` row is `imported-ledger`: the venue produced the CSV, but nothing verified the line on the way in, and that is not the same claim as a venue report. An attestation is `human-attested`, and the name of whoever swore to it is in the summary, because "who signed this" is the first question an auditor asks of one. A cycle is `engine-log`: the agent's own account, which nothing outside the process confirmed and nobody signed. `simulated` is the only provenance styled WARN. Not because synthetic evidence is worse -- an imported ledger line is not worse evidence than a venue report, it is different -- but because a paper fill sitting in a chronology beside real ones is the single row a reader must not skim past. THE EXPORT IS A SECURITY SURFACE, AND WAS TREATED AS ONE FROM THE FIRST LINE. This file is meant to be opened in Excel or Google Sheets by an auditor or a tax preparer, and both EXECUTE a cell whose text begins `=`, `+`, `-`, `@`, tab or carriage return. Several columns carry text keel did not write: a transaction's `notes` and `coinbase_id` come out of a venue's own CSV export, an attestation's `source` and `attested_by` were typed by a person. `csv_safe` applies OWASP's defence -- prefix with a single quote so the cell is inert text -- to EVERY cell rather than to a list of the risky ones, because a maintained list of which columns are attacker-influenced is exactly the thing that rots. Writing the end-to-end test for that found my own example was wrong: a hostile `notes` value is diluted by accident, because `summary` prefixes it with "deposit USD -- " and the cell no longer starts with a trigger. The real path is a field that lands at the START of a cell by itself, so the test drives a hostile `coinbase_id` into `reference`. Relying on the accidental prefix is exactly what applying the defence everywhere avoids. THE CSV ROUTE IS DELIBERATELY NOT AN `ApiRoute`. Everything in `API_ROUTES` is wrapped in the JSON envelope by `respond`, and every pin in `test_api.py` is parametrised over that table. A `text/csv` route inside it would force each of those to grow an exception, and an exception carved into a security pin is how the pin stops meaning anything. It gets its own handler branch -- inside the same loopback-plus-session admission, because an export of the whole audit trail must not be reachable more easily than the page it came from -- and its own header suite: `nosniff` (which matters MORE for a file a browser is told to save), `attachment` with a server-generated dated filename, `no-store`. The filename is never echoed from the query string: that would be the header-injection twin of the formula injection the body is defended against, and there is a test for it. NOTHING HERE IS TAMPER-EVIDENT, AND THE COLUMN SAYS SO. #703 asked each row to carry its hash. None of the four stores hashes its rows -- `orders`, `transactions` and both attestation tables have no hash column between them -- and the only hash-chained store in this codebase is the research trials ledger, which records experiments rather than trading activity. Folding it in to borrow its hashes would be provenance laundering, in the export whose entire purpose is provenance. So `row_hash` reads NOT RECORDED, never blank: blank invites the reader to assume the check passed. #721 files the engine work, and names the part that issue cannot pre-empt -- `orders` rows are UPDATED as a venue reports fills, so a chain over them breaks on every legitimate update. The chips come from the SCOPED window and are stored on the report, not derived from the rows on screen. Derived from those, pressing Flows would delete the Trades chip and leave no way back except knowing the empty string means all -- and a kind whose only rows fell past the cap would vanish from the bar while still being in the window. That bug was written, caught by its own test, and fixed here; it is the same one already fixed once on the Orders status tabs. The merged read is capped, because three of the four stores are unbounded and one request would otherwise cost the size of the deployment's whole history. The counts still describe the window, so a reader can see there is more. Co-Authored-By: Claude Opus 5 (1M context) --- keel/commands/timeline.py | 408 ++++++++++++++++++++++++++++ keel/web/api.py | 97 +++++++ keel/web/payload.py | 79 ++++++ keel/web/server.py | 38 +++ keel/web/static/index.html | 1 + keel/web/static/js/main.js | 11 + keel/web/static/js/render.js | 131 +++++++++ keel/web/staticfiles.py | 1 + tests/commands/test_timeline.py | 278 +++++++++++++++++++ tests/commands/test_timeline_csv.py | 91 +++++++ tests/web/test_api.py | 18 ++ tests/web/test_client_assets.py | 1 + tests/web/test_timeline_export.py | 139 ++++++++++ 13 files changed, 1293 insertions(+) create mode 100644 keel/commands/timeline.py create mode 100644 tests/commands/test_timeline.py create mode 100644 tests/commands/test_timeline_csv.py create mode 100644 tests/web/test_timeline_export.py diff --git a/keel/commands/timeline.py b/keel/commands/timeline.py new file mode 100644 index 0000000..97b5e46 --- /dev/null +++ b/keel/commands/timeline.py @@ -0,0 +1,408 @@ +"""One chronology over everything keel has done -- issue #703. + +Four stores record activity and none of them knew about the others: the engine's JSONL log +(cycles), the `orders` table (fills), the `transactions` ledger (cash flows), and the attestation +tables (what a human swore to). This module merges them into one timeline WITHOUT letting them +blur, which is the whole difficulty: a venue-reported fill, a line imported from a venue's CSV, +and a sentence a human typed are three different kinds of evidence, and a feed that presented +them identically would be worse than four separate tables. + +Every row therefore carries its PROVENANCE as a first-class field, from a closed vocabulary, and +the provenance is never inferred from the row's shape -- it is a property of which store the row +came out of, decided here, once. + +**Read-only, no broker, no network.** Same posture as every other service in this package. + +**Nothing here is tamper-evident, and the export says so.** #703 asked the CSV to carry each +row's hash. None of these four stores hashes its rows: `orders`, `transactions`, +`asset_attestations` and `instrument_attestations` have no hash column between them, and the only +hash-chained store in this codebase is the research trials ledger (`keel/research/ledger.py`), +which records experiments rather than trading activity and does not belong in this feed. So the +hash column is emitted as NOT RECORDED rather than left blank -- blank invites the reader to +assume the check passed -- and hashing these tables is filed as engine work. +""" + +from __future__ import annotations + +import csv +import io +from collections.abc import Iterable, Sequence +from dataclasses import dataclass +from decimal import Decimal +from typing import Any + +from keel.commands.orders import normalise_scope, scope_start_ts +from keel.data.repository import Repository + +#: The type chips, and the only words `kind` ever takes. +TIMELINE_KINDS: tuple[str, ...] = ("trade", "flow", "attestation", "system") + +#: How a row came to be known, as a closed vocabulary. The distinction this feed exists to keep: +#: +#: - `venue-reported` -- a live order; the venue told us it happened. +#: - `simulated` -- a paper order. The paper trader wrote it, no venue was involved, and calling +#: it venue-reported would put synthetic fills and real ones under one word. +#: - `imported-ledger` -- a `transactions` row, read out of a venue's own CSV export. +#: - `human-attested` -- someone typed it and signed their name to it. +#: - `engine-log` -- the agent's own structured log of what it did. +PROVENANCES: tuple[str, ...] = ( + "venue-reported", + "simulated", + "imported-ledger", + "human-attested", + "engine-log", +) + +#: What the hash column says until the engine records one. NOT blank: an empty cell in a column +#: headed `row_hash` reads as "nothing to report", and the honest reading is "nobody checked". +HASH_NOT_RECORDED = "NOT RECORDED" + +#: The characters a spreadsheet treats as the start of a formula. OWASP's list. +#: +#: `-` and `+` are here because they are formulas too, not only signs -- which means a negative +#: figure gets quoted. That is the correct trade for an audit export: a spreadsheet shows +#: `'-12.30` as text rather than evaluating it, the value is still readable and still +#: re-importable, and losing numeric typing is a smaller harm than executing a cell. +_FORMULA_TRIGGERS = ("=", "+", "-", "@", "\t", "\r") + + +def csv_safe(value: Any) -> str: + """One cell, safe to open in Excel or Google Sheets. + + **The security control on this export.** Both applications EXECUTE a cell whose text starts + with a formula trigger, and several columns here carry text keel did not write: a + transaction's `notes` (imported from a venue's CSV), an attestation's `source` and + `attested_by` (typed by a human), `product_id` and `rule_name` (config). A cell reading + `=cmd|...` in any of them runs when an auditor opens the file. + + Applied to EVERY text cell, not to a list of the risky ones: a maintained list of which + columns are attacker-influenced is exactly the thing that rots, and the cost of over-applying + is one leading quote on a figure. + + Quoting and comma/newline escaping belong to `csv.writer` and are deliberately not done here + -- doing both would double-escape every field. + """ + if value is None: + return "" + text = str(value) + if text.startswith(_FORMULA_TRIGGERS): + return "'" + text + return text + + +@dataclass(frozen=True) +class TimelineRow: + """One thing that happened, and how we know it happened.""" + + ts: int + #: One of `TIMELINE_KINDS` -- what the chips filter on. + kind: str + #: One of `PROVENANCES`. Never inferred from the row's shape by a renderer. + provenance: str + #: The store this came out of, named plainly (`orders`, `transactions`, ...), so a reader + #: chasing a row knows which table to open. + source: str + #: That store's own identifier for the row -- an `orders.id`, a `coinbase_id`, an asset. + reference: str + #: One line a human can read. Assembled here so both renderers say the same sentence. + summary: str + #: The product this concerns, or `""` where the record has none (a cash flow, an + #: asset-level attestation). + product_id: str + #: The figure this row is ABOUT, and what that figure is. `amount_kind` exists so a fill + #: price, a cash flow and a fee are never summed by a reader who assumed one column meant + #: one thing. + amount: Decimal | None + amount_kind: str + #: The row's own tamper-evidence, when its store records one. None of the four does today. + row_hash: str = HASH_NOT_RECORDED + + +@dataclass(frozen=True) +class TimelineReport: + now_ts: int + scope: str + scope_start_ts: int | None + #: The applied `kind` filter, echoed back, or `""` for every kind. + kind: str + limit: int + #: Rows across all four sources inside the scope, before `kind` and before `limit`. + scoped_count: int + #: Rows after `kind`, before `limit` -- what `rows` is a page of. + filtered_count: int + rows: tuple[TimelineRow, ...] + + #: Every kind present in the SCOPED set, in `TIMELINE_KINDS` order -- what a chip bar is + #: built from. STORED rather than derived from `rows`, because `rows` is what the chip and + #: the cap left: derived from those, selecting Flows would delete the Trades chip and leave + #: no way back, and a kind whose only rows fell past the limit would vanish from the bar + #: while still being in the window. A control that deletes its own alternatives is worse + #: than one that is sometimes empty. + #: + #: In the DECLARED order rather than first-seen: these four are a fixed vocabulary, and a + #: bar that reordered itself as history arrived would move under the reader. + kinds_present: tuple[str, ...] + + @property + def shown_count(self) -> int: + """How many rows this report carries. Derived rather than stored, and held here because + `keel/web/payload.py` may not call `len()` (Rule 6e).""" + return len(self.rows) + + + +#: The cap on one merged read. Four stores, three of them unbounded, joined into one response: +#: without a cap this route's cost is the size of the deployment's whole history. Newest-first +#: and capped means the page always answers, and the counts below say how much it did not show. +DEFAULT_TIMELINE_LIMIT = 200 +MAX_TIMELINE_LIMIT = 2000 + + +def _order_rows(repo: Repository, since_ts: int | None) -> list[TimelineRow]: + """`orders` -> trade rows. + + A PAPER order is `simulated`, not `venue-reported`. The paper trader wrote that row with no + venue involved, and one word covering both would put synthetic fills and real ones in the + same bucket -- which is the thing four separate tables at least never did. + """ + rows: list[TimelineRow] = [] + for raw in repo.get_orders(): + created = raw.get("created_at") + if created is None or (since_ts is not None and int(created) < since_ts): + continue + mode = str(raw.get("mode") or "") + side = str(raw.get("side") or "") + product = str(raw.get("product_id") or "") + status = str(raw.get("status") or "") + rows.append( + TimelineRow( + ts=int(created), + kind="trade", + provenance="simulated" if mode == "paper" else "venue-reported", + source="orders", + reference=str(raw.get("id") or ""), + summary=f"{status} {side} {product} ({mode})".strip(), + product_id=product, + amount=raw.get("actual_fill"), + amount_kind="fill price" if raw.get("actual_fill") is not None else "", + ) + ) + return rows + + +def _transaction_rows(repo: Repository, since_ts: int | None) -> list[TimelineRow]: + """`transactions` -> flow rows. + + `imported-ledger`, never `venue-reported`: these lines came out of a CSV the operator + downloaded, and that the venue produced the file does not make the row a report -- nothing + verified it on the way in. + """ + rows: list[TimelineRow] = [] + for raw in repo.get_transactions(): + ts = raw.get("ts") + if ts is None or (since_ts is not None and int(ts) < since_ts): + continue + kind_word = str(raw.get("type") or "") + asset = str(raw.get("asset") or "") + note = str(raw.get("notes") or "") + rows.append( + TimelineRow( + ts=int(ts), + kind="flow", + provenance="imported-ledger", + source="transactions", + reference=str(raw.get("coinbase_id") or raw.get("id") or ""), + summary=f"{kind_word} {asset}".strip() + (f" -- {note}" if note else ""), + product_id="", + amount=raw.get("total"), + amount_kind="flow total" if raw.get("total") is not None else "", + ) + ) + return rows + + +def _attestation_rows(repo: Repository, since_ts: int | None) -> list[TimelineRow]: + """The attestation tables -> attestation rows. + + `human-attested`: someone typed this and signed their name to it, which is a different kind + of evidence from anything a machine reported. The name is in the summary because "who swore + to this" is the first thing an auditor asks of an attestation. + """ + rows: list[TimelineRow] = [] + for raw in repo.get_asset_attestations(): + ts = raw.get("attested_at") + if ts is None or (since_ts is not None and int(ts) < since_ts): + continue + asset = str(raw.get("asset") or "") + rows.append( + TimelineRow( + ts=int(ts), + kind="attestation", + provenance="human-attested", + source="asset_attestations", + reference=asset, + summary=( + f"{asset} attested by {raw.get('attested_by') or 'unnamed'} " + f"(source: {raw.get('source') or 'unstated'})" + ), + product_id="", + amount=None, + amount_kind="", + ) + ) + for raw in repo.get_instrument_attestations(): + ts = raw.get("attested_at") + if ts is None or (since_ts is not None and int(ts) < since_ts): + continue + product = str(raw.get("product_id") or "") + venue = str(raw.get("venue") or "") + rows.append( + TimelineRow( + ts=int(ts), + kind="attestation", + provenance="human-attested", + source="instrument_attestations", + reference=f"{venue}:{product}", + summary=( + f"{product} on {venue} attested by " + f"{raw.get('attested_by') or 'unnamed'} " + f"(wrapper: {raw.get('wrapper') or 'unstated'})" + ), + product_id=product, + amount=None, + amount_kind="", + ) + ) + return rows + + +def _cycle_rows(cycles: Iterable[Any], since_ts: int | None) -> list[TimelineRow]: + """`ActivityCycle`s -> system rows. + + `engine-log`: the agent's own account of what it did. Distinct from `venue-reported` because + nothing outside this process confirmed it, and distinct from `human-attested` because nobody + signed it. + """ + rows: list[TimelineRow] = [] + for cycle in cycles: + ts = int(getattr(cycle, "started_ts", 0) or 0) + if since_ts is not None and ts < since_ts: + continue + products: tuple[str, ...] = tuple(getattr(cycle, "products", ()) or ()) + rows.append( + TimelineRow( + ts=ts, + kind="system", + provenance="engine-log", + source="engine log", + reference=str(getattr(cycle, "cycle_id", "") or getattr(cycle, "key", "")), + summary=( + f"cycle: {getattr(cycle, 'signals', 0)} signal(s), " + f"{getattr(cycle, 'entered', 0)} entered, " + f"{getattr(cycle, 'exited', 0)} exited, " + f"{getattr(cycle, 'errors', 0)} error(s)" + ), + product_id=products[0] if len(products) == 1 else "", + amount=None, + amount_kind="", + ) + ) + return rows + + +def gather_timeline( + repo: Repository, + *, + now_ts: int, + scope: str = "all", + kind: str = "", + limit: int = DEFAULT_TIMELINE_LIMIT, + cycles: Sequence[Any] = (), +) -> TimelineReport: + """One chronology over four stores, newest first, scoped, chip-filtered and capped. + + `cycles` is passed IN rather than read here: the engine log lives on disk behind a config + path, and `keel/commands/activity.py` already owns finding it, reading a bounded window of it + and parsing it. Re-doing any of that would be a second answer to "what did the agent do", and + the caller that has a config is the one that can supply them. + + The scope is `orders`' own `normalise_scope`/`scope_start_ts` rather than a third copy: this + is the same question those already answer, and a third implementation is a third thing to + drift. + + Filtering is server-side because the cap is: a client filtering the capped page would be + filtering the rows it happened to receive and calling the result "every flow this month". + """ + resolved_scope = normalise_scope(scope) + resolved_kind = (kind or "").strip().lower() + resolved_limit = max(1, min(int(limit), MAX_TIMELINE_LIMIT)) + since = scope_start_ts(resolved_scope, now_ts) + + scoped: list[TimelineRow] = [] + scoped.extend(_order_rows(repo, since)) + scoped.extend(_transaction_rows(repo, since)) + scoped.extend(_attestation_rows(repo, since)) + scoped.extend(_cycle_rows(cycles, since)) + + # Newest first, HERE -- so neither renderer sorts, and they cannot disagree about what "the + # latest thing" is. `reference` breaks a tie so two rows at the same second keep a stable + # order across repaints rather than an arbitrary one. + scoped.sort(key=lambda row: (row.ts, row.reference), reverse=True) + + filtered = [row for row in scoped if not resolved_kind or row.kind == resolved_kind] + present = {row.kind for row in scoped} + return TimelineReport( + now_ts=now_ts, + scope=resolved_scope, + scope_start_ts=since, + kind=resolved_kind, + limit=resolved_limit, + scoped_count=len(scoped), + filtered_count=len(filtered), + rows=tuple(filtered[:resolved_limit]), + kinds_present=tuple(kind for kind in TIMELINE_KINDS if kind in present), + ) + + +def to_csv(report: TimelineReport) -> str: + """The audit export: one row per event, every text cell neutralised (`csv_safe`). + + **Provenance is a column, not a footnote.** The point of this file is that a reader can tell + a venue-reported fill from a line someone imported from a spreadsheet -- so `provenance` and + `source` sit beside every figure, and `row_hash` says NOT RECORDED rather than being blank. + + `amount_kind` rides beside `amount` for the same reason: a fill price and a cash-flow total + in one column, with nothing saying which is which, is a column that will be summed. + """ + buffer = io.StringIO() + writer = csv.writer(buffer) + writer.writerow( + [ + "ts", + "kind", + "provenance", + "source", + "reference", + "product_id", + "amount", + "amount_kind", + "summary", + "row_hash", + ] + ) + for row in report.rows: + writer.writerow( + [ + csv_safe(row.ts), + csv_safe(row.kind), + csv_safe(row.provenance), + csv_safe(row.source), + csv_safe(row.reference), + csv_safe(row.product_id), + csv_safe("" if row.amount is None else format(row.amount, "f")), + csv_safe(row.amount_kind), + csv_safe(row.summary), + csv_safe(row.row_hash), + ] + ) + return buffer.getvalue() diff --git a/keel/web/api.py b/keel/web/api.py index c611268..74500d2 100644 --- a/keel/web/api.py +++ b/keel/web/api.py @@ -37,6 +37,7 @@ import time from collections.abc import Callable, Mapping, Sequence from dataclasses import dataclass, field +from datetime import UTC, datetime from typing import TYPE_CHECKING, Any from keel.web import payload @@ -358,6 +359,68 @@ def read_balances(cfg: ServeConfig, _query: Query, _state: Any, now_ts: int) -> return payload.balances_payload(report) +def _timeline_report(cfg: ServeConfig, query: Query, now_ts: int) -> Any: + """The merged timeline for one request. Shared by the JSON route and the CSV export so the + file an operator downloads is the same chronology the page showed them -- two builders would + be two answers to "what happened", and the export is the one that goes to an auditor.""" + from keel.commands.activity import ( + feed_from_lines, + read_log_window, + resolve_log_path, + ) + from keel.commands.timeline import ( + DEFAULT_TIMELINE_LIMIT, + MAX_TIMELINE_LIMIT, + gather_timeline, + ) + + config = load_config(cfg.config_path) + raw_limit = _first(query, "limit") + limit = ( + DEFAULT_TIMELINE_LIMIT if not raw_limit else _whole_number(raw_limit, MAX_TIMELINE_LIMIT) + ) + + # The engine log is read through `activity`'s own bounded window rather than re-parsed here: + # that module owns finding the file, reading a bounded tail of it and turning it into cycles, + # and a second implementation would be a second answer to "what did the agent do". + cycles: tuple[Any, ...] = () + try: + log_path = resolve_log_path(config) + window = read_log_window(log_path) + # `LogWindow` carries the lines and a read status, not the path -- `source` is the + # feed's own label for where the lines came from, so it is passed the path we resolved. + cycles = feed_from_lines( + window.lines, source=str(log_path), truncated=window.truncated + ).cycles + except OSError: + # No log yet, or an unreadable one. The timeline still has three other sources, and a + # missing log is not a reason to fail the whole page -- the `system` rows are simply + # absent, which is what an unread log honestly means. + cycles = () + + repo = open_repo(cfg.db_path) + try: + return gather_timeline( + repo, + now_ts=now_ts, + scope=_first(query, "scope") or "all", + kind=_first(query, "kind") or "", + limit=limit, + cycles=cycles, + ) + finally: + close_repo(repo) + + +def read_timeline(cfg: ServeConfig, query: Query, _state: Any, now_ts: int) -> dict[str, Any]: + """One chronology over the engine log, the orders book, the ledger and the attestations. + + READ ONLY, no broker, no network -- the same posture as every route here. `?kind=` is applied + rather than refused, `?scope=`'s own normalisation is reused, and both are echoed back. + """ + return payload.timeline_payload(_timeline_report(cfg, query, now_ts)) + + def read_insights(cfg: ServeConfig, _query: Query, _state: Any, now_ts: int) -> dict[str, Any]: """The per-rule track records, the promotion-gate distances, and the account-equity series. @@ -635,6 +698,12 @@ class ApiRoute: collection="assets", sortable=("product_id", "qty", "mark", "market_value"), ), + "/api/timeline": ApiRoute( + html_route="/timeline", + read=read_timeline, + collection="rows", + sortable=("ts", "kind", "provenance", "source", "product_id"), + ), "/api/rules": ApiRoute( html_route="/rules", read=read_rules, @@ -893,3 +962,31 @@ def action_document(result: Any) -> dict[str, Any]: def sortable_columns() -> Mapping[str, Sequence[str]]: """The declared sort surface, for a test to read rather than restate.""" return {path: route.sortable for path, route in API_ROUTES.items() if route.sortable} + + +#: The one path on this server that does not answer JSON (#703). +#: +#: Deliberately NOT an `ApiRoute`: every entry in `API_ROUTES` is wrapped in the JSON envelope by +#: `respond`, and `tests/web/test_api.py` parametrises the envelope, the no-JSON-number walk and +#: the JSON MIME assertions over that table. A CSV route in it would either break those or force +#: each of them to grow an exception -- and an exception inside a security pin is how the pin +#: stops meaning anything. It gets its own handler branch and its own header suite instead. +CSV_EXPORT_PATH = "/api/timeline/export.csv" + + +def export_timeline_csv(cfg: ServeConfig, query: Query) -> tuple[str, str]: + """`(csv_text, filename)` for the timeline export. + + Built from the SAME `_timeline_report` the JSON route uses, so the file an operator hands an + auditor is the chronology the page showed them. + + Every text cell goes through `csv_safe` (see `keel/commands/timeline.py`): the file is meant + to be opened in Excel or Sheets, both of which execute a cell beginning `=`, `+`, `-` or `@`, + and several columns carry text keel did not write. + """ + from keel.commands.timeline import to_csv + + now_ts = int(time.time()) + report = _timeline_report(cfg, query, now_ts) + stamp = datetime.fromtimestamp(now_ts, tz=UTC).strftime("%Y%m%d-%H%M%S") + return to_csv(report), f"keel-activity-{stamp}.csv" diff --git a/keel/web/payload.py b/keel/web/payload.py index 600ca23..fdab2df 100644 --- a/keel/web/payload.py +++ b/keel/web/payload.py @@ -145,6 +145,7 @@ SubscriptionStatusRow, WithdrawalAttestationStatus, ) + from keel.commands.timeline import TimelineReport, TimelineRow from keel.venue_readiness import VenueReadinessRow @@ -1716,6 +1717,84 @@ def balances_payload(report: BalancesReport) -> dict[str, Any]: } +#: How each provenance is styled (#703). NOT a judgement about quality -- an imported ledger line +#: is not "worse" evidence than a venue report, it is DIFFERENT evidence -- so nothing here is +#: `bad`. `simulated` is the one that warns, because a synthetic fill sitting in a chronology +#: beside real ones is the single thing a reader must not skim past. +_PROVENANCE_STATES: Mapping[str, str] = { + "venue-reported": NEUTRAL, + "simulated": WARN, + "imported-ledger": NEUTRAL, + "human-attested": NEUTRAL, + "engine-log": NEUTRAL, +} + +#: What each provenance MEANS, spelled out. The word is a term of art; the sentence is what a +#: reader who has not read `timeline.py` can act on -- and on an audit surface, "how do we know +#: this happened" is the question the whole page exists to answer. +_PROVENANCE_NOTES: Mapping[str, str] = { + "venue-reported": "the venue reported this fill", + "simulated": "the paper trader wrote this -- no venue was involved", + "imported-ledger": "imported from a venue CSV; nothing verified it on the way in", + "human-attested": "a person typed this and signed their name to it", + "engine-log": "the agent's own log of what it did", +} + + +def _timeline_row_payload(row: TimelineRow) -> dict[str, Any]: + """One event, placed. + + `provenance` is a `label` and not a bare string BECAUSE it carries a judgement -- `simulated` + warns -- and Rule 3 keeps that judgement here rather than letting a client infer it from the + word. `kind`, `source` and `reference` are bare: enum words and identifiers with nothing to + decide. + + `amount` rides with `amount_kind` for the reason the report holds them together: a fill price + and a cash-flow total in one column, with nothing saying which is which, is a column that + will be summed by someone. + """ + return { + "at": moment(row.ts), + "kind": row.kind, + "provenance": label( + row.provenance, + display=_PROVENANCE_NOTES.get(row.provenance, row.provenance), + state=_PROVENANCE_STATES.get(row.provenance, UNKNOWN), + ), + "source": row.source, + "reference": row.reference, + "product_id": row.product_id, + "amount": money(row.amount), + "amount_kind": row.amount_kind, + "summary": row.summary, + # A `label`, so the "we did not check" reading carries a state a client can style rather + # than a bare string it might render as though it were a hash. + "row_hash": label(row.row_hash, state=UNKNOWN), + } + + +def timeline_payload(report: TimelineReport) -> dict[str, Any]: + """`gather_timeline`'s `TimelineReport`, as JSON (#703). + + `kinds_present` is the chip bar and comes off the report, built from the SCOPED set -- a bar + built from the rows on screen would delete its own alternatives the moment one was chosen. + Every count comes off the report too (Rule 6e bans `len()` here). + """ + return { + "as_of": iso(report.now_ts), + "generated_at": moment(report.now_ts), + "scope": report.scope, + "scope_start_at": moment(report.scope_start_ts), + "kind": report.kind, + "kinds": [str(kind) for kind in report.kinds_present], + "limit": count(report.limit), + "scoped_count": count(report.scoped_count), + "filtered_count": count(report.filtered_count), + "shown_count": count(report.shown_count), + "rows": [_timeline_row_payload(row) for row in report.rows], + } + + # -- the envelope (#534) ------------------------------------------------------------------------- # # Every `GET /api/*` success is wrapped in the same four keys, so #536's single `fetch` wrapper diff --git a/keel/web/server.py b/keel/web/server.py index 2ab7968..fbc9296 100644 --- a/keel/web/server.py +++ b/keel/web/server.py @@ -390,6 +390,36 @@ def _send( if self.command != "HEAD": self.wfile.write(payload) + def _send_csv(self, text: str, filename: str) -> None: + """One CSV export, with its own headers (#703). + + `nosniff` matters MORE here than on the JSON routes, not less: this body is a file a + browser is being told to save, and a sniffing browser that decided some other type for it + would be deciding what a downloaded file IS. + + `Content-Disposition: attachment` is the second half of that. Without it a browser may + render the CSV inline, and an inline-rendered document from this origin is a different + security question from a saved file -- `attachment` keeps it a download, and names it so + the operator has a dated artefact rather than `export.csv` among ten others. + + `no-store` for the same reason every API response carries it: this is the operator's + whole audit trail, and a copy of it in a shared cache is a copy nobody chose to make. + + The filename is server-generated and never echoed from the query string -- a + caller-supplied one would put attacker-controlled text into a response header, which is + the header-injection version of the formula injection `csv_safe` already defends the body + against. + """ + body = text.encode("utf-8") + self.send_response(200) + self.send_header("Content-Type", "text/csv; charset=utf-8") + self.send_header("Content-Length", str(len(body))) + self.send_header("Content-Disposition", f'attachment; filename="{filename}"') + for name, value in _API_HEADERS: + self.send_header(name, value) + self.end_headers() + self.wfile.write(body) + def _send_json(self, code: int, document: dict[str, Any]) -> None: """One JSON response, with its own headers. @@ -839,6 +869,14 @@ def do_GET(self) -> None: # noqa: N802 - stdlib's naming, not ours # not exempt from the loopback-plus-session model for being machine-readable. What it # does NOT additionally require is `X-Keel-Client` -- that header gates POSTs, and its # docstring explains why a GET is not the gap it closes. + # #703's CSV export is the one path under `/api/` that does not answer JSON. It is + # checked HERE, inside the same admission the JSON routes passed, so it inherits the + # loopback-plus-session model unchanged -- an export of the whole audit trail is the + # last thing that should be reachable more easily than the page it came from. + if parsed.path == api.CSV_EXPORT_PATH: + text, filename = api.export_timeline_csv(self.cfg, query) + self._send_csv(text, filename) + return code, document = api.respond(self.cfg, parsed.path, query) self._send_json(code, document) return diff --git a/keel/web/static/index.html b/keel/web/static/index.html index b759e04..5a792d8 100644 --- a/keel/web/static/index.html +++ b/keel/web/static/index.html @@ -120,6 +120,7 @@
  • Orders
  • Positions
  • Balances
    • +
  • Timeline
  • Insights
  • Rules
  • Venues
    • diff --git a/keel/web/static/js/main.js b/keel/web/static/js/main.js index 752271a..c8b54cb 100644 --- a/keel/web/static/js/main.js +++ b/keel/web/static/js/main.js @@ -46,6 +46,7 @@ import { modeBadge, ordersView, balancesView, + timelineView, positionsView, refusedView, rulesView, @@ -101,6 +102,7 @@ const ROUTES = [ { name: "orders", label: "Orders", endpoints: ["orders"] }, { name: "positions", label: "Positions", endpoints: ["positions"] }, { name: "balances", label: "Balances", endpoints: ["balances"] }, + { name: "timeline", label: "Timeline", endpoints: ["timeline"] }, { name: "insights", label: "Insights", endpoints: ["insights", "journal"] }, { name: "rules", label: "Rules", endpoints: ["rules"] }, { name: "venues", label: "Venues", endpoints: ["venues"] }, @@ -430,6 +432,15 @@ function mount(route, readings) { } if (route.name === "positions") return positionsView(data, primary.sort, onSort); if (route.name === "balances") return balancesView(data, primary.sort, onSort); + if (route.name === "timeline") { + return timelineView(data, primary.sort, onSort, (kind) => { + // #703: the chip re-asks the SERVER, like the Orders status tabs. Filtering the + // capped page here would filter the rows that happened to arrive and present the + // result as "every flow this month". + paramsFor(route.endpoints[0]).kind = kind; + void paint(route, true, true); + }); + } if (route.name === "rules") return rulesView(data, primary.sort, onSort); if (route.name === "venues") return venuesView(data, primary.sort, onSort); if (route.name === "gates") return gatesView(data); diff --git a/keel/web/static/js/render.js b/keel/web/static/js/render.js index cda9ed6..c53ee21 100644 --- a/keel/web/static/js/render.js +++ b/keel/web/static/js/render.js @@ -1138,6 +1138,137 @@ function jobPanel(job) { * @param {(scope: string) => void} onScope * @returns {DocumentFragment} */ +/** + * The Timeline view (#703): one chronology over four stores that never knew about each other. + * + * ── PROVENANCE IS A COLUMN, NOT A FOOTNOTE ─────────────────────────────────────────────────── + * + * The engine's log, the orders book, an imported ledger and a human's attestation are four + * different kinds of evidence. Merging them into one feed is only an improvement if the feed + * keeps them apart -- otherwise it is four tables with their labels removed. So every row + * carries `provenance`, decided in `keel/commands/timeline.py` and styled by `payload.py`; + * `simulated` is the one that warns, because a paper fill sitting in a chronology beside real + * ones is the single row a reader must not skim past. + * + * ── THE CHIPS COME FROM THE WINDOW, AND FILTER ON THE SERVER ───────────────────────────────── + * + * `data.kinds` is what the SCOPED window holds, not the four kinds keel can emit and not what + * survived the current chip -- a bar built from the rows on screen deletes its own alternatives. + * Pressing one re-asks the server, because the page is capped and a client-side filter would be + * filtering the rows that happened to arrive. + * + * ── THE EXPORT IS A LINK, NOT A FETCH ──────────────────────────────────────────────────────── + * + * A plain `` to `/api/timeline/export.csv`, carrying the same `?scope=`/`?kind=` the + * page is showing so the file matches the screen. It is a navigation rather than a scripted + * download because the response is `Content-Disposition: attachment` and the browser's own + * handling of that is the behaviour we want -- and because this file writes no bytes itself. + * + * @param {any} data `/api/timeline`'s `data`. + * @param {any} sort + * @param {(column: string) => void} onSort + * @param {(kind: string) => void} onKind + * @returns {DocumentFragment} + */ +export function timelineView(data, sort, onSort, onKind) { + const fragment = document.createDocumentFragment(); + fragment.append(el("h1", undefined, "Timeline")); + + const sub = el("p", "sub"); + sub.append(field(data.generated_at), " · "); + sub.append(field(data.shown_count), " of ", field(data.filtered_count), " shown"); + fragment.append(sub); + + if (onKind) fragment.append(kindSwitch(plain(data.kind), data.kinds || [], onKind)); + + fragment.append( + gridCard([ + kv("scope", plain(data.scope)), + kv("since", data.scope_start_at), + kv("in this window", data.scoped_count), + kv("in this chip", data.filtered_count), + ]), + ); + + // The export, carrying the page's own scope and chip so the file matches the screen. + const actions = el("p", "note"); + const link = el("a", "chartaction", "Export CSV"); + link.setAttribute( + "href", + ["/api/timeline/export.csv?scope=", plain(data.scope), "&kind=", plain(data.kind)].join(""), + ); + link.setAttribute("download", ""); + actions.append(link); + actions.append(" — every row with its provenance; hashes are not recorded yet."); + fragment.append(actions); + + fragment.append(heading("h-timeline", "What happened")); + fragment.append( + table( + "h-timeline", + [ + { label: "when (UTC)", numeric: false, key: "ts" }, + { label: "kind", numeric: false, key: "kind" }, + { label: "how we know", numeric: false, key: "provenance" }, + { label: "source", numeric: false, key: "source" }, + { label: "reference", numeric: false }, + { label: "product", numeric: false, key: "product_id" }, + { label: "amount", numeric: true }, + { label: "what", numeric: false }, + ], + (data.rows || []).map(/** @param {any} row */ (row) => [ + row.at, + plain(row.kind) || "—", + row.provenance, + plain(row.source) || "—", + plain(row.reference) || "—", + plain(row.product_id) || "—", + row.amount, + plain(row.summary) || "—", + ]), + "Nothing recorded in this window.", + { sort: sort, onSort: onSort }, + ), + ); + + return fragment; +} + + +/** + * The Timeline's kind chips (#703). + * + * Built from `kinds` -- what the window holds -- with a leading "all" that clears the filter, + * the same shape `statusSwitch` takes for Orders and for the same reasons: a bar listing every + * kind keel can emit invites a reader into empty chips, and a filter with no way back is a trap. + * + * @param {string} current + * @param {string[]} kinds + * @param {(kind: string) => void} onKind + * @returns {HTMLElement} + */ +function kindSwitch(current, kinds, onKind) { + const wrap = el("nav", "scopes"); + wrap.setAttribute("aria-label", "Timeline kind"); + wrap.append(el("span", "k", "kind")); + const all = el("button", "scopekey", "all"); + all.setAttribute("type", "button"); + all.setAttribute("data-focus", "kind:"); + if (!current) all.setAttribute("aria-current", "true"); + all.addEventListener("click", () => onKind("")); + wrap.append(all); + for (const name of kinds) { + const button = el("button", "scopekey", name); + button.setAttribute("type", "button"); + button.setAttribute("data-focus", "kind:".concat(name)); + if (name === current) button.setAttribute("aria-current", "true"); + button.addEventListener("click", () => onKind(name)); + wrap.append(button); + } + return wrap; +} + + /** * The Balances view (#702): what the account holds, as the last cycle recorded it. * diff --git a/keel/web/staticfiles.py b/keel/web/staticfiles.py index 7b68307..094a0ba 100644 --- a/keel/web/staticfiles.py +++ b/keel/web/staticfiles.py @@ -143,6 +143,7 @@ def resolve_static_asset(root: Path, url_path: str) -> Path | None: "orders", "positions", "balances", + "timeline", "insights", "rules", "venues", diff --git a/tests/commands/test_timeline.py b/tests/commands/test_timeline.py new file mode 100644 index 0000000..ea43e59 --- /dev/null +++ b/tests/commands/test_timeline.py @@ -0,0 +1,278 @@ +"""The unified activity timeline -- issue #703. + +Four stores that never knew about each other, merged into one chronology: the engine's JSONL log, +the `orders` table, the `transactions` ledger, and the attestation tables. + +**The property under test throughout is that they merge WITHOUT blurring.** A venue-reported +fill, a line imported from a venue's CSV export, and a sentence a human typed and signed are +three different kinds of evidence. A feed that showed them identically would be less useful than +the four separate tables it replaced -- so every row carries its provenance, and the tests below +care more about that than about the ordering. +""" + +from __future__ import annotations + +from decimal import Decimal +from pathlib import Path +from typing import Any + +import pytest + +from keel.commands.timeline import PROVENANCES, TIMELINE_KINDS, gather_timeline +from keel.data.db import connect, migrate +from keel.data.repository import Repository + +NOW_TS = 1_800_000_000 +DAY = 86_400 +TODAY_START = 1_799_971_200 # 2027-01-15T00:00:00Z + + +@pytest.fixture() +def repo(tmp_path: Path) -> Repository: + conn = connect(str(tmp_path / "keel.db")) + migrate(conn) + return Repository(conn) + + +def _order(repo: Repository, **overrides: Any) -> int: + row: dict[str, Any] = { + "mode": "live", + "product_id": "BTC-USD", + "side": "buy", + "order_type": "market", + "qty": Decimal("0.01"), + "status": "filled", + "fee": Decimal("1.18"), + "expected_fill": Decimal("100000"), + "actual_fill": Decimal("100050"), + "created_at": NOW_TS - 3600, + "updated_at": NOW_TS - 3600, + } + row.update(overrides) + return repo.insert_order(row) + + +def _transaction(repo: Repository, **overrides: Any) -> None: + row: dict[str, Any] = { + "coinbase_id": "cb-1", + "source": "coinbase", + "type": "deposit", + "asset": "USD", + "ts": NOW_TS - 7200, + "qty": Decimal("500"), + "total": Decimal("500"), + "notes": "", + } + row.update(overrides) + repo.upsert_transaction(row) + + +def _attestation(repo: Repository, **overrides: Any) -> None: + row: dict[str, Any] = { + "asset": "BTC", + "sector": "payments", + "backing": "none", + "pays_yield": False, + "source": "whitepaper", + "attested_by": "operator", + "attested_at": NOW_TS - 10800, + } + row.update(overrides) + repo.upsert_asset_attestation(**row) + + +# -- the merge keeps the sources apart --------------------------------------------------------- + + +def test_the_four_sources_merge_into_one_chronology(repo: Repository, tmp_path: Path) -> None: + _order(repo) + _transaction(repo) + _attestation(repo) + + report = gather_timeline(repo, now_ts=NOW_TS, scope="all") + + kinds = {row.kind for row in report.rows} + assert {"trade", "flow", "attestation"} <= kinds + + +def test_rows_are_newest_first(repo: Repository, tmp_path: Path) -> None: + """One chronology, and the newest thing that happened is the thing an operator opened this + page to see.""" + _attestation(repo) # oldest + _transaction(repo) + _order(repo) # newest + + timestamps = [row.ts for row in gather_timeline(repo, now_ts=NOW_TS, scope="all").rows] + + assert timestamps == sorted(timestamps, reverse=True) + + +def test_a_live_fill_is_venue_reported_and_a_paper_fill_is_not( + repo: Repository, tmp_path: Path +) -> None: + """THE distinction this feed exists to preserve. A paper fill was written by the paper trader + with no venue involved; filing it under the same word as a real one would put synthetic and + real evidence in one bucket, which is the thing four separate tables at least never did.""" + _order(repo, mode="live", created_at=NOW_TS - 60) + _order(repo, mode="paper", created_at=NOW_TS - 120) + + rows = gather_timeline(repo, now_ts=NOW_TS, scope="all").rows + by_provenance = {row.provenance for row in rows} + + assert "venue-reported" in by_provenance + assert "simulated" in by_provenance + + +def test_an_imported_ledger_line_is_not_venue_reported(repo: Repository, tmp_path: Path) -> None: + """A `transactions` row came out of a CSV the operator downloaded. That the venue produced + the CSV does not make the row a venue REPORT -- nothing verified it on the way in.""" + _transaction(repo) + + row = gather_timeline(repo, now_ts=NOW_TS, scope="all").rows[0] + + assert row.kind == "flow" + assert row.provenance == "imported-ledger" + + +def test_an_attestation_is_marked_as_human(repo: Repository, tmp_path: Path) -> None: + _attestation(repo) + + row = gather_timeline(repo, now_ts=NOW_TS, scope="all").rows[0] + + assert row.kind == "attestation" + assert row.provenance == "human-attested" + assert "operator" in row.summary, "who swore to it belongs in the line" + + +def test_every_provenance_is_from_the_closed_vocabulary(repo: Repository, tmp_path: Path) -> None: + """Same discipline as the payload's `state` words: a provenance a renderer has to interpret + is one a renderer can get wrong.""" + _order(repo) + _transaction(repo) + _attestation(repo) + + for row in gather_timeline(repo, now_ts=NOW_TS, scope="all").rows: + assert row.provenance in PROVENANCES + assert row.kind in TIMELINE_KINDS + + +def test_each_row_names_the_store_it_came_from(repo: Repository, tmp_path: Path) -> None: + """A reader chasing a row needs to know which table to open. `reference` is that store's own + identifier, so the row is findable rather than merely described.""" + order_id = _order(repo) + + row = gather_timeline(repo, now_ts=NOW_TS, scope="all").rows[0] + + assert row.source == "orders" + assert row.reference == str(order_id) + + +# -- the hash column is honest ------------------------------------------------------------------ + + +def test_no_row_claims_a_hash_it_does_not_have(repo: Repository, tmp_path: Path) -> None: + """#703 asked for tamper-evidence. None of these four stores hashes its rows, so every row + says NOT RECORDED -- never blank, which a reader takes as "nothing to report", and never a + hash computed here, which would be this module attesting to its own output.""" + from keel.commands.timeline import HASH_NOT_RECORDED + + _order(repo) + _transaction(repo) + _attestation(repo) + + for row in gather_timeline(repo, now_ts=NOW_TS, scope="all").rows: + assert row.row_hash == HASH_NOT_RECORDED + + +# -- filtering and scoping ---------------------------------------------------------------------- + + +def test_the_kind_filter_narrows_server_side(repo: Repository, tmp_path: Path) -> None: + _order(repo) + _transaction(repo) + _attestation(repo) + + report = gather_timeline(repo, now_ts=NOW_TS, scope="all", kind="flow") + + assert [row.kind for row in report.rows] == ["flow"] + assert report.kind == "flow" + + +def test_the_scoped_count_counts_the_window_and_filtered_counts_the_chip( + repo: Repository, tmp_path: Path +) -> None: + """Two denominators, as on the Orders view: "3 of 12" under a Flows chip has to count flows, + and `scoped_count` has to keep meaning the window or nothing can say the window is empty.""" + _order(repo) + _transaction(repo) + _attestation(repo) + + report = gather_timeline(repo, now_ts=NOW_TS, scope="all", kind="flow") + + assert report.scoped_count == 3 + assert report.filtered_count == 1 + + +def test_the_scope_excludes_older_rows(repo: Repository, tmp_path: Path) -> None: + _order(repo, created_at=TODAY_START + 60) + _order(repo, created_at=TODAY_START - DAY) + + assert gather_timeline(repo, now_ts=NOW_TS, scope="today").scoped_count == 1 + + +def test_the_kinds_present_drive_the_chips(repo: Repository, tmp_path: Path) -> None: + """A chip bar built from what the window actually holds, in the declared order -- not one + chip per kind keel can produce, which would invite a reader into three empty tabs.""" + _order(repo) + _attestation(repo) + + assert gather_timeline(repo, now_ts=NOW_TS, scope="all").kinds_present == ( + "trade", + "attestation", + ) + + +def test_the_chips_survive_having_one_of_them_selected(repo: Repository, tmp_path: Path) -> None: + """A chip bar built from the SHOWN rows deletes its own alternatives: click Flows and the + Trades chip vanishes, leaving no way back except knowing the empty string means all. It has + to be built from the scoped set, before the chip narrows it.""" + _order(repo) + _transaction(repo) + _attestation(repo) + + filtered = gather_timeline(repo, now_ts=NOW_TS, scope="all", kind="flow") + + assert filtered.kinds_present == ("trade", "flow", "attestation") + + +def test_the_chips_survive_the_cap(repo: Repository, tmp_path: Path) -> None: + """Same failure through the other narrowing: a kind whose only rows fell past the limit is + still a kind this window holds.""" + _attestation(repo, attested_at=NOW_TS - 10800) + for index in range(3): + _order(repo, created_at=NOW_TS - index * 60) + + capped = gather_timeline(repo, now_ts=NOW_TS, scope="all", limit=2) + + assert capped.shown_count == 2 + assert "attestation" in capped.kinds_present + + +def test_the_limit_caps_the_rows_but_not_the_counts(repo: Repository, tmp_path: Path) -> None: + """The merged feed reads four unbounded stores; the cap is what keeps one request bounded. + The counts still describe the whole window, so a reader can see there is more.""" + for index in range(5): + _order(repo, created_at=NOW_TS - index * 60) + + report = gather_timeline(repo, now_ts=NOW_TS, scope="all", limit=2) + + assert report.shown_count == 2 + assert report.scoped_count == 5 + assert report.rows[0].ts == NOW_TS, "the cap keeps the NEWEST rows" + + +def test_an_empty_book_is_an_empty_timeline(repo: Repository, tmp_path: Path) -> None: + report = gather_timeline(repo, now_ts=NOW_TS, scope="all") + + assert report.rows == () + assert report.scoped_count == 0 diff --git a/tests/commands/test_timeline_csv.py b/tests/commands/test_timeline_csv.py new file mode 100644 index 0000000..44fd72f --- /dev/null +++ b/tests/commands/test_timeline_csv.py @@ -0,0 +1,91 @@ +"""CSV formula-injection defence for the activity export -- issue #703. + +**This is the security control on the export, not a formatting nicety.** The file is meant to be +opened in Excel or Google Sheets by an auditor, a tax preparer, or the operator -- and both +applications EXECUTE a cell whose text begins with a formula trigger. Several columns in this +export carry text keel did not write: a transaction's `notes` (imported from a venue CSV), an +attestation's `source` and `attested_by` (typed by a human), a `product_id` and `rule_name` +(config). A cell reading `=SUM(...)` or `@…` in any of those runs when the file is opened. + +The defence is OWASP's: prefix the cell with a single quote so the spreadsheet takes it as inert +text. It is applied to every text column rather than to a list of "risky" ones, because a +maintained list of which columns are attacker-influenced is exactly the thing that rots. +""" + +from __future__ import annotations + +import csv +import io + +import pytest + +from keel.commands.timeline import csv_safe + + +@pytest.mark.parametrize( + "dangerous", + [ + "=SUM(A1:A10)", + "@malicious", + "+1+1", + "-1+1", + "=cmd|' /C calc'!A0", + "\tleading tab", + "\rleading carriage return", + ], +) +def test_a_formula_trigger_is_neutralised(dangerous: str) -> None: + """Every trigger character OWASP names. `=` and `@` are the obvious ones; `+` and `-` are + formulas too, and a leading tab or carriage return can re-open the parse in some + spreadsheets.""" + escaped = csv_safe(dangerous) + + assert escaped.startswith("'"), f"{dangerous!r} was not neutralised" + assert escaped[1:] == dangerous, "the original text must survive intact after the quote" + + +@pytest.mark.parametrize( + "ordinary", + ["BTC-USD", "turtle_breakout", "a note", "", "0.01", "reward: staking", " spaced"], +) +def test_ordinary_text_is_left_exactly_as_it_is(ordinary: str) -> None: + """The defence must not corrupt the record it protects. An audit export whose every cell + gained a stray quote would be unusable as evidence of anything.""" + assert csv_safe(ordinary) == ordinary + + +def test_a_negative_number_is_still_neutralised_and_still_readable() -> None: + """`-12.30` is a formula trigger AND a real figure this export carries. Quoting it is the + correct trade: a spreadsheet shows `-12.30` as text rather than evaluating it, and the value + is still there to read and to re-import. Losing the minus sign would be worse than losing + numeric typing.""" + assert csv_safe("-12.30") == "'-12.30" + + +def test_the_escape_survives_a_real_csv_round_trip() -> None: + """The end-to-end property the acceptance criterion asks for: written by `csv.writer`, read + back by `csv.reader`, the cell still carries its quote and its original text.""" + payload = '=HYPERLINK("http://evil","click")' + buffer = io.StringIO() + csv.writer(buffer).writerow([csv_safe(payload), csv_safe("BTC-USD")]) + + row = next(csv.reader(io.StringIO(buffer.getvalue()))) + + assert row[0] == "'" + payload + assert row[1] == "BTC-USD" + + +def test_a_quote_and_a_comma_still_round_trip() -> None: + """`csv.writer` owns quoting and escaping; `csv_safe` must not double-handle it. A note + containing a comma and a double quote has to come back byte-identical.""" + payload = 'note with, a comma and a "quote"' + buffer = io.StringIO() + csv.writer(buffer).writerow([csv_safe(payload)]) + + assert next(csv.reader(io.StringIO(buffer.getvalue())))[0] == payload + + +def test_none_becomes_an_empty_cell_not_the_word_none() -> None: + """An absent value is an empty cell. `"None"` in an audit column is a value that looks like + data and is not.""" + assert csv_safe(None) == "" diff --git a/tests/web/test_api.py b/tests/web/test_api.py index 3893333..1085033 100644 --- a/tests/web/test_api.py +++ b/tests/web/test_api.py @@ -60,6 +60,7 @@ "/api/orders", "/api/positions", "/api/balances", + "/api/timeline", "/api/insights", "/api/journal", "/api/rules", @@ -68,6 +69,23 @@ ) +def test_the_csv_export_is_deliberately_outside_the_json_route_table() -> None: + """#703's export answers `text/csv`, so it must NOT be an `ApiRoute`. + + Everything in `API_ROUTES` is wrapped in the JSON envelope by `respond`, and every pin in + this module is parametrised over that table -- the envelope, the no-JSON-number walk, the + JSON MIME assertion. A CSV route inside it would either break those or force each one to + grow an exception, and an exception carved into a security pin is how the pin stops meaning + anything. Its headers are pinned separately, in `test_timeline_export.py`. + """ + from keel.web import api as web_api + + assert web_api.CSV_EXPORT_PATH not in web_api.API_ROUTES + assert web_api.CSV_EXPORT_PATH.startswith("/api/"), ( + "it still lives under /api/, so it still passes the same admission check" + ) + + def test_this_module_pins_every_route_the_server_serves() -> None: """`API_ROUTES` above is hand-written, and everything in this file is parametrised over it -- the envelope, the no-JSON-number walk, the cache headers, the nosniff header and the diff --git a/tests/web/test_client_assets.py b/tests/web/test_client_assets.py index 48fe04c..907bd97 100644 --- a/tests/web/test_client_assets.py +++ b/tests/web/test_client_assets.py @@ -1263,6 +1263,7 @@ def _status_view_keys() -> list[str]: ("ordersView", "data", "/api/orders"), ("positionsView", "data", "/api/positions"), ("balancesView", "data", "/api/balances"), + ("timelineView", "data", "/api/timeline"), ("insightsView", "insights", "/api/insights"), ("insightsView", "journal", "/api/journal"), ("rulesView", "data", "/api/rules"), diff --git a/tests/web/test_timeline_export.py b/tests/web/test_timeline_export.py new file mode 100644 index 0000000..6f75023 --- /dev/null +++ b/tests/web/test_timeline_export.py @@ -0,0 +1,139 @@ +"""The CSV export's own MIME and header suite -- issue #703. + +**This route is deliberately outside `test_api.py`'s `API_ROUTES` loop.** Everything in that +table answers the JSON envelope, and every pin there is parametrised over it; a `text/csv` route +inside it would force each of those to grow an exception, and an exception carved into a security +pin is how the pin stops meaning anything. So the export gets its own file, and the headers that +matter for a downloaded file get asserted here rather than assumed. + +What matters for a file a browser is told to save is not the same set that matters for JSON: +`nosniff` counts for MORE (a sniffing browser would be deciding what a downloaded file IS), +`Content-Disposition` has to make it a download rather than an inline render, and the filename +has to come from the server rather than from anything a caller sent. +""" + +from __future__ import annotations + +import csv +import io +from decimal import Decimal +from pathlib import Path +from typing import Any + +from keel.data.db import connect, migrate +from keel.data.repository import Repository +from tests.web.test_api import _get, _session, deployment, running # noqa: F401 + +EXPORT = "/api/timeline/export.csv" + + +def _csv(server: Any, path: str = EXPORT) -> tuple[int, dict[str, str], str]: + status, headers, body = _get(server, path, cookie=_session(server)) + return status, {k.lower(): v for k, v in headers.items()}, body + + +def test_the_export_answers_csv_and_not_json(running: Any) -> None: # noqa: F811 + status, headers, body = _csv(running) + + assert status == 200 + assert headers["content-type"] == "text/csv; charset=utf-8" + assert not body.lstrip().startswith("{"), "a JSON envelope would defeat the whole export" + + +def test_the_export_is_a_download_with_a_dated_server_chosen_name(running: Any) -> None: # noqa: F811 + """`attachment` keeps it a saved file rather than a document rendered inline from this + origin, and the date makes it an artefact an operator can file rather than the tenth + `export.csv` in their downloads folder.""" + _status, headers, _body = _csv(running) + + disposition = headers["content-disposition"] + assert disposition.startswith("attachment; filename=") + assert "keel-activity-" in disposition + assert disposition.endswith('.csv"') + + +def test_the_export_carries_nosniff_and_no_store(running: Any) -> None: # noqa: F811 + """`nosniff` matters more here than on the JSON routes: this body is a file a browser is + being told to save. `no-store` because it is the operator's whole audit trail, and a copy of + it in a shared cache is a copy nobody chose to make.""" + _status, headers, _body = _csv(running) + + assert headers["x-content-type-options"] == "nosniff" + assert "no-store" in headers["cache-control"] + + +def test_the_filename_cannot_be_chosen_by_the_caller(running: Any) -> None: # noqa: F811 + """A caller-supplied filename would put attacker-controlled text into a response header -- + the header-injection twin of the formula injection `csv_safe` defends the body against.""" + _status, headers, _body = _csv(running, EXPORT + '?filename=evil";DROP') + + assert "evil" not in headers["content-disposition"] + assert "DROP" not in headers["content-disposition"] + + +def test_the_export_requires_the_same_session_as_every_other_route(running: Any) -> None: # noqa: F811 + """An export of the whole audit trail must not be reachable more easily than the page it + came from. Checked inside the same admission the JSON routes pass.""" + status, _headers, _body = _get(running, EXPORT, cookie=None) + + assert status in (401, 403), "an unauthenticated export must be refused" + + +def test_the_export_has_a_header_row_naming_provenance(running: Any) -> None: # noqa: F811 + """The point of the file: a reader can tell a venue-reported fill from a line someone + imported from a spreadsheet. If provenance were not a column, the export would be a list of + events with no way to weigh any of them.""" + _status, _headers, body = _csv(running) + + header = next(csv.reader(io.StringIO(body))) + + assert "provenance" in header + assert "source" in header + assert "row_hash" in header + + +def test_a_hostile_value_that_starts_a_cell_cannot_execute(tmp_path: Path) -> None: + """End to end, through the real exporter. + + The cell that matters is one whose WHOLE content is attacker-influenced, because a + spreadsheet evaluates only text that BEGINS with a trigger. `coinbase_id` is exactly that: + it comes out of a venue's CSV export and lands in `reference` by itself. + + (A hostile `notes` value is diluted by accident -- `summary` prefixes it with "deposit USD + -- " so the cell no longer starts with `=`. That is not a defence to rely on, which is why + `csv_safe` is applied to every cell rather than to the ones currently reachable.) + """ + from keel.commands.timeline import gather_timeline, to_csv + + conn = connect(str(tmp_path / "keel.db")) + migrate(conn) + repo = Repository(conn) + repo.upsert_transaction( + { + "coinbase_id": '=cmd|" /C calc"!A0', + "source": "coinbase", + "type": "deposit", + "asset": "USD", + "ts": 1_800_000_000 - 60, + "qty": Decimal("1"), + "total": Decimal("1"), + "notes": "", + } + ) + + text = to_csv(gather_timeline(repo, now_ts=1_800_000_000, scope="all")) + rows = list(csv.reader(io.StringIO(text))) + reference = rows[1][rows[0].index("reference")] + + assert "=cmd" in reference, "the evidence must survive -- this is an audit record" + assert not reference.startswith("="), "and it must not be a formula when opened" + assert reference.startswith("'"), "OWASP's defence: quote it into inert text" + + +def test_a_negative_amount_is_inert_and_still_legible(tmp_path: Path) -> None: + """`-` is a formula trigger, and the amount column is full of real negative figures. The + export quotes them: a spreadsheet shows the text rather than evaluating it, and the value is + still there to read and to re-import.""" + from keel.commands.timeline import csv_safe + + assert csv_safe("-500.25") == "'-500.25"