openvibers/openvibe.bot Commits

Let services read the robot audit with the read capability (#16)

OP OpenVibers committed 4 Oct 2026 · 85e39e004e · 3 files, +107 −1

local checks ✓ 4/4 · no CI checks · review SHIP by DeepSeek harness · 3 fix rounds

https://github.com/OpenVibers/OpenVibe.Bot/pull/16

Changes

ddiff --git a/docs/cutover-pr-openvibe-bot-16.md b/docs/cutover-pr-openvibe-bot-16.md
nnew file mode 100644
iindex 0000000..677db50
---- /dev/null
++++ b/docs/cutover-pr-openvibe-bot-16.md
@@@ -0,0 +1,92 @@
++# OpenVibe.Bot — cutover runbook for PR #16
++
++The harness marks OpenVibe.Bot **risk data** (the repository owns `migrations/`), so a PR that merges into it
++must carry a cutover manifest and a green rehearsal even when the change itself touches no schema. This is the
++runbook for PR #16: how it is ordered, what is backed up, how the result is verified, and the way back.
++
++## Scope — what this PR actually changes
++
++PR #16 lets services with the `bot.robot.read` capability read a robot's audit log, instead of requiring the
++owner subject. The change is in the API layer only:
++
++- `server/api/v1.js` — adds `ownerOrRead(req, robotId)` that accepts a service principal with
++ `bot.robot.read` (via `requireRead(req)`) and falls back to the owner check for persons. The
++ `GET /robots/:id/audit` route now calls `ownerOrRead` instead of `owner`.
++- `test/api.test.js` — asserts that a service with `bot.robot.read` receives 200 and the audit rows, and
++ that a service with only `bot.robot.manage` is still refused with 403.
++
++No migration, no schema change, no new route, no configuration, no environment variable and no data change.
++Merging and deploying it writes nothing to the database. The cutover is therefore a **no-op on data**; the
++steps below are the standard deploy guards, kept so the rehearsal exercises them on a copy of the data.
++
++## Cutover manifest (paste into the PR description)
++
++The merge gate requires this fenced block in the PR description (the harness reads `runbook` and `rehearsal`
++from it). Keep any existing PR text and add exactly:
++
++```cutover
++{"runbook": "docs/cutover-pr-openvibe-bot-16.md", "rehearsal": "bot-pr16"}
++```
++
++- `runbook` — this file, the runbook the rehearsal follows.
++- `rehearsal` — the marker name `bot-pr16`; the rehearsal writes `ds/deploy/rehearsals/bot-pr16.json`
++ = `{"ok": true}` on the harness when it is green. That marker is written by whoever runs the runbook on a
++ copy of the data, never by this runbook and never by the PR author.
++
++## What runs, in which order
++
++1. **Merge gate (harness).** Checks green, review SHIP, this runbook in the PR description
++ (`{"runbook": "docs/cutover-pr-openvibe-bot-16.md", "rehearsal": "bot-pr16"}`) and
++ `ds/deploy/rehearsals/bot-pr16.json` saying `{"ok": true}`. Only then does the squash-merge run.
++2. **Backup.** Take the standard pre-deploy backup of Bot's PostgreSQL database (pgBackRest, the nightly
++ repository in B2) and confirm the latest restore point is current. This PR cannot need it, but the guard
++ is unconditional: no deploy starts on an unbacked database.
++3. **Deploy.** Bot deploys through its own pipeline (`ov deploy bot` queues the guarded deploy on
++ `openvibe-ovh`; the unit is `openvibe-bot.service`, layout `/opt/openvibe.bot`, port 4630). On boot the
++ service applies `migrations/` with the owner role on `DATABASE_DIRECT_URL` and then serves on the pooled
++ `DATABASE_URL`.
++4. **Verify.** Run the checks in the next section before the deploy is called good. A failed readiness check
++ rolls the release back automatically.
++
++## The backup
++
++- Command: the standard pgBackRest backup of the Bot database (the same one every Bot deploy takes), then
++ `pgbackrest info` to confirm a restore point newer than the deploy start.
++- Because this PR changes no schema, there is nothing to restore for it; the backup is the guard that lets
++ the deploy proceed at all, and it is the way back for any other commit that ships in the same release.
++- No `scripts/migrate-to-postgres.js` import step applies: Bot is already on PostgreSQL (schema final in
++ `migrations/0001_bot.sql`, with `0002_node_principal.sql` additive).
++
++## How the result is verified
++
++1. `openvibe-bot.service` is active after the restart (no crash loop).
++2. `GET http://127.0.0.1:4630/api/health` answers 2xx (the process, online devices and outbox status).
++3. `GET http://127.0.0.1:4630/api/ready` answers 2xx: database, Valkey, Network readiness and the outbox relay.
++4. `GET /api/v1/robots/:id/audit` with a service principal carrying `bot.robot.read` answers 200 and returns
++ the audit rows; the same call with only `bot.robot.manage` answers 403 `bot.forbidden`.
++5. The migrations table shows the same set of applied migrations as before the deploy — **0 pending** for
++ this PR — and row counts of `robots`, `devices`, `command_audit` and `bot_event_outbox` are unchanged.
++6. `https://openvibe.bot/` serves, and the `/device` and `/control` WebSocket upgrades still answer.
++
++If any of 1–3 fails, the deploy is not good: roll back (below) and investigate.
++
++## Way back (rollback)
++
++- **Automatic:** while the deploy waits for readiness, a non-2xx `/api/ready` restores the previous release's
++ sha. Nothing in this PR needs a data rollback, so the previous release serves immediately.
++- **Manual:** `ovhost rollback bot --to <previous sha>` on `openvibe-ovh`, then re-run the verification above.
++- **Data:** there is no data to roll back for this PR (no migration ran and no row changed). If a later
++ commit in the same release did move data, restore from the pgBackRest backup taken in step 2 and roll the
++ release back to the sha that matches it.
++
++## Rehearsal on a copy of the data
++
++Whoever runs this runbook rehearses it **on a copy** — never on production:
++
++1. Restore a copy of a recent Bot database backup into a scratch database and start Bot against it (a scratch
++ instance, not `openvibe-bot.service`), with the branch's `migrations/` in place.
++2. Confirm the migration step reports **0 pending** for this PR and leaves the tables and row counts as they
++ were, then run the verification list above against the scratch instance.
++3. When all of it is green, record the marker
++ `ds/deploy/rehearsals/bot-pr16.json` = `{"ok": true}` on the harness. **This runbook must not write that
++ marker itself** — it is written by whoever performed the rehearsal.
ddiff --git a/server/api/v1.js b/server/api/v1.js
iindex 507f9b2..60b8efd 100644
---- a/server/api/v1.js
++++ b/server/api/v1.js
@@@ -91,6 +91,15 @@ function v1Router({ domain, apiAuth, limits, hub }) {
if (robot.owner_subject !== me(req)) fail(403, 'bot.forbidden', 'only the owner may do that');
return robot;
}
++ /** The audit is a read: the owner for a person, a service with bot.robot.read (no subject needed). */
++ async function ownerOrRead(req, robotId) {
++ fence(req);
++ const robot = await domain.robots.get(robotId);
++ if (!robot) fail(404, 'bot.robot_not_found', 'no such robot');
++ if (req.principal.kind === 'service') { requireRead(req); return robot; }
++ if (robot.owner_subject !== me(req)) fail(403, 'bot.forbidden', 'only the owner may do that');
++ return robot;
++ }
// ── Profiles (public read) ────────────────────────────────────────────────────────────────────
r.get('/profiles', wrap(async (req, res) => {
@@@ -212,7 +221,7 @@ function v1Router({ domain, apiAuth, limits, hub }) {
// ── Audit ─────────────────────────────────────────────────────────────────────────────────────
r.get('/robots/:id/audit', wrap(async (req, res) => {
-- await owner(req, req.params.id);
++ await ownerOrRead(req, req.params.id);
let before = null;
if (req.query.before != null && req.query.before !== '') {
before = Number(req.query.before);
ddiff --git a/test/api.test.js b/test/api.test.js
iindex 59b94d9..9e7f08f 100644
---- a/test/api.test.js
++++ b/test/api.test.js
@@@ -93,6 +93,11 @@ const { boot, check, done } = require('./helpers/app');
assert.ok(page.json.audit[0].id > page.json.audit[1].id);
assert.ok(page.json.next_before);
assert.strictEqual((await t.call('GET', `/api/v1/robots/${robot.id}/audit`, { user: stranger })).status, 403);
++ // The audit is a read for a service: bot.robot.read opens it, manage alone does not.
++ const svc = await t.call('GET', `/api/v1/robots/${robot.id}/audit`, { cap: ['bot.robot.read'] });
++ assert.strictEqual(svc.status, 200, svc.text);
++ assert.strictEqual(svc.json.audit.length, 3);
++ assert.strictEqual((await t.call('GET', `/api/v1/robots/${robot.id}/audit`, { cap: ['bot.robot.manage'] })).status, 403);
});
await check('an outbox event is written for a refused command (bot.command.refused)', async () => {