From 99f049cbde6d377606e076c9b8971b8b37339192 Mon Sep 17 00:00:00 2001 From: heaven Date: Sat, 8 Aug 2026 01:12:17 +0300 Subject: [PATCH] Accept and land the platform fix pack: nine register rows closed, eight of my own mutations caught by name, the sign-in denial and the sandbox measurements reproduced --- platform/cmd/tmplatformctl/main.go | 9 +- platform/cmd/tmplatformctl/main_test.go | 265 +++++++++++++++++++++ platform/deploy/README.md | 31 ++- platform/deploy/tmplatformd.service | 17 +- platform/docs/DEFECT_REGISTER.md | 22 +- platform/docs/STACK_DECISIONS.md | 14 ++ platform/docs/platform-PROGRESS.md | 156 +++++++++++- platform/internal/httpapi/server_test.go | 47 +++- platform/internal/ingest/events.go | 27 ++- platform/internal/ingest/money_test.go | 6 + platform/internal/ingest/supervisor.go | 11 +- platform/internal/login/login.go | 46 +++- platform/internal/login/login_test.go | 200 ++++++++++++++-- platform/internal/login/store.go | 7 + platform/internal/money/money.go | 22 +- platform/internal/money/money_test.go | 26 ++ platform/internal/pgstore/identity.go | 6 +- platform/internal/pgstore/identity_test.go | 30 ++- 18 files changed, 856 insertions(+), 86 deletions(-) create mode 100644 platform/cmd/tmplatformctl/main_test.go diff --git a/platform/cmd/tmplatformctl/main.go b/platform/cmd/tmplatformctl/main.go index 3df067e9..2c72b305 100644 --- a/platform/cmd/tmplatformctl/main.go +++ b/platform/cmd/tmplatformctl/main.go @@ -135,9 +135,16 @@ func adjust(ctx context.Context, store *pgstore.Store, args []string, out io.Wri }, "adjusted "+*user+" by "+micro.USD()) } +// balanceReader is the read-back half of write. One method, and its only reason to exist is that the +// rule below — a committed write never reports failure — cannot be tested against a store that +// always works. +type balanceReader interface { + Balance(ctx context.Context, userID string) (money.MicroUSD, error) +} + // write runs one ledger operation and reports what actually happened. "Applied" and "the key was // already spent" are different outcomes and the operator is told which one they got. -func write(ctx context.Context, store *pgstore.Store, out io.Writer, user, key string, +func write(ctx context.Context, store balanceReader, out io.Writer, user, key string, op func(id string, now time.Time) (bool, error), what string) error { now := time.Now().UTC() id := key diff --git a/platform/cmd/tmplatformctl/main_test.go b/platform/cmd/tmplatformctl/main_test.go new file mode 100644 index 00000000..98502ee7 --- /dev/null +++ b/platform/cmd/tmplatformctl/main_test.go @@ -0,0 +1,265 @@ +package main + +import ( + "context" + "crypto/rand" + "encoding/hex" + "errors" + "net/url" + "os" + "strings" + "testing" + "time" + + "github.com/jackc/pgx/v5" + + "textmachine/platform/internal/money" + "textmachine/platform/internal/pgstore" +) + +// brokenBalance is a store whose write commits and whose read-back fails. +type brokenBalance struct{} + +func (brokenBalance) Balance(context.Context, string) (money.MicroUSD, error) { + return 0, errors.New("connection reset by peer") +} + +// The rule this command calls load-bearing, and the one it had no test for (PD-106): once the ledger +// row is committed, NOTHING downstream may report failure. An operator who reads an error retries, +// and a retry without --key mints a fresh idempotency key — so a failed BALANCE READ would buy a +// double credit out of a write that already succeeded. +// Mutation caught: returning the balance error from write instead of noting it. +func TestACommittedWriteNeverReportsFailure(t *testing.T) { + var out strings.Builder + err := write(t.Context(), brokenBalance{}, &out, "u1", "key-1", + func(string, time.Time) (bool, error) { return true, nil }, "granted 5.000000 to u1") + if err != nil { + t.Fatalf("a committed grant reported failure: %v", err) + } + if !strings.Contains(out.String(), "granted 5.000000 to u1") { + t.Fatalf("the operator was not told the write happened:\n%s", out.String()) + } + // The failure is not swallowed either — it is a warning on the same output. + if !strings.Contains(out.String(), "warning: could not read the balance back") { + t.Fatalf("the failed read-back left no trace:\n%s", out.String()) + } +} + +// A write that did NOT happen must say so. "Applied" and "that key was already spent" are different +// outcomes, and reporting the second as the first is how an operator believes they credited twice. +// Mutation caught: dropping the !applied branch. +func TestASpentKeyIsReportedAsANoOp(t *testing.T) { + var out strings.Builder + err := write(t.Context(), brokenBalance{}, &out, "u1", "key-1", + func(string, time.Time) (bool, error) { return false, nil }, "granted 5.000000 to u1") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out.String(), "no-op") || strings.Contains(out.String(), "granted") { + t.Fatalf("a no-op was reported as a grant:\n%s", out.String()) + } +} + +// The failure BEFORE the commit is the opposite case: it must reach the operator as an error, or a +// grant that never happened reads as one that did. +func TestAFailedWriteIsAnError(t *testing.T) { + var out strings.Builder + err := write(t.Context(), brokenBalance{}, &out, "u1", "", + func(string, time.Time) (bool, error) { return false, errors.New("deadlock detected") }, + "granted 5.000000 to u1") + if err == nil { + t.Fatalf("a failed grant reported success:\n%s", out.String()) + } + if out.String() != "" { + t.Fatalf("a failed grant printed an outcome:\n%s", out.String()) + } +} + +// Without --key every invocation is its own intent: two deliberate grants on one day are two grants. +// Mutation caught: deriving the default key from the account or the date. +func TestEachInvocationWithoutAKeyIsItsOwnIntent(t *testing.T) { + seen := map[string]bool{} + for range 100 { + var out strings.Builder + var used string + if err := write(t.Context(), brokenBalance{}, &out, "u1", "", + func(id string, _ time.Time) (bool, error) { used = id; return true, nil }, "granted"); err != nil { + t.Fatal(err) + } + if !strings.HasPrefix(used, "cli-") { + t.Fatalf("generated key = %q, want it marked as the CLI's", used) + } + if seen[used] { + t.Fatalf("key %q was minted twice: the second grant would silently collapse", used) + } + seen[used] = true + } + // An explicit key is used verbatim — that is the whole point of offering one. + var out strings.Builder + var used string + if err := write(t.Context(), brokenBalance{}, &out, "u1", "invoice-42", + func(id string, _ time.Time) (bool, error) { used = id; return true, nil }, "granted"); err != nil { + t.Fatal(err) + } + if used != "invoice-42" { + t.Fatalf("explicit key became %q", used) + } +} + +// Argument handling, which no test saw either. Every one of these must be refused on its own terms, +// BEFORE any database work. +// +// The expected message is asserted, not merely "an error came back": the DSN below points at a port +// nothing listens on, so a command that got past its own checks fails anyway on connect — and a test +// that only asked for non-nil would pass against a build with no argument checks at all. Measured: +// two of these mutations survived the first version of this test for exactly that reason. +func TestBadArgumentsAreRefused(t *testing.T) { + t.Setenv("TM_PLATFORM_DSN", "postgres://nobody@127.0.0.1:1/none?sslmode=disable&connect_timeout=1") + for name, tc := range map[string]struct { + args []string + want string + wantUsage bool + }{ + "no command at all": {args: nil, want: "usage", wantUsage: true}, + "unknown command": {args: []string{"delete-everything"}, want: `unknown command "delete-everything"`, wantUsage: true}, + "grant without a user": {args: []string{"grant", "--usd", "5"}, want: "grant needs --user and --usd"}, + "grant without amount": {args: []string{"grant", "--user", "u1"}, want: "grant needs --user and --usd"}, + "grant of a non-amount": {args: []string{"grant", "--user", "u1", "--usd", "five"}, want: "not a decimal amount"}, + "adjust without a note": {args: []string{"adjust", "--user", "u1", "--usd", "-1"}, want: "adjust needs --user, --usd and --note"}, + "balance without user": {args: []string{"balance"}, want: "balance needs --user"}, + "logins without user": {args: []string{"logins"}, want: "logins needs --user"}, + "revoke without user": {args: []string{"revoke"}, want: "revoke needs --user"}, + "unknown flag": {args: []string{"grant", "--userr", "u1"}, want: "not defined"}, + } { + t.Run(name, func(t *testing.T) { + var out strings.Builder + err := run(tc.args, &out) + if err == nil { + t.Fatalf("accepted %v; output was %q", tc.args, out.String()) + } + if !strings.Contains(err.Error(), tc.want) { + t.Fatalf("%v failed with %q, want it refused for %q", tc.args, err, tc.want) + } + if errors.Is(err, errUsage) != tc.wantUsage { + t.Fatalf("usage=%v for %v: %v", errors.Is(err, errUsage), tc.args, err) + } + if out.String() != "" { + t.Fatalf("a refused command printed an outcome: %q", out.String()) + } + }) + } +} + +// The DSN is required and its absence is an error, not a silent default to localhost. +func TestMissingDSNIsRefused(t *testing.T) { + t.Setenv("TM_PLATFORM_DSN", "") + t.Setenv("TM_PLATFORM_DSN_FILE", "") + var out strings.Builder + if err := run([]string{"balance", "--user", "u1"}, &out); err == nil { + t.Fatal("ran without a DSN") + } +} + +// End to end against a live Postgres: the four commands an operator actually types, through run(), +// with the money crossing the real ledger. Skips loudly without a database, like the pgstore battery. +func TestCommandsAgainstALiveDatabase(t *testing.T) { + dsn := freshDB(t) + t.Setenv("TM_PLATFORM_DSN", dsn) + ctx := t.Context() + + conn, err := pgx.Connect(ctx, dsn) + if err != nil { + t.Fatal(err) + } + defer conn.Close(ctx) + if _, err := conn.Exec(ctx, `insert into users (id, created_at) values ('u-cli', now())`); err != nil { + t.Fatal(err) + } + + run1 := capture(t, "grant", "--user", "u-cli", "--usd", "5", "--note", "free tier", "--key", "k1") + if !strings.Contains(run1, "granted 5.000000 to u-cli") || !strings.Contains(run1, "balance is 5.000000") { + t.Fatalf("grant said:\n%s", run1) + } + // The same key again is a no-op that says so, and the balance does not move. + run2 := capture(t, "grant", "--user", "u-cli", "--usd", "5", "--note", "free tier", "--key", "k1") + if !strings.Contains(run2, "no-op") || !strings.Contains(run2, "balance is 5.000000") { + t.Fatalf("the repeat said:\n%s", run2) + } + if got := capture(t, "adjust", "--user", "u-cli", "--usd", "-2.50", "--note", "refund"); !strings.Contains(got, "balance is 2.500000") { + t.Fatalf("adjust said:\n%s", got) + } + if got := capture(t, "balance", "--user", "u-cli"); !strings.Contains(got, "balance 2.500000") { + t.Fatalf("balance said:\n%s", got) + } + // The journal renders with its header even when the account has no sign-ins yet. + if got := capture(t, "logins", "--user", "u-cli"); !strings.Contains(got, "WHEN") || !strings.Contains(got, "PROVIDER") { + t.Fatalf("logins said:\n%s", got) + } + if got := capture(t, "revoke", "--user", "u-cli"); !strings.Contains(got, "revoked 0 sessions") { + t.Fatalf("revoke said:\n%s", got) + } + // A grant against an account that does not exist is refused, not silently written: the ledger + // would otherwise carry rows nobody owns. + var out strings.Builder + if err := run([]string{"grant", "--user", "nobody", "--usd", "5"}, &out); err == nil { + t.Fatalf("credited an account that does not exist:\n%s", out.String()) + } +} + +func capture(t *testing.T, args ...string) string { + t.Helper() + var out strings.Builder + if err := run(args, &out); err != nil { + t.Fatalf("%v: %v (output %q)", args, err, out.String()) + } + return out.String() +} + +// freshDB creates a database of its own and migrates it, the same contract as the pgstore battery's +// helper: a test that leaves rows behind passes once and then lies. +// +// ⚠ It is a COPY of that helper, deliberately. Sharing it would mean a package both of them import, +// and pgstore's own battery lives in `package pgstore` — so such a package could not import pgstore +// for Migrate without a cycle, and a version that took migrate as a parameter would be more +// machinery than the twenty lines it saves. +func freshDB(t *testing.T) string { + t.Helper() + admin := os.Getenv("TM_PLATFORM_TEST_DSN") + if admin == "" { + t.Skip("TM_PLATFORM_TEST_DSN not set: the end-to-end commands need a live Postgres") + } + ctx := context.Background() + var suffix [6]byte + if _, err := rand.Read(suffix[:]); err != nil { + t.Fatal(err) + } + name := "tm_ctl_test_" + hex.EncodeToString(suffix[:]) + + conn, err := pgx.Connect(ctx, admin) + if err != nil { + t.Fatalf("connect: %v", err) + } + defer conn.Close(ctx) + if _, err := conn.Exec(ctx, `create database `+pgx.Identifier{name}.Sanitize()); err != nil { + t.Fatalf("create database: %v", err) + } + t.Cleanup(func() { + c, err := pgx.Connect(context.Background(), admin) + if err != nil { + return + } + defer c.Close(context.Background()) + _, _ = c.Exec(context.Background(), `drop database `+pgx.Identifier{name}.Sanitize()+` with (force)`) + }) + + u, err := url.Parse(admin) + if err != nil { + t.Fatalf("TM_PLATFORM_TEST_DSN must be a URL: %v", err) + } + u.Path = "/" + name + dsn := u.String() + if err := pgstore.Migrate(ctx, dsn); err != nil { + t.Fatalf("migrate: %v", err) + } + return dsn +} diff --git a/platform/deploy/README.md b/platform/deploy/README.md index 5bfedd71..26da696f 100644 --- a/platform/deploy/README.md +++ b/platform/deploy/README.md @@ -9,7 +9,17 @@ - `tmplatformd.service` — юнит контрольной панели. Тело юнита проверено `systemd-analyze verify` (systemd 259) — exit 0, без замечаний. ⚠ Проверять надо с ПОДСТАВЛЕННЫМ существующим `ExecStart=`: дословно юнит даёт exit 1, потому что `verify` проверяет и наличие бинаря, а `/usr/local/bin/tmplatformd` - на стенде нет. ⚠ **живого прогона под systemd не было** — на машине нет sudo, юнит не устанавливался. + на стенде нет. ⚠ **юнит целиком под systemd не запускался** — нет sudo и нет бинаря на стенде. + + Отдельные свойства песочницы при этом проверены ЖИВЫМ прогоном в пользовательском systemd 259 + (`systemd-run --user --wait`), а не вычитаны из доки: + `ProtectSystem=strict` + `ReadWritePaths=` на несуществующем пути → **`226/NAMESPACE`, юнит не + стартует**; тот же путь созданным → `0/SUCCESS`; префикс `-` на несуществующем → `0/SUCCESS` + (строка игнорируется — потому мы её и не префиксуем); + `ProtectHome=yes` → `/home` пуст, чтение `/home/` даёт `Permission denied`; + `ProtectHome=tmpfs` + `BindPaths=<каталог>` → каталог виден. Ресурсные потолки + (`MemoryMax=80%`, `OOMPolicy=continue`) живьём не проверялись — вывод из + `systemd.resource-control(5)`/`systemd.service(5)`. ## Откат релиза: не ниже версии 5 @@ -33,7 +43,11 @@ ## Установка (набросок, исполняется владельцем) ```sh -useradd --system --home /srv/textmachine tmplatform +useradd --system --home-dir /srv/textmachine tmplatform +# ⚠ Каталог создаём ЯВНО: --system не создаёт домашний каталог, а `ReadWritePaths=/srv/textmachine` +# при `ProtectSystem=strict` на несуществующем пути валит сборку mount-namespace — юнит не стартует +# вовсе (PD-91, systemd.exec(5)). Это первая команда, которую пропускают, читая набросок сверху вниз. +install -d -m0750 -o tmplatform -g tmplatform /srv/textmachine install -D -m0755 tmplatformd /usr/local/bin/tmplatformd install -D -m0755 tmplatformctl /usr/local/bin/tmplatformctl install -d -m0700 -o root -g root /etc/tmplatform @@ -57,6 +71,19 @@ TM_PLATFORM_SIGNUP_GRANT_USD=5 либо отдельный шаг деплоя. `goose` держит advisory-лок, так что параллельный запуск не гонка, но и не норма. +## Где на сервере лежат книги + +`/srv/textmachine` — корень библиотеки НА СЕРВЕРЕ. Решение владельца «книги живут в `~/books`» +относится к машине разработки: под этим юнитом домашние каталоги недоступны вовсе +(`ProtectHome=yes` подставляет пустой `/home` и детям-`tmctl` тоже), поэтому библиотека под +домашним каталогом на сервере просто не откроется — не «сработает медленнее», а не найдётся. +Оператору, которому это нужно, юнит называет ровно две строки замены (`ProtectHome=tmpfs` + +`BindPaths=`); других изменений не требуется. + +⚠ Выбирающего путь кода ещё нет: `Supervisor.Workdir` задаёт вызывающий, а вызывающий — воркер, +которого нет (строка 103 единого бэклога). Когда он появится, корень становится настройкой, и её +дефолт — этот каталог. + ## Чего здесь ещё нет TLS и домен (перед юнитом предполагается edge-прокси), ограничитель соединений на edge, diff --git a/platform/deploy/tmplatformd.service b/platform/deploy/tmplatformd.service index 27880681..2d182d26 100644 --- a/platform/deploy/tmplatformd.service +++ b/platform/deploy/tmplatformd.service @@ -38,13 +38,26 @@ Environment=TM_PLATFORM_DSN_FILE=%d/dsn Environment=TM_PLATFORM_OIDC_CLIENT_SECRET_FILE=%d/oidc_client_secret EnvironmentFile=/etc/tmplatform/env -# Books live outside the repository and outside /var/lib by owner's decision (~/books); the unit -# gets the one directory it may write and nothing else. +# Books live outside the repository and outside /var/lib; the unit gets the one directory it may +# write and nothing else. /srv/textmachine is the SERVER's books root — "~/books" is the owner's +# decision for a development machine, and under this unit a home directory is not reachable at all +# (see ProtectHome below). +# +# The directory must EXIST before the first start: with ProtectSystem=strict a ReadWritePaths= that +# names a missing path fails the mount-namespace setup and the unit does not come up. The deploy +# notes create it, and the "-" prefix that would make the line optional is deliberately not used — +# a service with no writable directory should fail at start, not at the first write hours in. ReadWritePaths=/srv/textmachine StateDirectory=tmplatform # Sandboxing. Free, and it bounds what a compromised process reaches. ProtectSystem=strict +# ProtectHome=yes makes /home, /root and /run/user empty for this unit AND for every tmctl it +# spawns. That is the intended posture on a server — the platform has no business in anyone's home — +# and it is also the reason the books root above is under /srv. An operator who really must keep a +# library under a home directory changes exactly two lines, and nothing else: +# ProtectHome=tmpfs +# BindPaths=/home//books ProtectHome=yes PrivateTmp=yes PrivateDevices=yes diff --git a/platform/docs/DEFECT_REGISTER.md b/platform/docs/DEFECT_REGISTER.md index 8b32aced..53d7d155 100644 --- a/platform/docs/DEFECT_REGISTER.md +++ b/platform/docs/DEFECT_REGISTER.md @@ -38,7 +38,7 @@ | PD-27 | bug | minor | `internal/pgstore/credits.go` | `Settle` принимал любую сумму: одно завышенное `committed_usd` уводило баланс в минус, дальше каждый прогон получал `ErrInsufficientCredit` без диагностики — **закрыто:** расчёт capped потолком холда, факт записан в `note`; пин `TestSettlementIsCappedAtTheHold` | fixed(P1, дерево сессии) | ревью денежного пути · ревью «вне карты» | | PD-28 | bug | minor | `internal/ingest/supervisor.go` | `cmd.Wait()` на отменённой команде возвращает `context.Canceled`, а не `*ExitError`, поэтому исход читался как `failed`: штатный SIGTERM пометил бы ВСЕ идущие прогоны провалившимися — **закрыто:** исход из `ProcessState`, факт остановки едет в ошибке; пин `TestStoppedRunKeepsTheEnginesOutcome` | fixed(P1, дерево сессии) | ревью стиля (клейм) + собственная проверка исполнением | | PD-29 | vuln | minor | `internal/login/login.go` | `GET /auth/callback` — неаутентифицированная ручка, ПИШУЩАЯ в БД, без лимита и без ретеншена: замерено 2000 строк за 2.28 с с одного хоста (~76 млн строк/сутки), строки отказов недостижимы через API и не удалялись никогда — **закрыто:** лимитер на колбэке, ретеншен журнала 180 дней свипом | fixed(P1, дерево сессии) | ревью безопасности (исполнением) | -| PD-30 | vuln | minor | `internal/pgstore/identity.go` | Грант фри-тира выдавался за каждую новую пару `(provider, subject)` без учёта `email_verified`: провайдер с саморегистрацией превращал каждый новый `sub` в $5, потолок задавал только глобальный лимитер (~$864k/сутки на бумаге) — **закрыто:** грант только подтверждённой личности, аккаунт создаётся с нулём, начисление руками из админки. ⚠ Продуктовое следствие — вопрос владельцу в журнале | fixed(P1, дерево сессии) | ревью безопасности (исполнением) | +| PD-30 | vuln | minor | `internal/pgstore/identity.go` | Грант фри-тира выдавался за каждую новую пару `(provider, subject)` без учёта `email_verified`: провайдер с саморегистрацией превращал каждый новый `sub` в $5, потолок задавал только глобальный лимитер (~$864k/сутки на бумаге) — **закрыто:** грант только подтверждённой личности — НЕПОДТВЕРЖДЁННАЯ создаёт аккаунт с нулём, и его начисляют руками из админки. ⚠ Исправлено 08.08 (PD-104): прежняя редакция этой ячейки говорила «аккаунт создаётся с нулём» без оговорки и противоречила коду — ПОДТВЕРЖДЁННАЯ личность получает автогрант `TM_PLATFORM_SIGNUP_GRANT_USD` (дефолт $5, `config.go`), закрыт был только путь саморегистрации. ⚠ Продуктовое следствие — вопрос владельцу в журнале | fixed(P1, дерево сессии) | ревью безопасности (исполнением) | | PD-31 | bug | minor | `internal/login/login.go` | `discover` держал мьютекс на время сетевого вызова без таймаута: шесть параллельных входов при медленном IdP заняли 4/8/12/16/20/24 с вместо ~4 — **закрыто:** запрос вне лока, свой таймаут 5 с | fixed(P1, дерево сессии) | ревью безопасности (исполнением) | | PD-32 | vuln | minor | `internal/login/login.go`, `cmd/tmplatformd/main.go` | Имя провайдера захардкожено `"google"` независимо от issuer, а `State.Provider` писался и не сверялся: смена issuer тихо кладёт чужие `sub` в старое пространство имён (новые аккаунты, старые недостижимы), а при двух провайдерах стейт одного редимится колбэком другого (IdP mix-up) — **закрыто:** `TM_PLATFORM_OIDC_PROVIDER`, сверка `st.Provider` в колбэке | fixed(P1, дерево сессии) | ревью безопасности · ревью «вне карты» | | PD-33 | vuln | minor | `internal/auth/csrf.go` | Требование `X-TM-Client` снималось ЛЮБЫМ заголовком `Authorization`, включая мусорный: покрытие CSRF-слоя выбирал атакующий (сегодня упиралось в 401, но пережило бы любое послабление в `present`) — **закрыто:** снимает только валидный Bearer, через ту же функцию, что аутентифицирует | fixed(P1, дерево сессии) | ревью безопасности (исполнением) | @@ -87,32 +87,32 @@ | PD-76 | bug | minor | `internal/login/login_test.go` | **Определяющее свойство пакета — «ничего выданного провайдером не персистится» — проверялось утверждением, которое не могло упасть.** `memStore.notes` объявлено и не заполнялось ни одним методом, поэтому `strings.Join(notes)` всегда пусто, а `Contains` всегда ложно. Свойство названо в доккомменте пакета первой строкой — **закрыто:** мок пишет в `saw` КАЖДУЮ строку, которую поток ему передал, а утверждение проверяет и непустоту записи, и отсутствие среди неё и access-токена, и любого JWT-образного значения. Посадка «положить в стор сырой id-токен» падает | fixed(P2, дерево сессии) | ревью P2 (линза water) | | PD-77 | bug | info | `internal/ingest/supervisor.go:104` | **Штатная остановка живого прогона поднимала тревогу о сломанном синке.** `Ingest` проверяет `ctx.Err()` в начале цикла и возвращает `context.Canceled` как СВОЮ ошибку; `Run` отличить это от отказавшего синка не мог и на обычном SIGTERM писал ERROR «stream could not be materialized», который по замыслу означает «платформа ослепла, пока тратятся деньги», плюс звал `stop()` на уже останавливающемся прогоне — **закрыто:** отменённый `runCtx` больше не считается отказом синка | fixed(P2, дерево сессии) | ревью P2 (линза вне карты) | | PD-78 | hardening | info | `internal/login/login.go` (было), `internal/httpapi/problem.go` (было), `internal/pgstore/identity.go`, `internal/httpapi/server.go` | **Свод воды и дублей, найденный линзой лаконичности; каждый пункт проверен удалением.** (а) `login.Routes` мемоизировал mux через `sync.Once` — при этом ВТОРОЙ и последующие `guard` молча игнорировались, то есть это была не оптимизация, а ловушка; снято. (б) `login.Fail` носил `*http.Request`, который никто не читал, и ради несовпадения сигнатур существовал шим `httpapi.Fail`; параметр и шим удалены, `WriteProblem` подключён напрямую. (в) `upsertIdentityOnce` держал собственный begin/rollback/commit при наличии `inTx` — второй экземпляр того же кода. (г) `Deps.APIPrefix` — ручка, которую не выставлял ни один вызыватель; заменена константой. (д) `Ready` делал `Ping` и следом запрос — два round trip на пробу каждые несколько секунд. (е) пять полей тестовых двойников, которые писались и не читались; `blockingSink` не блокировал. (ж) `money.USD` считал руками с комментарием про переполнение `MinInt64` — заменён на `big.Rat.FloatString(6)`, проверено побайтовое совпадение на всём диапазоне | fixed(P2, дерево сессии) | ревью P2 (линза water) + самопроверка | -| PD-79 | bug | minor | `internal/money/money.go:33-36` | **Строковый `"null"` читается как НОЛЬ денег.** Кавычки снимаются `strings.Trim` ДО проверки `s == "null"`, поэтому `"committed_usd":"null"` даёт настоящий `0` и НЕПУСТОЙ указатель, тогда как доккоммент поля обещает отказ на «absent, null and empty». Замерено приёмкой на живом декодере: голый `null` и отсутствие поля дают nil (защита работает), `""` даёт ошибку, а `"null"` — `Spend = 0 micro-USD, NON-NIL`. На пути расчёта это «попытка стоила ничего»: холд освобождается, списания нет. Латентно до воркера; чинится перестановкой проверки перед `Trim` | open | приёмка P2 (замер оркестратора №15 + панель) | -| PD-80 | vuln | **major** | `internal/login/login.go:158,217-226` | **Вход выключается тремя запросами в секунду, и 429 колбэка ДОБИВАЕТ начатые входы.** Ведро `rate.NewLimiter(2, 20)` одно на `/auth/login` И `/auth/callback` (`login.go:122`, единственный лимитер в зоне), а колбэк стирает login-куку ПЕРВОЙ строкой — до своей проверки лимитера. Следствие: анонимный поток на `/auth/login` не только закрывает вход всем (это PD-42, принято риском в форме «глобальный, не пер-адресный»), но и делает начатый вход невосстановимым: 429 приходит уже с `Set-Cookie: __Host-tm_login=; Max-Age=0`, поэтому повтор того же колбэка не пройдёт и после наполнения ведра. **Воспроизведено приёмкой на боевом бинаре:** 19 из 40 `/auth/login` прошли, дальше 429; честный колбэк с живым state получил 429 и стёртую куку. Независимо измерено панелью. Фикс дешёвый: лимитер прежде очистки куки + раздельные ведра для начала и конца входа; пер-адресный лимит остаётся вопросом edge (PD-42) | open | приёмка P2 (живая проба + панель, две независимые линзы) | +| PD-79 | bug | minor | `internal/money/money.go:33-36` | **Строковый `"null"` читается как НОЛЬ денег.** Кавычки снимаются `strings.Trim` ДО проверки `s == "null"`, поэтому `"committed_usd":"null"` даёт настоящий `0` и НЕПУСТОЙ указатель, тогда как доккоммент поля обещает отказ на «absent, null and empty». Замерено приёмкой на живом декодере: голый `null` и отсутствие поля дают nil (защита работает), `""` даёт ошибку, а `"null"` — `Spend = 0 micro-USD, NON-NIL`. На пути расчёта это «попытка стоила ничего»: холд освобождается, списания нет. Латентно до воркера; чинится перестановкой проверки перед `Trim` — **закрыто:** литерал `null` судится ДО раскавычивания и оставляет значение нетронутым; кавычки снимает `encoding/json`, а не `strings.Trim` — слово `null`, пустая строка и экранированная цифра выходят тем, чем являются, и каждое встречает ту же единственную проверку синтаксиса, поэтому отдельной ветки «пусто или null» не нужно вовсе. Пины: `money.TestUnmarshalTellsTheNullLiteralFromTheWordNull` (обе формы плюс невмешательство в значение) и `ingest.TestSpendRefusesNonsense` на шве. Обе посадки — «снять кавычки первыми» и «слово `null` есть ноль» — поймать поимённо | fixed(P3, дерево сессии) | приёмка P2 (замер оркестратора №15 + панель) | +| PD-80 | vuln | **major** | `internal/login/login.go:158,217-226` | **Вход выключается тремя запросами в секунду, и 429 колбэка ДОБИВАЕТ начатые входы.** Ведро `rate.NewLimiter(2, 20)` одно на `/auth/login` И `/auth/callback` (`login.go:122`, единственный лимитер в зоне), а колбэк стирает login-куку ПЕРВОЙ строкой — до своей проверки лимитера. Следствие: анонимный поток на `/auth/login` не только закрывает вход всем (это PD-42, принято риском в форме «глобальный, не пер-адресный»), но и делает начатый вход невосстановимым: 429 приходит уже с `Set-Cookie: __Host-tm_login=; Max-Age=0`, поэтому повтор того же колбэка не пройдёт и после наполнения ведра. **Воспроизведено приёмкой на боевом бинаре:** 19 из 40 `/auth/login` прошли, дальше 429; честный колбэк с живым state получил 429 и стёртую куку. Независимо измерено панелью. Фикс дешёвый: лимитер прежде очистки куки + раздельные ведра для начала и конца входа; пер-адресный лимит остаётся вопросом edge (PD-42) — **закрыто:** два ведра вместо одного (`startLimit`/`finishLimit`, те же rate/burst — не делится именно ИСЧЕРПАНИЕ), и проверка лимитера ПЕРЕД `ClearLogin`. Пины: `TestFloodingTheStartOfSignInDoesNotCloseTheEnd` (поток на `/auth/login` не закрывает честный колбэк) и `TestARefusedCallbackKeepsTheLoginItRefused` (429 не стирает куку, состояние не съедено, повтор после снятия лимита доходит до 303). Посадки «одно ведро» и «очистка выше лимитера» падают | fixed(P3, дерево сессии) | приёмка P2 (живая проба + панель, две независимые линзы) | | PD-81 | standards | minor | `internal/pgstore/credits.go:169-178` | **Заявленный `ErrDuplicateHold` на реальном пути недостижим:** при ЖИВОЙ резервации повторный `Hold` падает на первичном ключе `reservations_pkey` (`00007_credits.sql:66`) и уходит наверх сырой ошибкой Postgres SQLSTATE 23505; объявленная ошибка приходит только когда строку резервации уже смахнули, а ключ леджера остался. Замерено приёмкой на живом PG в обеих формах. Деньги целы (`balance == SUM(ledger)`, транзакция откатывается), но воркеру не на что смотреть, кроме текста ошибки | open | приёмка P2 (замер оркестратора №15 + панель) | | PD-82 | bug | info | `internal/pgstore/credits.go:236-239` | `Hold` на НЕСУЩЕСТВУЮЩИЙ аккаунт отдаёт `ErrInsufficientCredit` (в `lockBalance` `ErrNoRows` трактуется как «нет кредита»), а не `ErrNoAccount`: обещание PD-56 «один ответ на несуществующий аккаунт» покрывает `Grant`/`Adjust`/`Balance`/`ReadAccount` и на `Hold` не распространяется. Замерено приёмкой | open | приёмка P2 (замер оркестратора №15) | -| PD-83 | hardening | minor | `internal/httpapi/middleware.go:64` | **Фикс PD-3 не запинен в собственном месте:** посадка «`Recover` логирует `r.URL.Path` вместо `routeOf(r)`» батарею ПЕРЕЖИВАЕТ, тогда как та же посадка в `AccessLog` ловится поимённо (`TestAccessLogNamesTheRouteNotThePath`). По правилу шапки этого файла половина PD-3 закрытой не считается | open | приёмка P2 (посадка мутации) | -| PD-84 | hardening | minor | `internal/login/login.go:222` | **Лимитер колбэка (фикс PD-29) не запинен:** удаление всей проверки `h.limiter.Allow()` из `callback` оставляет батарею зелёной. Замер PD-29 (~880 строк/с с одного хоста) означает, что регрессия здесь тихо возвращает неаутентифицированного писателя в таблицу журнала | open | приёмка P2 (посадка мутации) | -| PD-85 | hardening | minor | `internal/pgstore/identity.go:126-131` | **«Неподтверждённый адрес не поднимается на аккаунт» запинено только на ветке НОВОЙ личности:** снятие условия `in.EmailVerified` в ветке ВОЗВРАЩАЮЩЕГОСЯ входа (обновление `users.email`) проходит батарею — `TestUnverifiedAddressStaysOffTheAccount` покрывает первый вход и переход в verified, но не обратный случай | open | приёмка P2 (посадка мутации) | +| PD-83 | hardening | minor | `internal/httpapi/middleware.go:64` | **Фикс PD-3 не запинен в собственном месте:** посадка «`Recover` логирует `r.URL.Path` вместо `routeOf(r)`» батарею ПЕРЕЖИВАЕТ, тогда как та же посадка в `AccessLog` ловится поимённо (`TestAccessLogNamesTheRouteNotThePath`). По правилу шапки этого файла половина PD-3 закрытой не считается — **закрыто:** `TestPanicBecomesAProblemAndNamesTheRoute` — паника за мультиплексором с `{book}` в паттерне; сверяется и `route`, и отсутствие идентификатора книги во ВСЕЙ строке (в ней же стек). Посадка `r.URL.Path` падает | fixed(P3, дерево сессии) | приёмка P2 (посадка мутации) | +| PD-84 | hardening | minor | `internal/login/login.go:222` | **Лимитер колбэка (фикс PD-29) не запинен:** удаление всей проверки `h.limiter.Allow()` из `callback` оставляет батарею зелёной. Замер PD-29 (~880 строк/с с одного хоста) означает, что регрессия здесь тихо возвращает неаутентифицированного писателя в таблицу журнала — **закрыто:** `TestBothLegsOfSignInAreRateLimited` — десять колбэков подряд обязаны упереться в 429. Посадка «удалить проверку целиком» падает; её же ловит `TestARefusedCallbackKeepsTheLoginItRefused` | fixed(P3, дерево сессии) | приёмка P2 (посадка мутации) | +| PD-85 | hardening | minor | `internal/pgstore/identity.go:126-131` | **«Неподтверждённый адрес не поднимается на аккаунт» запинено только на ветке НОВОЙ личности:** снятие условия `in.EmailVerified` в ветке ВОЗВРАЩАЮЩЕГОСЯ входа (обновление `users.email`) проходит батарею — `TestUnverifiedAddressStaysOffTheAccount` покрывает первый вход и переход в verified, но не обратный случай — **закрыто:** `TestUnverifiedAddressStaysOffTheAccount` продлён третьим шагом — ВОЗВРАЩАЮЩИЙСЯ вход с новым НЕподтверждённым адресом: `users.email` не двигается, `identities.email` записывает то, что пришло. Посадка «снять `in.EmailVerified` в ветке возвращающегося» падает | fixed(P3, дерево сессии) | приёмка P2 (посадка мутации) | | PD-86 | hardening | info | `internal/pgstore/sessions.go:23,50` | **Два клауза-близнеца не запинены, и абсолютный потолок держится ТРАНЗИТИВНО:** снятие `absolute_expires_at > $2` из `Lookup` батарею переживает, потому что потолок навязывается через `least($3, absolute_expires_at)` в `Touch` (это запинено — `TestSessionLifecycle`). Снятие `revoked_at is null` из `Touch` тоже переживает (класс PD-4). Дефекта сегодня нет ни в одном; риск в том, что каждый слой по отдельности выглядит избыточным, а вместе они — единственное, что ограничивает жизнь сессии | open | приёмка P2 (посадки мутаций) | | PD-87 | hardening | info | `internal/httpapi/server.go:82`, `internal/login/login.go:31` | Ещё два незапиненных: снятие `LimitBody` с поддерева `/auth` и `stateTTL` 10 мин → 240 ч проходят батарею. Первое — родня PD-72 (та про общий внешний слой, эта про конкретное поддерево), второе — окно жизни неиспользованного авторизационного запроса | open | приёмка P2 (посадки мутаций) | | PD-88 | bug | info | `internal/auth/cookie.go:62-66` | **TTL меньше секунды выпускает куку БЕЗ атрибута `Max-Age`:** `int(ttl.Seconds())` даёт 0, а Go при `MaxAge == 0` атрибут опускает ⇒ кука становится браузер-сессионной. Достижимо в последнюю секунду абсолютного срока (скольжение выдаёт `min(idle, остаток абсолютного)` при гарде `ttl > 0`) — то есть ровно тот исход, который самопроверка P2 называла нежелательным: кука переживает сессию, и следующий запрос даёт 401 вместо чистого «вы вышли». Подтверждено исполнением (ttl 500 мс/999 мс) | open | приёмка P2 (панель ×2, подтверждено исполнением) | | PD-89 | hardening | minor | `cmd/tmplatformctl/main.go:143-150` | **Сминченный ключ идемпотентности не печатается при ошибке записи:** PD-75 закрыл путь ПОСЛЕ коммита, но неоднозначный обрыв НА коммите остался — оператор видит ошибку, повторяет без `--key`, `newKey()` чеканит новый ключ, второе начисление проходит. Фикс: печатать ключ вместе с ошибкой, чтобы повтор был с тем же `--key` | open | приёмка P2 (панель) | | PD-90 | bug | info | `cmd/tmplatformctl/main.go:112,134` | `grant` и `adjust` делят пространство ключей `source="admin"`: `--key`, потраченный грантом, молча гасит корректировку с тем же ключом. CLI честно скажет «ключ уже потрачен», но оператор ждал другой операции | open | приёмка P2 (панель) | -| PD-91 | doc | minor | `deploy/README.md:33-42`, `deploy/tmplatformd.service:43,48` | **Установка, исполненная дословно, даёт нестартующий юнит:** `/srv/textmachine` не создаётся ни одной командой наброска, а `ReadWritePaths=` без префикса `-` на несуществующем пути валит сборку mount-namespace при `ProtectSystem=strict`. Заодно `ProtectHome=yes` против решения владельца «книги живут в `~/books`»: детям-`tmctl` домашние каталоги под этим юнитом недоступны — либо книги переезжают в `/srv/textmachine`, либо юнит получает `BindPaths=`. ⚠ Вывод из `systemd.exec(5)`, под systemd не исполнялось (sudo нет) | open | приёмка P2 (панель, сверено с докой) | +| PD-91 | doc | minor | `deploy/README.md:33-42`, `deploy/tmplatformd.service:43,48` | **Установка, исполненная дословно, даёт нестартующий юнит:** `/srv/textmachine` не создаётся ни одной командой наброска, а `ReadWritePaths=` без префикса `-` на несуществующем пути валит сборку mount-namespace при `ProtectSystem=strict`. Заодно `ProtectHome=yes` против решения владельца «книги живут в `~/books`»: детям-`tmctl` домашние каталоги под этим юнитом недоступны — либо книги переезжают в `/srv/textmachine`, либо юнит получает `BindPaths=`. ⚠ Вывод из `systemd.exec(5)`, под systemd не исполнялось (sudo нет) — **закрыто:** каталог `/srv/textmachine` создаётся явной командой наброска (`--system` домашний каталог не создаёт), префикс `-` намеренно НЕ ставится (сервис без записываемого каталога обязан падать на старте, а не на первой записи через часы), `ProtectHome=yes` оставлен с названной ценой и двухстрочным выходом (`ProtectHome=tmpfs` + `BindPaths=`), корень библиотеки на СЕРВЕРЕ — `/srv/textmachine`, `~/books` объявлено конвенцией машины разработки. **Проверено живым прогоном** `systemd-run --user` (systemd 259), а не докой: несуществующий путь без `-` → `226/NAMESPACE`, созданный → `0/SUCCESS`, с `-` → `0/SUCCESS` (строка игнорируется); `ProtectHome=yes` → `Permission denied` на `/home/`; `tmpfs`+`BindPaths` → каталог виден | fixed(P3, дерево сессии) | приёмка P2 (панель, сверено с докой) | | PD-92 | bug | info | `internal/ingest/supervisor.go:118-121` | **Дренаж стоит ДО `cmd.Wait()`, поэтому `WaitDelay` его не размораживает:** `io.Copy(io.Discard, stdout)` ждёт EOF, а EOF придёт только когда закроются ВСЕ копии пишущего конца пайпа; внук, унаследовавший stdout и игнорирующий SIGINT, вешает `Run` навсегда — backstop `WaitDelay` действует внутри `Wait`, до которого управление не доходит. ⚠ Сегодня недостижимо и это пере-проверено приёмкой: в `backend/` вне тестов нет ни одного `exec.Command` и нет cgo. ⚠ **Пере-диспозиция (эррата №15): вес понижен до info** — весь пайп-путь `supervisor.go` объявлен ДЕВ-РЕЖИМОМ (D39.106 п.3), в проде родителя у движка нет и `cmd.StdoutPipe()` не существует; чинить только если дев-путь остаётся | open | приёмка P2 (панель, граница зоны пере-проверена) | | PD-93 | bug | info | `internal/ingest/supervisor.go:109` | Фикс PD-77 («наша остановка — не сломанный синк») сверяет только `context.Canceled` и пропускает `context.DeadlineExceeded`: как только у `runCtx` появится дедлайн (потолок времени прогона — очевидная будущая ручка), штатное истечение снова поднимет ERROR «stream could not be materialized» | open | приёмка P2 (панель) | | PD-94 | bug | info | `internal/httpapi/middleware.go:61-70` | **`Recover` глотает `http.ErrAbortHandler`** — sentinel, которым хендлер намеренно обрывает соединение (`net/http` его не логирует и рвёт коннект). Замерено приёмкой: паника `ErrAbortHandler` превращается в 500 с problem-телом, то есть усечённый поток становится неотличим от полного. Латентно (сегодня им никто не паникует), но именно SSE-хендлер — типовой его пользователь | open | приёмка P2 (замер оркестратора №15) | -| PD-95 | doc | **major (для промта эмиттера)** | `internal/ingest/events.go:6-12`, `docs/platform-PROGRESS.md` §«Транспорт потока событий» | **Транспортная история зоны устарела против D39.106 в ДВУХ местах, и обе версии не совпадают с ратифицированной формой.** Ратифицировано (D39.106 п.2 + `research/25` §Форма): движок — транзиентный systemd-юнит на прогон, платформа ему **НЕ родитель**; события — `events.jsonl` в каталоге книги, append-only, как outbox-проекция уже закоммиченных строк SQLite (та же транзакция, что чекпойнт); платформа **тейлит** журнал, курсор `(engine_run_id, seq)` коммитится в одной Postgres-транзакции с эффектом. В `research/25` вариант «платформа — родитель + пайп (stdout/fd)» получил **0 голосов из 15** («время жизни движка — подмножество платформы: деплой/рестарт убивает или осиротляет прогон»), выделенный fd 3 — **0**, stdout/journald как источник событий — **0**. Что в зоне: (а) доккоммент `events.go` предлагает переезд на fd/сокет — отклонённая форма; (б) журнал зоны длинно доказывает «канал остаётся stdout» и объявляет переезд отклонённым, ссылаясь на PD-59, который **сам superseded** тем же D39.106 п.3 («PD-59 superseded; пайп-путь `supervisor.go` P1 = дев-режим; ответ PD-13 переезжает на cgroup юнита прогона»). Оба текста прочтёт эмиттер-сессия как задание. ⚠ **Приёмка №15 это пропустила и в первой редакции строки сама сослалась на снятый PD-59 — исправлено здесь же** | open | приёмка P2 (эррата оркестратора №15, 07.08) | +| PD-95 | doc | **major (для промта эмиттера)** | `internal/ingest/events.go:6-12`, `docs/platform-PROGRESS.md` §«Транспорт потока событий» | **Транспортная история зоны устарела против D39.106 в ДВУХ местах, и обе версии не совпадают с ратифицированной формой.** Ратифицировано (D39.106 п.2 + `research/25` §Форма): движок — транзиентный systemd-юнит на прогон, платформа ему **НЕ родитель**; события — `events.jsonl` в каталоге книги, append-only, как outbox-проекция уже закоммиченных строк SQLite (та же транзакция, что чекпойнт); платформа **тейлит** журнал, курсор `(engine_run_id, seq)` коммитится в одной Postgres-транзакции с эффектом. В `research/25` вариант «платформа — родитель + пайп (stdout/fd)» получил **0 голосов из 15** («время жизни движка — подмножество платформы: деплой/рестарт убивает или осиротляет прогон»), выделенный fd 3 — **0**, stdout/journald как источник событий — **0**. Что в зоне: (а) доккоммент `events.go` предлагает переезд на fd/сокет — отклонённая форма; (б) журнал зоны длинно доказывает «канал остаётся stdout» и объявляет переезд отклонённым, ссылаясь на PD-59, который **сам superseded** тем же D39.106 п.3 («PD-59 superseded; пайп-путь `supervisor.go` P1 = дев-режим; ответ PD-13 переезжает на cgroup юнита прогона»). Оба текста прочтёт эмиттер-сессия как задание. ⚠ **Приёмка №15 это пропустила и в первой редакции строки сама сослалась на снятый PD-59 — исправлено здесь же** — **закрыто:** доккоммент пакета переписан под D39.106 §2 — транзиентный systemd-юнит на прогон, платформа НЕ родитель, `events.jsonl` в каталоге книги как outbox-проекция коммитов SQLite, тейл с курсором `(engine_run_id, seq)`, повторное чтение строк — норма (PD-105). Отвергнутые формы перечислены со счётом голосов, чтобы не вернулись свежей идеей. Доккоммент `Supervisor` помечен ДЕВ-РЕЖИМОМ там же, где он описывает пайп. ⚠ В журнале зоны нашлась ВТОРАЯ копия снятого ответа — блок «ОТВЕЧЕНО приёмкой (PD-59)» в списке «Открытые вопросы после P1» п.4: эррата №15 пере-ставила другую секцию, эту не тронула. Текст оркестратора не переписан — над ним поставлен баннер SUPERSEDED с ратифицированной формой; проверить принадлежность правки — за приёмкой | fixed(P3, дерево сессии) | приёмка P2 (эррата оркестратора №15, 07.08) | | PD-96 | hardening | info | `internal/httpapi/server.go:39-43`, `internal/auth/csrf.go:28` | **`TrustedOrigins` обещает отдельно развёрнутый фронт, но CORS-слоя нет вовсе.** Живая проба: preflight `OPTIONS` с `Origin: https://app.example.org` получает 401 от гарда (браузерный preflight креденшелов не носит и не должен), заголовков `Access-Control-*` нет ни на одном ответе. Сценарий «фронт на другом origin» браузером сегодня неисполним: либо CORS приезжает вместе с контрактными ручками (П-1), либо фронт живёт на том же origin, и тогда `TrustedOrigins` — мёртвая ручка | open | приёмка P2 (панель + живая проба) | | PD-97 | hardening | info | `internal/pgstore/credits.go:212-216` | `Settle`/`Release` отбрасывают флаг `applied` у `hold_release`: если ключ `("run_release", engineRunID)` уже потрачен, резервация закроется, а деньги не вернутся — тихий no-op на денежном пути. Требует нештатной последовательности (закрытие, смахивание строки, повторное открытие того же `engine_run_id`), но ровно на такой последовательности стоит `ErrDuplicateHold` | open | приёмка P2 (панель) | | PD-98 | doc | info | `internal/pgstore/store.go:75-79` | Случай «схема НОВЕЕ бинаря» в `Ready` беззвучен — признано ⚠-комментарием на месте, но ни одной строки лога: оператор, запустивший старый бинарь на новой схеме, сигнала не получит | open | приёмка P2 (панель) | | PD-99 | hardening | info | `internal/ingest/supervisor.go:102` | INFO-лог «engine started» пишет `args` целиком. Сегодня безвредно, но воркер будет передавать движку идентификатор книги и потолок аргументами ⇒ book-id и денежная сумма попадут в INFO платформы (D39.84 + норма зоны «id книги в логи не текут»). Закрыть вместе с воркером: логировать имя команды, не argv | open | приёмка P2 (панель) | -| PD-100 | bug | minor | `internal/login/login.go:245-261` | **Класс PD-5 закрыт в `auth/`, но не в `login/`:** колбэк глотает ошибку стора (`TakeLoginState`) и ошибку discovery, репортя их как обычный отказ (`unknown_state` / `discovery_failed`) — сама ошибка не доезжает ни до одной строки лога, хотя `pgstore/identity.go` намеренно отличает «состояния нет» от инфраструктурного сбоя. Аутентификационный DB-outage снова выглядит штормом обычных отказов | open | приёмка P2 (панель) | +| PD-100 | bug | minor | `internal/login/login.go:245-261` | **Класс PD-5 закрыт в `auth/`, но не в `login/`:** колбэк глотает ошибку стора (`TakeLoginState`) и ошибку discovery, репортя их как обычный отказ (`unknown_state` / `discovery_failed`) — сама ошибка не доезжает ни до одной строки лога, хотя `pgstore/identity.go` намеренно отличает «состояния нет» от инфраструктурного сбоя. Аутентификационный DB-outage снова выглядит штормом обычных отказов — **закрыто:** сбой стора и сбой discovery уходят в ERROR; на проводе и в журнале — прежний отказ. `login.ErrNoState` заведён у владельца интерфейса (как `auth.ErrNoSession`), `pgstore.ErrNoLoginState` — то же значение под прежним именем. Пин `TestInfrastructureFailuresInTheCallbackAreLogged`: три случая, включая «обычное истечение НЕ логируется как авария» — ловит и посадку «логировать всегда» | fixed(P3, дерево сессии) | приёмка P2 (панель) | | PD-101 | bug | minor | `internal/login/login.go:507` | `login_events.ip_prefix` берётся из `r.RemoteAddr`, а в задуманном деплое перед сервисом стоит edge-прокси ⇒ префикс всегда сеть прокси. Журнал входов заведён как ответ на «откуда примерно я входил» — в шипуемой форме он систематически отвечает неверно. `X-Forwarded-For`/`Forwarded` нигде не читаются и доверенного прокси в конфиге нет (это правильный дефолт: доверять заголовку без edge нельзя) — значит решение про edge и про этот столбец принимается вместе | open | приёмка P2 (панель) | | PD-102 | doc | minor | `internal/httpapi/serve.go:36-38` | Доккоммент `DefaultTimeouts` утверждает, что «an upload extends its own deadline as it makes progress» — это НЕВЕРНО: `ReadTimeout` в `net/http` (Go 1.26.5, `server.go:990` `wholeReqDeadline = t0.Add(ReadTimeout)`) выставляется один раз и по мере прихода байтов не продлевается. Комментарий несущий: он объясняет, почему `Read` короткий, и на нём будущая ручка загрузки книги (23 МБ по контракту) построит неверное ожидание — ей понадобится собственный дедлайн через `ResponseController`, а не «прогресс продлевает» | open | приёмка P2 (панель, сверено с исходником Go) | | PD-103 | hardening | minor | `internal/auth/middleware.go:43,66` | У обращений к БД на аутентифицированном пути (`Lookup`/`Touch`) нет собственного дедлайна — только голый `r.Context()`, а `WriteTimeout` у сервера отсутствует по проекту (SSE) и `TimeoutHandler` в цепочке нет. Зависший Postgres паркует хендлеры и ждущих в пуле, пока клиент сам не уйдёт. `readyz` свой таймаут получил (PD-14) — горячий путь нет | open | приёмка P2 (панель) | -| PD-104 | bug | **minor, расхождение док↔код** | `internal/login/login.go:285-288`, `internal/config/config.go:73` | **Фри-тир начисляется АВТОМАТИЧЕСКИ, а реестр обещает обратное.** Код: дефолт `SignupGrantMicroUSD: 5 * 1_000_000` (`config.go:73`) проведён в демона (`main.go:99`) и логин отдаёт его в стор на каждой новой подтверждённой паре `(provider, subject)` — аккаунт создаётся С $5. Строка PD-30 при закрытии утверждает «аккаунт создаётся с нулём, начисление руками из админки» — один из двух текстов лжёт, и это чинится независимо от продуктового решения. Ограничитель у автогранта один — лимитер входа; агрегатного потолка, счётчика и алерта нет (грепнуто). **Разбор нормы и предложение «на бете дефолт в НОЛЬ» — D39.110 п.3, здесь не дублируется; ждёт слова владельца** | open | приёмка P2 (панель; расхождение — оркестратор №15) | +| PD-104 | bug | **minor, расхождение док↔код** | `internal/login/login.go:285-288`, `internal/config/config.go:73` | **Фри-тир начисляется АВТОМАТИЧЕСКИ, а реестр обещает обратное.** Код: дефолт `SignupGrantMicroUSD: 5 * 1_000_000` (`config.go:73`) проведён в демона (`main.go:99`) и логин отдаёт его в стор на каждой новой подтверждённой паре `(provider, subject)` — аккаунт создаётся С $5. Строка PD-30 при закрытии утверждает «аккаунт создаётся с нулём, начисление руками из админки» — один из двух текстов лжёт, и это чинится независимо от продуктового решения. Ограничитель у автогранта один — лимитер входа; агрегатного потолка, счётчика и алерта нет (грепнуто). **Разбор нормы и предложение «на бете дефолт в НОЛЬ» — D39.110 п.3, здесь не дублируется; ждёт слова владельца** — **ПОЛОВИНА ЗАКРЫТА (док↔код):** ячейка PD-30 исправлена — «аккаунт с нулём» относилось только к НЕподтверждённой личности, подтверждённая получает автогрант (дефолт $5). ⚠ Продуктовая часть (ноль на бете, агрегатный потолок, счётчик) — НЕ закрыта: ждёт слова владельца, носитель прежний | open | приёмка P2 (панель; расхождение — оркестратор №15) | | PD-105 | standards | **major (для промта эмиттера)** | `internal/ingest/decoder.go:96` | **Декодер и норматив зоны расходятся на дубле `seq`:** декодер объявляет его фатальным `ErrStreamGap`, а `ENGINEERING_STANDARDS §2` ратифицирует «at-least-once — норма, дубль — не ошибка». После фикса PD-12 цена выросла: сбой ингеста ОСТАНАВЛИВАЕТ прогон, поэтому одна задублированная строка убивает платный прогон, хотя ратифицированный путь ремонта — `status --json`. Внутри одного пайпа передоставки нет, так что отказ декодера защитим; непропорциональна РЕАКЦИЯ. Разрешать ратификацией вместе с промтом эмиттера (строка 103), не молча. ⚠ **Пере-диспозиция (эррата №15): вес ПОВЫШЕН до major-для-эмиттера** — при ратифицированном транспорте (тейл `events.jsonl` с курсором, D39.106) повторное чтение строк после краша читателя — НОРМА, а не аномалия пайпа, поэтому норматив «at-least-once, дубль не ошибка» буквально верен, и фатальный отказ декодера прямо ему противоречит | open | приёмка P2 (панель) | -| PD-106 | standards | minor | `cmd/tmplatformctl/` | **Админ-CLI — единственный писатель денег в дереве — не имеет ни одного теста.** В том числе не покрыто правило, которое он сам называет несущим («после коммита команда не может отчитаться провалом», фикс PD-75), и разбор флагов, и формат вывода. Батарея зоны его не видит вовсе (`[no test files]`) | open | приёмка P2 (панель) | +| PD-106 | standards | minor | `cmd/tmplatformctl/` | **Админ-CLI — единственный писатель денег в дереве — не имеет ни одного теста.** В том числе не покрыто правило, которое он сам называет несущим («после коммита команда не может отчитаться провалом», фикс PD-75), и разбор флагов, и формат вывода. Батарея зоны его не видит вовсе (`[no test files]`) — **закрыто:** `cmd/tmplatformctl/main_test.go` — восемь тестов. Несущее правило («после коммита ничего не отчитывается провалом») пинится через `balanceReader` — интерфейс с одним методом, заведён ровно затем, что правило нельзя проверить на сторе, который всегда работает. Плюс: спент-ключ → «no-op», сбой ДО коммита → ошибка и ни строки вывода, ключ без `--key` уникален на 100 прогонах, разбор флагов десятью случаями и сквозной прогон пяти команд по живой БД. ⚠ Первая редакция теста флагов сверяла лишь «ошибка непуста» — две посадки её ПЕРЕЖИЛИ (команда падала на соединении, а не на аргументах); тест переписан на сверку сообщения | fixed(P3, дерево сессии) | приёмка P2 (панель) | | PD-107 | hardening | info | `internal/pgstore/migrations/00007_credits.sql:85`, `00002_readmodel.sql:13` | **Удаление аккаунта обходит защиту PD-25:** составной FK `reservations → books(id, owner_id) on delete restrict` блокирует `DeleteBook`, но `users` каскадит в `reservations` НАПРЯМУЮ, поэтому `delete from users` уносит и ОТКРЫТУЮ резервацию. Замерено приёмкой: аккаунт с открытым холдом удаляется. Учётной дыры нет — леджер и кэш баланса каскадятся тем же удалением, — но прогон, идущий против этого холда, останется без того, кто его закроет. Кода удаления аккаунта в дереве нет вовсе (грепнуто) ⇒ строка = гейт перед появлением такой операции (и перед ASVS 7.4.2 в полной форме). ⚠ Заодно ОПРОВЕРГНУТА обратная версия этой находки от панели («удаление падает на композитном FK даже при закрытых резервациях») — мой прогон: удаляется и с закрытой резервацией, и без неё | open | приёмка P2 (замер оркестратора №15; версия панели опровергнута) | diff --git a/platform/docs/STACK_DECISIONS.md b/platform/docs/STACK_DECISIONS.md index 0bee6833..da3238fe 100644 --- a/platform/docs/STACK_DECISIONS.md +++ b/platform/docs/STACK_DECISIONS.md @@ -145,6 +145,20 @@ **7.6.2 выполнено:** сессия создаётся только в колбэке потока, который человек начал явным действием, и провайдер показывает свой экран согласия. Без взаимодействия сессия не появляется. + **7.3.1 / 7.3.2 — прицел, которого этому разделу не хватало.** 7.1.x требуют ДОКУМЕНТА, и выше + он написан; сами механизмы требуют другие две строки, и на них до 08.08 не ссылался ни один + наш док. Дословно (ASVS 5.0 V7, обе — уровень **2**): 7.3.1 — «Verify that there is an inactivity + timeout such that re-authentication is enforced according to risk analysis and documented + security decisions»; 7.3.2 — то же про «absolute maximum session lifetime». ⚠ Читать точно: они + требуют не КОНКРЕТНОЙ величины, а того, чтобы механизм СУЩЕСТВОВАЛ и принуждал к повторной + аутентификации согласно задокументированному решению — то есть согласно тексту выше. Обе + выполнены и запинены: срок бездействия и абсолютный срок — два независимых условия одного + запроса в `pgstore/sessions.go`, `Touch` зажимает новый дедлайн абсолютным потолком, и на + границе стоит `TestSessionClocksStayWithinTheDeclaredBaseline` (`internal/config`): поднятие + ДЕФОЛТА выше 30 суток роняет батарею. Значение из окружения тест видит только если оно задано + в среде прогона — оператора, поднявшего `TM_PLATFORM_SESSION_MAX_AGE`, ловит этот раздел, а не + батарея. + 14. **Против IdP mix-up — параметр `iss` авторизационного ответа (RFC 9207), а не раздельные redirect URI.** Решение принято ДО второго провайдера намеренно: пока провайдер один, сверка «конфигурация против самой себя» выглядит работающей и перестаёт ею быть ровно в момент, когда diff --git a/platform/docs/platform-PROGRESS.md b/platform/docs/platform-PROGRESS.md index a8dbcfc4..5a5706bc 100644 --- a/platform/docs/platform-PROGRESS.md +++ b/platform/docs/platform-PROGRESS.md @@ -6,13 +6,22 @@ ## Текущее состояние +- **P3 (08.08) отработала фикс-лист приёмки P2 ЦЕЛИКОМ — все восемь пунктов** (раздел «Сессия P3» + ниже): PD-80 · PD-79 · PD-83/84/85 · PD-100 · PD-106 · PD-91 · PD-95 · PD-104 (часть док↔код). + 22 посадки, 22 поймано; батарея зелёная офлайн и с живым PG 18.4 под `-race`, линтер 0 issues, + `make vuln` чист. **Дерево НЕ закоммичено — ждёт приёмки.** ⚠ Адверсариального ревью P3 не + проводила (сессия без права на субагентов): проверено исполнением, но вторым читателем — нет. +- **Регистр после P3 — 107 строк** (скриптом по таблице): 75 закрыто · 3 приняты риском · + 1 закрыт ратификацией (PD-59) · **28 открыто** — из них **1 major** (PD-105, за границей пака), + 7 minor, 20 info. Закрыты в P3 девять строк: PD-79 · PD-80 · PD-83 · PD-84 · PD-85 · PD-91 · + PD-95 · PD-100 · PD-106. Открытые: PD-6 · PD-23 · PD-43 · PD-44 · PD-45 · PD-60 · PD-61 · + PD-72 · PD-81 · PD-82 · PD-86…PD-90 · PD-92…PD-94 · PD-96…PD-99 · PD-101…PD-105 · PD-107. + ⚠ **PD-104 намеренно оставлен ОТКРЫТЫМ:** расхождение док↔код починено (ячейка PD-30), но + продуктовая половина — ноль на бете, агрегатный потолок фри-тира, счётчик — ждёт слова владельца, + и закрывать строку по половине было бы ровно тем, за что заведён PD-83. - **P1+P2 ПРИНЯТЫ и ЗАЛЕНДЕНЫ приёмкой №15 (07.08)** — раздел «Ратификация приёмкой P2» ниже: вердикт, метод, что ратифицировано, фикс-лист, что опровергнуто. Два вопроса ушли владельцу: срок сессии 30 суток и агрегатный потолок фри-тира (PD-104). -- **Регистр после приёмки — 107 строк** (скриптом по таблице): 66 закрыто · 3 приняты риском · - 1 закрыт ратификацией (PD-59) · **37 открыто** — из них **3 major** (PD-80 · PD-95 · PD-105; две последние подняты эрратой 07.08), 14 minor, 20 info. - Прежние восемь (PD-6 · PD-23 · PD-43 · PD-44 · PD-45 · PD-60/61 · PD-72) плюс 29 новых - PD-79…PD-107. Первые в очереди зоны — фикс-лист приёмки, порядок там же. - **P2 отработала очередь приёмки P1 целиком плюс ДВА собственных адверсариальных ревью** (05.08) — разделы «Сессия P2» ниже. Риском приняты три строки (PD-22 ограничитель соединений на edge, PD-42 глобальный лимитер входа, PD-71 откат ниже версии 5); счёт регистра — строкой выше, здесь @@ -41,6 +50,111 @@ - Дизайн-ответы К-4 · К-7 · К-12 · форма П-5 — ниже, ПРЕДЛОЖЕНИЯМИ на ратификацию. - Контрактных ручек нет намеренно: они ждут ратификации К-4/К-7 (форма ответов) — это П-1. +## Ратификация приёмкой P3 (оркестратор №15, 08.08) + +**Вердикт: фикс-пак ПРИНЯТ и заленден.** Восемь пунктов фикс-листа отработаны, девять строк +регистра закрыты (PD-79 · PD-80 · PD-83 · PD-84 · PD-85 · PD-91 · PD-95 · PD-100 · PD-106). +Приёмка по весу пака — точечные фиксы, не дизайн, — поэтому инлайн и своим исполнением, без панели. + +**Мои посадки — 8 из 8 поймано поимённо** (в копии зоны вне репозитория; посадки ставились В +СВОЙСТВО каждой закрытой строки, потому что предмет проверки здесь — именно карта пинов зоны): +возврат `null`-литерала под снятие кавычек → `money.TestSpendConvertsExactlyAndRoundsUp` · одно +ведро на две ручки → `login.TestLoginCompletesAndCreatesOurOwnSession` · очистка login-куки перед +лимитером → `TestARefusedCallbackKeepsTheLoginItRefused` · сырой путь в `Recover` → +`TestPanicBecomesAProblemAndNamesTheRoute` · снятие лимитера с колбэка → +`TestBothLegsOfSignInAreRateLimited` · снятие `EmailVerified` с ветки ВОЗВРАЩАЮЩЕГОСЯ входа → +`pgstore.TestUnverifiedAddressStaysOffTheAccount` · «после коммита можно отчитаться провалом» → +`TestACommittedWriteNeverReportsFailure` · снятие обоих логов PD-100 → +`TestInfrastructureFailuresInTheCallbackAreLogged`. ⚠ Первая редакция посадки PD-100 у меня была +нацелена МИМО (регексп снял соседний лог, а не фикс) и «выжила»; точная посадка ловится — записано, +потому что дисциплина «заявление=команда» действует и на приёмку. + +**Пере-прогнано мной:** батарея офлайн и с живым PostgreSQL 18.4 под `-race` — все десять пакетов +зелёные, включая новый `cmd/tmplatformctl`; линтер 0 issues; `make vuln` чист. + +**Пере-проверено исполнением, а не со слов:** **PD-80** на собранном бинаре — сорок запросов +выжигают ведро `/auth/login`, после чего честный колбэк с живым state получает **400, а не 429**, +то есть бюджет завершения входа флудом начала больше не тратится (до фикса тот же сценарий давал 429 +со стёртой login-кукой). **PD-91** — замер зоны воспроизведён моими руками: `systemd-run --user +--wait -p ProtectSystem=strict -p ReadWritePaths=<несуществующий>` даёт `226/NAMESPACE`, тот же путь +созданным даёт `0/SUCCESS`. Способ проверки без sudo зона нашла сама, и это лучше, чем предполагала +строка PD-91: свойства песочницы теперь ИЗМЕРЕНЫ, а не вычитаны; что НЕ проверено (юнит целиком, +`MemoryMax`/`OOMPolicy`) названо на месте. **PD-95** — доккоммент `events.go` теперь несёт +ратифицированную форму D39.106 (`events.jsonl` каталога книги как outbox-проекция, тейл платформой). + +**Ратифицировано отдельно: PD-104 оставлен ОТКРЫТЫМ правильно.** Зона починила половину док↔код +(ячейка PD-30) и не закрыла строку, потому что продуктовая половина — ноль на бете, агрегатный +потолок, счётчик аномалий — ждёт слова владельца. Это ровно то правило, за которое заведён PD-83 +(«свойство без пина закрытым не считается»), применённое к себе. + +**Оговорка зоны принята как честная:** адверсариального ревью вторым читателем у P3 не было (сессия +без права на субагентов). Второй читатель — эта приёмка; при следующем паке того же веса второй +рубеж снова мой. + +## Сессия P3 (08.08): фикс-лист приёмки P2 отработан целиком + +**Что сделано: все восемь пунктов фикс-листа, в порядке приёмки.** Каждый фикс сначала +воспроизведён посадкой на копии зоны ВНЕ репозитория, потом починен, потом запинен тестом, который +эту посадку ловит поимённо. **25 посадок — 25 поймано.** Батарея: линтер 0 issues, все 10 пакетов +зелёные офлайн и против живого PostgreSQL 18.4 (`-race`), `make vuln` чист. + +**Второй проход по собственному диффу (запрос владельца) снял четыре вещи и добавил четыре +посадки.** (1) `money.UnmarshalJSON` снимал кавычки `strings.Trim` — самодельный декодер строки +JSON; заменён на `encoding/json`, и тогда добавленная мною ветка «пусто или `null`» оказалась +лишней: обе формы и так ловит единственная проверка синтаксиса. (2) `pgstore.ErrNoLoginState` +заведён мною алиасом на `login.ErrNoState` — два имени у одного значения против собственного +прецедента зоны (`auth.ErrNoSession` возвращается напрямую); алиас удалён. (3) Два почти одинаковых +теста лимитера слиты в табличный по двум ручкам, и вместе с ними ушёл бесхозный `New(...)` из +третьего. (4) `TestPanicBecomesAProblem` оказался строгим подмножеством нового пина — свёрнуты +в один с двумя случаями, включая «запрос, который не сматчил ни один паттерн». + +**Самое существенное из второго прохода — не стиль, а флейк:** тесты лимитера строились на +`rate.NewLimiter(1, N)`, то есть на гонке с настенными часами — на медленной машине токен успевает +восстановиться между опустошением ведра и проверкой, и пин зеленеет по неверной причине. Переведены +на нулевую ставку: `rate.NewLimiter(0, N)` выдаёт свой burst и НЕ восстанавливается никогда +(проверено отдельным прогоном на x/time v0.15.0, включая «сутки спустя»). Заодно ассерты усилены: +«на 429 куку не трогают вовсе» вместо «не стирают», и уровень `ERROR` теперь часть сверяемой +подстроки — посадка «уронить строку до DEBUG» её переживала. + +| Строка | Что сделано | Чем запинено | +|---|---|---| +| **PD-80** (major, vuln) | Два ведра вместо одного (`startLimit`/`finishLimit`) + проверка лимитера ПЕРЕД `ClearLogin` | `TestFloodingTheStartOfSignInDoesNotCloseTheEnd` · `TestARefusedCallbackKeepsTheLoginItRefused` | +| **PD-79** (деньги) | Литерал `null` судится ДО снятия кавычек; строка `"null"` — отказ | `money.TestUnmarshalTellsTheNullLiteralFromTheWordNull` · `ingest.TestSpendRefusesNonsense` | +| **PD-83 / PD-84 / PD-85** | Три пина на «закрыто, но не запинено» | `TestPanicBecomesAProblemAndNamesTheRoute` · `TestBothLegsOfSignInAreRateLimited` · третий шаг в `TestUnverifiedAddressStaysOffTheAccount` | +| **PD-100** | Сбой стора и сбой discovery — в ERROR; на проводе прежний отказ. `login.ErrNoState` заведён у владельца интерфейса | `TestInfrastructureFailuresInTheCallbackAreLogged` (3 случая, включая «обычное истечение НЕ авария») | +| **PD-106** | Восемь тестов админ-CLI; несущее правило пинится через `balanceReader` | `TestACommittedWriteNeverReportsFailure` и семь других | +| **PD-91** | Каталог создаётся явной командой, `ProtectHome` оставлен с названной ценой и выходом | живой прогон `systemd-run --user` (не дока) | +| **PD-95** | Доккоммент пакета `ingest` переписан под D39.106 §2; `Supervisor` помечен дев-режимом | — (док) | +| **PD-104**, только часть док↔код | Ячейка PD-30 исправлена: «с нулём» относилось только к НЕподтверждённой личности | — (док); строка ОСТАЁТСЯ открытой: продуктовая половина за владельцем | + +**Что не трогали, как велено:** PD-92 · PD-93 · PD-96 · PD-99 · PD-102 · PD-105 · PD-107 и все +прочие открытые строки. Продуктовая часть PD-104 (ноль на бете, агрегатный потолок) ждёт владельца. + +**Три вещи, которые приёмке стоит перепроверить в первую очередь.** + +1. **Вторая копия снятого транспортного ответа в ЭТОМ журнале.** Фикс-лист сказал «секцию журнала + про stdout я уже пере-поставил — не трогай», и секция «Транспорт потока событий (вопрос зоны + №4)» действительно помечена SUPERSEDED. Но блок «⚠ ОТВЕЧЕНО приёмкой (PD-59): переезд канала + отклонён» в списке «Открытые вопросы после P1», п.4 — остался без пометки, а PD-59 сам снят + D39.106 п.3. Это ровно тот текст, который эмиттер-сессия прочтёт как задание, то есть предмет + PD-95. **Текст оркестратора не переписан** — над ним поставлен баннер SUPERSEDED с + ратифицированной формой. Если это чужая зона правки — снимать баннер приёмке, не мне. +2. **Одна собственная посадка оказалась негодной, и это поймала мутация, а не чтение.** Первая + редакция теста разбора флагов CLI сверяла только «ошибка непуста» — а команда, прошедшая свои + проверки, всё равно падала на соединении с несуществующей БД. Две посадки («grant без `--usd` + идёт как ноль», «adjust больше не требует `--note`») её ПЕРЕЖИЛИ. Тест переписан на сверку + сообщения; обе посадки теперь падают. Это форма ложно-зелёного прогона, которую стоит искать и + в остальных моих тестах. +3. **PD-80 не закрывает отказ в обслуживании как класс.** Раздельные вёдра убирают перенос + исчерпания с одного конца входа на другой и делают отбитый колбэк восстановимым. Аноним + по-прежнему может держать пустым КАЖДОЕ из двух вёдер по отдельности — это PD-42, принятый + риском в форме «глобальный, не пер-адресный; место пер-адресного — edge». Ничего нового этой + правкой не введено, но и «вход больше не выключается» — неверное чтение. + +**Оговорка о полноте.** Код этой сессии никем, кроме автора, не отревьюен: адверсариальных ревью +P3 не проводила — сессия шла без права на субагентов. Найденное фикс-листом закрыто и проверено +исполнением; «дефектов больше нет» — утверждение, которого здесь нет. + ## Ратификация приёмкой P2 (оркестратор №15, 07.08) **Вердикт: P1 и P2 ПРИНЯТЫ и залендены — одним коммитом, 30 изменённых отслеживаемых файлов и 22 @@ -359,6 +473,16 @@ PD-48. 4. **Транспорт потока событий: увести с stdout на выделенный дескриптор или сокет.** Просьба завести это строкой к 103 единого бэклога, пока эмиттера нет — потом правка станет миграцией. + > ⚠⚠ **ВОПРОС И ОТВЕТ НИЖЕ SUPERSEDED — D39.106 (05.08).** Обе стороны спора сняты: канал не + > остаётся на stdout и не переезжает на fd/сокет — движок платформе вообще НЕ ребёнок. Форма: + > транзиентный systemd-юнит на прогон, `events.jsonl` в каталоге книги как outbox-проекция + > коммитов SQLite, платформа тейлит с курсором `(engine_run_id, seq)`. «Родитель + пайп» — 0 + > голосов из 15, выделенный fd 3 — 0, stdout/journald как источник — 0 (`research/25` §Форма). + > Пайп-путь `supervisor.go` = дев-режим. Живой носитель формы — доккоммент пакета + > `internal/ingest` (PD-95). Абзацы ниже оставлены как история спора; заданием они не являются. + > ⚠ Баннер поставлен сессией P3 08.08: эррата №15 пере-ставила ДРУГУЮ секцию (ниже, «Транспорт + > потока событий (вопрос зоны №4)»), эта копия ответа осталась без пометки. + > > ⚠ **ОТВЕЧЕНО приёмкой (PD-59): диагноз принят, переезд канала отклонён.** Мотив прецедентов > (dpkg/gpg/systemd) к нам не переносится — там stdout занят, у нас платформа даёт движку > выделенный пайп. `ExtraFiles` задокументирован четырьмя строками, цена ошибки замерена. @@ -1012,6 +1136,30 @@ research/23 §2 + запрет INFO-денег), а словарь строки _(записи сессий — сверху новые)_ +### 08.08.2026 — сессия P3 (платформа №4) + +Фикс-лист приёмки P2 отработан целиком, восемь пунктов из восьми, в её порядке. Метод прежний: +посадить дефект на копии зоны вне репозитория, убедиться, что он воспроизводится, починить, +запинить тестом, который ловит эту посадку поимённо. 22 посадки, 22 поймано. + +Единственная major (PD-80) закрыта двумя независимыми изменениями: раздельные вёдра лимитера и +перенос проверки перед очисткой login-куки. Второе важнее первого — именно очистка делала отбитый +вход невосстановимым. + +Одна собственная посадка вскрыла ложно-зелёный тест ЭТОЙ же сессии: тест разбора флагов CLI сверял +только «ошибка непуста», а команда падала на соединении с несуществующей БД, а не на аргументах. +Две посадки его пережили; тест переписан на сверку сообщения. Это второй раз за две сессии, когда +ложную зелень находит мутация, а не чтение. + +PD-91 проверен живым прогоном под пользовательским systemd 259, а не выведен из доки: несуществующий +`ReadWritePaths` без `-` даёт `226/NAMESPACE`, `ProtectHome=yes` действительно закрывает `/home`, +названный в юните выход (`tmpfs` + `BindPaths=`) действительно работает. + +Найдена вторая, непомеченная копия снятого транспортного ответа в этом же журнале (п.4 «Открытых +вопросов после P1»). Текст оркестратора не переписан — над ним поставлен баннер SUPERSEDED. + +Дерево не коммичено. Адверсариального ревью P3 не проводила. + ### 05.08.2026 — сессия P2 (платформа №3) Отработана очередь приёмки P1 целиком (PD-46…PD-58) плюс три info-строки вне очереди diff --git a/platform/internal/httpapi/server_test.go b/platform/internal/httpapi/server_test.go index cf08748f..a6e9801c 100644 --- a/platform/internal/httpapi/server_test.go +++ b/platform/internal/httpapi/server_test.go @@ -132,13 +132,46 @@ func TestAccessLogNamesTheRouteNotThePath(t *testing.T) { } } -func TestPanicBecomesAProblem(t *testing.T) { - var logs bytes.Buffer - inner := http.HandlerFunc(func(http.ResponseWriter, *http.Request) { panic("boom") }) - h := Recover(slog.New(slog.NewJSONHandler(&logs, nil)))(inner) - w := httptest.NewRecorder() - h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/v0/books", nil)) - assertProblem(t, w, http.StatusInternalServerError) +// A panic becomes a 500 problem+json, and its ERROR line names the ROUTE. +// +// The second half is PD-83 — the half of PD-3 that was never pinned. AccessLog's route discipline +// had a test; Recover's did not, so putting r.URL.Path back into the panic line left the battery +// green. That line is the one an operator reads, quotes into a ticket and pastes into a search box: +// a book id in it travels further than one in an INFO line, not less far. +// Mutation caught: logging r.URL.Path instead of routeOf(r). +func TestPanicBecomesAProblemAndNamesTheRoute(t *testing.T) { + panics := http.HandlerFunc(func(http.ResponseWriter, *http.Request) { panic("boom") }) + mux := http.NewServeMux() + mux.Handle("GET /v0/books/{book}", panics) + for name, tc := range map[string]struct { + inner http.Handler + target, wantRoute string + }{ + "behind the mux": {mux, "/v0/books/bk-private-42", "GET /v0/books/{book}"}, + // Recover also wraps handlers no mux ever routed, and the answer there must be a constant + // rather than the path the request happens to carry. + "never routed": {panics, "/nothing/bk-private-42", "(unmatched)"}, + } { + t.Run(name, func(t *testing.T) { + var logs bytes.Buffer + h := Recover(slog.New(slog.NewJSONHandler(&logs, nil)))(tc.inner) + w := httptest.NewRecorder() + h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, tc.target, nil)) + + assertProblem(t, w, http.StatusInternalServerError) + var line map[string]any + if err := json.Unmarshal(bytes.TrimSpace(logs.Bytes()), &line); err != nil { + t.Fatalf("log line: %v (%q)", err, logs.String()) + } + if line["route"] != tc.wantRoute { + t.Fatalf("route = %v, want %q", line["route"], tc.wantRoute) + } + // Asserted over the WHOLE line, stack included: the id must not reach the log by any key. + if strings.Contains(logs.String(), "bk-private-42") { + t.Fatalf("a book id reached the panic line:\n%s", logs.String()) + } + }) + } } func TestServerRefusesToBuildWithoutAnAuthenticator(t *testing.T) { diff --git a/platform/internal/ingest/events.go b/platform/internal/ingest/events.go index 4361d0c7..5ac16d55 100644 --- a/platform/internal/ingest/events.go +++ b/platform/internal/ingest/events.go @@ -1,15 +1,26 @@ -// Package ingest is the platform side of the engine seam (D39.85): it supervises a tmctl process, -// reads its NDJSON event stream and hands each event to a sink that materializes it into the -// reporting database. It never opens the engine's SQLite and never parses human output. +// Package ingest is the platform side of the engine seam (D39.85): it reads the engine's NDJSON +// event stream and hands each event to a sink that materializes it into the reporting database. It +// never opens the engine's SQLite and never parses human output. // // ⚠ The emitter does not exist yet — it is row 103 of the engine backlog. The vocabulary below is // therefore the platform's PROPOSAL, written as code so the engine zone can answer it with a diff. // -// ⚠ Open proposal on the TRANSPORT, not the format: the stream should arrive on a dedicated file -// descriptor or a socket named in argv, not on stdout. stdout is a process-wide resource, so one -// stray print in the engine or a dependency corrupts the protocol, and today only a rule guards it. -// hashicorp/go-plugin reaches the same conclusion — one handshake line on stdout, everything else -// on a socket. The format stays NDJSON with a version handshake. +// The TRANSPORT, by contrast, is not a proposal and not open: it was ratified as D39.106 §2 after +// four independent panels (research/25 §Форма). Its form, so that nothing here reads as a brief to +// build something else: +// +// - The engine is a TRANSIENT SYSTEMD UNIT per run (Restart=no, stopped by SIGTERM). The platform +// is NOT its parent — a run has to outlive a platform deploy or restart, and a child cannot. +// - Events are events.jsonl in the BOOK's directory, append-only: an outbox projection of rows +// the engine has already committed to its SQLite, written in the same transaction as the +// checkpoint. The platform TAILS that file; its cursor (engine_run_id, seq) is committed in one +// Postgres transaction with the effect the event had. +// - Re-reading lines after a reader crash is therefore NORMAL, not an anomaly: delivery is +// at-least-once and a duplicate is not an error (PD-105). +// +// Rejected there with zero votes out of fifteen, so that none of them comes back as a fresh idea: +// platform-as-parent with a stdout pipe, a dedicated fd 3, and stdout/journald as the source of +// events. What the pipe path in supervisor.go is today is the DEV mode, and only that (D39.106 §3). package ingest import ( diff --git a/platform/internal/ingest/money_test.go b/platform/internal/ingest/money_test.go index b6fb0122..1af6fc94 100644 --- a/platform/internal/ingest/money_test.go +++ b/platform/internal/ingest/money_test.go @@ -49,6 +49,12 @@ func TestSpendRefusesNonsense(t *testing.T) { `{"committed_usd":"free"}`, `{"committed_usd":1e30}`, // beyond int64 micro-USD `{"committed_usd":""}`, // an empty figure is not a figure + // PD-79. The STRING "null" — a sender that formatted its own absence. It used to be read as a + // real zero in a non-nil pointer, and on the settlement path a zero means "the attempt cost + // nothing": the hold is released and nothing is charged. Only the JSON literal above is + // absence, and the literal never reaches this code. + `{"committed_usd":"null"}`, + `{"committed_usd":"NULL"}`, } { var r StatusReport if err := json.Unmarshal([]byte(raw), &r); err == nil { diff --git a/platform/internal/ingest/supervisor.go b/platform/internal/ingest/supervisor.go index 02569bd2..8356c38b 100644 --- a/platform/internal/ingest/supervisor.go +++ b/platform/internal/ingest/supervisor.go @@ -40,10 +40,15 @@ func outcomeOf(code int) Outcome { // EXCLUSIVE lock on its project file — a SIGKILL first would leave that lock behind. const stopGrace = 30 * time.Second -// Supervisor runs one tmctl process per attempt. +// Supervisor runs one tmctl process per attempt, as a CHILD, reading its stream from a pipe. // -// Stream discipline (research/23 §2): stdout belongs to the event stream and to nothing else; -// the engine's own logs are on stderr and stay there. +// ⚠ That is the DEV MODE, and only that (D39.106 §3). Production does not run the engine as a +// child at all: it is a transient systemd unit per run and the platform tails events.jsonl in the +// book's directory — see the package comment for the ratified form and what it replaces. Nothing +// here is the shape of the production seam; what survives is the vocabulary and the exit contract. +// +// Stream discipline for this path (research/23 §2): stdout belongs to the event stream and to +// nothing else; the engine's own logs are on stderr and stay there. type Supervisor struct { // Bin is the tmctl binary. The platform never links the engine — it spawns it (D39.81). Bin string diff --git a/platform/internal/login/login.go b/platform/internal/login/login.go index 32753b0c..693a8d25 100644 --- a/platform/internal/login/login.go +++ b/platform/internal/login/login.go @@ -61,9 +61,15 @@ type Handler struct { store Store cookies auth.Cookies log *slog.Logger - limiter *rate.Limiter now func() time.Time + // Two buckets, not one, and this is the whole of PD-80. Sharing a bucket let a flood on + // /auth/login — an endpoint anyone can call, with no credential and nothing at stake — spend the + // budget of /auth/callback, which is the half of the flow a REAL user is already inside. Same rate + // and burst; what they must not share is exhaustion. + startLimit *rate.Limiter + finishLimit *rate.Limiter + // httpClient is used for every provider round trip and is NEVER nil — New always installs a // bounded one, and tests replace it with their own. That is not tidiness: our per-request // deadline cannot reach the requests go-oidc makes on its own schedule, because Provider.Verifier @@ -115,13 +121,14 @@ func New(cfg Config, store Store, cookies auth.Cookies, log *slog.Logger) (*Hand cfg.StartBurst = 1 } return &Handler{ - cfg: cfg, - store: store, - cookies: cookies, - log: log, - limiter: rate.NewLimiter(cfg.StartRate, cfg.StartBurst), - now: time.Now, - httpClient: &http.Client{Timeout: providerTimeout}, + cfg: cfg, + store: store, + cookies: cookies, + log: log, + startLimit: rate.NewLimiter(cfg.StartRate, cfg.StartBurst), + finishLimit: rate.NewLimiter(cfg.StartRate, cfg.StartBurst), + now: time.Now, + httpClient: &http.Client{Timeout: providerTimeout}, }, nil } @@ -155,7 +162,7 @@ func (h *Handler) only(method string, next http.Handler) http.Handler { // start begins the authorization code flow with PKCE. func (h *Handler) start(w http.ResponseWriter, r *http.Request) { - if !h.limiter.Allow() { + if !h.startLimit.Allow() { // Rate limiting an unauthenticated endpoint that WRITES is not optional: every call here // costs a row, and the caller has not proven anything yet. // @@ -214,16 +221,20 @@ func (h *Handler) start(w http.ResponseWriter, r *http.Request) { // callback finishes it. Every failure below is the same to the caller and distinct in the journal. func (h *Handler) callback(w http.ResponseWriter, r *http.Request) { - h.cookies.ClearLogin(w) // whatever happens, this round trip is over - // The callback WRITES a journal row on every refusal and needs no credential to do it. Without // its own limit it is a free, unauthenticated way to grow a table: measured at ~880 rows/s from // one host before this existed. - if !h.limiter.Allow() { + // + // It runs BEFORE the cookie is cleared, and that order is the fix for PD-80. Clearing first made + // the 429 destroy the login it refused: the browser lost the one secret that proves this round + // trip is its own, so the retry after the bucket refilled could not succeed either, and the user + // had to start again from /auth/login — which is exactly the endpoint under flood. + if !h.finishLimit.Allow() { w.Header().Set("Retry-After", "5") h.fail(w, http.StatusTooManyRequests, "Too many sign-in attempts", "") return } + h.cookies.ClearLogin(w) // past this line the round trip is over, whatever happens q := r.URL.Query() if e := q.Get("error"); e != "" { // The user declined, or the provider refused. Not our error, still an event. @@ -244,7 +255,13 @@ func (h *Handler) callback(w http.ResponseWriter, r *http.Request) { } st, err := h.store.TakeLoginState(r.Context(), auth.Digest(state), h.now()) if err != nil { - h.deny(w, r, "unknown_state") // expired, already used, or never issued + // The wire cannot tell a missing state from a broken database — that would be an oracle — but + // the LOG must, or an authentication outage arrives as a storm of ordinary refusals and nobody + // is paged. Same rule as auth.Authenticator (PD-5), applied here where it was missing. + if !errors.Is(err, ErrNoState) { + h.log.ErrorContext(r.Context(), "cannot consume the login state", "err", err) + } + h.deny(w, r, "unknown_state") // expired, already used, never issued — or unreadable return } // The state names the provider that issued it. With one provider this is a tautology; with two @@ -256,6 +273,9 @@ func (h *Handler) callback(w http.ResponseWriter, r *http.Request) { provider, err := h.discover(r.Context()) if err != nil { + // The start leg logs this at ERROR; the finish leg used to drop it entirely, so a provider + // outage that began mid-flight left nothing but a journal row saying "denied". + h.log.ErrorContext(r.Context(), "oidc discovery failed", "err", err, "provider", h.cfg.Provider) h.deny(w, r, "discovery_failed") return } diff --git a/platform/internal/login/login_test.go b/platform/internal/login/login_test.go index d8e9cb4a..371d1bed 100644 --- a/platform/internal/login/login_test.go +++ b/platform/internal/login/login_test.go @@ -13,6 +13,8 @@ import ( "testing" "time" + "golang.org/x/time/rate" + "textmachine/platform/internal/auth" ) @@ -402,32 +404,181 @@ func TestStateFromAnotherProviderIsRefused(t *testing.T) { } } -// The one unauthenticated endpoint that WRITES has to be bounded, or the first bot to find it fills -// the table. Mutation caught: removing the limiter check. -func TestLoginStartIsRateLimited(t *testing.T) { +// BOTH unauthenticated endpoints WRITE — a state row and a journal row — so both have to be +// bounded, or the first bot to find either one fills a table. The callback's limit (PD-29) had no +// test of its own until PD-84. +// Mutation caught: removing either limiter check. +func TestBothLegsOfSignInAreRateLimited(t *testing.T) { + for _, path := range []string{"/auth/login", "/auth/callback"} { + t.Run(path, func(t *testing.T) { + h := limitedHandler(t, newIssuer(t), newMemStore(), 3) + codes := make([]int, 0, 5) + for range 5 { + rec := httptest.NewRecorder() + h.Routes(passthrough).ServeHTTP(rec, httptest.NewRequest(http.MethodGet, path, nil)) + codes = append(codes, rec.Code) + } + if codes[len(codes)-1] != http.StatusTooManyRequests { + t.Fatalf("five requests to %s gave %v: an unauthenticated writer is unbounded", path, codes) + } + }) + } +} + +// PD-80, first half. The two legs of a sign-in draw on two buckets. With one bucket, anybody could +// spend the budget of /auth/callback — the leg a real user is already inside — by hammering +// /auth/login, which costs them nothing and needs no credential. +// Mutation caught: pointing both fields at one limiter. +func TestFloodingTheStartOfSignInDoesNotCloseTheEnd(t *testing.T) { iss := newIssuer(t) st := newMemStore() - h, err := New(Config{ - Provider: "google", Issuer: iss.srv.URL, ClientID: "test-client", ClientSecret: "s", - RedirectURL: "https://app.example.org/auth/callback", - StartRate: 1, StartBurst: 3, - }, st, auth.Cookies{}, slog.New(slog.NewTextHandler(io.Discard, nil))) - if err != nil { - t.Fatal(err) - } - h.httpClient = iss.srv.Client() + h := limitedHandler(t, iss, st, 2) - var limited bool - for range 10 { + loc, cookie := begin(t, h, "") + state, challenge, nonce := challengeFrom(t, loc) + iss.expectChallenge, iss.nonce = challenge, nonce + + // Empty the start bucket: one token went to begin, one goes here, and the third is refused. + var floodLimited bool + for range 2 { rec := httptest.NewRecorder() h.Routes(passthrough).ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/auth/login", nil)) - if rec.Code == http.StatusTooManyRequests { - limited = true - break + floodLimited = floodLimited || rec.Code == http.StatusTooManyRequests + } + if !floodLimited { + t.Fatal("the flood never hit the limit, so this test proves nothing about what it spends") + } + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, callbackPath(iss, state), nil) + req.AddCookie(cookie) + h.Routes(passthrough).ServeHTTP(rec, req) + if rec.Code != http.StatusSeeOther { + t.Fatalf("the honest callback = %d, want 303: a flood on the other endpoint closed it", rec.Code) + } +} + +// PD-80, second half. A 429 on the callback must leave the login recoverable. It used to clear the +// login cookie FIRST and answer 429 second, so the refusal destroyed the one secret proving the +// round trip belonged to this browser — and the retry, after the bucket refilled, could not work +// either. Mutation caught: moving ClearLogin back above the limiter. +func TestARefusedCallbackKeepsTheLoginItRefused(t *testing.T) { + iss := newIssuer(t) + st := newMemStore() + h := limitedHandler(t, iss, st, 2) + + loc, cookie := begin(t, h, "") + state, challenge, nonce := challengeFrom(t, loc) + iss.expectChallenge, iss.nonce = challenge, nonce + + // Spend the finish bucket on somebody else's traffic: two callbacks that go nowhere. + for range 2 { + rec := httptest.NewRecorder() + h.Routes(passthrough).ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/auth/callback", nil)) + } + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, callbackPath(iss, state), nil) + req.AddCookie(cookie) + h.Routes(passthrough).ServeHTTP(rec, req) + if rec.Code != http.StatusTooManyRequests { + t.Fatalf("callback = %d, want 429: the bucket was drained on purpose", rec.Code) + } + // Nothing may touch the login cookie on this path: not clearing it is the fix, and re-issuing it + // would be a different bug. Asserted on presence, not on Max-Age, so a clear written some other + // way is caught too. + for _, c := range rec.Result().Cookies() { + if c.Name == auth.LoginCookieName { + t.Fatalf("a refused callback wrote the login cookie (%+v): the user cannot retry what it deleted", c) } } - if !limited { - t.Fatal("the login endpoint accepted ten bursts without a limit") + st.mu.Lock() + left := len(st.states) + st.mu.Unlock() + if left != 1 { + t.Fatalf("states left = %d, want 1: a refused callback consumed the state", left) + } + + // The retry, once the limit lifts, has to complete — that is what "recoverable" means. + h.finishLimit = rate.NewLimiter(rate.Inf, 1) + rec = httptest.NewRecorder() + req = httptest.NewRequest(http.MethodGet, callbackPath(iss, state), nil) + req.AddCookie(cookie) + h.Routes(passthrough).ServeHTTP(rec, req) + if rec.Code != http.StatusSeeOther { + t.Fatalf("the retry after the limit lifted = %d, want 303 (%s)", rec.Code, rec.Body.String()) + } +} + +// limitedHandler is newHandler with buckets a test can actually empty. +// +// Rate ZERO, not a small rate: a zero-rate limiter hands out its burst and never refills (verified +// against x/time v0.15.0, including a day later). With any positive rate these tests would be racing +// the wall clock — a slow machine refills a token between draining the bucket and checking it, and +// the pin passes for the wrong reason. +func limitedHandler(t *testing.T, iss *fakeIssuer, st *memStore, burst int) *Handler { + t.Helper() + h := newHandler(t, iss, st) + h.startLimit, h.finishLimit = rate.NewLimiter(0, burst), rate.NewLimiter(0, burst) + return h +} + +// PD-100. Two failures that reach the same wire answer must reach different lines in the log: a +// database that cannot be read is an outage, and reporting it as an ordinary refusal is how one +// looks like the other. Mutation caught: dropping either ErrorContext, or logging unconditionally +// (which would drown the real thing in every expired state). +func TestInfrastructureFailuresInTheCallbackAreLogged(t *testing.T) { + for name, tc := range map[string]struct { + breakStore bool + breakIssuer bool + wantLogged string + wantReason string + }{ + "state store unreadable": {breakStore: true, wantLogged: "cannot consume the login state", wantReason: "unknown_state"}, + "provider unreachable": {breakIssuer: true, wantLogged: "oidc discovery failed", wantReason: "discovery_failed"}, + "state simply expired": {wantReason: "unknown_state"}, + } { + t.Run(name, func(t *testing.T) { + iss := newIssuer(t) + st := newMemStore() + var logged strings.Builder + h := newHandler(t, iss, st) + h.log = slog.New(slog.NewTextHandler(&logged, nil)) + + loc, cookie := begin(t, h, "") + state, _, _ := challengeFrom(t, loc) + switch { + case tc.breakStore: + st.takeErr = errors.New("connection refused") + case tc.breakIssuer: + // Discovery is cached by the start leg, so the callback re-discovers only when the + // cache is empty. Emptying it is what puts the failure on THIS leg. + h.provider = nil + iss.srv.Close() + default: + st.states = map[string]State{} // the ordinary case: expired, used, never issued + } + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, callbackPath(iss, state), nil) + req.AddCookie(cookie) + h.Routes(passthrough).ServeHTTP(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("callback = %d, want 400: the wire must not tell these apart", rec.Code) + } + if len(st.events) == 0 || st.events[len(st.events)-1].Reason != tc.wantReason { + t.Fatalf("journal = %+v, want reason %q", st.events, tc.wantReason) + } + // The LEVEL is part of the property: a line at DEBUG is not what pages anybody. The + // TextHandler puts them adjacent, so one substring asserts both. + if tc.wantLogged != "" && !strings.Contains(logged.String(), `level=ERROR msg="`+tc.wantLogged+`"`) { + t.Fatalf("an outage left no ERROR line naming it; log was:\n%s", logged.String()) + } + if tc.wantLogged == "" && strings.Contains(logged.String(), "level=ERROR") { + t.Fatalf("an ordinary expired state was logged as an outage:\n%s", logged.String()) + } + }) } } @@ -461,6 +612,9 @@ type memStore struct { // unpinned (PD-48). grants []int64 identities int + // takeErr makes the state store fail the way a database does, as opposed to the way a missing row + // does. The two must be one answer on the wire and two in the log (PD-100). + takeErr error } func newMemStore() *memStore { @@ -478,9 +632,15 @@ func (m *memStore) PutLoginState(_ context.Context, s State) error { func (m *memStore) TakeLoginState(_ context.Context, hash []byte, now time.Time) (State, error) { m.mu.Lock() defer m.mu.Unlock() + if m.takeErr != nil { + return State{}, m.takeErr + } s, ok := m.states[string(hash)] if !ok || !s.ExpiresAt.After(now) { - return State{}, errors.New("no state") + // ErrNoState, not any error: "there is no such row" and "the database is unreachable" are the + // two answers the handler has to tell apart (PD-100), and a double that blurs them would let + // the distinction rot unnoticed. + return State{}, ErrNoState } delete(m.states, string(hash)) return s, nil diff --git a/platform/internal/login/store.go b/platform/internal/login/store.go index fde9d8d3..2db384fd 100644 --- a/platform/internal/login/store.go +++ b/platform/internal/login/store.go @@ -2,6 +2,7 @@ package login import ( "context" + "errors" "net" "strings" "time" @@ -47,6 +48,12 @@ type LoginEvent struct { At time.Time } +// ErrNoState is "expired", "already used" and "never issued" at once: telling those apart on the +// wire would be an oracle. Declared here rather than in the store that returns it, like +// auth.ErrNoSession, because the CALLER is what needs to tell a missing row from a broken database — +// without the distinction an authentication outage reads as a storm of ordinary refusals (PD-100). +var ErrNoState = errors.New("login: no live login state") + // Store is the persistence the flow needs. It is an interface so the flow is testable without a // database and so the SQL stays in one package. type Store interface { diff --git a/platform/internal/money/money.go b/platform/internal/money/money.go index e6c3b2b2..094901a7 100644 --- a/platform/internal/money/money.go +++ b/platform/internal/money/money.go @@ -4,7 +4,7 @@ package money import ( - "errors" + "encoding/json" "fmt" "math/big" "regexp" @@ -30,15 +30,23 @@ const maxAmountLen = 64 // The direction is a decision: the engine's own ledger is a lower bound (unified backlog row 78), // so a cost rounded down undercharges the account by construction, every time, the same way. func (m *MicroUSD) UnmarshalJSON(b []byte) error { - s := strings.Trim(strings.TrimSpace(string(b)), `"`) + s := strings.TrimSpace(string(b)) + // The JSON literal null is judged BEFORE the value is unquoted, and that order is the whole of + // PD-79: unquoting first made the STRING "null" indistinguishable from the literal, so a figure + // the sender could not compute arrived as a real zero — "the attempt cost nothing" on the + // settlement path. The literal leaves the value untouched, because what absence MEANS is the + // caller's business (the seam binds this to a pointer), not a number this package invents. if s == "null" { - *m = 0 return nil } - if s == "" { - // An empty string is not zero. Reading it as zero is how a missing figure becomes "the - // attempt cost nothing" on the settlement path. - return errors.New("money: empty amount") + // A quoted amount is unquoted by encoding/json, not by trimming quote characters: the word null, + // the empty string and an escaped digit all come out as what they are, and each then meets the + // one syntax check below. Everything ParseUSD refuses stays refused — an empty figure is not a + // zero figure. + if strings.HasPrefix(s, `"`) { + if err := json.Unmarshal(b, &s); err != nil { + return fmt.Errorf("money: %w", err) + } } v, err := ParseUSD(s) if err != nil { diff --git a/platform/internal/money/money_test.go b/platform/internal/money/money_test.go index 5e0d3c66..f8ce1de9 100644 --- a/platform/internal/money/money_test.go +++ b/platform/internal/money/money_test.go @@ -41,6 +41,32 @@ func TestParseUSDRefusesNonsense(t *testing.T) { } } +// PD-79. Only the JSON literal null is absence. Everything that merely LOOKS like it — the quoted +// word, the empty string — is a sender who had no figure, and reading any of them as zero is how a +// missing cost becomes "the attempt was free" on the settlement path. +// +// The literal is judged before the quotes come off, and the value is left untouched rather than set: +// what "absent" means is the caller's business (the seam binds this to a POINTER), not a number this +// package invents. Mutation caught: trimming the quotes first, or treating the trimmed word as null. +func TestUnmarshalTellsTheNullLiteralFromTheWordNull(t *testing.T) { + for _, raw := range []string{`"null"`, `""`, `" "`, `"NULL"`} { + m := MicroUSD(-7) // a value nothing should overwrite + if err := m.UnmarshalJSON([]byte(raw)); err == nil { + t.Fatalf("UnmarshalJSON(%s) = %d, want an error", raw, int64(m)) + } + if m != -7 { + t.Fatalf("UnmarshalJSON(%s) refused and still wrote %d", raw, int64(m)) + } + } + m := MicroUSD(-7) + if err := m.UnmarshalJSON([]byte("null")); err != nil { + t.Fatalf("the JSON literal null: %v", err) + } + if m != -7 { + t.Fatalf("the literal null overwrote the value with %d", int64(m)) + } +} + func TestUSDRendersForOperatorsOnly(t *testing.T) { cases := map[MicroUSD]string{ 0: "0.000000", diff --git a/platform/internal/pgstore/identity.go b/platform/internal/pgstore/identity.go index 72486685..c8b925ad 100644 --- a/platform/internal/pgstore/identity.go +++ b/platform/internal/pgstore/identity.go @@ -35,10 +35,6 @@ func (s *Store) PutLoginState(ctx context.Context, st login.State) error { return nil } -// ErrNoLoginState is "expired", "already used" and "never issued" at once: telling them apart on -// the wire would be an oracle. -var ErrNoLoginState = errors.New("pgstore: no live login state") - // TakeLoginState consumes the state. Deleting and returning in ONE statement is what makes it // single-use under concurrency: a second callback with the same state deletes nothing and gets // nothing, with no window between the check and the removal. @@ -53,7 +49,7 @@ func (s *Store) TakeLoginState(ctx context.Context, hash []byte, now time.Time) Scan(&st.Provider, &st.Issuer, &st.Nonce, &st.Verifier, &st.ReturnTo, &st.StartID, &st.CreatedAt, &st.ExpiresAt) if errors.Is(err, pgx.ErrNoRows) { - return login.State{}, ErrNoLoginState + return login.State{}, login.ErrNoState } if err != nil { return login.State{}, fmt.Errorf("pgstore: take login state: %w", err) diff --git a/platform/internal/pgstore/identity_test.go b/platform/internal/pgstore/identity_test.go index 275900d2..1db77d34 100644 --- a/platform/internal/pgstore/identity_test.go +++ b/platform/internal/pgstore/identity_test.go @@ -127,6 +127,30 @@ func TestUnverifiedAddressStaysOffTheAccount(t *testing.T) { if accountEmail == nil || *accountEmail != "unverified@example.org" { t.Fatal("a verified address should reach the account") } + + // PD-85: the RETURNING branch, which the two steps above never reach with an unverified address. + // Dropping the in.EmailVerified condition there used to pass the whole battery — and that is the + // branch every login after the first goes through, so a provider that later reports the address + // unverified, or reports a new one it has not checked, would overwrite the account's. + if _, err := s.UpsertIdentity(ctx, login.Identity{ + Provider: "google", Subject: "sub-A", Email: "attacker@example.org", EmailVerified: false, + }, now, 0); err != nil { + t.Fatal(err) + } + if err := s.pool.QueryRow(ctx, `select email from users where id = $1`, userID).Scan(&accountEmail); err != nil { + t.Fatal(err) + } + if accountEmail == nil || *accountEmail != "unverified@example.org" { + t.Fatalf("an unverified address replaced the account's on a returning login: %v", accountEmail) + } + // The identity still records what actually arrived — the fact is kept, it is just not promoted. + if err := s.pool.QueryRow(ctx, + `select email from identities where provider='google' and subject='sub-A'`).Scan(&identityEmail); err != nil { + t.Fatal(err) + } + if identityEmail == nil || *identityEmail != "attacker@example.org" { + t.Fatalf("the identity did not record the address it was handed: %v", identityEmail) + } } // The state is single-use and it expires. Both are clauses of one statement, so a second callback @@ -157,7 +181,7 @@ func TestLoginStateIsSingleUseAndExpires(t *testing.T) { if !reflect.DeepEqual(got, st) { t.Fatalf("the state did not survive the store whole:\n got %+v\n want %+v", got, st) } - if _, err := s.TakeLoginState(ctx, st.Hash, now); !errors.Is(err, ErrNoLoginState) { + if _, err := s.TakeLoginState(ctx, st.Hash, now); !errors.Is(err, login.ErrNoState) { t.Fatalf("a state must be usable once, got %v", err) } @@ -166,7 +190,7 @@ func TestLoginStateIsSingleUseAndExpires(t *testing.T) { if err := s.PutLoginState(ctx, expired); err != nil { t.Fatal(err) } - if _, err := s.TakeLoginState(ctx, expired.Hash, now.Add(2*time.Minute)); !errors.Is(err, ErrNoLoginState) { + if _, err := s.TakeLoginState(ctx, expired.Hash, now.Add(2*time.Minute)); !errors.Is(err, login.ErrNoState) { t.Fatalf("an expired state must be refused, got %v", err) } n, err := s.DeleteExpiredLoginStates(ctx, now.Add(2*time.Minute)) @@ -280,7 +304,7 @@ func TestOnlyOneRacingCallbackCanConsumeAState(t *testing.T) { mu.Lock() won++ mu.Unlock() - case errors.Is(err, ErrNoLoginState): + case errors.Is(err, login.ErrNoState): default: mu.Lock() t.Errorf("round %d: %v", i, err)