SECURITY: 13 #[NoAdminRequired] endpoints unguarded — PUT /api/skills/{id} is persistent prompt injection into other users' agents (gate-7 reports 0; .github#365)

[rubenvdlinde]

Summary

13 #[NoAdminRequired] endpoints act on a caller-supplied agentId / skill id
/ template id with no ownership check. The worst converts a one-shot write into
persistent, fan-out prompt injection against other users' agents.

gate-7 reports 0 for hermiq and always has.

⚠️ This is NOT the #353/#360 false-positive class. I checked every
candidate against it first. canUserAccessAgent() exists twice
(lib/Controller/AgentsController.php:650,
lib/Controller/AgentVersionController.php:271) and is called at
AgentsController.php:164,221, AgentVersionController.php:251,
ChatStreamController.php:621. None of the 13 controllers below calls it —
MemoryController, SkillController and AgentTemplateController do not even
inject a service that could. Gate package measured is
ConductionNL/.github @ 112d4c9, i.e. post-#368.

Why this was never reported

.github#365 — gate-7 accepts Http::STATUS_UNAUTHORIZED / 401 as an
authorisation guard; 28 of hermiq's 43 controller files carry that preamble.

Compounding (.github#372): lib/Settings/hermiq_register.json declares 29
schemas, 1 with an authorization block — Agent: {"read": ["authenticated"]}.
AgentTemplate, Skill, Memory, Budget, SkillDraft are all null, so
OpenRegister grants open read and write on all of them
(ConductionNL/openregister#1955, #2011).

checker	findings
canonical (what CI reports today)	0
same run, UNAUTHORIZED/401 dropped from _GUARD_BODY_RE	35

20 take a caller-supplied id; hand-verification confirmed 13 REAL, 7
downgraded (listed at the bottom, in the interest of not repeating
.github#365's over-count).

Positive control — hermiq DOES own-check Skills, just not on these endpoints

// lib/Controller/SkillMaturityController.php:209
if ($this->seedCustody->actsAsOwner(owner: $skill->getOwner(), uid: $uid) === false) { return null; }
// -> lib/Service/SeedCustodyService.php:91
if ($stored === $uid && $uid !== '') { return true; }

SkillMaturityController::qualify gates on that. SkillController::update /
install / uninstall mutate the same Skill object with no such call. These
are omissions, not a missing convention.

🔴 Worst — PUT /api/skills/{id} (SkillController::update, :233-250)

No guard in the controller; lib/Service/SkillService.php:286-307
updateSkill() does getSkill() then saveObject(...) and never reads
owner/createdBy.

Skill bodies are not inert documents. lib/Service/Engine/Engine.php:

:339  $skillBundle = $this->contextAssembler->assembleSkillsForRun(...);
:344  $contextPreamble = ltrim($contextPreamble."\n\n".$skillText, "\n");

The rewritten skill text is folded straight into the system-prompt preamble of
every run of every agent that has the skill installed. So this is not a
one-shot write IDOR — it is persistent, fan-out prompt injection. The
attacker never touches the victim's agent object (which OR would refuse, since
Agent declares read but no write action); they poison a shared dependency the
victim's agent pulls in at run time.

Two things make it worse rather than better: the app already proves it knows the
correct predicate (SkillMaturityController.php:209) and omits it on the write
path, and GET /api/skills hands every authenticated user the complete list of
targets to choose from.

The rest

route	method	verdict	evidence
POST /api/agents/{agentId}/memory/consolidate	MemoryController::consolidate	REAL — destructive	:204-224 no guard; MemoryService.php:263 $data['entries'] = $normalised; — the caller-supplied entries array replaces the agent's entire memory. Wipe-or-rewrite of any agent's memory
PUT /api/agent-templates/{id}	AgentTemplateController::update	REAL — write	:231-247 no guard; AgentTemplateService.php:206-221 merges the caller payload and saveObjects it. createdBy is only written (:184), never compared
DELETE /api/agent-templates/{id}	AgentTemplateController::destroy	REAL — hard delete	:263-275; AgentTemplateService.php:235-242 is a bare deleteObject(uuid: $templateId, …). ⚠️ Contrast AgentTemplateController.php:404, where approve does gate via requireAction('agenttemplate.approve-quarantined')
POST /api/agents/{agentId}/memory	MemoryController::addMemory	REAL — write	:114-131 no guard; MemoryService.php:146 getMemory() filters on ['agentId' => $agentId] only. Attacker-authored "facts" injected into another user's agent run loop
POST /api/skills/{id}/install	SkillController::install	REAL — write	:272-294; SkillService.php:369-398 installOnAgent() mutates installedOn with no owner check. The agent-side syncAgentSkillInstalls (:395) is best-effort and OR would deny it for a foreign agent — but the Skill-side write lands
DELETE /api/skills/{id}/install/{agentId}	SkillController::uninstall	REAL — write	:310-330; SkillService.php:475-503 array_filters the agent out of installedOn, no owner check
POST /api/agents/{id}/run-on-object	AgentRunController::runOnObject	REAL — split	The object half is properly guarded: :251-257 find(id: $objectId, …, _rbac: true, _multitenancy: true) → :264 return null → :164 STATUS_NOT_FOUND. The agent half is not: :292 return $this->agentMapper->findByUuid($ref);, no canUserAccessAgent(). Any authenticated user can invoke a private agent — its prompt, model policy, tools and budget — they were never invited to
GET /api/agent-templates/from-agent/{agentId}/export	AgentTemplateController::export	REAL — read	:291-303 no guard; AgentTemplateService.php:273 'systemPrompt' => (string) ($data['prompt'] ?? '') plus tools/skillRefs. Dumps any agent's system prompt — including an isPrivate:true agent that AgentsController.php:221 would have refused
GET /api/agents/{agentId}/user-profiles	MemoryController::userProfiles	REAL — read	:147-160; listUserProfiles() filters on agentId only. Per-subject-user learned profiles — the most PII-dense object in the app
GET /api/agents/{agentId}/recall?q=	MemoryController::recall	REAL — read	:245-259 → recallSessions(agentId, query), agentId-only filter. Free-text search across another agent's conversation turns
GET /api/agents/{agentId}/memory	MemoryController::memory	REAL — read	:81-95 → MemoryService.php:148 findOne(schema: MEMORY_SCHEMA, filters: ['agentId' => $agentId])
GET /api/agents/{agentId}/sessions	MemoryController::sessions	REAL — read	:175-188 → listSessions(agentId), agentId-only filter

Downgraded — 7 candidates that are NOT IDORs

Recorded explicitly so this issue is not another over-count:

1. GET /api/agent-templates/{id} — ::show. AgentTemplateService.php:130-145 list() is an unfiltered findAll(config: ['limit' => 200]), so index already returns every template to every authenticated user. show crosses no new boundary.
2. GET /api/agent-templates/{id}/export — ::exportPackage. :299-307 serialises the same object show returns.
3. GET /api/skills/{id}/export — SkillController::export. skill#index (:143) already returns full skill objects. (Also mis-typed in my first pass as a write — it is GET, routes.php:308.)
4. GET /api/skills/{id}/drafts — SkillDraftController::index. Subsumed by the org-open Skill catalog. Flagged as the least confident downgrade — it is only "not a new leak" because SkillController::index is already open.
5. GET /api/analytics?agentId= — AnalyticsController::index. agentId is a pure narrowing filter: AnalyticsService.php:221 findAll(config: ['limit' => 1000]) (tenant-scoped, _multitenancy: true by default) then :232 if ($agentId !== null && … !== $agentId) { continue; }. Omitting it returns a superset.
6. GET /api/agents/{agentId}/budget-estimate — BudgetController::estimate. Same narrowing path via BudgetService.php:412.
7. GET /api/budgets/status — BudgetController::status. GUARDED-BELOW by OR multitenancy: BudgetService.php:796-799 findAll() runs _multitenancy: true, so the candidate set is already the caller's organisation; :811 can only narrow. A foreign org id yields the 'configured' => false stub at :508-518.

Also correcting my own first pass: none of these endpoints is unrouted. All 20
resolve in appinfo/routes.php @ a4eca493 (memory :201-211, analytics :214,
budget :265,270, skills :308,323,324,326, drafts :354, templates
:386-405, agentRun :77-109). My route parser missed them; the endpoints are
live.

Suggested remediation shape (measurement pass — not implemented here)

1. SeedCustodyService::actsAsOwner() already encodes the model for Skills.
   Call it on update / install / uninstall, not just on qualify.
2. canUserAccessAgent() already encodes it for Agents. Call it in
   MemoryController (all five methods), AgentTemplateController::export and
   AgentRunController::resolveAgent (:292). AgentRunController already
   guards the object half correctly — the agent half is one line.
3. Declare authorization blocks on Skill, Memory, AgentTemplate,
   SkillDraft (wave-12 follow-up: fleet sweep of *_register.json to add authorization blocks ahead of default-closed flip openregister#2011). Agent already shows the
   pattern works: it declares read and therefore correctly denies writes.
4. Treat PUT /api/skills/{id} as a prompt-injection vector, not just an
   IDOR — whatever guard lands should be accompanied by a test that a
   non-owner's skill edit cannot reach another agent's run preamble.

---

Confirmed by: canonical ConductionNL/.github @ 112d4c9 (post-#368);
hermiq @ origin/development a4eca493. Full harness and reproduction:
/home/rubenlinde/fleet-board/findings/gate7-fleet-reaudit.md.
Related: ConductionNL/.github#365, ConductionNL/.github#372,
ConductionNL/openregister#1955, ConductionNL/openregister#2011.

[rubenvdlinde]

The 13 are fixed in #188 — and the 7 downgrades are now MEASURED, not reasoned

The 13

All thirteen were reproduced on a two-account rig before any code changed (12 × 200, run-on-object 202, and the writes landed — a rewritten skill body, an emptied memory array, a hard-deleted template). After #188, on the same rig with the same accounts: all thirteen → 404, every positive control still 200, nothing written.

⚠️ One correction to my own first pass on POST /api/agents/{id}/run-on-object. It initially returned 404 to the attacker — and also to the owner and to admin. That is a null result, not a guard: the endpoint resolves via OpenRegister's native AgentMapper (oc_openregister_agents), while every other agent-scoped route in this app uses the ObjectEntity agent schema. Two identity spaces behind similar-looking route prefixes, and the native table was empty on a fresh instance. With the row present: owner 202, attacker 202. Had I not probed as the owner in the same run, I would have recorded a broken endpoint as a guarded one.

The 7 downgrades — confirmed by measurement

The downgrades were argued from code. I have now run each of the seven as the attacker and as the victim, back to back, and compared the responses:

endpoint	attacker	victim	same bytes?
GET /api/agent-templates/{id}	200	200	✅ identical
GET /api/agent-templates/{id}/export	200	200	✅ identical
GET /api/skills/{id}/export	200	200	✅ identical
GET /api/skills/{id}/drafts	200	200	✅ identical
GET /api/analytics?agentId=	200	200	✅ identical
GET /api/agents/{id}/budget-estimate	200	200	✅ identical
GET /api/budgets/status	200	200	✅ identical

The downgrade reasoning holds: none of these crosses a boundary that its own sibling list endpoint does not already cross, so guarding them individually would be theatre while index stays open. GET /api/skills/{id}/drafts — flagged as the least confident downgrade — behaves exactly like the rest.

🟡 But that is a product question, not a closed one

Two of those rows deserve to be read as more than "not a regression":

• GET /api/agent-templates/{id} and …/export return a template's systemPrompt to any authenticated user in the organisation, because AgentTemplateService::list() is an unfiltered findAll(config: ['limit' => 200]) and index already hands out every template.
• GET /api/skills/{id}/export returns the full skill body on the same basis.

That is the designed behaviour of an org-shared gallery and catalog, and #188 deliberately leaves it alone — narrowing a catalog is a product decision, and doing it inside a security fix would blur which change closed which hole.

⚠️ It is worth separating clearly from finding #11, which is fixed: GET /api/agent-templates/from-agent/{agentId}/export exports an Agent, including a isPrivate: true one that AgentsController::show() would have refused. Exporting a template is sharing something already shared; exporting an agent is not.

Open question for the product owner, not for this PR: should the template gallery and the skill catalog be org-wide readable, or scoped to a group / an explicit published state? If the answer is "scoped", that is its own change with its own migration — the field to scope on does not exist today, and inventing one inside an IDOR fix is exactly what should not happen.

Why gate-7 will not tell you any of this

gate-7 printed PASS on this repo both before and after the thirteen were closed — canonical package ConductionNL/.github @ 57bcb2b, identical scope on both trees. .github#365: the checker accepts Http::STATUS_UNAUTHORIZED / 401 as an authorisation guard, and 28 of hermiq's 43 controller files carry that preamble.