Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 37 additions & 1 deletion lib/Controller/StoreController.php
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,10 @@ private function installComponents(array $components): array {
$configKey = SchemaSlugMap::SLUG_TO_CONFIG_KEY[$slug];

try {
$this->registry->save(schemaConfigKey: $configKey, data: $object);
$this->registry->save(
schemaConfigKey: $configKey,
data: $this->asNewObject(object: $object)
);
$report[] = ['schema' => $slug, 'status' => 'installed', 'message' => ''];
} catch (Throwable $e) {
$failed = true;
Expand All @@ -357,6 +360,39 @@ private function installComponents(array $components): array {
return ['success' => ($failed === false && $report !== []), 'components' => $report];
}//end installComponents()

/**
* Strip every identity the remote payload carries, so an install CREATES.
*
* 🔴 WITHOUT THIS, "Install" IS AN OVERWRITE PRIMITIVE.
*
* OpenRegister's `saveObject()` resolves the object it is writing from the
* payload itself: `extractUuidAndNormalizeObject()` reads
* `$object['@self']['id'] ?? $object['id']` and treats a match as the uuid
* to UPDATE. So a store item whose component carried the uuid of this
* municipality's live case type would replace it rather than add one — and
* the write is PUT-semantic, so keys the payload omits are nulled, not left
* alone. The case type would not merely change, it would be gutted.
*
* The schema allowlist does not cover this. It governs WHICH schema a
* component may write, never whether the write creates or replaces, so an
* entirely legitimate `caseType` component is the attack.
*
* Identity is not the registry's to supply. An installed item is a NEW
* local object, and if install ever needs to be idempotent it must key on
* something dossiq controls rather than on a remote id.
*
* @param array<string, mixed> $object The component's object.
*
* @return array<string, mixed> The object with every identity key removed.
*
* @spec openspec/changes/dossiq-store-surface/specs/dossiq-store-surface/spec.md
*/
private function asNewObject(array $object): array {
unset($object['id'], $object['uuid'], $object['@self']);

return $object;
}//end asNewObject()

/**
* Read the registry connection.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,3 +148,31 @@ icon, in the footer section between Documentation and Reports.
- **THEN** the entry labelled `Store` MUST declare `icon: "StoreOutline"`
- **AND** it MUST sit in the `footer` section with an order between
Documentation and Reports

### Requirement: REQ-DSS-007 An install creates, and can never replace

The install action SHALL strip every identity key the remote payload carries
(`id`, `uuid`, `@self`) before writing, so an installed component is always a
NEW local object.

OpenRegister resolves the object it writes from the payload itself: `saveObject`
reads `@self.id` first and `id` second, and treats a match as the uuid to
UPDATE. The write is PUT-semantic, so keys the payload omits are nulled rather
than left alone. A store item carrying the uuid of a live case type would
therefore not merely change it, it would gut it.

The schema allowlist does NOT cover this. It governs which schema a component
may write, never whether the write creates or replaces, so a component naming a
perfectly legitimate configuration schema is the attack.

Identity is not a remote registry's to supply. If install ever needs to be
idempotent it SHALL key on something dossiq controls.

@e2e exclude Server-side, and the property is the ABSENCE of an addressed object. A browser sees an install succeed either way; only the payload handed to the write reveals which object it addressed. Proven by StoreControllerTest with a negative control: removing the strip makes the assertion fail.

#### Scenario: A component carrying an id installs as a new object

- **GIVEN** a store item whose component object carries `id`, `uuid` and `@self`
- **WHEN** it is installed
- **THEN** none of those keys MUST reach the write
- **AND** the rest of the component MUST still install
51 changes: 51 additions & 0 deletions tests/Unit/Controller/StoreControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,57 @@ public function testAFailedWriteIsReportedWithoutLeakingTheReason(): void {
}//end testAFailedWriteIsReportedWithoutLeakingTheReason()


/**
* 🔴 A component carrying an id must NOT overwrite a local object.
*
* OpenRegister resolves the object it writes from the payload:
* `saveObject()` reads `$object['@self']['id'] ?? $object['id']` and treats
* a match as the uuid to UPDATE. So a registry that shipped the uuid of
* this municipality's live case type would REPLACE it — and the write is
* PUT-semantic, so omitted keys are nulled rather than left alone.
*
* The schema allowlist does not cover this: the component names an allowed
* schema, which is exactly what makes it dangerous.
*
* @return void
*
* @spec openspec/changes/dossiq-store-surface/specs/dossiq-store-surface/spec.md#requirement-req-dss-003-install-accepts-configuration-and-refuses-records
*/
public function testAnInstalledComponentCannotOverwriteAnExistingObject(): void {
$this->storeService->method('resolve')->willReturn(
[
'slug' => 'hostile-item',
'components' => [
[
'schema' => 'caseType',
'object' => [
'id' => 'the-municipalitys-live-case-type',
'uuid' => 'the-municipalitys-live-case-type',
'@self' => ['id' => 'the-municipalitys-live-case-type'],
'title' => 'Replaced',
],
],
],
]
);

$written = null;
$this->registry->method('save')->willReturnCallback(
static function (string $key, array $data) use (&$written): array {
$written = $data;
return $data;
}
);

$this->controller->install(slug: 'hostile-item');

$this->assertIsArray($written);
$this->assertArrayNotHasKey('id', $written, 'a remote id must not address a local object');
$this->assertArrayNotHasKey('uuid', $written, 'nor a remote uuid');
$this->assertArrayNotHasKey('@self', $written, 'nor @self, which saveObject reads FIRST');
$this->assertSame('Replaced', $written['title'], 'the rest of the payload still installs');
}//end testAnInstalledComponentCannotOverwriteAnExistingObject()

/**
* The token never comes back out.
*
Expand Down
Loading