fix: enforce agent ownership on GET /api/v1/external/invocations/{id}

[kam-validmind]

Pull Request Description

What and why?

GET /api/v1/external/invocations/{id} had no ownership check, so any
agent with a valid bearer token could read any other agent's invocation
record — tool name, input, output, and error — just by knowing or guessing its
invocation_id. There is no list-all endpoint for non-admins, so this wasn't
blind-enumerable, but it's a real gap in access control: an invocation record
can carry secrets or internal system output that was only ever meant for the
agent that submitted it.

The write side of this same route family (PATCH .../invocations/{id}, handled
by RecordExecution) already enforces this — it checks that the caller's
authenticated agent identity matches inv.AgentID before allowing an update.
Service.Get (the read side) never got the equivalent check when it was
written, even though it sits one method away in the same file.

Fix: Extracted the ownership check out of RecordExecution into a shared
checkOwnership(ctx, inv) helper in internal/invocation/service.go, and
applied it to Service.Get too. Same rules as before:

• Only enforced when the request carries an authenticated agent identity
  (i.e. it went through the agent-runtime OAuth middleware). No-auth mode,
  the operator/review UI routes, and the in-process managed-agents watcher
  never carry an agent identity, so they're unaffected.
• Invocations submitted anonymously (no agent_id) are exempt — there's no
  owner to check against.

Before:

Caller	Owns the invocation?	GET response
Agent A	Yes	200, full record
Agent B	No	200, full record (bug)

After:

Caller	Owns the invocation?	GET response
Agent A	Yes	200, full record
Agent B	No	404 (identical body to a truly-missing invocation)

We deliberately made the "not yours" case return the exact same 404 body as
"doesn't exist," rather than a 403. A 403 would confirm to Agent B that the ID
is real, just not theirs — that's its own small information leak (existence
disclosure). A 404 tells them nothing either way.

Sequence diagram — before the fix (vulnerable)

Agent B never submitted this invocation and has no relationship to it, but the
API hands over Agent A's data anyway because Service.Get never checks who's
asking.

sequenceDiagram
    actor A as Agent A (owner)
    actor B as Agent B (attacker)
    participant API as Atryum API
    participant Svc as invocation.Service
    participant DB as Database

    A->>API: POST /api/v1/external/invocations<br/>(bearer token: agent A)
    API->>Svc: Submit(ctx[agent=A], ...)
    Svc->>DB: INSERT invocation (agent_id = "A")
    DB-->>Svc: invocation_id = inv_123
    Svc-->>API: inv_123
    API-->>A: 200 OK { invocation_id: "inv_123" }

    Note over B: Agent B somehow learns/guesses "inv_123"

    B->>API: GET /api/v1/external/invocations/inv_123<br/>(bearer token: agent B)
    API->>Svc: Get(ctx[agent=B], "inv_123")
    Svc->>DB: SELECT invocation WHERE id = "inv_123"
    DB-->>Svc: invocation (agent_id = "A", input, output, error)
    Note over Svc: ❌ no check that ctx's agent ("B")<br/>matches invocation's agent_id ("A")
    Svc-->>API: full invocation record
    API-->>B: 200 OK — Agent A's tool input/output/error

Loading

Sequence diagram — after the fix (protected)

Same request from Agent B now fails the ownership check inside Service.Get
before any data is returned, and gets back the same 404 shape a nonexistent ID
would.

sequenceDiagram
    actor A as Agent A (owner)
    actor B as Agent B (attacker)
    participant API as Atryum API
    participant Svc as invocation.Service
    participant DB as Database

    A->>API: POST /api/v1/external/invocations<br/>(bearer token: agent A)
    API->>Svc: Submit(ctx[agent=A], ...)
    Svc->>DB: INSERT invocation (agent_id = "A")
    DB-->>Svc: invocation_id = inv_123
    Svc-->>API: inv_123
    API-->>A: 200 OK { invocation_id: "inv_123" }

    Note over B: Agent B somehow learns/guesses "inv_123"

    B->>API: GET /api/v1/external/invocations/inv_123<br/>(bearer token: agent B)
    API->>Svc: Get(ctx[agent=B], "inv_123")
    Svc->>DB: SELECT invocation WHERE id = "inv_123"
    DB-->>Svc: invocation (agent_id = "A", input, output, error)
    Svc->>Svc: checkOwnership(ctx[agent=B], invocation)
    Note over Svc: ✅ "B" != "A" → return ErrNotOwner
    Svc-->>API: ErrNotOwner
    API-->>B: 404 Not Found { "error": "not found" }<br/>(same body as a nonexistent id)

    Note over A: Agent A can still GET inv_123 normally —<br/>ctx[agent=A] matches agent_id "A", check passes

Loading

How to test

Automated (this is what a reviewer should run):

go build ./...
go vet ./...
gofmt -l internal/invocation/service.go internal/invocation/record_execution_test.go \
         internal/api/handlers.go internal/api/handlers_test.go
go test ./internal/invocation/... ./internal/api/...

New tests added:

• internal/invocation/record_execution_test.go → TestGetEnforcesAgentOwnership
  (service-level, real in-memory DB): different agent rejected, owning agent
  allowed, no-identity caller exempt, anonymous invocation exempt. Mirrors the
  existing TestRecordExecutionEnforcesAgentOwnership for the write path.
• internal/api/handlers_test.go → TestExternalInvocationGetErrorStatusMapping
  (handler-level): asserts ErrNotOwner and "missing invocation" both produce
  status 404 with the byte-identical response body, so a future regression
  that makes them distinguishable would fail the test.

Manual local check, if you want to see it end-to-end:

The stock local dev stack (docker compose --profile dev up) ships
atryum.toml with every [[auth]] block commented out, so there's no OAuth
to configure — Atryum's noAuthAgentIDHint middleware (internal/api/handlers.go)
lets an unauthenticated caller simulate an agent identity with a plain
?agent_id= query param instead. That's what the checks below use, and it
exercises the exact same auth.AgentIDFromContext(ctx) path a real bearer
token's client_id claim would populate — no need to stand up Keycloak.

1. Rebuild/restart the service from this branch: docker compose --profile dev up -d --wait --build atryum.
2. Submit an invocation as agent A:

   curl -X POST "localhost:8090/api/v1/external/invocations?agent_id=agent-a" \
     -H "Content-Type: application/json" \
     -d '{"source":"demo","tool":"bash","input":{"cmd":"whoami"}}'
   # → note the returned invocation_id

3. Read it back as a different agent:

   curl -i "localhost:8090/api/v1/external/invocations/<that_id>?agent_id=agent-b"
   # before this fix: 200 with agent A's data. After: 404 {"error":{"message":"not found"}}

4. Repeat as the owner — still works:

   curl -i "localhost:8090/api/v1/external/invocations/<that_id>?agent_id=agent-a"
   # 200, unaffected

If your environment does have real OAuth configured (an active [[auth]]
block pointed at Keycloak/Auth0/etc.), the same check applies but swap the
?agent_id= query param for Authorization: Bearer <token> headers from two
different agents' client_credentials tokens — the code path is identical,
just sourced from the verified JWT's agent_id_claim instead of the query hint.

To see it in a browser instead of a terminal: open any page already served by
Atryum (e.g. http://localhost:8090/healthz) so DevTools' Console runs
same-origin, then paste:

const id = "<that_id>";
const asOwner = await fetch(`/api/v1/external/invocations/${id}?agent_id=agent-a`);
console.log("agent-a:", asOwner.status, await asOwner.json());
const asOther = await fetch(`/api/v1/external/invocations/${id}?agent_id=agent-b`);
console.log("agent-b:", asOther.status, await asOther.json());

What needs special review?

• Status code choice for GET (404, not 403). Worth a second opinion on
  whether hiding existence is the right call here vs. matching PATCH's 403.
  We chose 404 because a read endpoint returning 403 would still leak "this ID
  is real" to a caller who shouldn't know that.
• checkOwnership now sits on both the read and write path. Double-check
  the extraction didn't change RecordExecution's behavior — it's a straight
  lift of the same logic, but it's the write path for tool-execution results,
  so it's worth confirming nothing shifted.

Dependencies, breaking changes, and deployment notes

• No migrations, no schema changes, no new config/env vars.
• No dependent or blocking PRs.
• Behavior change: any caller previously relying on being able to GET
  an invocation belonging to a different agent identity will now get a 404
  instead of 200. This should never have worked in the first place, so we
  don't expect a legitimate integration depends on it, but flagging since it
  is a visible response-code change on a public API route.

Data impact

• Yes — this change is data-impacting
• No — this change is not data-impacting

Justification: This is an access-control check on an existing read endpoint.
No database schema, stored data, or business-process behavior changes — the
same invocation record is returned as before, just gated to its owning agent.

Release notes

Fixed an access-control gap where an agent could read another agent's
invocation details (tool name, input, output, error) through the external
invocations API if it obtained that invocation's ID. Reads are now scoped to
the invocation's owning agent, matching the access control already enforced
when reporting execution results.

Checklist

• What and why
• Screenshots or videos (Frontend) — N/A, backend-only change
• How to test
• What needs special review
• Dependencies, breaking changes, and deployment notes
• Labels applied — apply bug when opening the PR
• Data impact classified
• PR linked to Shortcut — add ticket link when opening the PR
• Unit tests added (Backend)
• Tested locally
• Documentation updated (if required) — N/A, no user-facing docs affected
• Environment variable additions/changes documented (if required) — N/A, none added

[mdeyell-valid-mind]

nice