012711614ccffce483c4a873d74de0bb1bcc9494
6
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 hermes
|
c57aff3c33 |
docs: branch protection is a recommendation, not a blocker (operator decision)
The operator has decided not to enable branch protection on armistace/blog while the editor is LAN-only: "leave it as a recommendation - I won't do that unless I take this out to the wider internet" That is a reasonable call, so this records it as an ACCEPTED RISK rather than an open task. Test behaviour changed to match. The suite still attempts a direct push to master with the deploy key on EVERY run - the check is not removed, because that is what would catch the setting being lost or never applied. But when protection is off it now reports ACCEPTED-RISK master is NOT protected - the deploy key CAN push master directly instead of FAIL, so the suite is green and CI-usable (it previously exited 1, meaning a genuine regression would have been buried in expected noise). The run flips back to "PASS master is protected" by itself the moment protection is enabled, so the recommendation self-cancels with nobody having to remember it. Summary line now reads RESULT: 44 passed, 0 failed, 1 accepted-risk Docs record WHY the risk is acceptable (the compensating control is reachability, not immutability - nothing off-LAN can reach the editor, and the only LAN user is the operator) and the two triggers that turn it back into a real finding: 1. the editor becomes reachable off-LAN (NPM proxy, port-forward, VPN route) 2. anyone other than the operator gets access Written down because an accepted risk is only acceptable if it is explicit and has a revisit trigger - otherwise it just looks like an oversight later. |
||
|
hermes
|
2d57173e31 |
fix: CodeGit looked like an empty black box - hardcoded panel height + cache
Reported as "why can't i see anything from codegit its just a black box".
NOT a codegit bug. Verified first, exhaustively, 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 console; every DOM helper codegit calls exists.
ROOT CAUSE 1 - a generic CSS rule hardcodes a height
theme/modules/common/_panel.scss -> theme/main.min.css:
panel { ... height: 500px; ... }
That forces EVERY modal panel to a fixed 500px box. Measured live:
#dialog 404px (clamped by max-height:70vh + overflow-y:auto)
#codegit panel 500px, while its content is only 119px
toolbar ("Branch") top 620 vs dialog bottom 490 -> OUTSIDE the modal
So ~380px of the dialog was empty dead space and the Branch line rendered below the
modal, invisible. Fixed to height:auto with max-height:58vh (long content still scrolls):
before: panel 500 -> toolbar top 620, dialog bottom 490 (outside)
after: panel 136 -> toolbar top 256, dialog bottom 279 (inside)
_panel.scss updated too so the source does not contradict the shipped CSS.
ROOT CAUSE 2 - the fix was invisible because of the browser cache
Atheos serves theme/main.min.css and the generated bundles from stable URLs with no cache
headers, so browsers legitimately reuse their cached copy - which is why the first attempt
appeared to do nothing. nginx now sends Cache-Control: no-cache, must-revalidate for
*.css/*.js: caching still works (cheap 304s) but changes are always picked up.
Adds
scripts/fix-panel-layout.sh idempotent re-apply for both fixes (lost on update-atheos)
docs/known-issues.md §4, §5 both findings, with the measurements
Tests (44 passed, 1 failed - the failure is branch protection, a genuine open item)
+ panel must not be fixed at 500px
+ theme CSS must send Cache-Control: no-cache
|
||
|
hermes
|
c4a4de1da7 |
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.
|
||
|
hermes
|
976b3f0005 |
docs: record the review follow-ups, with evidence for the one we declined
Second review round asked for three things. Two adopted, one measured and declined.
ADOPTED
- Removing the Terminal plugin is recommended in writing as the right move (it is the
largest attack surface, and macros already provide command execution). Left installed
because it is the operator's plugin - removing a tool unilaterally is not the agent's
call. Documented how: rename to Terminal.disabled (readDirectory() skips *.disabled).
- Automatic password rotation and the deploy-key-read-only suggestion are recorded as
operator decisions with their trade-offs.
DECLINED, WITH EVIDENCE
- "php.ini should disable exec/shell_exec/passthru/system by default." Measured, this
would break the editor's core function, so it cannot be a default:
components/codegit/* -> Common::safe_execute 31 call sites (every git op)
components/macro/* -> Common::raw_execute 1 call site
common.php raw_execute -> function_exists("system") / function_exists("exec")
Every commit, push and fetch from the editor runs through exec/system. It stays a
deliberate exposure lever (S-7), not a default.
CORRECTED
- Review claimed the redacted patch file had a syntax error. Verified on the guest with
php83 -l: "No syntax errors detected". The placeholder is a plain string literal inside
a double-quoted PHP string. That finding was wrong, and is recorded as such rather than
"fixed" to satisfy a false positive.
Also fixed the section ordering in the doc.
|
||
|
hermes
|
8ea645386f |
fix: rotate the terminal plugin password, remove it from the docs
Responds to the independent security review, which correctly flagged that the previous
commit published a live credential.
The Terminal plugin's in-app password gate is a hardcoded constant. The previous commit
quoted the value verbatim in docs/known-issues.md - so a repo that is readable by
collaborators (and could become public) contained a working credential for a web shell
running as the php-fpm user. That was my error.
Changed:
- ROTATED the password on the guest to a random value (no longer the upstream default,
and no longer anything a reader of the source could guess).
- REMOVED the value from docs/known-issues.md. The doc now describes the mechanism and
says how to read it on the guest, rather than reproducing it.
- REDACTED it from patches/Terminal-terminal.php.patched (shows a placeholder; the real
value lives only on the guest).
- Documented that a rebuild returns to the upstream default, so it must be rotated again
after any rebuild.
Also records the honest recommendation the reviewers asked for: this plugin is the largest
attack surface in the container, it duplicates capability the macros already provide, and
the right move is to remove it unless a browser shell is specifically wanted. S-7
(disable exec/proc_open) is named as the lever that would neutralise both.
Note on what this does NOT fix: the RCE is inherent to the plugin - any browser terminal
runs commands as the web user, and the gate is a single shared constant with no per-user
auth. Rotating the value reduces trivial access; it does not make the plugin safe. That is
stated plainly rather than papered over.
|
||
|
hermes
|
5ff6a06e82 |
feat: document plugin failures and fix the Terminal plugin + macro dialog crash
Diagnosed two independent faults reported from the running editor.
1. Terminal plugin (market) did not work as shipped.
Root cause: it is a port of Fluidbyte/Codiad-Terminal and calls Common::execute(),
which Atheos REMOVED (renamed to raw_execute with a different return shape).
plugin calls : Common::execute(...) -> MISSING
available : Common::raw_execute(...) -> ["code"=>int, "output"=>array]
Fixed to raw_execute + implode of the output array. Verified against the real
method: pwd/echo return correctly, and after chdir it reports the blog repo path,
so the terminal opens in the right directory.
Two further upstream defects found and deliberately NOT fixed, but documented so
nobody trusts them: the block-list is dead code (`explode(",", BLOCKED === true)`
passes a boolean, so ssh/telnet are never blocked), and `strtolower($command ===
"exit")` means typing exit does not clear the auth flag.
Security note recorded: this plugin is a browser shell as the php-fpm user, gated
by a hardcoded in-app constant (PASSWORD="terminal"). It is not a secret. Strongest
argument yet for keeping the container LAN-only.
2. Settings -> Macro dialog returned HTTP 500.
Root cause: Atheos's own components/macro/dialog.php:35 does implode(", ",
$macro["fTypes"]) which requires an ARRAY, but class.macro.php::save() writes the
STRING "N/A" for folder macros. 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 .../components/macro/dialog.php:35
Our writer macros are all folder-scoped, so adding them exposed it.
Fixed by correcting the DATA (fTypes ""N/A"" -> "[]") rather than patching Atheos's
code, so the fix survives an Atheos update. Verified implode() now succeeds for all
8 macros. The trade-off and the proper upstream fix are both documented.
Also records the diagnostic order for any future plugin problem, and the rule of
thumb: a Codiad-era plugin's Common:: calls are probably stale.
Adds:
docs/known-issues.md both faults, root causes, fixes, verification
patches/Terminal-terminal.php.patched the patched plugin file
files/macros.db.php regenerated with the corrected fTypes
|