fix: dialogs rendered EMPTY - relative project path caused a silent 403

THE ACTUAL CAUSE of "codegit is just a black box". Sections 4 and 5 were real bugs but
neither was it; this is.

CHAIN
  1. install/process.php stores a RELATIVE folder name -> projects.db {"Blog":"blog"}
  2. common.php::loadState() does $projectPath = reset($projectList) -> "blog", and
     SESSION("projectPath","blog") PERSISTS it, so the bad value sticks across logins
  3. filetree puts it straight into the DOM: <a id="project-root" data-path="blog">
  4. codegit sends repo="blog"; getWorkspacePath("blog") fails checkPath() ->
       traits/path.php:67  Common::send(403, "Client does not have access.")
  5. modal.js SILENTLY discards it:
       settled: function(reply,status){ if (status !== 200) return; ... }
     No toast, no console error, #content left empty.

That last step is why it presents as an empty black dialog rather than an error - the request
fails and nothing reports it. Which is exactly why it survived the CSS fixes.

PINNED DOWN BY CAPTURING THE REAL REQUEST, not by reading CSS:
  POST /dialog.php {"target":"codegit","action":"codegit","repo":"blog"} -> 403
  #content innerHTML length: 0
  getWorkspacePath("blog") -> 403
  projects store           -> {"Blog":"blog"}

FIX (scripts/fix-project-path.sh) - both parts required:
  A. resolve the path in loadState and re-store it, which also HEALS sessions already
     holding the relative value (and re-persists so it stays healed)
  B. make the projects store absolute, so array_search() can still map path -> name for
     the tree label. Fixing only (A) would silently break the label: the path would resolve
     but array_search would return false and the tree would show an empty project name.

VERIFIED from a deliberately poisoned session ("blog"): it self-heals to the absolute path,
dialog returns 200, #content fills with 1758 chars. An EXISTING browser session heals on its
next page load - no hard refresh or re-login needed.

Tests: 47 passed, 0 failed, 1 accepted-risk
  + projects store must hold an absolute path
  + loadState must resolve a relative projectPath

Also marks section 4 as superseded so a future reader doesn't stop at the CSS explanation.
This commit is contained in:
2026-09-29 16:55:08 +10:00
parent 48e439ed6e
commit be8c959de2
3 changed files with 207 additions and 10 deletions
+115 -10
View File
@@ -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* — **Symptom:** the CodeGit dialog appeared as a mostly-empty black rectangle. Nothing was *broken* —
it was unreadable, which reads as "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: all four panels return real content through the authenticated API:
@@ -328,7 +332,108 @@ 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;
`<a id="project-root" data-type="root" data-path="${path}">` // 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
`<section id="content">`.
### 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/<name>`. `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 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 its rightbar button is rendered by `components/sidebars/sb-right.php` from `plugin.json`. Both were
@@ -362,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 These came out of review. They are **operator decisions**, not agent ones — several change what the
editor can do. 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 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 "enable it before you expose this host" trigger are in
@@ -376,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 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. 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 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. 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. Review asked for `php.ini` to disable `exec`, `shell_exec`, `passthru`, `system` by default.
**This would break the editor's core function.** Evidence: **This would break the editor's core function.** Evidence:
@@ -398,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 you would also lose macros and git integration, which is why S-7 already says *"test, do not
blind-apply"*. 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 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 upstream default. Not implemented: it would hardcode a secret into a repo file, which is the same
class of problem this file already documents. 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** — 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. 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. currently applied to the data, so it survives updates, at the cost of being reversible from the UI.
+22
View File
@@ -191,6 +191,28 @@ fi
echo echo
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 "IDENTITY: git identity is SET by the operator (FR-11 superseded by explicit request)"
echo "==============================================================" echo "=============================================================="
# FR-11 originally required the agent to leave the identity EMPTY so it could not impersonate # FR-11 originally required the agent to leave the identity EMPTY so it could not impersonate
+70
View File
@@ -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: <a id="project-root" data-path="blog">
# 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/<name>, 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,"<?php/*|\n".json_encode($d, JSON_PRETTY_PRINT|JSON_UNESCAPED_SLASHES)."\n|*/?>");
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 ==="