fix(agent): serve only the columns of collections the caller may read

[PMerlet]

Why

A read is permission-checked on the root collection only. Every column a projection, a filter or a sort reaches through a relation path is served with no check on the collection it comes from.

A role with read on cards and nothing at all on holders gets this in full:

GET /forest/cards
Forest-Projection: id,holder:nationalId,holder:dateOfBirth

The header is not even required — GET /forest/cards with no fields[] returns the same columns, because ProjectionFactory.all expands every column of every to-one relation.

The filter is the sharper half. It never returns the column, yet it answers one guess per request:

GET /forest/cards?filters={"field":"holder:nationalId","operator":"starts_with","value":"1850"}

One row back or zero rows back is one digit. Ten iterations per character recovers a national id in full, from a collection with zero granted permissions, without the value ever appearing in a response.

fixes PRD-900

The rule

Check the collection each path ends on. Collections crossed on the way confer and require nothing — reaching one through a relation is a join, not a read — so account:organization:name needs read on organizations alone, and a ManyToMany through-collection stays out of it since it contributes no returned column.

What a denial does depends on who asked for the field:

Named by the caller (fields[], Forest-Projection)	Refused, listing every offending path in one message so a client drops them all and retries once
Never asked for (the ProjectionFactory.all default)	Dropped from the projection — refusing would turn an ordinary listing into a 403

Filters, sorts, extended searches and a chart's group-by or aggregated field are always refused. None has a prunable equivalent: dropping a condition widens the result set, dropping a sort clause silently reorders it, and a grouped-by key is chart output.

The check reads the caller's own query only. Scopes and segments are injected by the agent and may legitimately reference a collection the caller cannot read — a test locks that down.

One case the rule does not describe

A leaderboard counting a relation has no aggregate field, so no path traces back to the collection being counted. What it exposes is that collection's cardinality, which is what browse already governs on /forest/<collection>/count and /relationships/<name>/count — so that is what gets asserted, on the foreign collection rather than on the through-collection a ManyToMany aggregates.

Found by an adversarial review pass on the first revision of this branch, not by the original implementation.

Routes covered

get, list, csv, list-related, csv-related, count, count-related, the chart routes, and update.

update is a write, and in scope anyway. PUT /forest/<collection>/:id re-lists the row it just wrote with ProjectionFactory.all and serializes the result, so a role with edit on cards and no read on holders got holder.nationalId back from a no-op update — the same disclosure as GET /forest/cards/:id, and edit almost always comes with it. The projection there is entirely agent-chosen, so it is redacted, never refused: a write must not 403 because the row happens to carry a relation the caller cannot read.

The filter-shaped half on the destructive routes — delete.ts:45 and dissociate-delete-related.ts:122 accept the caller's filters on the same denied paths — is deliberately out of scope and split to PRD-1012. A delete answers 204 whatever it matched, so there is no response oracle and each probe destroys the rows it measures, while refusing would 403 any bulk delete filtered on a belongsTo sub-field, which the filter bar allows and no front change absorbs.

Sequencing — the front has landed

The front used to ask for these columns on every list and detail view. It stopped in ForestAdmin/forestadmin#9914, which prunes projections by canReadCollection and renders a denied belongsTo as restricted — merged into main on 2026-08-21 (d050ef96). That was the blocker on this PR: merging the agent first would have turned every list or detail view showing a belongsTo on a denied collection into a 403.

One path the front does not cover, by design: chart requests. See the dashboard leaderboard note under Residual cost.

Residual cost

Roles that display a related label today without read on the target lose it until an admin grants it. One permission sweep per project, visible and diagnosable rather than silent.

Dashboard leaderboards that count a relation break for existing roles. A Count leaderboard on holders counting cards, for a role without browse on cards, now returns 403 where it returned counts — and the widget still renders, since dashboard chart visibility follows the chart's own collection, so it shows an error rather than disappearing. ForestAdmin/forestadmin#9914 prunes record, list and export projections and does not touch chart requests, so the sequencing note below does not cover this path. It needs the same permission sweep as the rest, on browse rather than read.

Any client requesting relation fields its role may not read — a customer script, an agent-client integration, an MCP tool — starts getting a 403 where it got a 200. That is the fix working: only integrations running under a role that should never have had the data are affected, and the error names the field and the collection so they can be fixed rather than guessed at.

Not in scope

• browse still stands in for a denied read when the front resolves a get-one through the list route — tracked in PRD-990.
• On instantCacheRefresh: false, each denied check clears and refetches the whole permission cache; denials are routine on the redaction path, so that is one permission fetch per read — tracked in PRD-1002, to be fixed in forestadmin-client rather than here.
• Nothing else known. The extended-search sweep no longer derives its own set: getSearchedFields walks down the stack and the search decorator answers from childCollection, so a field hidden by .removeField is still checked and a replaced search is no longer refused.
• Whether a ManyToMany through-collection needs read of its own is left open, as PRD-900 states.

Tests

test/security/related-read-permissions.test.ts builds the ticket's schema and role, and each case fails against the previous implementation — verified by reverting the guard, not assumed.

🤖 Generated with Claude Code

Note

Enforce read permissions on query fields and projections across agent routes

• Introduces AuthorizationService.redactProjection to drop or reject projection fields whose owning related collections the caller cannot read; implicit projections drop fields, explicit field lists return 403
• Adds AuthorizationService.assertCanReadQueryFields to reject filter/sort/search terms that reference fields from unreadable related collections, applied to list, get, count, csv, chart, and related routes
• Adds FieldPathUtils.getLeafCollection to resolve the owning collection of a colon-separated field path, and SearchCollectionDecorator.getSearchedFields to surface which fields an extended search will read
• Updates CsvGenerator.filterHeader to keep CSV header labels aligned with redacted columns; UpdateRoute.handleUpdate now redacts unreadable fields from the post-update response
• Risk: callers that previously received data from unreadable related collections via implicit projections or filters now get redacted results or 403s — check redactProjection in authorization.ts and route handlers in packages/agent/src/routes/access/ for regressions in existing API consumers

Changes since #1840 opened

• Renamed the explicit property to namedByCaller in the RequestedProjection type and updated all usages throughout AuthorizationService.redactProjection, QueryStringParser.parseProjectionFromHeaderOrQuery, UpdateRoute.handleUpdate, and associated tests [680ec65]
• Updated an explanatory comment in the related-read-permissions.test test suite [aeb3c3c]
• Extended AuthorizationService.assertCanReadQueryFields method to accept optional consumes parameter of type QueryComponent[] with default value ['filter','sort','search'], and added exported QueryComponent type union and ALL_QUERY_COMPONENTS constant [9385592]
• Updated ChartRoute.makeChart, CountRelatedRoute.handleCountRelated, and CountRoute.handleCount handlers to pass ['filter','search'] as third argument to assertCanReadQueryFields [9385592]
• Added test case verifying count route ignores sort parameter on unreadable related fields [9385592]

^{Macroscope summarized 2e3e7b5.}

[linear-code]

PRD-900

[qltysh]

1 new issue

Tool	Category	Rule	Count
qlty	Structure	Function with many returns (count = 5): makeChart	1

[qltysh]

Coverage Impact

This PR will not change total coverage.

Modified Files with Diff Coverage (17)

Rating	File	% Diff	Uncovered Line #s
	packages/agent/src/routes/access/count-related.ts	100.0%
	packages/agent/src/services/authorization/authorization.ts	100.0%
	packages/agent/src/routes/access/list-related.ts	100.0%
	...ages/datasource-toolkit/src/decorators/collection-decorator.ts	0.0%	44
	packages/agent/src/utils/query-string.ts	100.0%
	packages/agent/src/routes/access/get.ts	100.0%
	packages/agent/src/utils/csv-generator.ts	100.0%
	packages/datasource-toolkit/src/index.ts	100.0%
	packages/agent/src/routes/access/csv.ts	100.0%
	packages/agent/src/routes/access/chart.ts	100.0%
	packages/datasource-customizer/src/index.ts	100.0%
	...ages/datasource-customizer/src/decorators/search/collection.ts	100.0%
	packages/agent/src/routes/access/csv-related.ts	100.0%
	packages/agent/src/routes/access/count.ts	100.0%
	packages/agent/src/routes/access/list.ts	100.0%
	packages/agent/src/utils/field-path.ts	100.0%
	...ges/datasource-customizer/src/decorators/search/field-paths.ts	96.4%	43
	Total	98.4%

🤖 Increase coverage with AI coding...

In the `fix/prd-900-agent-read-permission-on-projected-collections` branch, add test coverage for this new code:

- `packages/datasource-customizer/src/decorators/search/field-paths.ts` -- Line 43
- `packages/datasource-toolkit/src/decorators/collection-decorator.ts` -- Line 44

🚦 See full report on Qlty Cloud »

🛟 Help

• Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

• Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

• File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

  • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

[hercemer42]

Validator review — PR #1840

Spec (PRD-900): conforms on every route the ticket's What shipped section lists, and the three documented divergences (leaf-only checking, refuse-named / redact-default, no redaction announcement) are implemented as written — with one gap: the ticket's own read 3 oracle stays reachable through search, not filters, because the guard only inspects search when searchExtended is also set. See the must-fix on authorization.ts:112. The scope/segment exemption is implemented and tested as specified.

---

Claude Opus 5 (claude-opus-5): Should fix

Applies to: packages/agent/src/routes/modification/update.ts:40-46 (not in this diff)

PUT /forest/<collection>/:id re-lists the record with ProjectionFactory.all(this.collection) and serializes the result straight into the response, so a role with edit on cards and no read on holders gets holder.nationalId back from a no-op update. It is the same disclosure this PR closes on GET /forest/cards/:id, one HTTP verb away, and a role with edit almost always has it.

modification/delete.ts:43-45 and dissociate-delete-related.ts:119-122 accept filters on the same denied related paths — destructive, so a poor oracle, but the guard is absent there too. create.ts:41 serializes only caller-supplied PKs, so it does not leak.

Routing update.ts through redactProjection with explicit: false is a two-line change and closes the read-shaped half. The filter-shaped half on the modification routes is a follow-up worth naming in the ticket's Not in scope, since "serve only the columns of collections the caller may read" reads as a claim about the whole agent.

---

Claude Opus 5 (claude-opus-5): Should fix

Applies to: the PR as a whole

No ADR records the decisions this diff embeds, and agent-nodejs has no docs/adr directory at all — the org-wide corpus is four records, none touching permissions (control query verified, so this is an absence rather than a failed search). Three calls here are hard to reverse, surprising without context, and a real trade-off:

• explicitly named denied paths are refused while the implicit ProjectionFactory.all expansion is silently redacted — one class of request 403s, another is quietly narrowed;
• browse stands in for read on a Count leaderboard;
• rendering scopes, segments and a customized replaceSearch are exempt.

PRD-900 carries the rationale and the comments at authorization.ts:55-60,88-92,148 carry part of it, but this is a cross-SDK contract — the ticket says agent-ruby should be built from it, and agent-python/php have no record either. Worth running /adr in this repo so the next implementer inherits the reasoning rather than the behaviour.

[PMerlet]

Replying to the two findings in the validator review body, which have no inline thread.

update.ts:40-46 — fixed

Verified independently before fixing, and your map of the three sites is exact: ProjectionFactory.all appears in exactly one route (update.ts:43), create.ts:41 serializes only caller-supplied data, and the two filter-shaped sites are delete.ts:45 and dissociate-delete-related.ts:122.

Fixed in bc3925a as suggested — routed through redactProjection with explicit: false. Redaction rather than refusal is load-bearing here: the projection is entirely agent-chosen, so a PUT must not 403 because the row it wrote happens to carry a relation the caller cannot read. Two tests, one per side of the branch: the redacted case, and a readable relation surviving so the fix cannot be satisfied by dropping relations wholesale.

You were right that the PR title reads as a claim about the whole agent. The "Routes covered" section now names update and says why a write is in it.

The filter-shaped half is deferred, not overlooked — PRD-1012, and named in PRD-900's Not in scope. Two reasons, and the first weakens your "poor oracle" reading further: a delete answers 204 whatever it matched, so there is no response channel at all — the caller learns only by observing which rows vanished, and each probe destroys the rows it is measuring. Second, assertCanReadQueryFields is a one-line addition to both sites, but it would 403 any bulk delete filtered on a belongsTo sub-field, which the filter bar lets a user build and which #9914 does not prune. That is the same breaking-change class as the leaderboard, on a destructive path, and it needs its own sequencing decision rather than riding on this one.

No ADR — declined, deliberately

Not adding one. PRD-900 carries all three decisions with their counter-arguments, and its What shipped section is explicitly marked as authoritative over the earlier decision for whoever ports this — that is the artefact agent-ruby will be built from, and it is linked from this PR.

The gap you name is real: Linear is less discoverable from a repo than docs/adr would be. But agent-nodejs has no ADR directory, so this would create the convention rather than follow it, and a corpus of one that nobody else maintains is worse than a ticket people already read. Worth raising as its own decision for the SDK repos rather than settling it inside a security fix that is waiting on a front PR to land.

qlty structure findings

• lenientGetSchema complexity 16 / nesting level 4 — fixed in bc3925a. The real branch is on suffix, so it is now taken once at the top instead of inside the loop. continue guards were the first attempt and ESLint rejects them (no-continue), which is why it reads this way. A named SEARCHABLE_THROUGH constant with a type predicate replaces the inline three-way type disjunction.
• makeChart many returns (5) — left as is. git show origin/main confirms it already had 5 returns; it is flagged only because this PR added a line to the function. It is a switch dispatching one return per chart type, which is the clearest form available.

[hercemer42]

Validator re-review — PR #1840

Follow-up to run 1840-20260820144736-hercemer42, re-verified against bc3925a4. All nine inline findings from that review are addressed: seven fixed, one deferred to PRD-1002 with reasoning I accept, and the update.ts disclosure fixed in bc3925a4 — the exact-projection tests on both sides of the branch are the right way to pin it. The lenientGetSchema refactor is semantically identical to what it replaced. CI is green on this head.

Spec (PRD-900): the code conforms. The ticket does not — see the finding below; it is a documentation gap with a code consequence for the SDK ports, not a defect in this diff.

---

Claude Opus 5 (claude-opus-5): Should fix

Applies to: PRD-900

The ticket does not record the search fix. No mention of the relation.column:term dot syntax, of getSearchedFields, or of the oracle that motivated a88dbfed / 78944c53 — and What shipped still describes the implementation those commits replaced:

Filters, sorts, extended search, and a chart's group-by or aggregated field are always refused

A customized replaceSearch is exempt, deliberately. Its condition tree is built inside SearchCollectionDecorator.refineFilter, long after the route has run its check, so a handler searching holder:nationalId is never verified.

Both sentences reason from a route that derives the check from the schema. The route no longer derives anything: it asks the stack, and the exemption is now the decorator returning null because it owns the decision — which is a better rule, stated nowhere.

This matters because of the argument for not adding an ADR, which I accept on its merits: PRD-900 carries the decisions, and it is what agent-ruby will be built from. It now carries three of four, and the missing one is the largest change in the follow-up. Built from the ticket as it stands, the Ruby port lands on the original design — enumerate to-one relations at the route, gate on searchExtended, never look at the search string — which is the hole this PR closed. The dot syntax is documented end-user behaviour (/product/execute/browse § Advanced search syntax), so the port would ship it reachable.

The other three follow-ups are recorded well: update in Routes covered, the dashboard leaderboard under Newly settled, the delete filters split to PRD-1012 in Not in scope. This is the one gap, and it is the one that carries a vulnerability into the next SDK.

[hercemer42]

Three inline threads from the re-review are verified at aeb3c3c2 and resolved. The getLeafCollectionName fix went further than the finding asked — you closed it instead of accepting the reachability argument, and the withPks comment now names a case I had not raised (a foreignKeyTarget that is not the primary key, where account:id genuinely carries something the row does not). Both correct.

Pushing back on the one body finding, which is a Should fix and still open.

PRD-900 does not record the search fix. Re-checked just now: no mention of the relation.column:term dot syntax, of getSearchedFields, or of the oracle anywhere in the description. The ticket was edited at 08:45Z, after the re-review posted, so this is not a lag — and both passages that describe the deleted implementation are still there verbatim:

Filters, sorts, extended search, and a chart's group-by or aggregated field are always refused

A customized replaceSearch is exempt, deliberately. Its condition tree is built inside SearchCollectionDecorator.refineFilter, long after the route has run its check, so a handler searching holder:nationalId is never verified.

The second one is the sharper half, and it is not only stale wording — the reason is now wrong. The exemption no longer exists because the route's check runs too early; it exists because SearchCollectionDecorator.getSearchedFields returns null, deliberately, as the layer that owns the decision. A port that implements the recorded reason builds the check at the route, sees that a replaced search is unverifiable there, and concludes it has parity — while the dot-syntax path it never looked at stays open. That path is documented end-user syntax (/product/execute/browse § Advanced search syntax), so it ships reachable.

This is the same ticket you offered in place of an ADR, and I accepted that on its merits — a corpus of one is worse than a ticket people read. The trade only holds while the ticket is complete. It now carries update, the dashboard leaderboard and the delete split, and not the largest change of the three rounds.

Concretely, what agent-ruby needs from What shipped: the route asks the collection what a search will reach rather than deriving it from the schema; the answer is resolved below the publication and renaming layers; relation.column:term reaches a relation with no searchExtended; to-many hops resolve too; and an unknown answer serves the request, which is where the replaceSearch exemption now lives. Happy to be told this belongs in an ADR after all, or in the Ruby ticket instead — but not nowhere.