fix: rDelete fataled on musl (GLOB_BRACE) - broke ALL deletion and plugin removal

Reported as "deleting terminal just cycles in the ui". Root cause is much larger than the
plugin: it was a platform bug that broke deletion everywhere.

CAUSE
  traits/helpers.php::rDelete() did:
      glob($target . "{*,.[!.]*,..?*}", GLOB_BRACE|GLOB_MARK);
  GLOB_BRACE is a GLIBC extension. Alpine is musl, where the constant is undefined,
  and referencing an undefined constant is a FATAL in PHP 8:
      PHP Fatal error: Uncaught Error: Undefined constant "GLOB_BRACE"
      in traits/helpers.php:138
      #0 components/market/class.market.php(178): Common::rDelete()
      #1 components/market/controller.php(46): Market->remove()
  The fatal kills the request before Common::send(200), so the front end never gets a
  settle response and the UI spins forever. A hang, not an error - which is why it
  presented as "cycles in the ui".

BLAST RADIUS (measured, two call sites)
  components/market/class.market.php:178   removing a plugin from the market
  components/filetree/class.filetree.php:59  DELETING ANY FILE OR FOLDER IN THE EDITOR
  The second is core functionality, so this was never a plugin bug - installing the
  Terminal plugin is just what made it visible.

FIX
  patches/helpers-rDelete-GLOB_BRACE.diff - expand the brace pattern by hand when
  GLOB_BRACE is undefined, keeping upstream behaviour where it is defined. Verified:
  nested dirs + dotfiles + single files all delete correctly now.
  Upstream has NOT fixed this (checked main) so it cannot be resolved by updating.

PLUGIN REMOVED
  Once deletion worked, removal via the market succeeded and the plugin is now gone:
    {"text":"Plugin successfully removed.","status":200}
  That is the right end state - a browser shell was the largest attack surface, and
  macros already provide command execution. Removed through the market's own code path
  so the plugin list, addons cache and bundle stayed consistent.

TESTS (now 42 passed, 1 failed - the failure is branch protection, a genuine open item)
  + a GLOB_BRACE regression guard that fails loudly if the patch is lost to an update
  + the identity guard is INVERTED: FR-11 required an EMPTY identity so the agent could
    not impersonate the operator, but the operator has since asked for it to be set, so
    the test asserting emptiness was asserting the opposite of the current design. It
    now requires the identity to be present and to NOT look agent-generated.

docs/known-issues.md: new section 1 documents the GLOB_BRACE fault in full, including the
honest note that this is the real cost of choosing Alpine over a glibc base image.

docs/known-issues.md

@@ -0,0 +1,302 @@
+# Known Issues & Patches
+
+Upstream Atheos is an actively-moving project with **no releases**, and its plugin market ships
+plugins that are **ports from Codiad** (Atheos's predecessor). Two of those plugins use APIs that
+Atheos has since removed or changed. This file records what we hit, what we changed, and why the
+fix is done the way it is.
+
+Pinned Atheos commit: `f90cd1806dbe0e25673a77e27390e8b1a240c3f6` (build 611).
+
+---
+
+## 1. 🔴 **Deleting any file or folder was broken** — `GLOB_BRACE` does not exist on musl
+
+**Symptom:** removing the Terminal plugin from the market **cycled forever in the UI** (never
+completed, never errored). Deleting a file or folder in the editor was broken by the same cause.
+
+**Root cause — a glibc-only constant referenced on musl.** `traits/helpers.php::rDelete()`:
+
+```php
+$files = glob($target . "{*,.[!.]*,..?*}", GLOB_BRACE|GLOB_MARK);
+```
+
+`GLOB_BRACE` is a **glibc extension**. Alpine uses **musl**, where the constant is *undefined* —
+and touching an undefined constant is a **fatal error** in PHP 8 (a warning in older versions):
+
+```
+PHP Fatal error: Uncaught Error: Undefined constant "GLOB_BRACE"
+in /var/www/atheos/traits/helpers.php:138
+#0 components/market/class.market.php(178): Common::rDelete()
+#1 components/market/controller.php(46): Market->remove()
+```
+
+Because the fatal kills the request before `Common::send(200)`, the front end never receives a
+settle response — so the UI **spins indefinitely**. A hang, not an error.
+
+**Blast radius — measured, two call sites:**
+
+| Caller | What breaks |
+|---|---|
+| `components/market/class.market.php:178` | removing a plugin from the market |
+| `components/filetree/class.filetree.php:59` | **deleting any file or folder in the editor** |
+
+The second is core editor functionality, so this is **not** a plugin bug — installing the Terminal
+plugin is merely what made it visible. Verified directly: with the original code, `rDelete()` fatals
+and the target is left on disk.
+
+**Fix applied** (`patches/helpers-rDelete-GLOB_BRACE.diff`) — expand the brace pattern by hand when
+`GLOB_BRACE` is absent, keeping upstream's exact behaviour where it *is* defined:
+
+```php
+if (defined("GLOB_BRACE")) {
+    $files = glob($target . "{*,.[!.]*,..?*}", GLOB_BRACE | GLOB_MARK);
+} else {
+    $files = array_merge(
+        glob($target . "/*",        GLOB_MARK) ?: [],
+        glob($target . "/.[!.]*",   GLOB_MARK) ?: [],
+        glob($target . "/..?*",     GLOB_MARK) ?: []
+    );
+}
+```
+
+The three patterns are precisely what the brace expansion produces (`*`, dotfiles, and `.`/`..`-like
+names), so the directory listing is equivalent.
+
+**Verified** — `php83 -l` clean, then a real deletion of a tree containing nested dirs and two
+dotfiles:
+
+```
+entries before: 10
+rDelete() returned: true
+exists after: gone (OK)
+single file: rDelete=true exists=gone (OK)
+```
+
+**Upstream has NOT fixed this** — the same line is still in `main` (checked directly), so it cannot
+be resolved by updating Atheos. Re-apply this patch after any update (`scripts/update-atheos.sh`).
+
+**Consequence for the platform choice:** this is the real cost of Alpine. A glibc-based image
+(Debian/Ubuntu) would not have hit it. It is patchable in one place, so Alpine stays — but it is worth
+knowing that `GLOB_BRACE` is the kind of thing musl breaks.
+
+---
+
+## 2. 🔴 Plugin market: the **Terminal** plugin does not work as shipped
+
+**Symptom:** installed the Terminal plugin from the market; it failed / did not function.
+
+**Root cause — a removed API.** `plugins/Terminal/terminal.php` calls `Common::execute(...)`, which
+**no longer exists** in Atheos (it was renamed):
+
+```
+plugin calls : Common::execute(...)      ->  MISSING
+available    : Common::raw_execute(...)  ->  returns ["code" => int, "output" => array]
+                Common::safe_execute(...)
+```
+
+The plugin's header says it is a port of `Fluidbyte/Codiad-Terminal`, so it targets the old API. Its
+return-shape assumption is also stale (the old `execute()` returned `text`; `raw_execute()` returns
+`output` as an array).
+
+**Fix applied** (`patches/Terminal-terminal.php.patched`):
+
+```diff
+-  $output = Common::execute("pwd");
++  $output = Common::raw_execute("pwd");
+-  $_SESSION["activeDir"] = array([$_SESSION["project"]] => $output["text"]);
++  $_SESSION["activeDir"] = array([$_SESSION["project"]] => implode("\n", (array)$output["output"]));
+
+-  $output = Common::execute($cmd);
+-  return $output["text"];
++  $output = Common::raw_execute($cmd);
++  return implode("\n", (array)$output["output"]);
+```
+
+**Verified** against the real method (`php83 -l` clean; both call patterns exercised):
+
+| Call | Result |
+|---|---|
+| `raw_execute("pwd")` | `code=0`, `output=["/root",""]` |
+| `raw_execute("echo ...")` | `code=0`, returns the text |
+| after `chdir` to the repo | returns `/var/www/atheos/workspace/blog` — so the terminal opens in the blog |
+
+**Re-apply after an Atheos update** — `scripts/update-atheos.sh` replaces the application files, so
+this patch is lost on update. Re-apply it, or drop the plugin.
+
+### Two further defects in the same plugin (NOT fixed — upstream, and mostly harmless)
+
+1. **`terminal.php:95`** — `$blocked = explode(",", BLOCKED === true);` passes a **boolean** to
+   `explode()`. This evaluates to `explode(",", false)`, so the intended block-list
+   (`define("BLOCKED","ssh,telnet")`) **never actually blocks anything**. The plugin's own guard is
+   dead code. Cosmetic in effect (the command would just fail on its own), but worth knowing rather
+   than trusting.
+2. **`terminal.php:124`** — `if (strtolower($command === "exit"))` passes a boolean to
+   `strtolower()`. Should be `strtolower($command) === "exit"`. Result: typing `exit` does not clear
+   `term_auth`; the session stays authenticated until the browser session ends.
+
+Left as-is: they are upstream bugs, and fixing them changes the plugin's behaviour beyond the
+minimum needed to make it run. Recorded here so nobody "fixes" them by accident or trusts the
+block-list.
+
+### ⚠️ Security note
+
+This plugin gives a **browser terminal that executes shell commands as the php-fpm user**
+(`atheos`). Two things matter:
+
+1. **The plugin ships with a well-known, hardcoded gate constant.** Upstream is literally
+   `define("PASSWORD", "<the default value in the plugin source>");`. Anyone who can reach the page
+   can read the source on GitHub and unlock it. **We have rotated it to a random value** — retrieve
+   it from the `PASSWORD` define in `plugins/Terminal/terminal.php` on the guest. It is deliberately
+   **not recorded in this repo** (see below).
+2. **It is a shell, so treat it as a privilege decision, not a convenience one.** Installing it
+   means the editor can run arbitrary commands as `atheos` — the same user that owns the deploy key.
+
+Because of (1) and (2), the in-app password is treated as a **secret**:
+
+- it is **not** written into this repo, the README, or the RUNBOOK
+- to read it: `ssh root@192.168.178.57 'grep PASSWORD /var/www/atheos/plugins/Terminal/terminal.php'`
+- it changes per install; `scripts/provision.sh` does not set one, so a rebuild returns to the
+  upstream default — **rotate it again after a rebuild**
+
+**Outcome: the plugin has been REMOVED.** Once the `GLOB_BRACE` fault (§1) was fixed, removal through
+the market succeeded — and that is the right end state: a browser shell is the single largest attack
+surface in the container, and **Atheos macros already provide command execution**, so it added little
+you did not already have. Removal was done by invoking the market's own `remove()` path, so the plugin
+list, the `addons` cache and the generated bundle all stayed consistent:
+
+```
+{"text":"Plugin successfully removed.","status":200}
+plugins/ -> (only README.md remains)
+addons cache references to Terminal: 0
+```
+
+The notes above are kept because they document what the plugin *is* and how it failed — and because
+the same stale-API pattern will recur with any other Codiad-era plugin. To reinstall, use the
+market: it will re-introduce the `Common::execute` fault, so `patches/Terminal-terminal.php.patched`
+was the fix — **it has been deleted along with the plugin**. If you ever reinstall, re-derive it:
+`grep -rhoE 'Common::[a-zA-Z_]+' plugins/Terminal/` and expect `Common::execute` to be missing.
+
+`docs/security-notes.md` §2 S-7 covers disabling `exec`/`proc_open`, which would neutralise **both**
+this plugin and the macros. That is the correct lever if you ever expose this container.
+
+---
+
+## 3. 🟠 The Macro dialog crashed when folder-scoped macros existed
+
+**Symptom:** the Settings → Macro dialog returned HTTP 500.
+
+**Root cause — invalid data by Atheos's own design.** `components/macro/dialog.php:35` does:
+
+```php
+implode(", ", $macro["fTypes"])     // requires an ARRAY
+```
+
+but `components/macro/class.macro.php` `save()` writes the **string** `"N/A"` for folder macros:
+
+```php
+"fTypes" => $type === "file" ? explode(",", $fTypes) : "N/A",
+```
+
+So **any folder-type macro** makes that dialog fatal:
+
+```
+PHP Fatal error: Uncaught TypeError: implode(): Argument #2 ($array) must be of type ?array,
+string given in /var/www/atheos/components/macro/dialog.php:35
+```
+
+**Fix applied — correct the DATA, not Atheos's code.** The stored `fTypes` for folder macros is now
+`[]` (an empty array) instead of `"N/A"`:
+
+```sh
+sed -i 's/"fTypes": "N\/A"/"fTypes": []/g' /var/www/atheos/data/macros.db.php
+```
+
+Verified: `implode()` now succeeds for all 8 macros.
+
+**Why data rather than code:** patching `dialog.php` would be overwritten by `update-atheos.sh`
+(and by any Atheos update), whereas valid data keeps working. The trade-off is that the UI's own
+"save" path could re-introduce `"N/A"` if a macro is edited in the UI — see the follow-up below.
+
+**Follow-up (not done):** the real upstream fix is
+`$type === "file" ? explode(",", $fTypes) : []` in `class.macro.php::save()`. Applying it would stop
+the UI re-introducing the bad value, at the cost of a patch that must be re-applied after updates.
+Worth doing if the dialog starts 500-ing again after editing a macro in the UI.
+
+---
+
+## 4. Checking for plugin problems in future
+
+The plugin's JS is bundled into `public/plugins.min.js` (built by `classes/sourcemanager.php`), and
+its rightbar button is rendered by `components/sidebars/sb-right.php` from `plugin.json`. Both were
+verified working:
+
+```
+plugins.min.js contains 'plugins/Terminal/init.js'   -> YES   (bundle is fine)
+rightbar HTML                                        -> <a onclick="atheos.Terminal.open();">
+                                                        <i class="fas fa-terminal"></i>Terminal</a>
+```
+
+So when a plugin misbehaves, check in this order:
+
+```sh
+# 1. did it install?
+tail /var/www/atheos/data/log/market.log.php
+
+# 2. is its JS in the served bundle?
+grep -c "plugins/<Name>/init.js" /var/www/atheos/public/plugins.min.js
+
+# 3. is it a stale-API port? (Codiad-era plugins are the risk)
+grep -rhoE 'Common::[a-zA-Z_]+' /var/www/atheos/plugins/<Name>/ | sort -u
+#    then check each exists:  grep -rn "function <name>" /var/www/atheos/common.php /var/www/atheos/traits/
+
+# 4. is there a PHP fatal?
+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.
+
+---
+
+## 5. 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.
+
+### 5.1 Remove the Terminal plugin — ✅ DONE
+
+Removed through the market's own code path once §1 was fixed (see §2). A browser shell was the largest
+attack surface in the container and macros already cover command execution.
+
+### 5.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"*.
+
+### 5.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.
+
+### 5.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.
+
+### 5.5 The real upstream fix for §3
+
+`class.macro.php::save()` should write `[]` instead of `"N/A"` for folder macros. See §3 — the fix is
+currently applied to the data, so it survives updates, at the cost of being reversible from the UI.

files/macros.db.php

@@ -25,7 +25,7 @@
         "uuid": "macro-writer-0001-start",
         "title": "1. Start/Join Writer Branch",
         "type": "folder",
-        "fTypes": "N/A",
+        "fTypes": [],
         "command": "cd %PATH% && (git checkout atheos-writer 2>/dev/null || git checkout -b atheos-writer) && git push -u origin atheos-writer",
         "icon": "fas fa-code-branch",
         "owner": "armistace"
@@ -34,7 +34,7 @@
         "uuid": "macro-writer-0002-commit",
         "title": "2. Commit + Push Writer Branch",
         "type": "folder",
-        "fTypes": "N/A",
+        "fTypes": [],
         "command": "cd %PATH% && (git config --get user.email >/dev/null || { echo 'ERROR: no git identity set. Run over SSH:  git config --global user.name \"Your Name\" && git config --global user.email \"you@example.com\"   (RUNBOOK STEP 1)'; exit 1; }) && git add -A && (git diff --cached --quiet || git commit -m \"writer: $(date +%F_%H%M)\") && git push origin atheos-writer",
         "icon": "fas fa-cloud-upload-alt",
         "owner": "armistace"
@@ -43,7 +43,7 @@
         "uuid": "macro-writer-0003-reset",
         "title": "3. DESTRUCTIVE: Reset Writer Branch to Master",
         "type": "folder",
-        "fTypes": "N/A",
+        "fTypes": [],
         "command": "cd %PATH% && git fetch origin && git reset --hard origin/master && git push --force-with-lease origin atheos-writer",
         "icon": "fas fa-exclamation-triangle",
         "owner": "armistace"
@@ -52,7 +52,7 @@
         "uuid": "macro-writer-0004-status",
         "title": "4. Status + Recent Log",
         "type": "folder",
-        "fTypes": "N/A",
+        "fTypes": [],
         "command": "cd %PATH% && git status --short --branch && echo '--- recent commits:' && git log --oneline -8 && echo '--- local branches:' && git branch",
         "icon": "fas fa-info-circle",
         "owner": "armistace"
@@ -61,7 +61,7 @@
         "uuid": "macro-writer-0005-refresh",
         "title": "5. Refresh Master (ff-only)",
         "type": "folder",
-        "fTypes": "N/A",
+        "fTypes": [],
         "command": "cd %PATH% && git checkout master && git pull --ff-only && git checkout atheos-writer",
         "icon": "fas fa-sync-alt",
         "owner": "armistace"
@@ -70,7 +70,7 @@
         "uuid": "macro-writer-0006-pullmaster",
         "title": "6. Merge Master Into Writer Branch",
         "type": "folder",
-        "fTypes": "N/A",
+        "fTypes": [],
         "command": "cd %PATH% && (git config --get user.email >/dev/null || { echo 'ERROR: no git identity set. Run over SSH:  git config --global user.name \"Your Name\" && git config --global user.email \"you@example.com\"   (RUNBOOK STEP 1)'; exit 1; }) && git fetch origin && git merge origin/master -m \"merge master into atheos-writer\" && git push origin atheos-writer",
         "icon": "fas fa-code-merge",
         "owner": "armistace"

patches/helpers-rDelete-GLOB_BRACE.diff

@@ -0,0 +1,22 @@
+--- /var/www/atheos/traits/helpers.php.orig
++++ /var/www/atheos/traits/helpers.php
+@@ -135,7 +135,18 @@
+ 
+         if (is_dir($target)) {
+ 
+-            $files = glob($target . "{*,.[!.]*,..?*}", GLOB_BRACE|GLOB_MARK); //GLOB_MARK adds a slash to directories returned
++            // GLOB_BRACE is a glibc extension and is UNDEFINED under musl (Alpine), where
++            // referencing it is a fatal error in PHP 8. Expand the brace pattern by hand
++            // so this works on both libcs.
++            if (defined("GLOB_BRACE")) {
++                $files = glob($target . "{*,.[!.]*,..?*}", GLOB_BRACE | GLOB_MARK);
++            } else {
++                $files = array_merge(
++                    glob($target . "/*", GLOB_MARK) ?: [],
++                    glob($target . "/.[!.]*", GLOB_MARK) ?: [],
++                    glob($target . "/..?*", GLOB_MARK) ?: []
++                );
++            }
+             // $files = glob($target . "/*");
+ 
+             foreach ($files as $file) {

scripts/build-and-test.sh

@@ -108,7 +108,29 @@ done
 
 echo
 echo "=============================================================="
-echo "GUARD: is master PROTECTED on the blog repo? (the writer-branch model depends on it)"
+echo "REGRESSION: rDelete works (GLOB_BRACE is glibc-only; musl lacks it)"
+echo "=============================================================="
+# traits/helpers.php::rDelete() used GLOB_BRACE, which is undefined under musl -> PHP 8 fatal.
+# That broke deleting ANY file/folder in the editor AND removing a market plugin (the UI
+# would spin forever because the fatal killed the response before Common::send()).
+# docs/known-issues.md §1. This test fails loudly if the patch is lost (e.g. by an update).
+rm -rf /tmp/rdel_guard 2>/dev/null
+if mkdir -p /tmp/rdel_guard/a/b/c /tmp/rdel_guard/.hd 2>/dev/null \
+   && touch /tmp/rdel_guard/f.txt /tmp/rdel_guard/.dot /tmp/rdel_guard/a/b/c/deep.txt; then
+  ROUT=$(php83 -r 'require_once("/var/www/atheos/common.php"); echo Common::rDelete("/tmp/rdel_guard/") ? "ok" : "fail";' 2>&1 | grep -viE 'deprecat|warning|notice' | tail -1)
+  if [ "$ROUT" = "ok" ] && [ ! -e /tmp/rdel_guard ]; then
+    ok "rDelete removed a nested tree incl. dotfiles (delete works)"
+  else
+    if echo "$ROUT" | grep -q 'GLOB_BRACE'; then
+      bad "rDelete FATALS on GLOB_BRACE - re-apply patches/helpers-rDelete-GLOB_BRACE.diff"
+    else
+      bad "rDelete failed (got: $ROUT)"
+    fi
+  fi
+else
+  bad "could not build the rDelete test tree"
+fi
+rm -rf /tmp/rdel_guard 2>/dev/null
 echo "=============================================================="
 # The whole branch-based workflow assumes the deploy key CANNOT push master. That is a
 # repo-side setting we cannot verify from here by inspection - so test it the way an
@@ -128,10 +150,22 @@ fi
 
 echo
 echo "=============================================================="
-echo "FR-11 GUARD: agent created NO identity or armistace credential"
+echo "IDENTITY: git identity is SET by the operator (FR-11 superseded by explicit request)"
 echo "=============================================================="
-chk "git user.name empty"  "$(asuser 'git config --get user.name'  2>/dev/null)" ""
-chk "git user.email empty" "$(asuser 'git config --get user.email' 2>/dev/null)" ""
+# FR-11 originally required the agent to leave the identity EMPTY so it could not impersonate
+# the operator. The operator has since explicitly asked for it to be set, so the requirement is
+# superseded - and the guard is inverted: the identity must now be present and must be the
+# OPERATOR's, not an agent-invented one. Guarding the old expectation would assert the opposite
+# of the current design and fail forever (which is what it did).
+NAME=$(asuser 'git config --get user.name'  2>/dev/null)
+MAIL=$(asuser 'git config --get user.email' 2>/dev/null)
+if [ -n "$NAME" ]; then ok "git user.name set ($NAME)"; else bad "git user.name is empty - macro 2 will refuse to commit"; fi
+if [ -n "$MAIL" ]; then ok "git user.email set ($MAIL)"; else bad "git user.email is empty"; fi
+case "$MAIL" in
+  *hermes*|*bot*|*noreply@anthropic*|*agent*) bad "identity looks agent-generated: $MAIL" ;;
+  "") ;;
+  *) ok "identity is an operator address, not an agent-invented one" ;;
+esac
 if [ -f /home/atheos/.ssh/id_ed25519_blog ]; then ok "deploy key present"; else bad "deploy key missing"; fi
 chk "deploy key mode" "$(stat -c '%a' /home/atheos/.ssh/id_ed25519_blog 2>/dev/null)" "600"
 chk "deploy key owner" "$(stat -c '%U' /home/atheos/.ssh/id_ed25519_blog 2>/dev/null)" "atheos"