feat(config): expose allow_subadmins via OWNCLOUD_ALLOW_SUBADMINS

[DeepDiver1975]

Summary

Exposes the new allow_subadmins ownCloud config setting through a new OWNCLOUD_ALLOW_SUBADMINS environment variable.

ownCloud core PR owncloud/core#41634 disables the subadmin (group-admin) feature by default as a security mitigation, introducing the allow_subadmins config key (defaults to false). This change lets operators opt the feature back in (or explicitly out) without bind-mounting a custom config.

Follows the existing boolean env-var idiom used throughout getConfigFromEnv() (e.g. OWNCLOUD_VERSION_HIDE → version.hide). When unset, nothing is written and core's own default applies.

Scope: allow_subadmins is an ownCloud 11 only setting, so this applies to v24.04 only. v20.04 and v22.04 are intentionally left unchanged.

Changes

• v24.04/overlay/etc/templates/config.php — map OWNCLOUD_ALLOW_SUBADMINS to allow_subadmins
• ENVIRONMENT.md — document the new variable

Testing

• php -l passes
• OWNCLOUD_ALLOW_SUBADMINS=true → allow_subadmins is bool(true)
• OWNCLOUD_ALLOW_SUBADMINS=false → bool(false)
• unset → key absent (core default applies)

🤖 Generated with Claude Code

[DeepDiver1975]

🤖 Automated review by Claude Code review agent.

Overview

This PR exposes ownCloud core's new allow_subadmins config flag (introduced in owncloud/core#41634, merged 2026-06-18) through a new OWNCLOUD_ALLOW_SUBADMINS environment variable, scoped to v24.04 only. The change is small (6 additions, 2 files) and squarely on-target: it lets operators opt the subadmin/group-admin feature back in (or explicitly out) without bind-mounting a custom config.php.

I verified the change against core #41634 directly: core sets 'allow_subadmins' => false as the secure default and gates behavior on a strict boolean (IConfig-injected isEnabled() helper). This PR maps cleanly onto that model.

Verdict: correct, idiomatic, and safe to merge. Comments below are minor/optional.

Code quality / style

• v24.04/overlay/etc/templates/config.php:85-87 follows the exact established idiom of every other boolean setting in getConfigFromEnv() — guard with getenv(...) != '', then assign getenv(...) === 'true'. It sits naturally alongside OWNCLOUD_ALLOW_USER_TO_CHANGE_MAIL_ADDRESS directly above it. Consistency is spot-on.
• ENVIRONMENT.md:21-22 is inserted in correct alphabetical position (between OWNCLOUD_ADMIN_USERNAME and OWNCLOUD_ALLOW_USER_TO_CHANGE_DISPLAY_NAME) and uses the same KEY= \ description formatting as its neighbours.

Boolean / default-value correctness (verified against core)

This is the security-critical part, and it is handled correctly:

• Unset → secure default preserved. When OWNCLOUD_ALLOW_SUBADMINS is unset (or empty), the if (getenv(...) != '') guard is false, so the key is never written to config.php. Core then applies its own default of false (subadmins OFF). The image does not silently enable the feature. This is the right behavior and matches core's mitigation intent.
• "true" → true, everything else → false. getenv(...) === 'true' is a strict string comparison, which produces a real PHP bool. This matches what core now expects (strict boolean check). "false", "0", "1", "yes", etc. all map to false — i.e. only the literal string true enables the feature. That is the conservative, fail-safe direction for a security-gated flag, and it is identical to how all sibling boolean vars in this file behave.

Idempotency

No concern. config.php is a template regenerated by the entrypoint on each container start; this block deterministically derives the value from the env var with no read-modify-write of existing config, so re-running the entrypoint cannot duplicate or corrupt the key.

Specific suggestions (all optional / non-blocking)

1. "1"/"0" do not work, unlike some operators may expect. Because the check is strict === 'true', a user setting OWNCLOUD_ALLOW_SUBADMINS=1 gets false, not true. This is consistent with the rest of the repo (not a regression), so I would not change it here — but it is the single most likely support-ticket footgun. Consider a follow-up that documents "use the literal value true" repo-wide, or normalizes boolean parsing across all vars. Out of scope for this PR.

2. Doc wording could be slightly sharper. ENVIRONMENT.md:22 says "Enable or disable the subadmin (group admin) feature." Since only the literal true enables it and the core default is OFF, a small addition like "Set to true to enable; unset leaves it disabled (core default)." would preempt confusion. Optional.

3. Deep-linkable anchor. The neighbouring ALLOW_USER_TO_CHANGE_* entries link to a specific #anchor on the docs page; the new entry links to the page root. If/when the docs publish an allow_subadmins anchor, updating the link would match the surrounding style. Minor.

Potential issues / risks

• Scope correctness confirmed. The PR intentionally touches only v24.04. allow_subadmins is an ownCloud-core-11-era setting and adding it to v20.04/v22.04 would be wrong, so leaving them untouched is correct.
• No entrypoint shell changes — the wiring is entirely in the PHP config template, which is the right layer; there is no shell-quoting or default-value risk to evaluate in .sh entrypoints for this change.
• Low blast radius. A purely additive, guarded config mapping with a fail-safe default. No migration, no breaking change to existing deployments (operators who never set the var see core's new default, which is the intended security posture).

Nice, minimal, and correctly fail-safe change. Approve.

[DeepDiver1975]

🤖 Automated review by Claude Code review agent.

Re-review at head 091d32e6 (prior review was at 704f1fa4).

Overview — what changed since the prior review

Exactly one new commit since the last review: 091d32e6 "docs: clarify OWNCLOUD_ALLOW_SUBADMINS usage and deep-link docs". It is documentation-only — the only file touched between 704f1fa4..091d32e6 is ENVIRONMENT.md. The PHP/shell mapping in v24.04/overlay/etc/templates/config.php is byte-for-byte unchanged from the previously reviewed revision.

The ENVIRONMENT.md line changed from:

Enable or disable the subadmin (group admin) feature. Disabled by default in ownCloud core (see documentation).

to:

Enable or disable the subadmin (group admin) feature. Set to true to enable; unset leaves it disabled (ownCloud core default) (see documentation#allow-or-disallow-the-group-administrator-subadmin-feature).

This addresses the prior OPTIONAL documentation suggestion: it now (a) explicitly states only true enables the feature, (b) makes clear that unset preserves core's secure OFF default, and (c) deep-links to the specific config sample parameter anchor, matching the style of the neighbouring OWNCLOUD_ALLOW_USER_TO_CHANGE_* entries.

Code quality / style

• The mapping

  if (getenv('OWNCLOUD_ALLOW_SUBADMINS') \!= '') {
    $config['allow_subadmins'] = getenv('OWNCLOUD_ALLOW_SUBADMINS') === 'true';
  }

  is idiomatic and identical in form to every sibling boolean env var in getConfigFromEnv() (e.g. allow_user_to_change_mail_address, strict_login_enforced). Consistency: good.
• Placement is logical (grouped with the other OWNCLOUD_ALLOW_* entries).
• Doc entry is alphabetically ordered and formatted like its neighbours.

Specific suggestions

• The second optional suggestion from the prior review — handling "1"/"0" string values — remains not addressed, and that is the correct decision. The strict === 'true' comparison is the established convention for all boolean vars in this file; accepting "1" here would create an inconsistency with every other variable and is out of scope. No change recommended.
• No new actionable items.

Potential issues / risks

• Fail-safe boolean mapping: confirmed. Only the literal string true sets the value to PHP true; any other non-empty value (including "1", "yes", "True", "false", "0") maps to false. This is fail-safe for a security-sensitive flag that defaults OFF.
• Unset preserves core's secure default: confirmed. The outer \!= '' guard means an unset/empty var writes nothing to $config, so ownCloud core's own allow_subadmins=false default applies — no silent enable.
• Idempotency: confirmed. The template renders the same config deterministically from the same env on every container start.
• Scope: confirmed. Change is limited to v24.04 (ownCloud 11 only), consistent with the PR description; v20.04/v22.04 intentionally untouched.

Verdict

LGTM. The new commit cleanly resolves the prior documentation suggestion, and the security-relevant behaviour (fail-safe boolean, unset → OFF default, idempotent, consistent with sibling vars) is unchanged and correct.