Skip to content
This repository was archived by the owner on May 29, 2026. It is now read-only.

fix(security): wave-3 critical fixes#183

Merged
rubenvdlinde merged 1 commit into
developmentfrom
fix/wave3-critical-c1-c4
May 28, 2026
Merged

fix(security): wave-3 critical fixes#183
rubenvdlinde merged 1 commit into
developmentfrom
fix/wave3-critical-c1-c4

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Summary

Fixes all 4 wave-3 CRITICAL findings verified in /tmp/triage-openbuilt.md.

  • C1 (ApplicationInsightsService): Inject SchemaMapper, resolve schema slugs to integer IDs via SchemaMapper::find() before passing to AuditTrailMapper. array_map('intval', $schemaIds) converted every slug string to 0, silently zeroing all KPI dashboard values on every install.

  • C2 (ManifestResolverService + ApplicationsController): Replace three 'application-version' string literals with ApplicationVersionService::APPLICATION_VERSION_SCHEMA ('applicationVersion'). SchemaMapper lowercases before compare — 'applicationversion' ≠ 'application-version' — so every ?_version= manifest route and the diff endpoint silently returned null (DoesNotExistException swallowed as 404).

  • C3 (RunExportJob + ExportJobService): Add ExportJobService::loadJob(), call it at the top of executePipeline() to read the real applicationUuid/applicationVersion/applicationSlug/license from the queued OR record. Old code passed $jobUuid as applicationUuid and hardcoded 'exported-app'/'0.1.0', making every export produce the same generic stub.

  • C4 (ApplicationsController::persistApplication): Populate permissions.owners = ['user:<ownerUid>'] on the cloned Application. Without this, PermissionResolver::matchesCaller returned false on empty permissions, immediately 403-ing non-admin citizen-developers from their own template clones.

Test plan

  • PHPStan: no errors in changed files (56 pre-existing baseline errors unaffected)
  • PHPUnit: all 262 unit tests pass
  • C2 runtime: GET /api/apps/{slug}/manifest?_version={versionSlug} returns 200
  • C3 runtime: exported ZIP file name and content reflect actual app slug + version
  • C4 runtime: non-admin user who clones a template can read/edit/delete the clone

C1 — ApplicationInsightsService: inject SchemaMapper, resolve schema
slugs to integer IDs via SchemaMapper::find() before passing to
AuditTrailMapper. Removes array_map('intval', ...) that converted every
slug to 0, silently zeroing all KPI dashboard values on every install.

C2 — ManifestResolverService + ApplicationsController: replace three
'application-version' string literals with
ApplicationVersionService::APPLICATION_VERSION_SCHEMA ('applicationVersion').
SchemaMapper::find() lowercases before compare, so 'applicationversion' ≠
'application-version', causing DoesNotExistException on every ?_version=
route and the diff endpoint.

C3 — RunExportJob: add ExportJobService::loadJob(), call it at the top
of executePipeline() to read the real applicationUuid/applicationVersion/
applicationSlug/license from the queued OR record. Removes hardcoded
'exported-app' / '0.1.0' context and job UUID passed as applicationUuid.

C4 — ApplicationsController::persistApplication: populate
permissions.owners = ['user:<ownerUid>'] on the cloned Application so
PermissionResolver::matchesCaller returns true and non-admin
citizen-developers are not immediately 403'd from their own clones.

Tests: update RunExportJobTest to mock loadJob with a job fixture for
all pipeline-exercising paths; add SchemaMapper mock to
ApplicationInsightsServiceTest setUp.
@rubenvdlinde rubenvdlinde merged commit 30abf33 into development May 28, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant