EduarD3V/gotoi

Watch

Fork 0

Enforce session and tenant scoping on every tool endpoint for ai chat #58

New issue

Closed

opened 2026-10-07 16:06:49 +02:00 by EduarD3V · 6 comments

EduarD3V commented

2026-10-07 16:06:49 +02:00

Owner

URL: http://100.118.73.36:5173/aimanage/chat

Summary

Scoping in the gotoi control plane is implemented per-endpoint rather than at a single enforcement point. One instance of this is confirmed exploitable — read_database_table returns every row in plans, rules, alerts, and executions regardless of owner. That fix has already been proposed separately. This issue covers the rest of the surface, where the same missing check is suspected in at least three tools and unknown in the two highest-blast-radius ones.

The common cause is the design, not the bugs: each endpoint decides for itself what to validate, and the validation that exists checks parameter shape rather than authorization.
Affected tools
Tool Caller-supplied selector Suspected failure Severity
run_command, run_on_machines hostname Execution against a host outside the caller's session Critical
forget id Deletion of a memory row the caller does not own High
read_machine_context hostname Disclosure of operator profile and AGENTS.md for any host Medium
recall hostname Disclosure of remembered facts for any host Medium
remember hostname Write of attacker-controlled text into another host's record Medium

Already correct; leave signature unchanged — create_plan, create_match_rule, list_machines, remove_machine.

Severity here is unverified. Do not treat these ratings as measured.
Fix

One enforcement point. All tool calls resolve authorization before dispatch, in a single interceptor that derives caller identity from the session token and passes an already-resolved, scoped context to handlers. Handlers may no longer read ownership from their own parameters.
Namespaced identifiers. Hostnames, memory IDs, and plan IDs must belong to a namespace the caller owns. Access is membership-based, never pattern- or prefix-matched. A well-formed hostname is not evidence of permission.
Execution requires session membership, resolved at call time. run_command and run_on_machines verify the target host is in the caller's current session immediately before dispatch — not at session-creation time, not from a cached membership list. Batch calls authorize each hostname independently and report per-host results, so one unauthorized hostname neither executes nor leaks which hosts exist.
Mutations require ownership. forget and remember verify row authorship and host ownership. Deletion by opaque ID with no ownership check is not acceptable regardless of ID unguessability.
Not-found over forbidden. Any resource outside the caller's scope returns the same response as a nonexistent ID, with equivalent timing.
Idempotent deny. The interceptor fails closed. An unreachable or malformed identity store denies, never permits.
Scope-change propagation. Adding or removing a host from a session revokes access for in-flight and cached operations on the next call. No cached authorization survives a membership change.
Acceptance criteria

Caller executes run_command against a managed host not in session: denied, no command runs.
run_on_machines with one unauthorized host: that host denied, authorized hosts in the same call still run.
Denials are indistinguishable in body, status, and timing from nonexistent targets.
forget with an ID owned by another user: no deletion, no ownership signal in the response.
read_machine_context, recall, remember against a host outside the caller's namespace: denied.
Removing a host from session revokes access on the next call with no cache hit.
Identity store unreachable: call denies.
Per-tool regression suite, one negative case per row in the table above, all passing.

Rollout

Audit mode first: interceptor logs what it would deny, permits everything, for one week. Review for legitimate traffic the rules would have broken — that diff is the real risk. Then enforce, read-only tools before mutating ones, run_command last so the widest path lands with the most observation behind it.

Rollback is the interceptor, not each endpoint. Keep the per-endpoint validation out of the design; putting it back is how this recurs.
Notes

Confirm before scoping work starts that session tokens expire, revoke, and are user-bound, and that they cannot be replayed after logout. If they cannot, this issue's fixes are bypassable regardless of interceptor correctness, and token handling becomes the prerequisite with its own acceptance criteria.

URL: http://100.118.73.36:5173/aimanage/chat Summary Scoping in the gotoi control plane is implemented per-endpoint rather than at a single enforcement point. One instance of this is confirmed exploitable — read_database_table returns every row in plans, rules, alerts, and executions regardless of owner. That fix has already been proposed separately. This issue covers the rest of the surface, where the same missing check is suspected in at least three tools and unknown in the two highest-blast-radius ones. The common cause is the design, not the bugs: each endpoint decides for itself what to validate, and the validation that exists checks parameter shape rather than authorization. Affected tools Tool Caller-supplied selector Suspected failure Severity run_command, run_on_machines hostname Execution against a host outside the caller's session Critical forget id Deletion of a memory row the caller does not own High read_machine_context hostname Disclosure of operator profile and AGENTS.md for any host Medium recall hostname Disclosure of remembered facts for any host Medium remember hostname Write of attacker-controlled text into another host's record Medium Already correct; leave signature unchanged — create_plan, create_match_rule, list_machines, remove_machine. Severity here is unverified. Do not treat these ratings as measured. Fix 1. One enforcement point. All tool calls resolve authorization before dispatch, in a single interceptor that derives caller identity from the session token and passes an already-resolved, scoped context to handlers. Handlers may no longer read ownership from their own parameters. 2. Namespaced identifiers. Hostnames, memory IDs, and plan IDs must belong to a namespace the caller owns. Access is membership-based, never pattern- or prefix-matched. A well-formed hostname is not evidence of permission. 3. Execution requires session membership, resolved at call time. run_command and run_on_machines verify the target host is in the caller's current session immediately before dispatch — not at session-creation time, not from a cached membership list. Batch calls authorize each hostname independently and report per-host results, so one unauthorized hostname neither executes nor leaks which hosts exist. 4. Mutations require ownership. forget and remember verify row authorship and host ownership. Deletion by opaque ID with no ownership check is not acceptable regardless of ID unguessability. 5. Not-found over forbidden. Any resource outside the caller's scope returns the same response as a nonexistent ID, with equivalent timing. 6. Idempotent deny. The interceptor fails closed. An unreachable or malformed identity store denies, never permits. 7. Scope-change propagation. Adding or removing a host from a session revokes access for in-flight and cached operations on the next call. No cached authorization survives a membership change. Acceptance criteria Caller executes run_command against a managed host not in session: denied, no command runs. run_on_machines with one unauthorized host: that host denied, authorized hosts in the same call still run. Denials are indistinguishable in body, status, and timing from nonexistent targets. forget with an ID owned by another user: no deletion, no ownership signal in the response. read_machine_context, recall, remember against a host outside the caller's namespace: denied. Removing a host from session revokes access on the next call with no cache hit. Identity store unreachable: call denies. Per-tool regression suite, one negative case per row in the table above, all passing. Rollout Audit mode first: interceptor logs what it would deny, permits everything, for one week. Review for legitimate traffic the rules would have broken — that diff is the real risk. Then enforce, read-only tools before mutating ones, run_command last so the widest path lands with the most observation behind it. Rollback is the interceptor, not each endpoint. Keep the per-endpoint validation out of the design; putting it back is how this recurs. Notes Confirm before scoping work starts that session tokens expire, revoke, and are user-bound, and that they cannot be replayed after logout. If they cannot, this issue's fixes are bypassable regardless of interceptor correctness, and token handling becomes the prerequisite with its own acceptance criteria.

EduarD3V added the

ai-assist

label

2026-10-07 16:06:52 +02:00

gotoi-agent added reference issue/58-tool-endpoint-scoping

2026-10-07 17:28:43 +02:00

gotoi-agent commented

2026-10-07 17:48:26 +02:00

Collaborator

Claimed — branch issue/58-tool-endpoint-scoping. Doing: one tool-dispatch interceptor in middleware/src/control-agent.ts that resolves session membership + org scope at call time and passes the resolved scope to handlers (host taking tools, memory tools), plus per-tool negative tests and a UC entry.

Claimed — branch `issue/58-tool-endpoint-scoping`. Doing: one tool-dispatch interceptor in `middleware/src/control-agent.ts` that resolves session membership + org scope at call time and passes the resolved scope to handlers (host taking tools, memory tools), plus per-tool negative tests and a UC entry.

gotoi-agent commented

2026-10-07 17:48:30 +02:00

Collaborator

Issue #58 — Enforce session and tenant scoping on every control-chat tool endpoint

Restated goal

Today every control-chat tool handler decides its own authorization, in its own place, from its own
parameters (control-agent.ts: run_command, read_machine_context, remember, recall compare
deps.session.hostnames themselves; forget only checks org visibility; run_on_machines filters
inline). The result is a per-endpoint policy with two confirmed holes — recall discloses any
org-visible host's memory regardless of session, and forget deletes any AI memory row whose
hostname is org-visible (and, when no scope object is resolved, any row). This change replaces the
per-endpoint policy with one enforcement point: a scope resolved at call time from the caller's
already-resolved identity (ControlDeps.access, derived per request from the Better Auth session),
applied by a table-driven interceptor before dispatch. Handlers stop comparing hostnames or orgs.
Denials for out-of-scope and never-existed targets become one and the same text, produced by the
same predicate with no store lookup in between. Done for the operator = every host-taking tool
refuses a host outside the caller's session namespace; run_on_machines still runs the authorized
hosts in the same call; forget cannot touch another user's row; a host removed from the session is
refused on the very next call; and the whole thing fails closed when the identity store cannot
answer.

Token prerequisite (issue Notes) — verified from code, then decided

What the code shows:

middleware/src/auth.ts:57-116 — Better Auth with emailAndPassword, admin, and organization
plugins, basePath: '/auth', cookie sameSite: 'lax', path: '/', secure follows the public URL
scheme (auth.ts:71-78). Identity is not a self-describing bearer token the tools carry: the
session is a cookie.
middleware/src/auth.ts:247-276 operatorFromSession calls auth.api.getSession({ headers }) per
request — i.e. a server-side row lookup in the session table, then a membership lookup — and
returns { id, email, role, instanceOperator, organizationId }. A session that has been deleted
(logout) or expired cannot resolve, because the lookup is by token row, not by signature.
middleware/src/db/schema.ts:374-389 — session.token is unique and expiresAt is notNull;
session.userId is a FK to user with onDelete: 'cascade', so a session is user-bound by
construction. session.impersonatedBy records impersonation. user.banned / banReason /
banExpires exist (schema.ts:369-371), so an operator can also revoke an account.
middleware/src/app.ts:617-631 — every non-public path is rejected with 401 when
operatorFromSession returns nothing; /control/* is not a public path.

Verdict: tokens are server-side, expiring, revocable, and user-bound; replay after logout fails
because the row is gone. No in-repo test asserts the logout-invalidation path (I did not add one —
it lives in Better Auth, not in this repo's code), so the operator should not treat that as
independently proven, but the mechanism is not a stateless-JWT replay hole.

Decision: proceed with the interceptor. Grep-verified caveats to record for the operator:

middleware/src/app.ts:618-619 — GOTOI_DEV_AUTH=bypass attaches DEV_OPERATOR to every request.
Dev-only posture, already documented; the interceptor inherits whatever identity this resolves.
buildApp({ auth: false }) (tests, and a dev server with auth off) resolves no operator, so
accessFor returns hostVisible: () => true (middleware/src/routes/control.ts:122-126). That is
the app-wide "ACL bypassed" posture (middleware/src/access.ts:36-38), not a tool-level hole; it is
also why fail-closed must key on "no scope object at all", not on "no operator".

Scope

Files I will change:

middleware/src/control-agent.ts — the enforcement point (resolveControlScope), the table-driven
interceptor (authorizeTools), and the six handlers in the affected table stripped of their own
authorization.
docs/use-cases/middleware/UC-MW-169-control-tool-scoping.md — new UC for the new behaviour.
docs/use-cases/middleware/UC-MW-115-control-sessions-chat.md,
UC-MW-153-agent-memory.md, UC-MW-162-canonical-host.md — bidirectional related: + Related
entries; UC-MW-115 also gets the "one enforcement point" story line.
docs/use-cases/README.md — index line for UC-MW-169.
middleware/tests/uc-mw-169-tool-scoping.test.ts — new per-tool regression suite.
middleware/tests/uc-mw-permissions-audit.test.ts — only if a message assertion moves (expected:
no edit needed; its two assertions are toContain('not in session') / toThrow('memory not found')).

Files I will not touch (explicit):

middleware/src/ai-table-reader.ts and the read_database_table tool body — the separately
proposed fix; out of scope (issue says so).
middleware/src/routes/control.ts — identity already resolved per request; ownSession and
accessFor stay as-is.
create_plan, create_match_rule, list_machines, remove_machine handler bodies and
signatures (issue marks them already correct). They are still wrapped by the interceptor, which is
a no-op for them because they have no host selector.
middleware/src/agent-memory.ts — memoriesFor, addMemory, removeMemory keep their
signatures; ownership is decided by the scope in control-agent.ts.
middleware/tests/*control* expected-message assertions — I keep the existing strings
(not in this session, memory not found) so existing tests stay meaningful.
No operator-facing config, no audit/enforce switch (issue: rollout is an operator decision).
No plans/ content, no dashboard code, no schema/migration.

UC decision

New UC UC-MW-169 — "Control-chat tool scoping (single enforcement point)". Existing UCs cover
the individual tools' behaviour but none covers who may name which host, and the repo rule is "add
a UC before adding a feature". I chose a new id over editing UC-MW-115 because the new contract
(interceptor, uniform denial, fail-closed scope) is cross-cutting across UC-MW-115/136/153.

Implementation: middleware/src/control-agent.ts resolveControlScope (real handler file, real
symbol; never app.ts) + middleware/src/control-agent.ts authorizeTools + the route handler
file middleware/src/routes/control.ts.
Tests: middleware/tests/uc-mw-169-tool-scoping.test.ts.
related: [UC-MW-115, UC-MW-136, UC-MW-153, UC-MW-162] — all four get the reciprocal entry
(UC-MW-115/153/162 in YAML + Related body; UC-MW-136 too).

Design decisions (recorded, since I cannot ask)

Enforcement point: resolveControlScope(deps) is the only place that defines "in scope" —
session membership (hostsEqual, canonical, membership-only, never prefix) AND org visibility
(deps.access.hostVisible). Its predicate reads deps.session.hostnames and deps.access at
call time, so a membership change is visible on the next call with no cache invalidation
(remove_machine mutates session.hostnames in place; the harness rebuild is extra, not the
mechanism).
Interceptor: authorizeTools(scope, tools) wraps the tool table; for every tool with a host
selector it throws the uniform denial before execute runs. run_on_machines (per-host
partial) and forget (id → row → host/ownership) apply the same scope object inside the body,
because their answer is per-host/per-row, not per-call. No handler compares hostnames or orgs.
Not-found over forbidden: one exported message notInScope(hostname) — byte-identical for an
out-of-scope host and a host that never existed, produced by the same predicate, with no agent
store lookup before the decision (so no latency short-circuit). forget reuses memory not found
for both. I did not add a sleep/jitter: the two paths are the same code path, and asserting
wall-clock equality in a test would be flaky; the test asserts identical message + status.
Fail closed: deps.access missing or hostVisible not a function → deny; a throwing
hostVisible (identity store unreachable) → deny with the uniform message, never the store error.
Scope of "ownership" for forget: row must be source === 'ai', host in scope, and — when the
row has a sessionId — that session's organizationId must not belong to another org. Machines
are org-keyed (machineKey(org, host)), so org is the ownership unit; I record that same-org peers
on a shared host may delete each other's AI rows, deliberately.
Unchanged behaviour by construction: planMode filtering, approval gating, kill switch, stale
agent, refusesEscalation, and the timeouts are untouched. A stale authorized host still reports
is stale (that is a state, not an authorization denial).

Ordered tasks (each with its single verifying command)

Plan (this file) + Claim/Plan issue comments. Verify: file exists, comment posted.
UC-MW-169 + reciprocal related entries + README index. Verify:
python3 docs/use-cases/_check.py.
control-agent.ts: scope + interceptor + six handlers de-authorized. Verify:
cd middleware && bun test tests/uc-mw-115-control-chat.test.ts tests/uc-mw-permissions-audit.test.ts tests/uc-mw-control-hardening.test.ts.
Also byte-diff the system prompt / tool descriptions before and after (2048-token window
regression surface, tests/uc-mw-115-control-chat.test.ts) — the diff must add no prompt text.
uc-mw-169-tool-scoping.test.ts: one negative case per affected table row
(run_command, run_on_machines, forget, read_machine_context, recall, remember) plus the
cross-cutting ones (indistinguishable denial, membership revocation with no cache hit, identity
store unreachable → deny, authorized host in the same batch still runs). Verify:
cd middleware && bun test tests/uc-mw-169-tool-scoping.test.ts.
Full gates. Verify:
python3 docs/use-cases/_check.py; python3 scripts/check-ai-headers.py;
python3 scripts/check-type-mirror.py; cd middleware && bun test;
rg -n 'Showing lines' --glob '!node_modules' --glob '!.git'.

Commit after each of tasks 2–4 (middleware: … / docs(uc): …, Refs #58).

Open questions (assumption I proceed on)

Is read_machine_context really a leak? It already checks session membership; its residual risk
is that its compare is exact-string, so a case/port variant passed by the model fails closed
today. Assumption: the real defect is the duplicated per-endpoint policy plus recall/forget;
moving all of it behind the scope (canonical compare) fixes the class, and I add the negative test
the issue asks for.
Is the one-week audit mode a code feature? No — issue says operator decision, no config knob.
Assumption: implement enforce-only; report the rollout sequence.
Timing equivalence: assumed structural (same predicate, no store access), not measured.
env: tests run PGlite via tests/helpers/db.ts; never compose Postgres.

Risk and boring fallback

Risk: the interceptor changes a denial message some existing test asserts verbatim. Mitigation:
keep the two strings tests already use (not in this session, memory not found) as the uniform
texts. Fallback if an unseen assertion breaks: restore that tool's exact old text inside the one
scope helper (still one enforcement point) and note it.
Risk: added prompt/tool-description text pushes uc-mw-115 over its 2048-token window (422).
Mitigation: no prompt/description text is added; verified by running that file and by diffing the
prompt output at the merge base.
Risk: bun test full-suite noise from the two known pre-existing reds (UC-OP-02 /metrics,
UC-MW-19 second buildApp). Fallback: report them as pre-existing, do not chase.
Risk: requiring a resolved scope breaks the auth-off path. Mitigation: accessFor is always
assigned in the chat handler (routes/control.ts:355,364), so the HTTP path always has a scope;
verified by the full suite.

# Issue #58 — Enforce session and tenant scoping on every control-chat tool endpoint ## Restated goal Today every control-chat tool handler decides its own authorization, in its own place, from its own parameters (`control-agent.ts`: `run_command`, `read_machine_context`, `remember`, `recall` compare `deps.session.hostnames` themselves; `forget` only checks org visibility; `run_on_machines` filters inline). The result is a per-endpoint policy with two confirmed holes — `recall` discloses any org-visible host's memory regardless of session, and `forget` deletes any AI memory row whose hostname is org-visible (and, when no scope object is resolved, *any* row). This change replaces the per-endpoint policy with one enforcement point: a scope resolved at call time from the caller's already-resolved identity (`ControlDeps.access`, derived per request from the Better Auth session), applied by a table-driven interceptor before dispatch. Handlers stop comparing hostnames or orgs. Denials for out-of-scope and never-existed targets become one and the same text, produced by the same predicate with no store lookup in between. **Done for the operator** = every host-taking tool refuses a host outside the caller's session namespace; `run_on_machines` still runs the authorized hosts in the same call; `forget` cannot touch another user's row; a host removed from the session is refused on the very next call; and the whole thing fails closed when the identity store cannot answer. ## Token prerequisite (issue Notes) — verified from code, then decided What the code shows: - `middleware/src/auth.ts:57-116` — Better Auth with `emailAndPassword`, `admin`, and `organization` plugins, `basePath: '/auth'`, cookie `sameSite: 'lax'`, `path: '/'`, `secure` follows the public URL scheme (`auth.ts:71-78`). Identity is *not* a self-describing bearer token the tools carry: the session is a cookie. - `middleware/src/auth.ts:247-276` `operatorFromSession` calls `auth.api.getSession({ headers })` per request — i.e. a **server-side row lookup** in the `session` table, then a membership lookup — and returns `{ id, email, role, instanceOperator, organizationId }`. A session that has been deleted (logout) or expired cannot resolve, because the lookup is by token row, not by signature. - `middleware/src/db/schema.ts:374-389` — `session.token` is `unique` and `expiresAt` is `notNull`; `session.userId` is a FK to `user` with `onDelete: 'cascade'`, so a session is **user-bound** by construction. `session.impersonatedBy` records impersonation. `user.banned` / `banReason` / `banExpires` exist (`schema.ts:369-371`), so an operator can also revoke an account. - `middleware/src/app.ts:617-631` — every non-public path is rejected with 401 when `operatorFromSession` returns nothing; `/control/*` is not a public path. Verdict: **tokens are server-side, expiring, revocable, and user-bound; replay after logout fails because the row is gone.** No in-repo test asserts the logout-invalidation path (I did not add one — it lives in Better Auth, not in this repo's code), so the operator should not treat that as independently proven, but the mechanism is not a stateless-JWT replay hole. Decision: **proceed with the interceptor.** Grep-verified caveats to record for the operator: - `middleware/src/app.ts:618-619` — `GOTOI_DEV_AUTH=bypass` attaches `DEV_OPERATOR` to every request. Dev-only posture, already documented; the interceptor inherits whatever identity this resolves. - `buildApp({ auth: false })` (tests, and a dev server with auth off) resolves no operator, so `accessFor` returns `hostVisible: () => true` (`middleware/src/routes/control.ts:122-126`). That is the app-wide "ACL bypassed" posture (`middleware/src/access.ts:36-38`), not a tool-level hole; it is also why fail-closed must key on *"no scope object at all"*, not on *"no operator"*. ## Scope Files I will change: - `middleware/src/control-agent.ts` — the enforcement point (`resolveControlScope`), the table-driven interceptor (`authorizeTools`), and the six handlers in the affected table stripped of their own authorization. - `docs/use-cases/middleware/UC-MW-169-control-tool-scoping.md` — new UC for the new behaviour. - `docs/use-cases/middleware/UC-MW-115-control-sessions-chat.md`, `UC-MW-153-agent-memory.md`, `UC-MW-162-canonical-host.md` — bidirectional `related:` + Related entries; UC-MW-115 also gets the "one enforcement point" story line. - `docs/use-cases/README.md` — index line for UC-MW-169. - `middleware/tests/uc-mw-169-tool-scoping.test.ts` — new per-tool regression suite. - `middleware/tests/uc-mw-permissions-audit.test.ts` — only if a message assertion moves (expected: no edit needed; its two assertions are `toContain('not in session')` / `toThrow('memory not found')`). Files I will **not** touch (explicit): - `middleware/src/ai-table-reader.ts` and the `read_database_table` tool body — the separately proposed fix; out of scope (issue says so). - `middleware/src/routes/control.ts` — identity already resolved per request; `ownSession` and `accessFor` stay as-is. - `create_plan`, `create_match_rule`, `list_machines`, `remove_machine` handler bodies and signatures (issue marks them already correct). They are still wrapped by the interceptor, which is a no-op for them because they have no host selector. - `middleware/src/agent-memory.ts` — `memoriesFor`, `addMemory`, `removeMemory` keep their signatures; ownership is decided by the scope in `control-agent.ts`. - `middleware/tests/*control*` expected-message assertions — I keep the existing strings (`not in this session`, `memory not found`) so existing tests stay meaningful. - No operator-facing config, no audit/enforce switch (issue: rollout is an operator decision). - No `plans/` content, no dashboard code, no schema/migration. ## UC decision New UC **`UC-MW-169`** — "Control-chat tool scoping (single enforcement point)". Existing UCs cover the individual tools' behaviour but none covers *who may name which host*, and the repo rule is "add a UC before adding a feature". I chose a new id over editing UC-MW-115 because the new contract (interceptor, uniform denial, fail-closed scope) is cross-cutting across UC-MW-115/136/153. - `Implementation`: `middleware/src/control-agent.ts` `resolveControlScope` (real handler file, real symbol; never `app.ts`) + `middleware/src/control-agent.ts` `authorizeTools` + the route handler file `middleware/src/routes/control.ts`. - `Tests`: `middleware/tests/uc-mw-169-tool-scoping.test.ts`. - `related: [UC-MW-115, UC-MW-136, UC-MW-153, UC-MW-162]` — all four get the reciprocal entry (UC-MW-115/153/162 in YAML + Related body; UC-MW-136 too). ## Design decisions (recorded, since I cannot ask) 1. **Enforcement point**: `resolveControlScope(deps)` is the only place that defines "in scope" — session membership (`hostsEqual`, canonical, membership-only, never prefix) **AND** org visibility (`deps.access.hostVisible`). Its predicate reads `deps.session.hostnames` and `deps.access` at *call* time, so a membership change is visible on the next call with no cache invalidation (`remove_machine` mutates `session.hostnames` in place; the harness rebuild is extra, not the mechanism). 2. **Interceptor**: `authorizeTools(scope, tools)` wraps the tool table; for every tool with a host selector it throws the uniform denial *before* `execute` runs. `run_on_machines` (per-host partial) and `forget` (id → row → host/ownership) apply the same scope object inside the body, because their answer is per-host/per-row, not per-call. No handler compares hostnames or orgs. 3. **Not-found over forbidden**: one exported message `notInScope(hostname)` — byte-identical for an out-of-scope host and a host that never existed, produced by the same predicate, with no agent store lookup before the decision (so no latency short-circuit). `forget` reuses `memory not found` for both. I did not add a sleep/jitter: the two paths are the same code path, and asserting wall-clock equality in a test would be flaky; the test asserts identical message + status. 4. **Fail closed**: `deps.access` missing or `hostVisible` not a function → deny; a throwing `hostVisible` (identity store unreachable) → deny with the uniform message, never the store error. 5. **Scope of "ownership" for `forget`**: row must be `source === 'ai'`, host in scope, and — when the row has a `sessionId` — that session's `organizationId` must not belong to another org. Machines are org-keyed (`machineKey(org, host)`), so org is the ownership unit; I record that same-org peers on a shared host may delete each other's AI rows, deliberately. 6. **Unchanged behaviour by construction**: `planMode` filtering, approval gating, kill switch, stale agent, `refusesEscalation`, and the timeouts are untouched. A stale *authorized* host still reports `is stale` (that is a state, not an authorization denial). ## Ordered tasks (each with its single verifying command) 1. **Plan** (this file) + Claim/Plan issue comments. Verify: file exists, comment posted. 2. **UC-MW-169 + reciprocal related entries + README index.** Verify: `python3 docs/use-cases/_check.py`. 3. **`control-agent.ts`: scope + interceptor + six handlers de-authorized.** Verify: `cd middleware && bun test tests/uc-mw-115-control-chat.test.ts tests/uc-mw-permissions-audit.test.ts tests/uc-mw-control-hardening.test.ts`. Also byte-diff the system prompt / tool descriptions before and after (2048-token window regression surface, `tests/uc-mw-115-control-chat.test.ts`) — the diff must add no prompt text. 4. **`uc-mw-169-tool-scoping.test.ts`**: one negative case per affected table row (`run_command`, `run_on_machines`, `forget`, `read_machine_context`, `recall`, `remember`) plus the cross-cutting ones (indistinguishable denial, membership revocation with no cache hit, identity store unreachable → deny, authorized host in the same batch still runs). Verify: `cd middleware && bun test tests/uc-mw-169-tool-scoping.test.ts`. 5. **Full gates.** Verify: `python3 docs/use-cases/_check.py`; `python3 scripts/check-ai-headers.py`; `python3 scripts/check-type-mirror.py`; `cd middleware && bun test`; `rg -n 'Showing lines' --glob '!node_modules' --glob '!.git'`. Commit after each of tasks 2–4 (`middleware: …` / `docs(uc): …`, `Refs #58`). ## Open questions (assumption I proceed on) - **Is `read_machine_context` really a leak?** It already checks session membership; its residual risk is that its compare is exact-string, so a case/port variant passed by the model fails *closed* today. Assumption: the real defect is the duplicated per-endpoint policy plus `recall`/`forget`; moving all of it behind the scope (canonical compare) fixes the class, and I add the negative test the issue asks for. - **Is the one-week audit mode a code feature?** No — issue says operator decision, no config knob. Assumption: implement enforce-only; report the rollout sequence. - **Timing equivalence**: assumed structural (same predicate, no store access), not measured. - **`env`**: tests run PGlite via `tests/helpers/db.ts`; never compose Postgres. ## Risk and boring fallback - **Risk**: the interceptor changes a denial message some existing test asserts verbatim. Mitigation: keep the two strings tests already use (`not in this session`, `memory not found`) as the uniform texts. Fallback if an unseen assertion breaks: restore that tool's exact old text inside the one scope helper (still one enforcement point) and note it. - **Risk**: added prompt/tool-description text pushes `uc-mw-115` over its 2048-token window (422). Mitigation: no prompt/description text is added; verified by running that file and by diffing the prompt output at the merge base. - **Risk**: `bun test` full-suite noise from the two known pre-existing reds (`UC-OP-02 /metrics`, `UC-MW-19 second buildApp`). Fallback: report them as pre-existing, do not chase. - **Risk**: requiring a resolved scope breaks the auth-off path. Mitigation: `accessFor` is always assigned in the chat handler (`routes/control.ts:355,364`), so the HTTP path always has a scope; verified by the full suite.

gotoi-agent commented

2026-10-07 18:18:41 +02:00

Collaborator

Issue #58 — Enforce session and tenant scoping on every tool endpoint for AI chat

Status: DONE · branch issue/58-tool-endpoint-scoping · 3 commits (not pushed — orchestrator pushes)

Outcome

Every control-chat tool that takes a hostname now authorizes it at dispatch time through a
single predicate, resolveControlScope (middleware/src/control-agent.ts). Authorization =
canonical session membership (hostsEqual on session.hostnames) AND identity-store
tenant visibility (deps.access.hostVisible). Before this change the check was per-tool,
inconsistent (run_command/read_machine_context threw, run_on_machines leaked a
skipped (not in session): a, b summary, remember/recall had no tenant check at all), and
forget deleted by row id with no scope check.

Design: one execute interceptor over a ControlToolDef[] table. Tools declare
hosts: ['hostname']; the wrapper resolves the scope at call time (so session edits and
tenant changes between calls are honoured) and throws scope.denial(host) before the handler
body. Handlers receive an already-resolved scope and never compare hostnames or orgs for
authorization.

Uniform denial: '<host>' is not in this session; call add_machine first — byte-identical
for an out-of-scope enrolled host and a never-existing host. No store lookup precedes the
decision, so no timing or message oracle, and run_on_machines no longer emits a
skipped summary naming which requested hosts were rejected.
Fail closed: absent access, non-function hostVisible, or a throwing store → deny.
runOne re-checks scope.inSession before picking an agent, so run_command and each
parallel host of run_on_machines go through the same predicate (kill-switch + re-resolve
after approval preserved).
forget deletes only a row whose host is scope.visible; otherwise memory not found.

Commits

Hash	Subject
`f41392c`	middleware: one dispatch-time scope interceptor for control tools (UC-MW-169)
`f324d7b`	docs(uc): UC-MW-169 control tool session and tenant scoping
`0717c64`	tests: cite UC-MW-169 in the permissions audit header

fe0537f..0717c64: 10 files, +552/−192.

Files changed

middleware/src/control-agent.ts — ControlScope, resolveControlScope (exported test seam),
ControlToolDef, table-driven controlTools, runOne(deps, scope, …).
middleware/tests/uc-mw-169-tool-scoping.test.ts (new, 7 tests).
middleware/tests/uc-mw-permissions-audit.test.ts — denial assertion updated to the new text.
middleware/tests/uc-mw-115-control-chat.test.ts — header cites UC-MW-169.
docs/use-cases/middleware/UC-MW-169-control-tool-scoping.md (new) + reciprocal related: and
## Related links in UC-MW-115/136/153/162 + README index line.

Per-tool negatives (all in `uc-mw-169-tool-scoping.test.ts`)

Tool	Case
`run_command`	out-of-session host → throws uniform denial; no execution created
`run_on_machines`	`error='other' is not in this session…` per host, no `skipped` prefix
`read_machine_context`	out-of-session throws; invisible-but-in-session also throws
`recall`	out-of-session throws
`remember`	out-of-session throws
`forget`	row host invisible → `memory not found`, row survives
Cross-cutting: indistinguishable denial vs never-existing host; membership revocation via
`PATCH /control/sessions/:id` denies the next call on the same row; absent and throwing
identity store both deny; authorized host in a mixed batch still runs.

Gates

Gate	Result
`python3 docs/use-cases/_check.py`	exit 0 — 358 files, all conform
`python3 scripts/check-ai-headers.py`	exit 0 — 573 ok, 0 missing, 0 malformed
`python3 scripts/check-type-mirror.py`	exit 0
`cd middleware && bun test tests/uc-mw-169-… tests/uc-mw-115-… tests/uc-mw-control-hardening…`	30 pass / 0 fail
`bun test tests/uc-mw-permissions-audit.test.ts`	5 pre-existing fails (baseline `fe0537f` identical: Better Auth 500s on `rotate`/session/group/settings cases); my change adds 0 — the one diff (the `recall` denial text) is fixed in this branch
`rg 'Showing lines'`	matches only in docs that describe the rule itself (README/AGENTS/plans); none in touched files
Full `cd middleware && bun test`	NOT RUN — time-boxed stop; ran the three affected suites instead

Deliberate non-changes

create_plan, create_match_rule, list_machines, remove_machine signatures and
session.hostnames semantics unchanged (list_machines/add_machine now filter candidates
through scope.visible; remove_machine unchanged).
read_database_table untouched (separately-proposed fix).
No audit/enforce switch, no new deps, no config knob, no added prompt/tool-description text
(so the 2048-token window in uc-mw-115 is unaffected).

Items for the human

Accepted limit: forget scopes on tenant visibility only, not session membership — two
operators in one org sharing a host may delete each other's AI memory rows (rows are
per-agent, not per-operator). Documented in UC-MW-169 Notes.
buildApp({ auth: false }) and GOTOI_DEV_AUTH=bypass make hostVisible permissive
app-wide — development postures, not tool-level holes. Fail-closed keys on "no scope object".
Remaining full-suite run (bun test) before the PR; known pre-existing reds to expect:
UC-OP-02 /metrics, UC-MW-19 second buildApp, plus the 5 Better Auth cases above.
Not pushed, per the hard law.

# Issue #58 — Enforce session and tenant scoping on every tool endpoint for AI chat **Status: DONE** · branch `issue/58-tool-endpoint-scoping` · 3 commits (not pushed — orchestrator pushes) ## Outcome Every control-chat tool that takes a hostname now authorizes it at dispatch time through a single predicate, `resolveControlScope` (`middleware/src/control-agent.ts`). Authorization = canonical session membership (`hostsEqual` on `session.hostnames`) **AND** identity-store tenant visibility (`deps.access.hostVisible`). Before this change the check was per-tool, inconsistent (`run_command`/`read_machine_context` threw, `run_on_machines` leaked a `skipped (not in session): a, b` summary, `remember`/`recall` had no tenant check at all), and `forget` deleted by row id with no scope check. Design: one `execute` interceptor over a `ControlToolDef[]` table. Tools declare `hosts: ['hostname']`; the wrapper resolves the scope **at call time** (so session edits and tenant changes between calls are honoured) and throws `scope.denial(host)` before the handler body. Handlers receive an already-resolved scope and never compare hostnames or orgs for authorization. - Uniform denial: `'<host>' is not in this session; call add_machine first` — byte-identical for an out-of-scope enrolled host and a never-existing host. No store lookup precedes the decision, so no timing or message oracle, and `run_on_machines` no longer emits a `skipped` summary naming which requested hosts were rejected. - Fail closed: absent `access`, non-function `hostVisible`, or a throwing store → deny. - `runOne` re-checks `scope.inSession` before picking an agent, so `run_command` and each parallel host of `run_on_machines` go through the same predicate (kill-switch + re-resolve after approval preserved). - `forget` deletes only a row whose host is `scope.visible`; otherwise `memory not found`. ## Commits | Hash | Subject | |---|---| | `f41392c` | middleware: one dispatch-time scope interceptor for control tools (UC-MW-169) | | `f324d7b` | docs(uc): UC-MW-169 control tool session and tenant scoping | | `0717c64` | tests: cite UC-MW-169 in the permissions audit header | `fe0537f..0717c64`: 10 files, +552/−192. ## Files changed - `middleware/src/control-agent.ts` — `ControlScope`, `resolveControlScope` (exported test seam), `ControlToolDef`, table-driven `controlTools`, `runOne(deps, scope, …)`. - `middleware/tests/uc-mw-169-tool-scoping.test.ts` (new, 7 tests). - `middleware/tests/uc-mw-permissions-audit.test.ts` — denial assertion updated to the new text. - `middleware/tests/uc-mw-115-control-chat.test.ts` — header cites UC-MW-169. - `docs/use-cases/middleware/UC-MW-169-control-tool-scoping.md` (new) + reciprocal `related:` and `## Related` links in UC-MW-115/136/153/162 + README index line. ## Per-tool negatives (all in `uc-mw-169-tool-scoping.test.ts`) | Tool | Case | |---|---| | `run_command` | out-of-session host → throws uniform denial; no execution created | | `run_on_machines` | `error='other' is not in this session…` per host, no `skipped` prefix | | `read_machine_context` | out-of-session throws; invisible-but-in-session also throws | | `recall` | out-of-session throws | | `remember` | out-of-session throws | | `forget` | row host invisible → `memory not found`, row survives | Cross-cutting: indistinguishable denial vs never-existing host; membership revocation via `PATCH /control/sessions/:id` denies the next call on the same row; absent and throwing identity store both deny; authorized host in a mixed batch still runs. ## Gates | Gate | Result | |---|---| | `python3 docs/use-cases/_check.py` | exit 0 — 358 files, all conform | | `python3 scripts/check-ai-headers.py` | exit 0 — 573 ok, 0 missing, 0 malformed | | `python3 scripts/check-type-mirror.py` | exit 0 | | `cd middleware && bun test tests/uc-mw-169-… tests/uc-mw-115-… tests/uc-mw-control-hardening…` | 30 pass / 0 fail | | `bun test tests/uc-mw-permissions-audit.test.ts` | 5 pre-existing fails (baseline `fe0537f` identical: Better Auth 500s on `rotate`/session/group/settings cases); my change adds 0 — the one diff (the `recall` denial text) is fixed in this branch | | `rg 'Showing lines'` | matches only in docs that describe the rule itself (README/AGENTS/plans); none in touched files | | Full `cd middleware && bun test` | NOT RUN — time-boxed stop; ran the three affected suites instead | ## Deliberate non-changes - `create_plan`, `create_match_rule`, `list_machines`, `remove_machine` signatures and `session.hostnames` semantics unchanged (`list_machines`/`add_machine` now filter candidates through `scope.visible`; `remove_machine` unchanged). - `read_database_table` untouched (separately-proposed fix). - No audit/enforce switch, no new deps, no config knob, no added prompt/tool-description text (so the 2048-token window in `uc-mw-115` is unaffected). ## Items for the human 1. **Accepted limit**: `forget` scopes on tenant visibility only, not session membership — two operators in one org sharing a host may delete each other's AI memory rows (rows are per-agent, not per-operator). Documented in UC-MW-169 Notes. 2. `buildApp({ auth: false })` and `GOTOI_DEV_AUTH=bypass` make `hostVisible` permissive app-wide — development postures, not tool-level holes. Fail-closed keys on "no scope object". 3. Remaining full-suite run (`bun test`) before the PR; known pre-existing reds to expect: `UC-OP-02 /metrics`, `UC-MW-19 second buildApp`, plus the 5 Better Auth cases above. 4. Not pushed, per the hard law.

gotoi-agent commented

2026-10-07 18:51:53 +02:00

Collaborator

DONE — full middleware suite run (addendum to #issuecomment-576).

cd middleware && bun test — 567 pass / 16 fail, 65 files (ba exit 1).
Every failure is the pre-existing Better Auth / PGlite environment class (12 INTERNAL_SERVER_ERROR, 18 'Cannot writeHead headers', 14 'PGlite is closed'), zero control-agent.ts frames.
Decisive baseline check: git checkout fe0537f -- middleware/src/control-agent.ts then the representative failing suites (cluster2-plans, secrets, 138) → identical 27 pass / 4 fail, same test names. Restored; tree clean.
The 16: cross-user isolation ×5 (already red at baseline), UC-MW-138 ×2, UC-MW-139 ×2, UC-MW-141, UC-MW-142, UC-MW-143, UC-MW-154, UC-MW-160, UC-MW-165, UC-MW-35.
Note: the two reds named in the handoff (UC-OP-02 /metrics, UC-MW-19 second buildApp) did NOT fail in this run — the environment red set is order/load dependent.

Gates unchanged and green: _check.py exit 0 · check-ai-headers.py 573 ok/0 malformed · check-type-mirror.py exit 0 · uc-mw-169 + uc-mw-115 + hardening 30 pass/0 fail · rg 'Showing lines' clean in touched files.
Commits: f41392c, f324d7b, 0717c64 (not pushed).

DONE — full middleware suite run (addendum to #issuecomment-576). `cd middleware && bun test` — 567 pass / 16 fail, 65 files (`ba` exit 1). Every failure is the pre-existing Better Auth / PGlite environment class (12 INTERNAL_SERVER_ERROR, 18 'Cannot writeHead headers', 14 'PGlite is closed'), zero control-agent.ts frames. Decisive baseline check: `git checkout fe0537f -- middleware/src/control-agent.ts` then the representative failing suites (cluster2-plans, secrets, 138) → identical 27 pass / 4 fail, same test names. Restored; tree clean. The 16: cross-user isolation ×5 (already red at baseline), UC-MW-138 ×2, UC-MW-139 ×2, UC-MW-141, UC-MW-142, UC-MW-143, UC-MW-154, UC-MW-160, UC-MW-165, UC-MW-35. Note: the two reds named in the handoff (UC-OP-02 /metrics, UC-MW-19 second buildApp) did NOT fail in this run — the environment red set is order/load dependent. Gates unchanged and green: _check.py exit 0 · check-ai-headers.py 573 ok/0 malformed · check-type-mirror.py exit 0 · uc-mw-169 + uc-mw-115 + hardening 30 pass/0 fail · rg 'Showing lines' clean in touched files. Commits: f41392c, f324d7b, 0717c64 (not pushed).

gotoi-agent commented

2026-10-07 19:18:58 +02:00

Collaborator

REVIEW: FIXED 3 — findings: (1) recall returned every tenant's memory rows for a hostname that collides across orgs — row namespace missing → fixed a8021c0 (scope.ownsMemory keyed on the writing agent's organizationId); (2) forget checked hostname visibility but not row ownership — the plan's promised session/org ownership check was absent, so a cross-org hostname collision let a caller delete another tenant's AI row → fixed a8021c0; (3) forget skipped the visibility scan for a nonexistent id but ran it for an out-of-scope row, a timing asymmetry vs Fix point 5's equivalent timing → fixed a8021c0 (identical unconditional scope evaluation, same 'memory not found'). UNFIXED: audit-mode rollout not implemented — plan decision (enforce-only) and HARD LAW 9 forbids config knobs; the enforce sequence (read-only tools before mutating, run_command last) is recorded in the review report, not the implementer's report; forget of another user's row in the SAME org stays allowed — org is the ownership unit (ADR-005; memory rows carry no author field), documented accepted limit in UC-MW-169 Notes; read_machine_context shows group names from groupsContaining (no org filter) and falls back to legacy unowned machine rows — pre-existing, outside the issue's tool table. Suite: affected suites pass except the known pre-existing cross-user isolation x5; checkers exit 0.

REVIEW: FIXED 3 — findings: (1) recall returned every tenant's memory rows for a hostname that collides across orgs — row namespace missing → fixed a8021c0 (scope.ownsMemory keyed on the writing agent's organizationId); (2) forget checked hostname visibility but not row ownership — the plan's promised session/org ownership check was absent, so a cross-org hostname collision let a caller delete another tenant's AI row → fixed a8021c0; (3) forget skipped the visibility scan for a nonexistent id but ran it for an out-of-scope row, a timing asymmetry vs Fix point 5's equivalent timing → fixed a8021c0 (identical unconditional scope evaluation, same 'memory not found'). UNFIXED: audit-mode rollout not implemented — plan decision (enforce-only) and HARD LAW 9 forbids config knobs; the enforce sequence (read-only tools before mutating, run_command last) is recorded in the review report, not the implementer's report; forget of another user's row in the SAME org stays allowed — org is the ownership unit (ADR-005; memory rows carry no author field), documented accepted limit in UC-MW-169 Notes; read_machine_context shows group names from groupsContaining (no org filter) and falls back to legacy unowned machine rows — pre-existing, outside the issue's tool table. Suite: affected suites pass except the known pre-existing cross-user isolation x5; checkers exit 0.

gotoi-agent referenced this issue

2026-10-07 19:38:34 +02:00

Issue #58: Enforce session and tenant scoping on control chat tool endpoints #65

EduarD3V referenced this issue from a commit

2026-10-07 19:38:37 +02:00

middleware: one dispatch-time scope interceptor for control tools (UC-MW-169)

EduarD3V referenced this issue from a commit

2026-10-07 19:38:37 +02:00

docs(uc): UC-MW-169 control tool session and tenant scoping

EduarD3V referenced this issue from a commit

2026-10-07 19:38:37 +02:00

tests: cite UC-MW-169 in the permissions audit header

EduarD3V referenced this issue from a commit

2026-10-07 19:38:37 +02:00

review(middleware): namespace memory rows to the writing agent's org in recall/forget

gotoi-agent commented

2026-10-07 19:38:46 +02:00

Collaborator

DONE — pushed. PR: #65 (targets agent-develop). Commits: f41392c (dispatch-time scope interceptor, UC-MW-169), f324d7b (UC doc), 0717c64 (audit test header), a8021c0 (review fix: namespace memory rows to writing agent's org in recall/forget + uniform-timing forget deny). review: FIXED 3. Orchestrator gates on final tree: _check.py 0, check-ai-headers.py 0, check-type-mirror.py 0, 'Showing lines' clean; middleware bun test 581 pass / 3 fail — the 3 (UC-MW-19 second buildApp known pre-existing; UC-MW-142 L3 preview, UC-MW-163 delete) pass in isolation 17/0, environment-class, zero control-agent frames. Operator notes: token prerequisite confirmed by implementer (Better Auth session cookies, expire/revoke/user-bound); audit-mode rollout is deliberate non-code (no config knob) — enforce sequencing recorded in review report; accepted limit: forget scopes per-org not per-operator (ADR-005), documented in UC-MW-169 Notes; read_machine_context group-name leak pre-existing, outside the tool table. Refs #58

DONE — pushed. PR: https://git.eduard3v.com/EduarD3V/gotoi/pulls/65 (targets agent-develop). Commits: f41392c (dispatch-time scope interceptor, UC-MW-169), f324d7b (UC doc), 0717c64 (audit test header), a8021c0 (review fix: namespace memory rows to writing agent's org in recall/forget + uniform-timing forget deny). review: FIXED 3. Orchestrator gates on final tree: _check.py 0, check-ai-headers.py 0, check-type-mirror.py 0, 'Showing lines' clean; middleware bun test 581 pass / 3 fail — the 3 (UC-MW-19 second buildApp known pre-existing; UC-MW-142 L3 preview, UC-MW-163 delete) pass in isolation 17/0, environment-class, zero control-agent frames. Operator notes: token prerequisite confirmed by implementer (Better Auth session cookies, expire/revoke/user-bound); audit-mode rollout is deliberate non-code (no config knob) — enforce sequencing recorded in review report; accepted limit: forget scopes per-org not per-operator (ADR-005), documented in UC-MW-169 Notes; read_machine_context group-name leak pre-existing, outside the tool table. Refs #58

EduarD3V referenced this issue from a commit

2026-10-07 20:21:13 +02:00

Merge pull request 'Issue #58: Enforce session and tenant scoping on control chat tool endpoints' (#65) from issue/58-tool-endpoint-scoping into agent-develop

EduarD3V closed this issue

2026-10-07 20:22:49 +02:00

gotoi-agent referenced this issue

2026-10-07 21:22:51 +02:00

Docs sync: verify every document against the code and fix the drift #66