diff --git a/docs/known-issues.md b/docs/known-issues.md index b22c6be..840c883 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -229,7 +229,11 @@ Worth doing if the dialog starts 500-ing again after editing a macro in the UI. **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: +> **⚠️ Superseded by §6.** This section fixed a real layout bug, but it was NOT the cause of the +> empty CodeGit dialog. The actual cause was a relative project path making the request 403, which +> the modal silently swallowed. Both are fixed; if a dialog is ever blank again, read §6 first. + +**This was NOT a codegit bug in the JS sense.** Verified exhaustively first: ``` all four panels return real content through the authenticated API: @@ -280,23 +284,156 @@ after: panel 136px → toolbar top 256, dialog bottom 279 (inside) ## 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 above looked -like it had not worked at all. +**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?" -**Root cause:** Atheos serves `theme/main.min.css` and the generated `public/*.min.js|css` bundles -from **stable URLs with no cache headers**, so nginx adds only `Last-Modified`/`ETag` and browsers -legitimately reuse their cached copy. +**It was not Firefox.** Verified in a real, scripted Firefox (155.0) against the live server with the +fix applied: -**Fix:** nginx now sends `Cache-Control: no-cache, must-revalidate` for `*.css`/`*.js`. Note this does -**not** disable caching — it forces **revalidation**, so normal loads still get a cheap 304 while any -change is guaranteed to be picked up on the next load. +``` +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 +``` -**If you ever see a stale UI again:** force a reload (Ctrl/Cmd-Shift-R). That is the first thing to try -before assuming a fix failed. +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 + + +``` + +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=`). 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 +## 6. 🔴 Every dialog rendered EMPTY (CodeGit "just a black box") + +**Symptom:** CodeGit — and, it turns out, the Macro / Market / Settings dialogs — opened as an empty +black area. No error, no text, nothing. Reported as *"codegit still no go just a black box"*, twice, +and it survived the CSS fix in §4, because it was never a CSS problem. + +**This is the bug that actually mattered.** §4 and §5 were real defects, but neither was the cause. + +### The chain + +1. **The installer stores a RELATIVE path.** `components/install/process.php` keeps the folder name + as given, so the project store held `{"Blog": "blog"}` — not an absolute path. +2. **`loadState()` hands it to the UI unmodified and persists it** (`common.php`): + + ```php + $projectPath = reset($projectList); // "blog" + ... + SESSION("projectPath", $projectPath); // stored -> the bad value STICKS across logins + ``` + +3. **The file tree puts it straight into the DOM** (`components/filetree/init.js`): + + ```js + let path = atheos.current.projectPath; + `` // data-path="blog" + ``` + +4. **CodeGit then sends `repo="blog"`**, and `getWorkspacePath("blog")` fails its access check: + + ``` + traits/path.php:67 Common::send(403, "Client does not have access."); + ``` + +5. **`modal.js` silently discards the failure** — this is what makes it look like a "black box" + rather than an error: + + ```js + settled: function(reply, status) { + if (status !== 200) return; // no message, no toast, no console error + content.html(reply); + ``` + +So the request 403s, nothing is rendered, and nothing is reported. The dialog is left with an empty +`
`. + +### How it was pinned down + +Not by reading CSS. By capturing the real request/response from a scripted browser: + +``` +POST /dialog.php {"target":"codegit","action":"codegit","repo":"blog"} +-> 403 (and #content innerHTML length: 0) +``` + +A relative path was reproduced directly: + +``` +is_dir("blog") from the app root -> NO +getWorkspacePath("blog") -> 403 Client does not have access. +projects store -> {"Blog": "blog"} +``` + +### Fix (`scripts/fix-project-path.sh`) + +A relative value in the store is **legitimate** — Atheos's own project-creation code resolves +relative → `WORKSPACE/`. `loadState()` simply never does. Two changes, both required: + +**A. Resolve at load time, and re-store it** — this heals sessions already holding the bad value: + +```php +if (!Common::isAbsPath($projectPath)) { + $projectPath = Common::getWorkspacePath($projectPath, true); + SESSION("projectPath", $projectPath); +} +``` + +**B. Make the store absolute** (`{"Blog": "/var/www/atheos/workspace/blog"}`) so that +`array_search($projectPath, $projectList)`, which `loadState()` uses to derive the project *name* +for the tree label, keeps working. Fixing only (A) would silently break the label — the path would +resolve but `array_search` would return `false`, so the tree would show an empty name. + +**Verified** by starting from a deliberately poisoned session (`projectPath="blog"`) and confirming +it self-heals: + +``` +projectPath : /var/www/atheos/workspace/blog <- healed from "blog" +data-path : /var/www/atheos/workspace/blog +dialog.php : 200 with content +#content : 1758 chars +visible : CodeGit Overview Log Transfer Configure Commit Amend ... +``` + +**An existing browser session heals on its next page load** — no hard refresh or re-login needed. + +### Worth remembering + +A 403 from `getWorkspacePath()` presents as *"the dialog is empty"*, with no error anywhere. If any +Atheos dialog is ever blank again, check the **project path** first, then the response code — not the CSS. + +--- + +## 7. 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 @@ -330,12 +467,12 @@ verify them before trusting it. --- -## 7. Follow-ups not done (recorded so they are not forgotten) +## 8. 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 +### 8.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 @@ -344,12 +481,12 @@ Deliberately **not** enabled while the editor is LAN-only (operator decision). F 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 +### 8.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 +### 8.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: @@ -366,18 +503,18 @@ turns the editor into a text editor that cannot commit. So it stays **a delibera 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 +### 8.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 +### 8.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 +### 8.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 +`class.macro.php::save()` should write `[]` instead of `"N/A"` for folder macros. See §4 — the fix is currently applied to the data, so it survives updates, at the cost of being reversible from the UI. diff --git a/scripts/build-and-test.sh b/scripts/build-and-test.sh index 21ffe68..5f2e88e 100644 --- a/scripts/build-and-test.sh +++ b/scripts/build-and-test.sh @@ -130,6 +130,20 @@ else fi echo echo "==============================================================" +echo "REGRESSION: asset URLs are versioned (cache-busting)" +echo "==============================================================" +# Stable asset URLs with no version are why a CSS/JS fix stayed invisible behind the browser cache, +# in every browser incl. Firefox. docs/known-issues.md §5. +ASSETS=$(wget -qO /tmp/bt_page.html --header='Host: 127.0.0.1' http://127.0.0.1/ 2>/dev/null; grep -oE '(href|src)="[^"]+\.(css|js)"' /tmp/bt_page.html 2>/dev/null | wc -l | tr -d ' ') +VER=$(grep -oE '(href|src)="[^"]+\.(css|js)\?v=[0-9]+"' /tmp/bt_page.html 2>/dev/null | wc -l | tr -d ' ') +if [ "${VER:-0}" -ge 4 ]; then + ok "asset URLs are versioned ($VER asset tags carry ?v=)" +elif [ "${ASSETS:-0}" -eq 0 ]; then + note "could not read the served page from the guest (unauthenticated: expected)" +else + bad "only $VER of $ASSETS asset URLs are versioned - a CSS fix can stay hidden in the browser cache" +fi +echo "==============================================================" echo "REGRESSION: rDelete works (GLOB_BRACE is glibc-only; musl lacks it)" echo "==============================================================" # That broke deleting ANY file/folder in the editor AND removing a market plugin (the UI @@ -177,6 +191,28 @@ fi echo echo "==============================================================" +echo "REGRESSION: project path is ABSOLUTE (relative => empty dialogs)" +echo "==============================================================" +# A relative project path makes every getWorkspacePath() call fail its access check with 403, +# which modal.js silently swallows - so CodeGit/Macro/Market/Settings render EMPTY with no error. +# docs/known-issues.md S6. This is the bug that presented as "just a black box". +PP=$(php83 -r ' +$raw=@file_get_contents("/var/www/atheos/data/projects.db.json.php"); +if(!$raw) exit; +preg_match("/<\?php\/\*\|(.*)\|\*\/\?>/s",$raw,$m); +$d=json_decode($m[1],true); +if(is_array($d)) echo reset($d);' 2>/dev/null) +case "$PP" in + /*) ok "projects store holds an absolute path ($PP)" ;; + "") bad "could not read the projects store" ;; + *) bad "projects store holds a RELATIVE path ('$PP') - dialogs will render empty; re-run scripts/fix-project-path.sh" ;; +esac +if grep -q 'isAbsPath($projectPath)' /var/www/atheos/common.php 2>/dev/null; then + ok "loadState resolves a relative projectPath (heals poisoned sessions)" +else + bad "loadState does NOT resolve relative project paths - re-run scripts/fix-project-path.sh" +fi +echo "==============================================================" echo "IDENTITY: git identity is SET by the operator (FR-11 superseded by explicit request)" echo "==============================================================" # FR-11 originally required the agent to leave the identity EMPTY so it could not impersonate diff --git a/scripts/fix-panel-layout.sh b/scripts/fix-panel-layout.sh index bc4e753..e05afa9 100644 --- a/scripts/fix-panel-layout.sh +++ b/scripts/fix-panel-layout.sh @@ -73,4 +73,39 @@ EOF || echo " !! nginx -t failed - restore $NGX.orig" fi +echo "=== 3. cache-busting versioned asset URLs ===" +# Atheos serves theme/main.min.css and the generated bundles from STABLE paths with no version: +# +# A browser then keeps its cached copy across a change, so a CSS/JS fix appears not to work. +# Appending each file's mtime means a changed file gets a NEW url (fetched immediately) and an +# unchanged one keeps its url (stays cached). Better than headers alone: it also defeats a +# browser cache that was already poisoned before any header was set. +SM=$ATH/classes/sourcemanager.php +IX=$ATH/index.php +if grep -q 'filemtime($path)' "$SM" 2>/dev/null; then + echo " getTag already versioned" +else + [ -f "$SM.orig" ] || cp -a "$SM" "$SM.orig" + php83 -r ' +$f=$argv[1]; $s=file_get_contents($f); +$old = " function getTag(\$type, \$path) {\n return \$type === \"css\" ? \"\\t\\n\": \"\\t\\n\";\n }"; +if (strpos($s,$old) === false) { fwrite(STDERR, " !! getTag signature changed - inspect $f\n"); exit(1); } +$new = " function getTag(\$type, \$path) {\n // Version the URL by mtime so a changed asset is fetched immediately instead of being\n // served from the browser cache (docs/known-issues.md §5).\n if (file_exists(\$path)) {\n \$path .= \"?v=\" . filemtime(\$path);\n }\n return \$type === \"css\" ? \"\\t\\n\": \"\\t\\n\";\n }"; +file_put_contents($f, str_replace($old,$new,$s)); echo " versioned getTag\n";' "$SM" + php83 -l "$SM" >/dev/null 2>&1 || echo " !! SYNTAX ERROR in $SM - restore $SM.orig" +fi + +if grep -q 'theme/main.min.css?v=' "$IX" 2>/dev/null; then + echo " index.php already versioned" +else + [ -f "$IX.orig" ] || cp -a "$IX" "$IX.orig" + php83 -r ' +$f=$argv[1]; $s=file_get_contents($f); +$old = " echo(\"\\t\\n\");"; +if (strpos($s,$old) === false) { fwrite(STDERR, " !! theme echo line changed - inspect $f\n"); exit(1); } +$new = " // Version the theme URL by mtime; otherwise a browser keeps its cached copy across a\n // theme change and the fix appears not to work (docs/known-issues.md §5).\n \$themeV = file_exists(\"theme/main.min.css\") ? filemtime(\"theme/main.min.css\") : time();\n echo(\"\\t\\n\");"; +file_put_contents($f, str_replace($old,$new,$s)); echo " versioned index.php theme link\n";' "$IX" + php83 -l "$IX" >/dev/null 2>&1 || echo " !! SYNTAX ERROR in $IX - restore $IX.orig" +fi + echo "=== done ===" diff --git a/scripts/fix-project-path.sh b/scripts/fix-project-path.sh new file mode 100644 index 0000000..350ce43 --- /dev/null +++ b/scripts/fix-project-path.sh @@ -0,0 +1,70 @@ +#!/bin/sh +# Fix: dialogs (CodeGit, Macro, Market, Settings) rendered EMPTY. +# +# Root cause: the project was registered with a RELATIVE path, and the UI used it verbatim. +# +# 1. install/process.php stores a relative folder name: projects.db -> {"Blog": "blog"} +# 2. common.php::loadState() does $projectPath = reset($projectList) -> "blog" +# and persists it to the session, so the bad value STICKS. +# 3. filetree puts it straight into the DOM: +# 4. codegit sends repo="blog"; getWorkspacePath("blog") fails checkPath() -> 403 +# 5. modal.js does `if (status !== 200) return;` -> SWALLOWS the error -> #content stays empty +# +# That last step is why it presents as an empty/black dialog rather than an error: the request +# fails, nothing is rendered, and nothing is reported. +# +# Atheos's own project-creation code resolves relative -> WORKSPACE/, so a relative value in +# the store is legitimate - loadState() simply fails to resolve it. Both fixes are needed: +# A. resolve at load time (this also heals sessions already holding the relative value) +# B. store absolute paths, so array_search() can still map path -> name for the tree label +# +# Usage: run ON the guest as root. Idempotent. +set -e +A=/var/www/atheos + +echo "=== A. resolve a relative projectPath in loadState() ===" +S=$A/common.php +if [ ! -f "$S" ]; then echo " !! $S not found"; exit 1; fi +if grep -q 'isAbsPath($projectPath)' "$S"; then + echo " already applied" +else + [ -f "$S.orig" ] || cp -a "$S" "$S.orig" + php83 -r ' +$f=$argv[1]; $s=file_get_contents($f); +$old = " \$openFiles_db = Common::getObjStore(\"activeFiles\", \"users/\" . \$activeUser);"; +if (strpos($s,$old) === false) { fwrite(STDERR," !! anchor not found in loadState\n"); exit(1); } +$new = " // A project may be registered with a RELATIVE folder name (the installer stores it\n // that way). Handing that to the UI yields data-path=\"blog\", and every later\n // getWorkspacePath() call then fails its access check with 403 - which the modal\n // silently swallows, leaving the dialog empty. Resolve it once here, and re-store it\n // so sessions already holding the relative value heal on their next load.\n if (!Common::isAbsPath(\$projectPath)) {\n \$projectPath = Common::getWorkspacePath(\$projectPath, true);\n SESSION(\"projectPath\", \$projectPath);\n }\n\n" . $old; +file_put_contents($f, str_replace($old,$new,$s)); echo " patched loadState\n";' "$S" + php83 -l "$S" >/dev/null 2>&1 || { echo " !! SYNTAX ERROR - restore $S.orig"; exit 1; } +fi + +echo "=== B. store absolute paths in the projects store ===" +P=$A/data/projects.db.json.php +if [ ! -f "$P" ]; then + echo " !! $P not found" +elif grep -q '"\/' "$P"; then + echo " already absolute" +else + cp -a "$P" "$P.bak-$(date +%Y%m%d-%H%M%S)" + php83 -r ' +$f=$argv[1]; $ws="/var/www/atheos/workspace"; +$raw=file_get_contents($f); +preg_match("/<\?php\/\*\|(.*)\|\*\/\?>/s",$raw,$m); +$d=json_decode($m[1],true); +if(!is_array($d)){ fwrite(STDERR," !! store unreadable\n"); exit(1); } +$c=false; +foreach($d as $n=>$p){ if(!preg_match("#^(/|[A-Za-z]:)#",$p)){ $d[$n]=$ws."/".ltrim($p,"/"); $c=true; } } +if(!$c){ echo " already absolute\n"; exit(0); } +file_put_contents($f,""); +echo " store rewritten -> ".json_encode($d)."\n";' "$P" + chown atheos:atheos "$P" 2>/dev/null || true +fi + +echo "=== verify ===" +php83 -r ' +$raw=file_get_contents("/var/www/atheos/data/projects.db.json.php"); +preg_match("/<\?php\/\*\|(.*)\|\*\/\?>/s",$raw,$m); $d=json_decode($m[1],true); +$p=reset($d); $n=array_search($p,$d); +printf(" path=%s name=%s\n",$p,var_export($n,true)); +printf(" absolute: %s\n", preg_match("#^(/|[A-Za-z]:)#",$p) ? "yes" : "NO");' 2>&1 +echo "=== done ==="