Skip to content

gate-17: three 'pass-through' methods all have callers, and one carries an _rbac:false policy the detector strips #182

Description

@rubenvdlinde

gate-17 redundant-controller reports 3 pass-through methods on hermiq. All three have real callers, and for one of them the detector strips the very argument that makes it not a pass-through. Filing rather than deleting working code.

Measured on development @ cd23f547, run 31490144919; reproduced locally against ConductionNL/.github@main.

lib/Service/AgentTemplateService.php:157 method=get     rule=pass-through-to-ObjectService
lib/Service/AgentTemplateService.php:235 method=delete  rule=pass-through-to-ObjectService
lib/Service/BudgetService.php:659        method=delete  rule=pass-through-to-ObjectService

The gate's premise does not hold here

detect-redundant-controllers.py documents its rationale as ADR-022: the frontend already reaches OpenRegister CRUD directly via useObjectStore, so a per-schema wrapper "ships dead code — observed on decidesk#60: 260 lines with zero callers from the frontend."

Callers exist for all three:

lib/Controller/AgentTemplateController.php:179   $this->templateService->get(templateId: $id)
lib/Controller/AgentTemplateController.php:270   $this->templateService->delete(templateId: $id)
lib/Service/AgentTemplateService.php:208,301,394,454   $this->get(templateId: $templateId)
lib/Controller/BudgetController.php:279          $this->budgetService->delete(budgetId: $budgetId)

AgentTemplateController is a guarded endpoint gating through ActionAuthService::requireAction('agenttemplate.approve-quarantined') per ADR-023 — a check the generic OpenRegister object-patch path does not express. So the frontend does not reach this through useObjectStore, and deleting these methods would inline objectService->find(...) plus the REGISTER_SLUG / TEMPLATE_SCHEMA constants into six call sites.

BudgetService::delete() is not a pass-through at all

public function delete(string $budgetId): void
{
    $this->objectService->deleteObject(
        uuid: $budgetId,
        register: self::REGISTER_SLUG,
        schema: self::BUDGET_SCHEMA,
        _rbac: false,
        _multitenancy: false
    );
}

_rbac: false, _multitenancy: false is a deliberate policy decision, paired with BudgetController::destroy() performing its own organisation-ownership check via mayAdminister() immediately before the call. The detector reduces the body to "one call to a canonical ObjectService method" and never looks at the arguments, so a deliberate RBAC bypass and a plain CRUD wrapper are indistinguishable to it.

That one is worth a second look regardless of this gate: it is the shape most worth keeping in ONE place rather than inlining into a controller.

Suggested gate change (ConductionNL/.github)

A body whose single ObjectService call passes _rbac: false or _multitenancy: false is asserting something about authorisation and should not be classified as a pure pass-through — rule 3 already excludes methods with an authorisation branch, and this is the same claim expressed as an argument rather than an if.

Not done, on purpose

No baseline, no exclude, and no deletion of methods that have callers. gate-17 stays at 3 on hermiq.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions