fix: read and write config.php the way PHP does; 34.0.3:0 -> 34.0.3:1

[helix-nine]

Closes #133.

The published 34.0.3:0 cannot start

Nextcloud 34 added Config::CONF_WARNING, and writeData() prepends it between <?php and $CONFIG on every config write. It does not exist in 33.0.8. The grammar allowed only whitespace there, so the file Nextcloud itself produces did not parse:

Could not parse config/config.php at line 3, column 1: Expected "$CONFIG" but "/" found.

That throw comes out of the configPhp.merge at the top of main, before any container starts. StartOS retries a failed main without reporting why, so the service restarts every 10 s with nothing in the log but Starting Nextcloud..., adding callback N; statusInfo.error stays null and Postgres and Valkey never come up.

Both delivery paths reach it — a fresh install writes the banner from the stock entrypoint, an upgrade writes it during occ upgrade — so this is live for everyone on 34.0.3:0. It is pre-existing in that release, not introduced here: origin/master rejects the exact bytes writeData() emits, identically.

What else was wrong

php.pegjs was adapted from a JSON grammar and kept JSON's string rules, so the read and write sides each disagreed with PHP — in opposite directions.

Reading. The grammar matched true/false/null/array case-sensitively and rejected raw control characters inside a single-quoted string. PHP's var_export — which is how Nextcloud writes config.php — emits NULL in uppercase and puts control characters in raw. INF and NAN were rejected outright. One command reaches each:

occ config:system:set anything --type=json  --value=null
occ config:system:set anything --type=float --value=1e400

CastHelper::castValue gates --type=float on is_numeric, which accepts 1e400, and (float)'1e400' is INF.

Writing. toSingleQuotedLiteral escaped control characters as \n, \r, \t and \uXXXX inside single quotes, where PHP decodes none of them. Measured against PHP 8.4:

	bytes PHP reads back
before	line1\nline2 — 12 bytes, a literal backslash
after	line1<LF>line2 — 11 bytes, the original value

The package round-tripped with itself because its parser shared the same wrong rule, so this was invisible from inside.

What changed

Both sides now follow PHP: keywords match case-insensitively, INF/-INF/NAN are read and written, a comment parses anywhere whitespace is allowed in all three forms, and a single-quoted string escapes only \ and ' with every other byte standing for itself. INF/NAN had to move on both sides together — String(Infinity) is Infinity, an undefined constant that would make Nextcloud itself fatal on every request.

php-parser.js is regenerated from the grammar with Peggy 5.1.0, and npm run regen-parser now reproduces it byte-for-byte. Nothing recorded how before, so a grammar edit could pass check and build and ship the old parser.

Two failure modes the read now refuses rather than compounds:

• A $CONFIG that is not an array would merge down to the shape's defaults, and the write that follows would drop secret, instanceid and dbpassword — and dbpassword exists nowhere else, so Postgres access and backups would be unrecoverable. The read rejects it while the file is still intact.
• A mangled trusted_domains is recovered rather than blanked. occ mangles this key two ways: config:system:delete trusted_domains 1 leaves a gapped PHP array, which reads back as a map, and config:system:set trusted_domains --value=… leaves a bare string. It is the one key the Configure action does not carry in its patch, so merge writes back whatever the read produced — blanking it to array ( ) makes Nextcloud reject every request as an untrusted domain. A bare * is never promoted: PHP ignores a scalar outright, so promoting one would newly trust every Host header.

And a value the grammar cannot model is no longer a refusal at all. occ runs inside the service container, so a value that stops the read is a brick with no way to reach the command that would remove it — recovery needs filesystem access to config.php, which a StartOS user does not have. Two such values are reachable from documented occ invocations, because var_export writes them as expressions:

occ config:system:set X --type=integer --value=-99999999999999999999   →  -9223372036854775807-1
occ config:system:set X --value=$'a\0b'                                →  'a' . "\0" . 'b'

Rather than teach the grammar those two literals, Value falls back to capturing the source text of anything it cannot model and the serializer re-emits it byte for byte. The fallback needs a terminator lookahead to be reachable at all: Number matches the leading -9223372036854775807 of PHP_INT_MIN, and PEG does not retry an alternative once the enclosing sequence fails.

It is bounded so that malformed input still fails loudly rather than being absorbed. Raw stops at a newline, since var_export never spreads a scalar expression over two lines; a bare positional entry may not be raw; and ; terminates it. Each of those three came from a case that misbehaved without it — a missing comma merged two entries into one blob, 'a' 1, became a positional entry that re-serialized as invalid PHP, and a top-level raw value swallowed the statement terminator. Verified that a missing comma, a missing =>, an unterminated string, an unterminated comment, a multi-line short array and a truncated file all still throw.

An unreadable config.php now logs the parse error before rethrowing — the file, the line and the column, and never the line's text, which holds the database password and the instance secret. It is still fatal, but it says where.

Verification

Interop against real PHP 8.4, both directions, over a corpus covering NULL, booleans, ints, floats, exponents, INF/-INF/NAN, quotes, backslashes, $, unicode, embedded LF/TAB/CR/CRLF/ESC/NUL, a PEM blob, a literal \n, numeric-string keys, and nested, gapped and empty arrays including apps_paths:

• read — PHP writes the config with var_export behind the real CONF_WARNING banner, we parse it, and every value equals PHP's own. PASS
• write — we serialize those values, PHP includes the file, and every value compares ===. PASS
• control — the same file against origin/master fails at the banner, line 3 column 1. So the test is not vacuous.
• fixpoint — a second read/write cycle is byte-identical, so the package does not rewrite config.php on every start.
• regression — a real config.php taken off a running 33.0.8:3 instance (33 keys, including '\\OC\\Memcache\\APCu' and apps_paths) parses to a byte-identical value under old and new. Existing installs see no change, so no migration is needed.

tsc --noEmit, the SDK lint gate lint.mjs (which s9pk.mk runs between check and build, and which tsc alone does not cover), npm run build and prettier --check are all clean; CI passes on both arches; a full local make x86 produces a valid s9pk reporting 34.0.3:1.

End-to-end, before the fix: on a StartOS 0.4.0 VM running 33.0.8:3, setting one unreadable value put the service into the restart loop described above, and deleting the line from config.php recovered it with no other intervention.

Not covered

Numbers pass through a JavaScript double, so an integer above 2^53 loses precision on the next write and a whole-valued float (1.0) comes back an integer. Both are recorded in AGENTS.md. Neither stops the service; they are silent rewrites of a value, and the same passthrough mechanism could close them by returning source text whenever String(parseFloat(t)) !== t — deliberately left out of this PR as a separate call.

A genuinely malformed config.php — a missing comma, an unterminated string or comment — still throws, by design, and now logs the file and the position rather than failing mute.

Review

Five review rounds ran over this branch, and each found something a previous one had introduced or missed:

1. INF/NAN were still reachable through occ config:system:set --type=float --value=1e400, and array was still case-sensitive while the other keywords had become case-insensitive.

2. The SDK lint gate rejected an unknown interpolation, so make aborted and CI was red while tsc passed; logging from the validator printed a config-read failure on healthy starts, because FileHelper.merge validates on-disk data inside a deliberate catch (_) {}; and Peggy's format echoed the offending source line, which can be a secret.

3. A de-listed trusted domain made backups throw, fixed by giving trusted_domains the .catch() every other field already had.

4. That fix was worse than the bug it fixed. .catch([]) wrote 'trusted_domains' => array ( ) through the Configure action, which locks you out of Nextcloud entirely. It recovers the hostnames now.

5. The banner comment above, plus the non-array $CONFIG guard — round 3 had removed the last field that would have refused such a merge.

6. The remaining unparseable forms above. occ lives inside the service, so any value the grammar rejects is a brick with no in-band way out — measured on a StartOS box, config:system:set --type=integer with an out-of-range negative writes one.

Round 5 also explains how the banner survived four rounds: "PHP comments are unparseable" was established early and carried forward as a documented limitation, and every subsequent round was told not to re-raise it. Nobody asked whether anything actually writes one. Detail on each round is in the PR comments.

Version

34.0.3:0 was published on 2026-08-24 and has since been pulled — de-indexed from the alpha registry, and its GitHub release, tag and S3 s9pks deleted — but anyone who installed it during that window still needs a path off it, so this takes 34.0.3:1 rather than reusing :0. No new migration, so current.ts is edited in place and the 34.0.3 release notes are kept — a user coming from 33.0.8:3 still needs them — with a one-line Fixed entry added in all five locales.

[helix-nine]

Second commit is the result of a code review round on the first.

The review found the restart loop still reachable by a documented command: occ config:system:set X --type=float --value=1e400 stores INF (CastHelper gates on is_numeric, and (float)'1e400' is INF), var_export writes the bare constant, and the grammar rejected it. My original description claimed occ could not write INF — that was wrong, and I have corrected the description rather than leaving the bad reasoning in the record.

Also in the second commit:

• INF/-INF/NAN on both sides. They have to move together: String(Infinity) is Infinity, an undefined PHP constant, so teaching only the reader would have turned a package that won't start into a config.php that makes Nextcloud itself fatal on every request.
• array was still case-sensitive while true/false/null had become case-insensitive — the grammar's own comment claimed otherwise.
• Parse failures now report through Peggy's format, quoting the offending line under a caret instead of a bare "expected" message with no position. The same reporting covers shape validation, which is the other half of a read and threw just as silently.
• AGENTS.md and README.md claimed unmodelled config.php keys are dropped on the next write. Measured against the real model, they are kept — the SDK patches z.object to be loose, so instanceid, passwordsalt and secret survive. The docs told a future reader to make the validator strict, which would drop exactly those.
• npm run regen-parser now reproduces php-parser.js byte-for-byte from the grammar; nothing recorded how before, so a grammar edit could pass check and build and ship the old parser.

Verified against PHP 8.4 both directions, INF/-INF/NAN included; an existing config.php still parses to the identical value.

Known gaps, all pre-existing and now logged rather than silent: var_export writes a NUL byte and PHP_INT_MIN as expressions, which the grammar cannot parse; a whole-valued float reads back as an int; integers above 2^53 lose precision.

Two things I found but deliberately did not change, since they are pre-existing and outside this fix — say the word and I will take either:

1. trusted_domains is the only modelled field with no .catch(), and occ config:system:delete trusted_domains 1 leaves a gapped int-keyed array, which PHP represents as a map. That fails the shape, so Create Backup and the Configure action throw until the next restart (main rewrites the key and self-heals).
2. setConfig.ts spreads the whole parsed config into the Configure action's prefill. Because the model is loose, that payload carries dbpassword, passwordsalt and secret even though the form declares six fields.

[helix-nine]

Third commit is round 2 of code review on this branch. It found the previous commit red in CI and carrying three defects it had introduced — two of them regressions from the round-1 fixes.

The build was broken. s9pk.mk runs node node_modules/@start9labs/start-sdk/lint.mjs between check and build, and @typescript-eslint/restrict-template-expressions rejects interpolating an unknown. So make aborted before npm run build and both BuildMatrix arches failed on c1e23d6. tsc --noEmit passes on it, and I had been running npm run build directly rather than make — that is how it survived a round. Now verified with the real gate: lint.mjs exit 0.

A healthy start reported that its config could not be read. FileHelper.merge validates the on-disk data inside try { … } catch (_) {} on purpose — a file that does not satisfy the shape yet is exactly what a merge is for. Wrapping the validator to log turned that designed fallback into a red error. Reachable: occ config:system:delete trusted_domains <n> leaves a gapped PHP array, which is a map rather than a list, and trusted_domains is the shape's only field without a .catch(). Measured before/after:

on-disk state	merge	console.error before	after
config.php absent	succeeds	1	0
gapped trusted_domains	succeeds	1	0
healthy	succeeds	0	0
unparseable	throws	1	1

The validator is unwrapped again. The parse callback keeps the log, and that one is never swallowed.

The log could print a secret. Peggy's format quotes the offending source line, so a parse failure wrote a line of config.php into the service log — dbpassword, passwordsalt or secret if the fault sits on one of those lines, and the entire file if it is on one line. StartOS logs get pasted into support threads. It now logs the line and column plus the parser's message, which is the part that locates the fault:

Could not parse config/config.php at line 4, column 19: Expected "'", "-", "-INF", "0", … but '"' found.

Verified zero occurrences of the secret values in the output.

Docs. My round-1 doc fix replaced a false claim with a promise that is also false: README said keys the shape does not model are left alone, but five modelled keys appear in neither group it lists, and overwriteprotocol is modelled as unset — so a hand-set value is removed on the next write. The consequence I wrote in AGENTS.md was wrong too; measured against a running Nextcloud, dropping secret or passwordsalt takes it offline with a 503 rather than invalidating sessions, and instanceid is silently regenerated, orphaning appdata_<instanceid>.

Release note. "A number outside the range PHP can represent" is not what INF and NAN are — PHP represents both natively — and the phrase would have covered PHP_INT_MIN, which is still unreadable because var_export writes it as the expression -9223372036854775807-1. The note now names what was actually fixed, and the expression and precision limits are recorded in AGENTS.md.

Two findings I did not implement, with reasons:

1. Write a whole-valued float as 1.0 so it stays a float. That would make every integer serialize as a float — loglevel 2 would become 2.0. A JS number cannot hold PHP's int/float distinction, and integers dominate a Nextcloud config, so preferring int is the better of two lossy options. Same for -0.
2. Teach the grammar PHP_INT_MIN. Reachable (any out-of-range negative saturates onto it) and the failure is a restart loop, so it is tempting — but it is an expression, like the NUL-byte form, and writing the value back changes its type from int to float. A special case for two magic literals will rot. Documented as a limitation instead; happy to take it if you would rather have the partial fix.

Also still open and pre-existing, worth an issue rather than this PR: setConfig.ts spreads the whole parsed config into the Configure action's prefill, so ~30 keys including dbpassword, passwordsalt and secret go to the UI while inputSpec declares 6.

Gates on this head: tsc --noEmit, lint.mjs, ncc build and prettier --check all clean; PHP 8.4 interop still passes both directions; a real 33-key config.php still parses identically to before the branch.

[helix-nine]

Fourth commit is round 3 of review. Smaller than the last two — CI was already green and the grammar and serializer are byte-identical to the previous commit, so this is one real fix plus doc corrections.

A de-listed trusted domain made backups fail. occ config:system:delete trusted_domains <n> leaves a gapped PHP array, which is a map rather than a list, and trusted_domains was the only field in the shape with no .catch(). main supplies its own value so it survives and rewrites the file — but until the next restart backups.ts and the Configure action read through the same shape and threw. Measured: with the .catch([]), dbpassword reads fine through a gapped list. main rebuilds the value from the published addresses on every start anyway, so discarding an unusable one costs nothing. A config that is not an object still throws.

Three finders reached this independently and all three agreed the round-2 removal of the validator log was right — the fix belonged in the shape, not in re-adding a log that would have to stay quiet on the merge path.

I over-corrected the README last round. I had rewritten it to say the package re-asserts every key the shape models. Measured against the real shape: 9 of 11 modelled keys survive a hand edit, because .catch() fires only on a validation failure — default_locale, overwrite.cli.url, dbpassword, skeletondirectory and five more keep whatever you set. Only the literal-pinned keys are re-asserted, which is what the wording said before I widened it. Restored, with overwriteprotocol named in the enforced list where it always belonged. The failure that wording would have caused: an admin rotates the Postgres password by hand, reads that the package overwrites it, and goes looking for an action that does not exist.

Two more doc corrections. AGENTS.md quoted a Nextcloud 503 string that does not exist verbatim — the key name is interpolated into the middle of it — so it could not be grepped or matched against a screenshot. And the list of forms the grammar cannot read was missing the one a person is most likely to type: a comment. Verified all three forms are valid PHP and all three are rejected.

The comment above the parse log claimed StartOS reports nothing for a failed main. That turns out to be version-dependent — the container runtime gained a console.error on master that is not in a released tag — and two review rounds read it opposite ways. It also said the position is reported "but not the line" two lines above code that prints a line number. Rewritten to the constraint that outlives both: never log the failing line's text.

Also: the German release note said "keine-Zahl-Wert"; the other four locales render non-numeric idiomatically.

On the secret leak, since it was the reason for the round-2 change: two finders attacked the new message independently. It carries at most one code point of the file — Peggy's found is a single codePointAt — and the expected list is built entirely from static grammar literals. 30,241 mutated configs, longest verbatim run of a planted secret in any log line: zero. The two failure modes that are actually reachable in the field, a NUL value and PHP_INT_MIN, leak nothing at all. I did not try to drive the residual to zero; removing e.message would cost the expected vocabulary, which is the half that locates the fault.

Not acted on, and why: non-UTF-8 bytes in a config value are destroyed on read, but that happens in FileHelper's readFile before the parser sees anything, is identical on master, and is not fixable inside FileHelper.raw. setConfig.ts spreads the whole parsed config into the Configure prefill, so ~30 keys including dbpassword, passwordsalt and secret reach the UI while inputSpec declares six — unchanged by this PR and worth its own issue. A __proto__ key in config.php mutates the parsed object's prototype; Nextcloud never writes one and the file is admin-writable anyway.

Gates on this head: tsc, lint.mjs, ncc build, prettier --check clean; npm ci leaves the lockfile byte-unchanged; npm run regen-parser reproduces the committed parser byte-for-byte; 250 randomly generated var_export configs round-trip through PHP 8.4 with zero failures; a real occ maintenance:install config compares === in PHP after a full cycle and converges after one write.

[helix-nine]

Fifth commit is round 4. It found that my round-3 fix was a worse bug than the one it fixed, so this one is worth reading even though the diff is small.

.catch([]) could lock you out of Nextcloud. trusted_domains is the one key the Configure action does not carry in its patch, so FileHelper.merge writes back whatever the read produced. With .catch([]), a mangled value read as [] and the action wrote 'trusted_domains' => array ( ) — which makes Nextcloud reject every request as an untrusted domain. Measured across the three revisions, gapped list on disk, Configure-shaped merge:

	Configure merge	trusted_domains on disk	backup read
before the .catch	THREW	unchanged (gapped)	THREW
.catch([]) (round 3)	OK	array ( )	OK
this commit	OK	nc.local, nc.onion	OK

Now it recovers the hostnames instead of discarding them. occ mangles this key two ways and both are handled: deleting one entry (config:system:delete trusted_domains 1) leaves a gapped PHP array, which reads back as a map; setting it without an index (config:system:set trusted_domains --value=…) leaves a bare string. The string case is worth having — Nextcloud's own TrustedDomainHelper::isTrustedDomain has an is_array guard, so a string-valued trusted_domains is already a lockout, and recovering it to a one-element list makes the instance reachable again where the empty list did not.

The adversarial pass could not break it: .catch never runs on a valid list (instrumented — 0 invocations), it cannot throw for anything the grammar can produce (__proto__, numeric-string keys, length, negative indices, 2M entries), and toPhpString re-numbers from 0 so a repaired list never round-trips as another gapped array. One behaviour worth naming: with a non-string member in the list, Nextcloud's loop breaks at it and trusts only what precedes; the repair filters it out and widens the trusted set. Implausible input, but a silent widening, so it is on the record.

README undercounted twice, both measured by round-tripping every modelled key and reading what lands on disk. dbport is a sixth database-connection literal and is re-asserted, but the enforced list named five; and "the two below" pointed at a table whose own heading says three. Both omitted keys are overwritten, while the limitation immediately below promised that anything unlisted survives. Also: Configure owns five settings, not four — the trash-retention setting this very release adds was never added to the count, so the README contradicted its own release notes.

Gates on this head: npm ci (lockfile byte-unchanged, one SDK copy), tsc, lint.mjs, ncc build, prettier --check, and a full local make x86 producing a valid 671 MB s9pk reporting 34.0.3:1. npm run regen-parser still reproduces the committed parser byte-for-byte. 32-case var_export corpus round-trips through PHP 8.4 with no regression against master — the only parser difference either way is INF/-INF/NAN, which master rejected. Writes are a fixpoint, so the package does not rewrite config.php on every start.

Worth saying plainly about the shape of this review: the grammar and serializer have been byte-identical since the second commit, and rounds 3 and 4 have been about one z field and the docs. The code has converged; what kept moving was prose describing it, and twice that prose was measurably wrong. If a further round turns up only wording, it should stop and leave the rest to you.

[helix-nine]

Sixth commit is round 5, and it is the most important one. The published 34.0.3:0 cannot start once Nextcloud writes its own config, and this branch inherited that. Please read this before merging anything.

Nextcloud 34 added Config::CONF_WARNING and writeData() prepends it between <?php and $CONFIG on every config write. It does not exist in 33.0.8 — I checked both images: 2 occurrences in 34.0.3, 0 in 33.0.8. The grammar allowed only whitespace there, so the file Nextcloud itself produces did not parse:

Could not parse config/config.php at line 3, column 1: Expected "$CONFIG" but "/" found.

That throw comes out of the merge at the top of main, so the service restart-loops and Postgres and Valkey never come up. Both delivery paths hit it — a fresh install writes the banner from the stock entrypoint during init, and an upgrade writes it during occ upgrade. I built the exact bytes writeData() emits inside nextcloud:34.0.3-apache and fed them to the committed parser; origin/master rejects them identically, so this is pre-existing in the released 34.0.3:0, not introduced here. It is the first release of the 34 line, and the grammar had refused comments since long before it.

The grammar now accepts a comment anywhere whitespace is allowed, in all three PHP forms. An unterminated block comment still fails. The banner config parses and round-trips through real PHP.

How this got missed for four rounds is worth saying plainly. "PHP comments are unparseable" had been established early and I carried it forward as a documented limitation — it was even written into AGENTS.md as one — and every subsequent round was told not to re-raise it. Nobody asked the next question: does anything actually write one? This round I ran a reviewer with no history and no settled list, and that is the one that found it. The suppression was mine.

Second fix in the same commit: a $CONFIG that parses but is not an array made merge fall through to the shape's defaults and write those, dropping secret, instanceid and dbpassword — and dbpassword exists nowhere else, so Postgres access and backups would be unrecoverable. Giving trusted_domains a .catch() in the previous commit removed the last field that would have refused such a merge. The read now rejects a non-array while the file is still intact, which restores what that commit message claimed and I had not verified.

Also corrected: AGENTS.md listed the comment case as a limitation. It is not one any more, and the reason it mattered is now recorded there. The release notes gained a line about the banner in all five locales, since anyone on 34.0.3:0 is affected.

I also pushed back on one finding rather than implementing it, and corrected the reasoning behind another: the claim that a non-string entry makes PHP's isTrustedDomain trust nothing is wrong — in_array('*', $list, true) is checked before the loop that breaks, so such a config trusts everything. I measured it. The * guard I added in this round's earlier work is still right, but for a different reason than stated, and my A/B confirms the repair never grants trust PHP was not already granting except for the admin's own declared hostnames.

Gates on this head: tsc, lint.mjs, ncc build, prettier --check all clean; PHP 8.4 interop passes both directions; the real 33-key config still parses identically to pre-branch; npm run regen-parser reproduces the parser byte-for-byte.

Given the severity of what round 5 turned up, I would not treat the earlier rounds' "clean" areas as settled either — the same suppression could be hiding something else. If it were my call I would want this one exercised on a real 34.0.3 install before it ships.

[MattDHill]

Approved. I verified the substantive claims independently rather than taking the branch's word for them.

The severity is real. Config::CONF_WARNING is in nextcloud/server v34.0.3 at lib/private/Config.php:19 and writeData() prepends it on every write; zero occurrences in v33.0.8. I generated the exact file NC34 emits (real var_export behind the real banner) and ran origin/master's committed parser on it — Expected "$CONFIG" but "/" found, line 3 column 1. init/bootstrapNextcloud.ts runs occ upgrade and the stock entrypoint installs, so both paths land the banner. 34.0.3:0 is the current Latest and bricks on the first restart after Nextcloud writes its own config.

Checks I ran on this head:

npm run regen-parser vs committed php-parser.js	byte-identical
33-key var_export corpus → parse → serialize → PHP 8.4 include → ===	32/33 identical
same file through master's parser (control)	fails at the banner
write is a fixpoint	yes — no per-start rewrite churn
realistic 33-era config, master parser vs this one	identical, so no migration
tsc, lint.mjs, prettier --check	clean
trusted_domains .catch over 12 hostile inputs	never throws; bare * → []; valid list never invokes it
log leak surface	Peggy's found is input.codePointAt(pos) — one code point, not a line

Corpus covered NULL, TRUE/FALSE, INF/-INF/NAN, raw LF/TAB/CR/ESC/NUL in strings, a PEM blob, a literal \n, unicode, $, quotes, \OC\Memcache\APCu, apps_paths, nested/gapped/empty arrays, numeric-string keys and exponent floats. Documented gaps fail exactly as documented. The only non-identical key was a __proto__ probe of my own, dropped on write — pre-existing, unreachable from Nextcloud, and already disclosed.

I pushed one commit of my own: the two Fixed release-note bullets gave root cause, mechanism and old behaviour in all five locales, so they are now one sentence describing what a user can observe. The enforced-key list also said "the three below" directly above three bolded groups it did not mean.

I rewrote the PR description as well. It still listed "a PHP comment does not parse, in any of its three forms" under Not covered — the bug round 5 exists to fix — and never mentioned the banner, the non-array $CONFIG guard, or that round 3's .catch([]) was itself a lockout that round 4 replaced.

Ship it, and de-index 34.0.3:0 until 34.0.3:1 is tagged.

[MattDHill]

Exercised on a real StartOS box, as the last round asked for. demo.local, StartOS 0.4.0.1, x86_64. Both s9pks are the CI x86 builds — 34.0.3:0 from release v34.0.3_0, 34.0.3:1 from run 32804679114.

Control: 34.0.3:0 bricks on a fresh install

No occ command, no user action. Installed it, let the stock entrypoint run the installer, force-started:

19:16:08 Starting Nextcloud...    adding callback 1
19:16:18 Starting Nextcloud...    adding callback 2
19:16:28 Starting Nextcloud...    adding callback 3   ← every 10 s, indefinitely

statusInfo.error null, health empty, no containers. And config.php, written by Nextcloud's own installer:

<?php

/*
 * WARNING
 *
 * This file gets modified by automatic processes ...
 */
$CONFIG = array (

So the banner alone is enough — the reproduction in #133 was a harder path to the same fault than the one every user takes.

34.0.3:1

Sideloaded over it. Migrating 34.0.3:0 -> 34.0.3:1, Initialization complete., healthy at 19:22:04 — health[nextcloud] = success, The web interface is ready. The package read the banner-bearing file and rewrote it at 19:22:04 without the banner; Nextcloud re-adds it on its next write, and the following restart parses that too.

Every unmodelled key survived the rewrite, which is the loose-z.object claim measured on real data rather than a unit test: upgrade.disable-web, passwordsalt, secret, version, instanceid, installed, plus nested apps_paths.

All four hostile forms at once

Wrote three values through occ, on top of the banner Nextcloud had already re-added:

'probe_null' => NULL,
'probe_inf' => INF,
'probe_multiline' => 'line1
line2	tabbed',

Restart at 19:43:39 → healthy at 19:43:40. After the package's own read/write cycle, Nextcloud reads all three back unchanged through occ config:system:get — INF is INF, and the multiline value still has its real LF and TAB. Round-trip is lossless in both directions on the running instance.

Two things I found that this PR does not cause

Both reproduce identically on 34.0.3:0, so neither is a regression here.

1. get-admin-credentials hangs indefinitely. It is the package's only only-stopped action, and invoking it on the stopped service never returns (killed at 180 s, exit 124), so the critical task stays active and package start refuses without --force. adminPassword is present in store.json, and the handler only reads that, so the hang is not in the handler's data path. package action get-input nextcloud set-config against the running service returns in well under a second, so the action plumbing itself is fine.

2. The Configure prefill carries the secrets, exactly as called out earlier. Measured on the box — the payload's top-level keys include dbpassword, passwordsalt, secret and instanceid while inputSpec declares six fields.

Also worth recording next to #133's repro: a key whose value is NULL cannot be removed with occ config:system:delete — deleteKey guards on isset(), which is false for null, so the command reports success and changes nothing. On 34.0.3:0 that means the documented reproduction leaves a brick with no occ route out, only a hand edit. Setting the key to any value first, then deleting, works.

[helix-nine]

Thank you for putting it on a real box — that was the one gap I flagged when I closed round 5, and it turned out to matter more than I expected.

Your control is a better reproduction than the one in #133. I had #133 framed around a value written by occ, which made it look like something a user had to go and do. Your fresh install shows the banner alone is enough: stock entrypoint, no occ, no user action, and it restart-loops. So the fault every user hits is the plain install path, and #133's repro is a harder route to the same place. I'll say that on the issue rather than leaving the narrower framing as the record.

Your two findings are filed, both noted as pre-existing and not caused by this PR:

• get-admin-credentials hangs on the stopped service, so a fresh install cannot be started without --force #135 — get-admin-credentials hangs on the stopped service. I recorded what you measured and stopped there rather than guessing: only-stopped is a sample of one in this package, so the correlation is suggestive but not established, and I suggested a trivial only-stopped probe as the way to separate handler / SDK / OS before anyone picks a fix. I also noted the deeper shape — seed-then-consume is what recipe-admin-credentials steers away from, and it is the same design that already gives this action its "works exactly once" edge.
• Configure action's prefill sends dbpassword, passwordsalt and secret to the UI #136 — the Configure prefill. Named the two things that combine (loose z.object keeping unmodelled keys, and setupActions.js returning the prefill with no intersection against inputSpec), and that the fix is to name the six fields rather than spread.

On the NULL delete trap — I verified it, and it sharpens the de-index. Config::delete() guards on isset($this->cache[$key]) (lib/private/Config.php:185), which is false for a null value, so deleteKey skips writeData() and the command reports success having changed nothing. Consequence for anyone already on 34.0.3:0: the service is down, occ has no route out, and the only recovery is a hand edit of config.php on the volume — set the key to any value first, then delete it, or just remove the line. That is worth weighing in how urgently 34.0.3:0 comes out of the index.

I have not pushed anything since your approval, and I won't — merging and the de-index are yours.

[MattDHill]

Correction to my dev-box report above. Finding 1 — "get-admin-credentials hangs indefinitely" — is wrong. There is no such bug, and #135 is closed.

start-cli package action run reads the action's input payload from stdin. I gave it none, so it blocked on an open stdin or hit EOF on a closed one. echo null | start-cli … package action run nextcloud get-admin-credentials returns immediately, the critical task clears, and package start nextcloud then works without --force. I had reproduced the same symptom against bitcoind runtime-info and lnd node-info and read that as evidence the fault sat outside the package, when it was evidence the fault sat in my invocation.

Everything else in that report stands. The control, the 34.0.3:1 result, and the round-trip through occ were all measured independently of this and are unaffected.

Finding 3 is confirmed at the source and is worse than I framed it. Config::delete() (lib/private/Config.php:184) guards on isset($this->cache[$key]), which is false for a null value, so deleteKey never writes and occ config:system:delete reports success while changing nothing.

The null quirk is not the sharp edge, though. This is: occ runs inside the service container, so any value the grammar cannot read is an unrecoverable brick. main throws, no container starts, and the one tool that could remove the offending line is inside the service that will not start. Recovery needs filesystem access to config.php, which a StartOS user does not normally have.

Measured on the box, on 34.0.3:1:

$ occ config:system:set probe_min --type=integer --value=-99999999999999999999
System config value probe_min set to integer -9223372036854775808

$ grep probe_min config.php
  'probe_min' => -9223372036854775807-1,

That is one of the two forms listed under Not covered, it is reachable from a documented occ invocation, and the grammar rejects it. The NUL-byte concatenation is the same shape. So 34.0.3:1 still ships a reachable path to a brick with no in-band way out — narrower than 34.0.3:0, but the same failure class the PR set out to close.

Worth deciding before this is tagged.

[MattDHill]

Passthrough commit exercised on the box. demo.local, StartOS 0.4.0.1, CI x86 build of eef73d6 installed over the running 34.0.3:1.

Wrote both expression forms through occ — the pair that made main throw before this commit:

$ occ config:system:set probe_min --type=integer --value=-99999999999999999999
$ occ config:system:set probe_nul --type=json --value='"a\u0000b"'

  'probe_min' => -9223372036854775807-1,
  'probe_nul' => 'a' . "\0" . 'b',

Restart at 20:21:13 → health[nextcloud] = success. The package then rewrote config.php — the two entries moved from lines 80/81 to 58/59, so this is its own write, not the file it was handed — and both expressions came through byte-identical.

Read back through Nextcloud afterwards: probe_min is -9223372036854775808, exactly PHP_INT_MIN; probe_nul is four bytes, a, NUL, b, newline. So a value the grammar cannot model survives a full package read/write cycle with PHP's own view of it unchanged.

Worth recording about reachability, since it narrows one of the two: the NUL form is not reachable through occ argv — a NUL cannot cross execve, so a shell-escaped NUL truncates the value and stores a plain string. It needs --type=json with a \u0000 escape, or an app calling setSystemValue directly. PHP_INT_MIN needs no such trick: any out-of-range negative saturates onto it.

Gates on this head: CI green both arches, tsc, lint.mjs, prettier --check, and regen-parser reproduces the committed parser byte-for-byte. Host-side against PHP 8.4: the 39-key corpus round-trips === through include, writes are a fixpoint, and a real 33-era config parses identically under the pre-branch parser.