Skip to content

ObjectService::find() called positionally with register/schema in the $_extend/$files slots — contract transition 404s and Customer 360 denies every read #794

Description

@rubenvdlinde

Summary

Two call sites pass register and schema positionally as the 2nd and 3rd arguments to OpenRegister's ObjectService::find(). The real signature puts $_extend and $files there:

openregister@a4dd9067, lib/Service/ObjectService.php:635-644:

public function find(
    int|string $id,
    ?array $_extend = [],                       // <-- 2nd
    bool $files = false,                        // <-- 3rd
    Register|string|int|null $register = null,  // <-- 4th
    Schema|string|int|null $schema = null,      // <-- 5th
    ...
): ?ObjectEntity

A string into ?array $_extend is a TypeError under any coercion mode — string → array is never coercible — and both call sites are in files carrying declare(strict_types=1). Both are wrapped in catch (Throwable), so the TypeError is swallowed and turned into "not found".

The two, and what each one breaks

1. lib/Controller/ContractController.php:248 — every contract 404s

$object = $objectService->find($id, $registerId, $schemaId);

loadContract() returns null for every existing contract, so POST /api/contracts/{id}/transition is dead — it answers 404 for every id. Pinned by ContractControllerTest::testTransitionResolvesTheContractAgainstTheUpstreamFindSignature (markTestSkipped, message names the bug).

2. lib/Controller/Customer360Controller.php:142 — Customer 360 denies every read

$object = $objectService->find($clientId, $register, $schema);

This is canReadClient(), the per-object read guard. It fails closed by design — the docblock says so explicitly:

Fails closed — an OR outage or missing config denies the read rather than granting it, since this is the caller's only defense against reading another client's data.

So this is not a security hole; it is the safe direction. But it means the guard denies every caller and the Customer 360 surface is entirely non-functional.

⚠️ A test would have gone green over this

tests/Stubs/Service/ObjectService.php:47 declares:

find(string $id, string $register = '', string $schema = '')

— i.e. the wrong signature, matching the buggy call rather than the real one. Any test using the bundled stub passes over this bug. The pinning test above deliberately declares the upstream signature instead.

🔑 That is the reusable part: a test stub that mirrors the CALLER instead of the CALLEE cannot fail on a signature mismatch, which is the one class of bug a stub is otherwise ideal for catching. The stub's own docblock claims it "mirrors the real OR ObjectService signature" — for findAll/count it does; for find it does not. Worth fixing the stub in the same change, and re-running the suite to see what else it was hiding.

The positive control — the app knows the right form

38 call sites in lib/ use named arguments (->find(id: …, register: …, schema: …)), and several more use the correct positional order (find($id, [], false, $registerId, $schemaId) — e.g. ScheduledTaskService.php:274, ContactVcardService.php:247). A full scan of lib/ for positional ->find() calls with more than one argument turned up 25 sites, of which only these two are on OpenRegister's ObjectService with register/schema in the wrong slots; the rest are either correct or are calls to a different class (PortalObjectRepository::find($schema, $id), SchemaMapper::find(...)) with its own signature.

Found while writing gate-25 contract tests.

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