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.