Skip to content

Commit eef73d6

Browse files
MattDHillclaude
andcommitted
fix: carry a value the grammar cannot model through as source text
occ runs inside the service container, so a value main cannot 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: PHP_INT_MIN, which `config:system:set --type=integer` saturates any out-of-range negative onto, and a NUL byte. 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. Bounded so malformed input still fails loudly rather than merging the entries around it: Raw stops at a newline, since var_export never spreads a scalar expression over two lines, and a bare positional entry may not be raw. Verified against PHP 8.4 that a missing comma, a missing =>, an unterminated string, an unterminated comment and a truncated file all still throw. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 7ebec69 commit eef73d6

5 files changed

Lines changed: 555 additions & 139 deletions

File tree

AGENTS.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ verified, tried, and decided belongs in the commit message and the PR body.
3232
- **Long-running `occ` work must be queued, never run inline in an action handler** — it would block past the action timeout. Adding one means matching entries in `OCC_ARGS`, `TASK_NOTICE`, and a conditional health check, all keyed off `ACTION_IDS`.
3333
- **`startos/fileModels/php-parser.js` is generated — edit `php.pegjs` and run `npm run regen-parser`.** Nothing in the build reads the grammar, so a grammar edit that skips the regeneration passes `check` and `build` and ships the old parser.
3434
- **`config.php` keeps keys the shape does not model.** The SDK patches `z.object` to be loose, so validation keeps unknown keys and the serializer writes them back — a key a user or a Nextcloud app adds survives. Model a key when the package needs to read or default it, not to preserve it. Do not make the validator strict: `instanceid`, `passwordsalt` and `secret` are unmodelled. Dropping `secret` or `passwordsalt` takes Nextcloud offline with a 503 `config variable is not configured in the config.php file` page, and dropping `instanceid` makes Nextcloud mint a new one, orphaning `appdata_<instanceid>`.
35-
- **The `config.php` grammar reads literals, not expressions, and numbers go through a JavaScript double.** Comments do parse, and they have to: Nextcloud 34 prepends a banner comment between `<?php` and `$CONFIG` on every config write, so rejecting one meant the service could not start. `var_export` renders a NUL byte and `PHP_INT_MIN` as expressions (`'a' . "\0" . 'b'`, `-9223372036854775807-1`), and neither parses, so `main` will not start until the value is gone — `occ config:system:set <key> --type=integer` saturates any out-of-range negative onto `PHP_INT_MIN`. An integer above 2^53 loses precision on the next write, and a whole-valued float comes back an integer.
35+
- **A value the `config.php` grammar cannot model is carried through as source text — never narrow `Raw` into a hard failure.** `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. Malformed input must still fail loudly, which is what bounds `Raw` to a single line: `var_export` never spreads a scalar expression over two, so a run that reaches a newline is a broken file rather than an unmodelled value. A carried-through value is `{ __raw: <source> }`, so a PHP array whose only key is `__raw` and whose value is a string is indistinguishable from one.
36+
- **Comments have to parse.** Nextcloud 34 prepends a banner comment between `<?php` and `$CONFIG` on every config write.
37+
- **`config.php` numbers go through a JavaScript double.** An integer above 2^53 loses precision on the next write, and a whole-valued float comes back an integer.
3638
- **The File Browser mount's `idmap` (uid 1000 → `www-data` 33) is what makes the integration work at all**, and it needs StartOS 0.4.0-beta.10+. Files other services drop into File Browser's volume under a different uid surface as `nobody` until those services idmap their own mount to 1000 too.
3739
- **Adding an external-storage source is a registry edit in `startos/externalStorage.ts` plus a typed mount.** File Browser is the shared hub most services route through, so a direct source is worth adding only for a service whose files live browsably on its own volume.

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ Two models, and only one of them is upstream's.
7575
| `config/config.php` | `nextcloud` | PHP | Yes — `FileHelper.raw` | Every start, and the Configure action |
7676
| `store.json` | `main` | JSON | Yes — `FileHelper.json` | Install, and several actions |
7777

78-
`config.php` is PHP, not a config format any parser handles, so the model carries a **PEG grammar** (`php.pegjs`) to read it and a serializer to write it back. **A key outside the shape survives**: the SDK's `z.object` is loose, so `instanceid`, `passwordsalt`, `secret` and anything a Nextcloud app or an admin adds are read, kept and written back. A key the shape models keeps its value too, unless the shape rejects it or pins it to a literal — the enforced list below. Numbers pass through a JavaScript double, so an integer above 2^53 loses precision and a whole-valued float comes back an integer.
78+
`config.php` is PHP, not a config format any parser handles, so the model carries a **PEG grammar** (`php.pegjs`) to read it and a serializer to write it back. **A key outside the shape survives**: the SDK's `z.object` is loose, so `instanceid`, `passwordsalt`, `secret` and anything a Nextcloud app or an admin adds are read, kept and written back. A key the shape models keeps its value too, unless the shape rejects it or pins it to a literal — the enforced list below. A value written in a form the grammar does not model is left exactly as it was found, so a setting the package cannot interpret can never stop it from starting. Numbers pass through a JavaScript double, so an integer above 2^53 loses precision and a whole-valued float comes back an integer.
7979

8080
**Enforced** — re-asserted whenever the package writes: the database connection (type, name, host, port, user, table prefix), the Valkey memcache trio and its connection, `datadirectory`, `trusted_proxies` (the service bridge's subnet), `filelocking.enabled`, `check_for_working_wellknown_setup`, `overwriteprotocol` (held unset, so a value set by hand is removed), and the three in the table below.
8181

startos/fileModels/config.php.ts

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -75,28 +75,34 @@ function toSingleQuotedLiteral(str: string) {
7575
return "'" + str.replace(/[\\']/g, (c) => '\\' + c) + "'"
7676
}
7777

78+
// The parser's marker for a value it could not model, holding that value's
79+
// source text.
80+
const isRaw = (v: object): v is { __raw: string } =>
81+
Object.keys(v).length === 1 &&
82+
typeof (v as { __raw?: unknown }).__raw === 'string'
83+
7884
function toPhpString(value: unknown, indent = 0): string {
7985
switch (typeof value) {
8086
case 'object':
81-
return value == null
82-
? 'null'
83-
: `array (\n${
84-
Array.isArray(value)
85-
? value
86-
.filter((x) => x !== undefined)
87-
.reduce(
88-
(acc, x, idx) =>
89-
`${acc}${' '.repeat(indent + 1)}${idx} => ${toPhpString(x, indent + 1)},\n`,
90-
'',
91-
)
92-
: Object.entries(value)
93-
.filter(([k, v]) => k !== undefined && v !== undefined)
94-
.reduce(
95-
(acc, [key, value]) =>
96-
`${acc}${' '.repeat(indent + 1)}${toPhpString(key)} => ${toPhpString(value, indent + 1)},\n`,
97-
'',
98-
)
99-
}${' '.repeat(indent)})`
87+
if (value === null) return 'null'
88+
if (isRaw(value)) return value.__raw
89+
return `array (\n${
90+
Array.isArray(value)
91+
? value
92+
.filter((x) => x !== undefined)
93+
.reduce(
94+
(acc, x, idx) =>
95+
`${acc}${' '.repeat(indent + 1)}${idx} => ${toPhpString(x, indent + 1)},\n`,
96+
'',
97+
)
98+
: Object.entries(value)
99+
.filter(([k, v]) => k !== undefined && v !== undefined)
100+
.reduce(
101+
(acc, [key, value]) =>
102+
`${acc}${' '.repeat(indent + 1)}${toPhpString(key)} => ${toPhpString(value, indent + 1)},\n`,
103+
'',
104+
)
105+
}${' '.repeat(indent)})`
100106
case 'string':
101107
return toSingleQuotedLiteral(value)
102108
case 'number':
@@ -132,7 +138,7 @@ export const configPhp = FileHelper.raw<z.infer<typeof shape>>(
132138
// A `$CONFIG` that is not an array would merge down to the shape's
133139
// defaults, and the write that follows would drop `secret`, `instanceid`
134140
// and `dbpassword`. Refuse it while the file is still intact.
135-
if (typeof parsed !== 'object' || parsed === null)
141+
if (typeof parsed !== 'object' || parsed === null || isRaw(parsed))
136142
throw new Error('$CONFIG is not an array')
137143
return parsed
138144
} catch (e) {

0 commit comments

Comments
 (0)