Repository navigation
Security review fixes (v1.10.2) - #2
Merged
Merged
Conversation
With the documented `-v ~:/opt` mount, /opt is the operator's home directory, and entry.sh reset it to 755 on every start. In the Compose example the node's wallet directory also sat inside that mount. entry.sh now only adds traverse permission to /opt (so legacy ~:/opt mounts keep working) and makes the config directory itself readable. README mounts only ~/nanoNodeMonitor for the monitor and ~/Nano for the node (same host locations as before) and notes that old setups keep working but should switch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The monitor needs only read-only RPC. Tell operators to keep enable_control off and never expose the RPC port publicly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The backup used to be config.php.bak-<ts>, which a web server serves as plain text when the config lives in the webroot (manual installs), and it was created with the default umask. Use a .php name next to the config, copy the original's permission bits, restore the backup when writing the new config fails, and ignore the backups in git and the Docker build context. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The custom-logic scanner in migrate-config.php was a denylist, so
getenv(), backticks, new, eval, ?:, fn, match and any function call
passed, were evaluated as root at container start and written back as
literals (materializing secrets, or producing a config that fatals).
After the existing denylist, require every token to be on an allowlist
of plain assignments (literals, arrays, true/false/null, defined
constants, non-superglobal variables, = ; , ( ) [ ] . - +) and reject
"(" directly after a value, which would be a call. Anything else is
treated as scripted and left untouched. config.sample.php and plain
configs migrate exactly as before.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The default file cache lives in the shared system temp dir. A local user could pre-create it (world-writable or as a symlink) and plant cache entries, which reached the page unescaped via getVersionInformation(), or plant symlinks so writes clobber other files. FileCache now only uses its base directory when it is a real directory (not a symlink), owned by the effective user and not world-writable, and never reads or writes a cache file that is a symlink. An unsafe directory disables the cache (NullCache behaviour, logged), never fatal. The latest monitor version is escaped with e(); normal version strings render byte-identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
While an entry was expired every concurrent request rebuilt it, so a burst of api.php requests fired about 11 RPCs each at the node. FileCache::fetch() now rebuilds under an exclusive flock() on <cachefile>.lock and re-reads after acquiring it; ApcuCache::fetch() does the same with apcu_add() as an atomic lock (released on exit too). Waiting is bounded to 10 s, after which, or when locking is impossible, both fall back to the previous read-callback-write behaviour. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Direct requests to modules/*.php leaked paths through PHP errors, and installs that git-clone into the webroot served .git/, the docs and the Docker files. modules/.htaccess and scripts/.htaccess deny all requests (nothing the browser loads lives there). The root .htaccess answers 404 for .git/, .github/, dotfiles, *.md/yml/yaml/sh/ps1 and Dockerfile using only mod_alias, so it needs no AuthConfig override and cannot 500 the page on hosts that only allow FileInfo. .dockerignore now keeps the root .htaccess in the image. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The error handler assigned the raw response body to innerHTML, so a proxy/CDN error page or any future error path echoing data would be injected as markup. Extract the text with DOMParser and show it in the same .myError box via textContent (fallback 'API error'). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
X-Frame-Options/frame-ancestors are intentionally not set so operators can keep embedding the page. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The GitHub lookup rendered in the footer used a 30 s timeout, so index.php could block for about 32 s when GitHub is slow. Use EXTERNAL_TIMEOUT like the other external lookups. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The publish job holds packages: write, so a moved tag on a third-party action could push a malicious :latest image. Pin every action to its full commit SHA (version in a trailing comment; Dependabot keeps SHA pins updated) and stop persisting the checkout token. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes from a full security review of the fork. The review's headline result: nothing a web visitor sends can reach the node's RPC. Every RPC action is hard-coded and read-only, and the page reads no request input. The fixes below close the remaining gaps. Existing installs keep working unchanged.
~/nanoNodeMonitor:/opt/nanoNodeMonitorinstead of your whole home directory, and the node mounts~/Nano:/root/Nano. Before, the node's wallet files sat inside the monitor container.entry.shno longer runschmod 755on the mount root; it grants traverse-only access. Older-v ~:/optsetups keep working.enable_controloff, and keep RPC off public interfaces.config.backup-<ts>.php, with the original file's permissions. The oldconfig.php.bak-*files could be downloaded as plain text on manual installs. A failed write now really restores the backup.getenv(), backticks,new,evalor?:was executed and rewritten with the evaluated values baked in. Such configs are now skipped: left as-is, and still working.flock, APCu lock). Before, bursts of visitors multiplied RPC load on a slow node.modules/andscripts/now return 403. Repo metadata and docs return 404 when the repo is cloned into the webroot.innerHTML.X-Content-Type-Options: nosniffandReferrer-Policy. Framing is still allowed so operators can embed the page.persist-credentials: false.PROJECT_VERSIONis now 1.10.2.Verification
Lint.
php -lis clean.Config migration.
--dry-runoutput is identical before and after forconfig.sample.phpand for typical plain configs.Local smoke test against a mock node:
/api.phpreturns 503.Cache tests:
Docker image:
/modules/config.php,/modules/and/scripts/return 403./README.md,/.git/and/Dockerfilereturn 404.🤖 Generated with Claude Code