Skip to content

fix: Bypass RBAC and multitenancy checks in CLI context#978

Open
rubenvdlinde wants to merge 4 commits intodevelopmentfrom
fix/973-cli-rbac-bypass
Open

fix: Bypass RBAC and multitenancy checks in CLI context#978
rubenvdlinde wants to merge 4 commits intodevelopmentfrom
fix/973-cli-rbac-bypass

Conversation

@rubenvdlinde
Copy link
Contributor

@rubenvdlinde rubenvdlinde commented Mar 24, 2026

Summary

  • Fixes RBAC permission checks that block app configuration imports when running from CLI (occ commands, repair steps, cron jobs)
  • In CLI context, there is no user session or active organisation, so hasRbacPermission() returned false for both checks
  • Now detects \OC::$CLI === true and allows access for these trusted system operations

Fixes #973

Test plan

  • Run docker exec nextcloud php occ maintenance:repair with Pipelinq installed — schemas should load without "Access denied" errors
  • Run docker exec nextcloud php occ app:disable pipelinq && docker exec nextcloud php occ app:enable pipelinq — same result
  • Verify web UI RBAC is unaffected (non-CLI context still enforces permissions)
  • Run Pipelinq Playwright CI — tests should pass now that schemas load

When running from CLI (occ commands, repair steps, cron jobs),
there is no user session or active organisation. The RBAC check
in hasRbacPermission() returned false in both cases, blocking
app configuration imports via repair steps.

Now checks OC::$CLI and allows access when running from the
command line, since these are trusted system operations.

Fixes #973
@github-actions
Copy link
Contributor

Quality Report

Repository ConductionNL/openregister
Commit 29d84f3
Branch 978/merge
Event pull_request
Generated 2026-03-24 09:45 UTC
Workflow Run https://github.com/ConductionNL/openregister/actions/runs/23482955037

Summary

Group Result
PHP Quality FAIL
Vue Quality PASS
Security PASS
License PASS
PHPUnit SKIP
Newman SKIP
Playwright SKIP

PHP Quality

Tool Result
lint PASS
phpcs FAIL
phpmd PASS
psalm PASS
phpstan PASS
phpmetrics PASS

Vue Quality

Tool Result
eslint PASS
stylelint PASS

Security

Ecosystem Result
composer PASS
npm PASS

License Compliance

Ecosystem Result
composer PASS
npm PASS

composer dependencies (147 total)

Metric Count
Approved (allowlist) 146
Approved (override) 1
Denied 0

npm dependencies (586 total)

Metric Count
Approved (allowlist) 585
Approved (override) 1
Denied 0

PHPUnit Tests

PHPUnit tests were not enabled for this run.

Integration Tests (Newman)

Newman integration tests were not enabled for this run.

E2E Tests (Playwright)

Playwright E2E tests were not enabled for this run.


Generated automatically by the Quality workflow.

Download the full PDF report from the workflow artifacts.

rjzondervan
rjzondervan previously approved these changes Mar 24, 2026
$activeOrg = $this->organisationService->getActiveOrganisation();
if ($activeOrg === null) {
// CLI context — no active organisation is expected. Allow access.
if (\OC::$CLI === true) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small check: Is there no other way of accessing this information? Looking at the trends in deprecation by Nextcloud they seem not too happy about the \OC::$something accessors

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! PHP_SAPI === 'cli' is a pure PHP constant — no Nextcloud dependency at all, future-proof. Let me update the fix.

\OC::$CLI is a legacy Nextcloud accessor that may be deprecated.
PHP_SAPI === 'cli' is a pure PHP constant with no framework
dependency, making it future-proof.
@github-actions
Copy link
Contributor

Quality Report

Repository ConductionNL/openregister
Commit 3acf5c0
Branch 978/merge
Event pull_request
Generated 2026-03-24 10:03 UTC
Workflow Run https://github.com/ConductionNL/openregister/actions/runs/23483695167

Summary

Group Result
PHP Quality FAIL
Vue Quality PASS
Security PASS
License PASS
PHPUnit SKIP
Newman SKIP
Playwright SKIP

PHP Quality

Tool Result
lint PASS
phpcs FAIL
phpmd PASS
psalm PASS
phpstan PASS
phpmetrics PASS

Vue Quality

Tool Result
eslint PASS
stylelint PASS

Security

Ecosystem Result
composer PASS
npm PASS

License Compliance

Ecosystem Result
composer PASS
npm PASS

composer dependencies (147 total)

Metric Count
Approved (allowlist) 146
Approved (override) 1
Denied 0

npm dependencies (586 total)

Metric Count
Approved (allowlist) 585
Approved (override) 1
Denied 0

PHPUnit Tests

PHPUnit tests were not enabled for this run.

Integration Tests (Newman)

Newman integration tests were not enabled for this run.

E2E Tests (Playwright)

Playwright E2E tests were not enabled for this run.


Generated automatically by the Quality workflow.

Download the full PDF report from the workflow artifacts.

@github-actions
Copy link
Contributor

Quality Report

Repository ConductionNL/openregister
Commit 8620abe
Branch 978/merge
Event pull_request
Generated 2026-03-25 12:44 UTC
Workflow Run https://github.com/ConductionNL/openregister/actions/runs/23541525372

Summary

Group Result
PHP Quality PASS
Vue Quality PASS
Security PASS
License PASS
PHPUnit SKIP
Newman SKIP
Playwright SKIP

PHP Quality

Tool Result
lint PASS
phpcs PASS
phpmd PASS
psalm PASS
phpstan PASS
phpmetrics PASS

Vue Quality

Tool Result
eslint PASS
stylelint PASS

Security

Ecosystem Result
composer PASS
npm PASS

License Compliance

Ecosystem Result
composer PASS
npm PASS

composer dependencies (147 total)

Metric Count
Approved (allowlist) 146
Approved (override) 1
Denied 0

npm dependencies (595 total)

Metric Count
Approved (allowlist) 592
Approved (override) 3
Denied 0

PHPUnit Tests

PHPUnit tests were not enabled for this run.

Integration Tests (Newman)

Newman integration tests were not enabled for this run.

E2E Tests (Playwright)

Playwright E2E tests were not enabled for this run.


Generated automatically by the Quality workflow.

Download the full PDF report from the workflow artifacts.

@github-actions
Copy link
Contributor

Quality Report

Repository ConductionNL/openregister
Commit ec6fb03
Branch 978/merge
Event pull_request
Generated 2026-03-25 15:52 UTC
Workflow Run https://github.com/ConductionNL/openregister/actions/runs/23550288430

Summary

Group Result
PHP Quality PASS
Vue Quality PASS
Security PASS
License PASS
PHPUnit SKIP
Newman SKIP
Playwright SKIP

PHP Quality

Tool Result
lint PASS
phpcs PASS
phpmd PASS
psalm PASS
phpstan PASS
phpmetrics PASS

Vue Quality

Tool Result
eslint PASS
stylelint PASS

Security

Ecosystem Result
composer PASS
npm PASS

License Compliance

Ecosystem Result
composer PASS
npm PASS

composer dependencies (147 total)

Metric Count
Approved (allowlist) 146
Approved (override) 1
Denied 0

npm dependencies (595 total)

Metric Count
Approved (allowlist) 592
Approved (override) 3
Denied 0

PHPUnit Tests

PHPUnit tests were not enabled for this run.

Integration Tests (Newman)

Newman integration tests were not enabled for this run.

E2E Tests (Playwright)

Playwright E2E tests were not enabled for this run.


Generated automatically by the Quality workflow.

Download the full PDF report from the workflow artifacts.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants