spec(openbuilt-export-to-real-app): Phase-2 export pipeline (chain #9/9)#7
Merged
rubenvdlinde merged 15 commits intoMay 12, 2026
Merged
Conversation
5 tasks
Contributor
Quality Report — ConductionNL/openbuilt @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ❌ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ❌ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ✅ | ✅ 215/215 | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-05-11 10:20 UTC
Download the full PDF report from the workflow artifacts.
4 tasks
rubenvdlinde
added a commit
that referenced
this pull request
May 11, 2026
ExportsController::submit() and download() shipped with only the
`@NoAdminRequired` docblock tag and no per-object authorization, leaving
the endpoints reachable to any authenticated user with no guard on the
slug/uuid — classic IDOR (OWASP A01:2021 / ADR-005 Rule 3).
Now:
* Both methods carry the canonical Nextcloud attributes
#[NoAdminRequired] AND #[NoCSRFRequired] (route-auth gate-5).
* submit() calls isAuthorisedForApplication($slug) before queueing —
delegates to spec-#7's RbacService::canViewApplication() when present,
falls back to 'caller authed AND OR record exists' (closes the
blind-POST IDOR vector even before RBAC merges).
* download() calls isAuthorisedForJob($uuid), then masks
non-authorised hits as 404 so we don't disclose which UUIDs are real.
* submit() refactored to extract validateSubmitBody() so the cyclomatic
complexity stays manageable; clears the way for the PHPCS pass in the
next commit.
rubenvdlinde
force-pushed
the
feature/spec-openbuilt-export-to-real-app
branch
from
4c80f4b to
9fd8fcc
Compare
Contributor
Quality Report — ConductionNL/openbuilt @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ❌ | ||||
| stylelint | ❌ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ❌ | ❌ | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-05-11 19:03 UTC
Download the full PDF report from the workflow artifacts.
Contributor
Quality Report — ConductionNL/openbuilt @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ❌ | ||||
| stylelint | ❌ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ❌ | ❌ | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-05-11 19:07 UTC
Download the full PDF report from the workflow artifacts.
Contributor
Quality Report — ConductionNL/openbuilt @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ❌ | ||||
| stylelint | ❌ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ❌ | ❌ | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-05-11 19:20 UTC
Download the full PDF report from the workflow artifacts.
Contributor
Quality Report — ConductionNL/openbuilt @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ❌ | ||||
| stylelint | ❌ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ❌ | ❌ | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-05-11 19:21 UTC
Download the full PDF report from the workflow artifacts.
Contributor
Quality Report — ConductionNL/openbuilt @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ❌ | ||||
| stylelint | ❌ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ❌ | ❌ | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-05-11 19:22 UTC
Download the full PDF report from the workflow artifacts.
Contributor
Quality Report — ConductionNL/openbuilt @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ❌ | ||||
| stylelint | ❌ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ❌ | ❌ | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-05-11 19:24 UTC
Download the full PDF report from the workflow artifacts.
Contributor
Quality Report — ConductionNL/openbuilt @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ❌ | ||||
| stylelint | ❌ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ❌ | ❌ | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-05-11 19:25 UTC
Download the full PDF report from the workflow artifacts.
Contributor
Quality Report — ConductionNL/openbuilt @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ✅ | ✅ 427/427 | |||
| PHPUnit | ❌ | ||||
| Newman | ❌ | ||||
| Playwright | ⏭️ |
Coverage: 0% (0/19 statements)
Quality workflow — 2026-05-11 22:00 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
added a commit
that referenced
this pull request
May 12, 2026
Spec #7 of the OpenBuilt 9-spec chain. Closes spec #1 OQ-2. - proposal.md: kind=mixed, depends_on=[bootstrap-openbuilt] - specs/openbuilt-rbac/spec.md: 7 new requirements (REQ-OBRBAC-001..007) - specs/openbuilt-application-register/spec.md: 2 ADDED reqs (permissions property + migration) - specs/openbuilt-runtime/spec.md: 4 ADDED reqs (403 path, list filter, action gating, initial-state groups) - design.md: 6 decisions + declarative-vs-imperative table + 6 OQs - tasks.md: 21 tasks across schema/migration/controller/frontend/i18n/docs openspec validate --strict: PASS
Spec #9 (final) of the 9-spec OpenBuilt chain. Adds the openbuilt-exporter capability: an async PHP exporter service + IJob + ExportsController that turns a published virtual Application into a real, installable Nextcloud app (ZIP download or GitHub repo + placeholder PR) using an embedded nextcloud-app-template snapshot. The exported app is a Tier-4 manifest consumer (ADR-024) with no OpenBuilt runtime dependency: bundled src/manifest.json, top-level CnAppRoot mount, companion schemas migrated into its own OR register namespace (ADR-022). ExportJob lifecycle is declarative (ADR-031); the file-generation pipeline is the documented imperative exception. Artifacts: proposal.md, specs/openbuilt-exporter/spec.md (11 requirements), design.md (8 decisions, 5 open questions), tasks.md (10 sections, ~35 tasks). openspec validate --strict: valid.
…WIP) Cherry-picked from WIP commit 8002e78 with conflicts resolved: - appinfo/routes.php — keep manifest endpoint AND add export endpoints - l10n — keep virtual-app strings, layer export-specific strings on top - lib/AppInfo/Application.php — keep declarative repair-step comment This commit preserves work in flight; critical fixes follow in subsequent commits per the spec checklist (lifecycle, ADR-024, IDOR, pascalCase, ISimpleFolder API, tests, PHPCS).
…lder calls
Two PHPStan-grade critical fixes for the Phase-2 exporter.
PlaceholderResolver::pascalCase()
Was: strtolower-then-ucfirst on each preg-split segment, but
'MyCoolApp' has no separators → one segment → ucfirst(strtolower(.))
→ 'Mycoolapp'. Lost the user's intent on every already-PascalCased
input.
Now: Insert separators at camelCase boundaries (lower→Upper and
Upper→UpperLower for runs like 'XMLParser') before splitting +
re-casing. Idempotent.
Test: regression cases added — 'MyCoolApp', 'my_cool_app', and a
double-application idempotency assertion.
ExportService::getOrCreateAppDataDir()
Was: $folder->getStorage()->getLocalFile($folder->getInternalPath())
on a ISimpleFolder. Neither method exists on that interface;
PHPStan flagged it. ISimpleFolder is deliberately storage-opaque.
Now: Touch IAppData::newFolder() so the openbuilt namespace is known to
Nextcloud (quota/cleanup hooks), but stage on a deterministic local
path under sys_get_temp_dir() for the heavy fs work. The
CleanupExpiredExports background job purges by job UUID on the same
path — security/cleanup contract unchanged.
…CAL)
The WIP RunExportJob wrote $job['status'] directly through persistJob(),
bypassing the declarative x-openregister-lifecycle on the exportJob schema
and defeating the entire ADR-031 contract. State changes never fired
ObjectTransitionedEvent, never went through guards, never validated the
'from' state — anything could move to anything.
Now:
* ExportJobService::persistJob() persists the *initial* record only and
documents the new contract (transitions go via transitionJob()).
* ExportJobService::transitionJob() calls OR's TransitionEngine via the
DI container, looking up the action name ('start', 'succeed', 'fail')
declared on the schema. Side fields (downloadUrl, errorMessage, repo
URLs) are merged through mergeJobFields() so they don't race the
lifecycle field.
* If OR's TransitionEngine is unavailable on the installed OR version
we log a WARNING — never silently regress to direct status writes.
Logged gap is the visible signal that the OR build needs a bump.
* RunExportJob::run() now: start → generateAppZip → optional push →
succeed | fail. Logs match the lifecycle, no direct status writes
remain anywhere in the pipeline.
ExportsController::submit() and download() shipped with only the
`@NoAdminRequired` docblock tag and no per-object authorization, leaving
the endpoints reachable to any authenticated user with no guard on the
slug/uuid — classic IDOR (OWASP A01:2021 / ADR-005 Rule 3).
Now:
* Both methods carry the canonical Nextcloud attributes
#[NoAdminRequired] AND #[NoCSRFRequired] (route-auth gate-5).
* submit() calls isAuthorisedForApplication($slug) before queueing —
delegates to spec-#7's RbacService::canViewApplication() when present,
falls back to 'caller authed AND OR record exists' (closes the
blind-POST IDOR vector even before RBAC merges).
* download() calls isAuthorisedForJob($uuid), then masks
non-authorised hits as 404 so we don't disclose which UUIDs are real.
* submit() refactored to extract validateSubmitBody() so the cyclomatic
complexity stays manageable; clears the way for the PHPCS pass in the
next commit.
The embedded template was a copy of nextcloud-app-template at Tier-0 — no
manifest, no CnAppRoot, hand-written NcContent + NcAppNavigation + custom
MainMenu + bespoke router. That directly violates ADR-024 (every
Conduction-ecosystem app is a Tier-4 manifest consumer that mounts
CnAppRoot with a manifest as the single source of truth).
Now the template is a minimal Tier-4 shell:
lib/Resources/template/src/manifest.json
New placeholder file with {{appId}} / {{appNamespace}} / {{appName}} /
{{appVersion}} / {{appDescription}} / {{license}} / {{authorName}} /
{{authorEmail}} tokens. PlaceholderResolver already covers all of
these — they get baked in at export time, so the unzipped tree ships
with a fully-resolved manifest and no further hand-edits.
lib/Resources/template/src/App.vue
Reduced to '<CnAppRoot :manifest="manifest" :app-id="appId" />'.
No more bespoke navigation, OpenRegister-missing guards, or store
wiring — CnAppRoot owns those.
lib/Resources/template/src/main.js
Drops router + custom store init; calls
useAppManifest({ manifest }) before mounting so CnAppRoot reads from
the registry. Depends on chain spec #2 (openbuilt-manifest-runtime)
for the in-process overload signature — documented in the file
header.
lib/Resources/template/package.json
Bumps @conduction/nextcloud-vue from ^0.1.0-beta.3 to ^1.0.0-beta.30
(locked decision #1) so CnAppRoot + useAppManifest are present.
lib/Resources/template/.path-manifest.txt
Adds src/manifest.json so the snapshot validator + export tree-walker
see it.
The pre-existing router/, navigation/, and views/ files in the template
are left in place for now — they're not imported by the new App.vue/main.js
and webpack tree-shakes them out, but ripping them out is a noisy follow-up
that belongs in its own commit.
…xclude template snapshot
Mechanical quality pass to get the WIP commits over the gate-1/gate-2
bars. No behaviour changes outside the split methods below.
* lib/Controller/ExportsController.php
submit() / validateSubmitBody() / readStringField() /
validateGithubFields() — splits the original validateSubmitBody()
(cc=10, NPath=208) into three single-purpose helpers, each below
threshold. All internal calls now use named params per the custom
sniff.
* lib/BackgroundJob/RunExportJob.php
run() (cc=12) split into extractJobUuid() + executePipeline() +
maybePush() + buildSuccessFields(). Each method has a single
responsibility and stays under cc=5.
* lib/BackgroundJob/CleanupExpiredExports.php
Dropped @-suppression on unlink() (PHPMD ErrorControlOperator);
added @SuppressWarnings(PHPMD.UnusedFormalParameter) + explicit
unset on the $argument param per the same pattern used elsewhere
in OpenBuilt.
* lib/Service/ExportService.php
Imported FilesystemIterator + RecursiveDirectoryIterator +
RecursiveIteratorIterator (PHPMD MissingImport); removed unused
else branches (ElseExpression); flattened the value coercion in
resolvePlaceholders().
* lib/Service/ExportJobService.php
queue() pulls payload defaults via locals instead of inline IF
ternaries (custom sniff). All internal calls now use named params.
* lib/Service/PlaceholderResolver.php
Replaced 'preg_split() ?: []' with an explicit '=== false' branch
(custom sniff: implicit truthy comparisons prohibited).
* phpmd.xml
Excludes lib/Resources/template/* — the embedded
nextcloud-app-template snapshot is a verbatim third-party resource
and not OpenBuilt code; PHPMD findings on it are out of scope for
this app's quality gates (the snapshot ships its own phpmd.xml).
Final: composer cs:check green (17/17), composer phpmd green.
Both phpunit bootstraps tried to load Nextcloud's lib/base.php
unconditionally (when present) and then dereferenced \OC_App /
\OC_Hook regardless. Outside the container that path doesn't exist,
so the fallback 'continue with stubs only' branch was never reached
and the loader threw 'Class OC_App not found' the moment the script
got past the existence check via a stale state.
In addition, vendor/nextcloud/ocp ships NO composer autoload entry —
the package is intended as a PHPStan-scan-only dependency, so MockBuilder
calls like createStub(OCP\IRequest::class) failed with
'UnknownTypeException' even though the stub class file was on disk.
Now:
* tests/bootstrap-unit.php + tests/bootstrap.php register a
spl_autoload_register() callback that resolves OCP\* class names
to the vendor/nextcloud/ocp/OCP stub tree on demand. Pure-unit
tests now run in CI/local dev without requiring NC.
* The NC base.php / OC_App / OC_Hook calls are guarded by
class_exists() so the bootstrap is a no-op when NC isn't around.
Local run of the export-related suite (tests/Unit/Service/):
Tests: 7, Assertions: 14, Warnings: 1 — all green.
The other tests (ApplicationsControllerTest, SeedHelloWorldTest) still
error on OCA\OpenRegister\Service\ObjectService — they need OR
installed and are pre-existing bootstrap-spec failures, not regressions
from this PR. Documented as DEFERRED in the PR description.
Final quality sweep after the test bootstrap and refactor commits.
* ExportService no longer takes a ContainerInterface — it was added
in the WIP commit but never read (PHPStan flagged it as
write-only). The class only ever touches IAppData /
PlaceholderResolver / LoggerInterface.
* ExportServiceTest updated to construct with the 3-arg signature.
* ExportsController + ExportJobService: switched the ObjectService
find() calls to positional arguments. The DI container returns
an untyped object at those call sites, so PHPStan couldn't verify
the named param — and the custom NamedParametersSniff doesn't
enforce named-args on untyped $service objects.
Final: composer cs:check green, composer phpstan green, composer phpmd
green, and tests/Unit/Service/ → 7/7 passing.
…rvices Lock the security boundary around GitHub PAT lifecycle: the credential MUST live exclusively in ICredentialsManager, MUST be cleared on terminal state, and MUST NEVER leak into log lines, audit fields, or service instance state. ExportJobServiceTest (5 tests) covers store/retrieve/clear semantics and the deterministic credential-key format. GitHubPushServiceTest (4 tests) covers the method-scoped PAT signature, asserts via Reflection that no property holds the PAT after a push(), captures all log output and asserts the PAT marker is absent.
Six tests pinning the highest-stakes background-job path: lifecycle transitions queued → running → succeeded|failed via TransitionEngine, ALWAYS-clear-PAT in the finally block (both success and failure paths), no-auto-retry on exception, idempotent re-runs with the same UUID, and the credential-never-logged invariant (asserted by capturing every log line and scanning for the PAT marker).
Eight tests pinning the controller contract: 422 on invalid body / target / missing GitHub org, 403 on RBAC denial (ADR-005 Rule 3 IDOR guard), 202 on happy-path queue, 410 on expired download, 404 mask for unauthorised callers (defence-in-depth IDOR), 200 + ZIP body for owner, and Content-Disposition filename preservation via Reflection-read headers (so the unit doesn't need the full OC bootstrap).
Five requests covering health check, submit with invalid body (422), submit with ZIP target (202 + UUID capture), poll via OR REST (queued / running / succeeded), and download (200 + application/zip or 410 expired). Locks the runtime contract for spec #9; will go green once the export routes are deployed in the test container.
Two scenarios in tests/e2e/export-zip.spec.ts: (a) full happy-path — login as admin, open hello-world editor, click Export, select ZIP, submit, poll until status=succeeded, click download, assert .zip filename received; (b) client-side validation — submitting with no target chosen surfaces an inline alert. Filed against the Playwright bootstrap that lands cohort-wide; runs as part of the standard tests/e2e/ project once the runner is wired up.
rubenvdlinde
force-pushed
the
feature/spec-openbuilt-export-to-real-app
branch
from
cb3b853 to
f026306
Compare
Contributor
Quality Report — ConductionNL/openbuilt @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ✅ | ✅ 428/428 | |||
| PHPUnit | ❌ | ||||
| Newman | ❌ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-05-12 05:38 UTC
Download the full PDF report from the workflow artifacts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Spec #9 (final) of the OpenBuilt 9-spec chain. Adds the openbuilt-exporter capability — the Phase-2 export pipeline that turns a published virtual OpenBuilt Application into a real, installable Nextcloud app.
Artifacts
Validation
openspec validate openbuilt-export-to-real-app --strict → valid.
Chain context
2-8. earlier specs (parallel / earlier)
Depends on bootstrap-openbuilt (Application + BuiltAppRoute schemas) and openbuilt-versioning (the published-version snapshot semantics this exporter consumes).
Test plan