Skip to content

Disable Non-default Apps runs every occ app:disable concurrently; one failure aborts the rest #127

Description

@helix-nine

Disable Non-default Apps fans every occ app:disable out concurrently in one subcontainer, and a single failure aborts the rest and destroys the container under them.

startos/actions/maintenance/disableUnstableApps.ts:102-111 maps over the enabled apps into Promise.all, so every php occ app:disable starts at once. Three consequences, all traced through the installed SDK (start-sdk 2.0.9):

  1. A timeout destroys the volume-bound subcontainer mid-write. execFail passes no timeoutMs, so exec's 30 s default applies (SubContainer.js:425) and the process is SIGKILLed. res.throw() throws on signal as well as a non-zero code (SubContainer.js:511-517), Promise.all rejects, and withTemp's finally { await sub.destroy() } runs. exec takes no hold, so destroy() goes straight to killLeader() + destroyFs() while sibling occ processes are mid-OC\Config::writeData() — a read-modify-write of the whole config/config.php. A truncated config.php means Nextcloud will not boot at all, which is strictly worse than the Internal Server Error the action exists to escape.

  2. Concurrent occ runs lose each other's writes even without a timeout: two overlapping processes both read and both rewrite config.php, so one app's enabled=false is dropped and a broken app stays enabled while the action reports success.

  3. One non-zero exit discards the whole report. disabledApps.push(app) records an app before its disable is confirmed, and the return { message: 'The following apps have been disabled: …' } never executes if any app rejects — the common case, since a fataling app is exactly what this action targets. The other apps were disabled, but the user gets a bare ExitError and cannot tell what state the instance is in.

Running the disables sequentially with exec (which does not throw) and reporting a per-app pass/fail line would fix all three.

Two smaller hardening items in the same handler:

  • JSON.parse(res.stdout) at :98 has no tolerance for a non-JSON prefix. Any PHP startup or deprecation notice on stdout ahead of occ's JSON makes it die with SyntaxError: Unexpected token 'P', saying nothing about the real cause. Slicing from the first {, or surfacing res.stderr in the thrown error, is cheap.
  • parsed.enabled is asserted by cast and never checked, so a missing key gives Object.keys(undefined)TypeError with no context. parsed.enabled ?? {} covers it.

All pre-existing. Raised now because the Nextcloud 34 release notes point users at this action as the recovery path for an app broken by 34's removed interfaces, so this is the release that drives traffic to it — on exactly the instances (many third-party apps, some fataling) where the fan-out is widest.

Found while reviewing #131.

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