Access, tenant and credential checks that can be bypassed

Fixes every finding in #3620606, one commit per finding, each with a test seen to fail against unmodified code.

The request no longer decides the step's terms

The entity task screen took its form operation from the query string. A form operation selects the form handler the entity type builds, and node declares a delete form, so a task candidate holding edit access could append op=delete and be handed a working delete form for content they had no delete permission on. Both values the request supplied (the form operation and the attachment key) are the step's own configuration, so they are read from the definition the instance runs, through the existing declared-interaction accessor. The redirect that reaches the screen no longer carries them at all: left there they would read as the source of the choice to whoever came next, which is how the choice ended up in a query string in the first place. A step naming a form the entity type does not declare now answers 404 and logs the step, rather than reaching the form builder as an unknown handler and leaving the actor a 500.

The comment page took "require a comment" from the query string, so an actor who removed the argument removed the requirement and the form validated nothing. The page already reads that step for its outcomes; it now reads the requirement there too, through the interaction resolver, which also covers a comment interaction declared by a chain rather than by the node. An embedder still passes it as an argument, which is code and not the actor. The argument is gone rather than ignored.

Tenant confinement

Three paths reached a token or a row outside the tenant the request was acting in:

  • The operation resumer. A continuation handle is client-supplied, and the only authorization was "parked" plus the assignment matcher, which is tenant-blind by construction: it intersects the node's candidates with the viewer's tokens, and those are site-wide, so a role that staffs a step in one tenant matches the same step in another. Both sibling paths already gated on the tenant.
  • The resume event, the integration-neutral seam, which carries an opaque id. The ECA action and the local API client both confine first; this did not.
  • The delegation entity's access handler. Every listing is scoped to the acting tenant, so an id in a URL was the one way to reach a row from the wrong realm.

Delegation

Only the delegate's right to act was ever checked. The form refuses a delegate who cannot process tasks, and the "Delegated to me" route gates on the same permission for the same stated reason, but nothing asked whether the delegator may act on their own work. Both delegation permissions are independent of that one, so a delegator could declare cover having never held it, or lose it afterwards, and the delegate kept reaching that person's work — including the pools that delegator's roles staff, which the delegate need not be in.

The rule lands in the resolver, the one seam every consumer goes through: a declared delegator who is blocked, or who does not hold the permission, is not returned. The row is read, never rewritten, so the cover works again the moment the permission comes back. The answer now varies with those accounts, so the resolver accumulates their cache tags the way it already accumulates the nearest date boundary. Four kernel classes were resting on the defect (their fixtures granted no permission to anyone) and now hold it.

The rest

  • Injected token syntax was evaluated in Easy Email notifications. The channel pre-renders the Orchestra tokens, and Easy Email renders the same subject and bodies twice more at send time, so token syntax that arrived in a value (a process variable filled from a web form, a label a submitter chose) was evaluated by those later passes, and had already been counted by Easy Email's own unsafe-token check, which exists to judge author-written text. Core's token service takes a callback for exactly this: the bracket that opens a token is escaped in the markup body and dropped from a token-shaped run in the subject and plain body, both repeating until nothing matches. Without it the new test's subject reads Pay site@example.com today for a label of Pay [site:mail] today.
  • The Reference and Trace Views columns declared no cacheability at all, while rendering a link only to a viewer who may open the page behind it. A field handler reaches a display's cache metadata only by implementing the interface, because FieldPluginBase, unlike FilterPluginBase, declares none of its own; the two siblings that do the same work already declare it. Every shipped display carrying either column also carries an exposed filter and stores a zero max-age, so nothing shipped was cached wrongly: what this protects is the view a site builds itself.
  • An anonymous account counted as the assignee of a task recording 0 as its assignee, on the two routes that carry no permission requirement of their own. Nothing writes that today, but the module already states elsewhere that an anonymous account is never an assignee nor a completer. Three surfaces ran the same two-line comparison to ask this, so the rule now lives in one named predicate they share.
  • The reassign permission is described as reassigning a task and returning it to its pool, and the action gate treats it as a full bypass. That is right, because a holder can reassign any task to themselves and then act on it, so gating the actions separately would add a step and refuse nothing. Nothing said so: not the permission, not the docs, not the bypass, which carried no comment. All three now do.

Docs and the French catalog move with the changes; a new log line is recorded as a log message rather than translated.

Edited by Frank Mably

Merge request reports

Loading
Loading