diff --git a/docs/known-issues.md b/docs/known-issues.md index 3b01a03..a516b75 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -170,3 +170,53 @@ grep -E 'PHP Fatal' /var/log/nginx/atheos.error.log | tail **Rule of thumb:** if a plugin was ported from Codiad, assume its `Common::` calls are stale and verify them before trusting it. + +--- + +## 4. Follow-ups not done (recorded so they are not forgotten) + +These came out of review. They are **operator decisions**, not agent ones — several change what the +editor can do. + +### 4.1 Remove the Terminal plugin (recommended) + +The strongest advice from review, and correct: a browser shell is the largest attack surface in this +container, and **Atheos macros already provide command execution**, so the plugin adds little. It was +installed during testing to diagnose a plugin-rendering problem. Removing the plugin directory (or +renaming it to `Terminal.disabled` — `readDirectory()` skips `*.disabled`) removes the surface. + +Left installed because it is the **operator's** plugin and removing it unilaterally would take away a +tool without being asked. + +### 4.2 ❌ `disable_functions` cannot be "on by default" here — measured + +Review asked for `php.ini` to disable `exec`, `shell_exec`, `passthru`, `system` by default. +**This would break the editor's core function.** Evidence: + +``` +components/codegit/* → Common::safe_execute (31 call sites — every git operation) +components/macro/* → Common::raw_execute (1 call site) +common.php raw_execute → function_exists("system") / function_exists("exec") +``` + +Every git commit, push, fetch and status from the editor goes through `exec`/`system`. Disabling them +turns the editor into a text editor that cannot commit. So it stays **a deliberate exposure lever** +(S-7 in `docs/security-notes.md`), to be applied only when the container is exposed — at which point +you would also lose macros and git integration, which is why S-7 already says *"test, do not +blind-apply"*. + +### 4.3 Rotate the terminal password automatically + +If the plugin is kept, `provision.sh` should generate a random password rather than leaving the +upstream default. Not implemented: it would hardcode a secret into a repo file, which is the same +class of problem this file already documents. + +### 4.4 Deploy key could be read-only + +Review suggested making the deploy key read-only if writes are unnecessary. **They are necessary** — +that is the whole point of the writer-branch workflow (macro 2 pushes). Not applicable. + +### 4.5 The real upstream fix for §2 + +`class.macro.php::save()` should write `[]` instead of `"N/A"` for folder macros. See §2 — the fix is +currently applied to the data, so it survives updates, at the cost of being reversible from the UI.