Reported again as "code git still no go just a black box is it because I'm on firefox?"
It is NOT Firefox. Tested properly: installed Firefox 155 and drove the live server with
Playwright, authenticated. Result:
assets Firefox requested: theme/main.min.css?v=1790663824 <- versioned
applied panel rule: height: auto; max-height: 58vh <- the fix IS applied
dialog 193px | panel 138px | toolbar INSIDE | JS errors: none
CodeGit renders correctly in Firefox. Firefox was simply still running the OLD CSS.
WHY THE EARLIER FIX DIDN'T TAKE
Atheos emits asset URLs with no version:
<link rel="stylesheet" href="theme/main.min.css">
<script src="public/modules.min.js"></script>
Same path before and after a change, and originally no cache headers at all. So the browser
keeps its cached copy and never even asks the server. Changing the CSS server-side cannot
help when the browser isn't fetching it - which is why the previous Cache-Control fix was
necessary but NOT sufficient against a cache that was already poisoned.
FIX - versioned URLs
SourceManager::getTag() and the theme link in index.php now append the file mtime:
theme/main.min.css?v=1790663824
A changed file gets a NEW URL, so it is fetched immediately; an unchanged file keeps its URL
and stays cached. This works even against an already-stale cache, which headers alone cannot
do. The page itself is served no-store, so the next normal reload picks up new URLs
automatically - no hard refresh needed by the user.
Both parts are now in scripts/fix-panel-layout.sh (idempotent, verified):
1. Cache-Control: no-cache for *.css/*.js (revalidation; keeps 304s fast)
2. mtime-versioned asset URLs (the part that actually defeats a poisoned cache)
Tests: 45 passed, 0 failed, 1 accepted-risk
+ asset URLs must carry ?v= (guards against this regressing and silently hiding future fixes)
416 lines
18 KiB
Markdown
416 lines
18 KiB
Markdown
# 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. 🟠 The CodeGit panel looked like an empty black box
|
|
|
|
**Symptom:** the CodeGit dialog appeared as a mostly-empty black rectangle. Nothing was *broken* —
|
|
it was unreadable, which reads as "broken".
|
|
|
|
**This was NOT a codegit bug.** Verified exhaustively first:
|
|
|
|
```
|
|
all four panels return real content through the authenticated API:
|
|
overview 200 940 bytes
|
|
log 200 79596 bytes (full commit history)
|
|
transfer 200 1460 bytes
|
|
config 200 1174 bytes
|
|
no JS errors in the browser console; all DOM helpers (replaceClass etc.) exist
|
|
```
|
|
|
|
**Root cause — the generic `panel` rule hardcodes a height.** `theme/modules/common/_panel.scss`
|
|
(after minification, in `theme/main.min.css`):
|
|
|
|
```css
|
|
panel { padding: 5px; width: calc(100% - 200px); overflow: auto; overflow-y: scroll;
|
|
height: 500px; position: relative; border-left: var(--borderLight); }
|
|
```
|
|
|
|
Measured live, that produces:
|
|
|
|
| Element | Height | Problem |
|
|
|---|---|---|
|
|
| `#dialog` | 404px | clamped by `max-height: 70vh` + `overflow-y: auto` |
|
|
| `#codegit panel` | 500px | **forced**, while its content is 119px |
|
|
| `toolbar` ("Branch: …") | top **620** vs dialog bottom **490** | **pushed OUTSIDE the modal** |
|
|
|
|
So ~380px of the dialog was **empty dead space**, and anything after the panel — including the
|
|
Branch line — rendered *below* the modal, invisible. A generic `panel { height: 500px }` is exactly
|
|
the kind of rule that looks like a void.
|
|
|
|
**Fix applied:**
|
|
|
|
```css
|
|
panel { ... height: auto; max-height: 58vh; overflow-y: auto; ... }
|
|
```
|
|
|
|
Content-sized, capped so long content (the 79KB log) still scrolls. Verified live:
|
|
|
|
```
|
|
before: panel 500px → toolbar top 620, dialog bottom 490 (outside)
|
|
after: panel 136px → toolbar top 256, dialog bottom 279 (inside)
|
|
```
|
|
|
|
`theme/modules/common/_panel.scss` is updated too, so the source does not contradict the shipped CSS
|
|
(there is no Sass compiler on the guest, so `main.min.css` is edited directly).
|
|
|
|
---
|
|
|
|
## 5. 🟠 CSS/JS changes were invisible behind the browser cache
|
|
|
|
**Symptom:** a fix to the theme did not appear until a hard refresh — so the panel fix in §4 looked
|
|
like it had not worked at all. Reported twice, the second time with "is it because I'm on Firefox?"
|
|
|
|
**It was not Firefox.** Verified in a real, scripted Firefox (155.0) against the live server with the
|
|
fix applied:
|
|
|
|
```
|
|
assets Firefox requested: theme/main.min.css?v=1790663824 <- versioned
|
|
applied panel rule: height: auto; max-height: 58vh <- the fix
|
|
dialog 193px | panel 138px | toolbar INSIDE | JS errors: none
|
|
```
|
|
|
|
The panel rendered correctly and the Branch toolbar sat inside the modal. Firefox behaves identically
|
|
to Chromium here; it was simply still running the **old cached stylesheet**.
|
|
|
|
**Root cause — stable asset URLs, with nothing to bust them.** Atheos emits:
|
|
|
|
```html
|
|
<link rel="stylesheet" href="theme/main.min.css"> <!-- no version -->
|
|
<script src="public/modules.min.js"></script> <!-- no version -->
|
|
```
|
|
|
|
Same path before and after a change, and originally no cache headers at all. A browser therefore keeps
|
|
its cached copy across an update, and never even asks the server. Changing the CSS on the server
|
|
cannot fix that — the browser is not fetching it.
|
|
|
|
**Fix (two parts):**
|
|
|
|
1. **nginx** sends `Cache-Control: no-cache, must-revalidate` for `*.css`/`*.js`. This does **not**
|
|
disable caching — it forces **revalidation**, so normal loads still get a cheap 304 while changes
|
|
are picked up.
|
|
2. **Versioned URLs.** `SourceManager::getTag()` and the theme link in `index.php` now append the
|
|
file's mtime (`?v=<mtime>`). A changed file gets a **new URL** so it is fetched immediately; an
|
|
unchanged file keeps its URL and stays cached.
|
|
|
|
Part 2 is the one that actually mattered here: it works **even against an already-poisoned cache**,
|
|
which part 1 alone cannot do. Once the *page* refreshes (it is served `no-store`), every asset URL it
|
|
references is new, so the stale CSS is bypassed with no manual clearing.
|
|
|
|
**If you ever see a stale UI again:** force a reload (Ctrl/Cmd-Shift-R). If that fixes it, this
|
|
section is why.
|
|
|
|
**Forcing this one time:** if a browser holds an old copy, any of these work —
|
|
Ctrl/Cmd-Shift-R, `about:blank` then back, a private window, or `rm -rf` the profile's cache. Or just
|
|
change the file's mtime (`touch`) and the URL changes by itself.
|
|
|
|
---
|
|
|
|
## 6. 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.
|
|
|
|
---
|
|
|
|
## 7. 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.
|
|
|
|
### 7.0 Branch protection on `master` — ACCEPTED RISK
|
|
|
|
Deliberately **not** enabled while the editor is LAN-only (operator decision). Full reasoning and the
|
|
"enable it before you expose this host" trigger are in
|
|
[`docs/writer-branch-workflow.md` §2](writer-branch-workflow.md#2-protect-master-on-the-blog-repo--recommendation-deliberately-not-enabled).
|
|
|
|
The suite still probes for it on every run and reports `ACCEPTED-RISK` (not a failure) while it is
|
|
off, so the recommendation self-cancels the moment it is enabled.
|
|
|
|
### 7.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.
|
|
|
|
### 7.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"*.
|
|
|
|
### 7.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.
|
|
|
|
### 7.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.
|
|
|
|
### 7.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.
|