PROTECTED_ROOTS covered only the running /opt/serverkit-<x>, so the
previous version's .env (JWT key) in the sibling slot was readable by
any role with files.read, including the default viewer.
The C1/C2 reference conversion: FileService raises typed errors
(PermissionDenied/NotFound/Conflict/Validation) and returns bare data;
files.py stops translating {'success': bool} envelopes and stops choosing
status codes by sniffing error prose. Denials are now 403 regardless of
wording, missing paths 404, already-exists 409. Success bodies keep the
compatibility 'success': True key so consumers are unchanged. The plan's
'eight callers' blocker was a substring-grep artifact — FileService has
exactly one consumer. Sniffing census 23 -> 11.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot review on the GHSA-rm3m-9mvw-68fh patch caught two issues:
- _BACKEND_DIR/_INSTALL_DIR were one level too shallow (backend/app and
backend/ instead of backend/ and the install root), which left
<install>/.env and frontend/dist reachable on the deployed layout —
the exact PoC path from the advisory. _BACKEND_DIR stays in
PROTECTED_ROOTS so flat layouts (code at /app) are covered too.
- The ALLOWED_ROOTS check used a bare startswith, so e.g. /optfoo
matched the /opt root. Both checks now require an exact match or a
path-boundary prefix.
New tests pin the level math (the existing cases computed the expected
layout independently and passed on Windows for the wrong reason) and
simulate the deployed layout with the install dir inside an allowed root.
The 1.7.68 RBAC fix enforced files.read on read endpoints, but the default
viewer role legitimately has files.read=true, and on the documented
install.sh layout the panel install dir (/opt/serverkit) sits under the
allowed root /opt. A viewer could read the backend .env, steal
JWT_SECRET_KEY / SERVERKIT_ENCRYPTION_KEY, and forge admin sessions.
FileService now carries PROTECTED_ROOTS (the panel install dir and
SERVERKIT_CONFIG_DIR), checked inside is_path_allowed before the
ALLOWED_ROOTS test, so every file-manager operation (read, write, browse,
download, upload, ...) refuses panel-internal paths for every role.
Regression tests: protected paths rejected by is_path_allowed; viewer
read/download of the backend .env returns 403; allowed roots keep working.
Co-authored-by: CaptBoykin <CaptBoykin@users.noreply.github.com>
- log_service: grep the service/unit filter with -F so names like
'nginx.service' are matched literally instead of as a regex.
- log_service: clamp the in-Python tail like 'tail -n' — lines=0 sliced
as [-0:] == all lines, negatives kept nearly everything.
- test_files_rbac: add files/upload and files/s3/upload to WRITE_CASES,
and disk-usage/disk-mounts/analyze/type-breakdown to READ_CASES, so the
permission gates on the full /api/v1/files surface are covered.
Follow-ups from the GHSA-4wqh-7f4f-5qmx / GHSA-mc93-rc3x-fpgq review:
- File Manager read endpoints (browse/info/read/search/disk-*/analyze/
type-breakdown/download + s3/browse, s3/read, s3/download-url) only
required a valid JWT, so a user whose files.read permission is revoked
via custom permissions could still read and download files. They now
require permission_required('files', 'read'), matching the write side.
- LogService._read_syslog() interpolated the unit name into a bash -c
string using subprocess.list2cmdline(), which is cmd.exe-style quoting
and does not stop $(...)/backtick expansion under bash. grep now runs
as a plain argv list (with --) and the tail happens in Python.
Reachable only on non-systemd hosts by admins, but the same bug class
as the Postfix advisory.
Regression tests: revoked files.read 403s every read endpoint; the
syslog filter never invokes a shell and passes payloads verbatim.
Mutating File Manager endpoints (write/create/mkdir/delete/rename/copy/
move/chmod/upload and the S3 equivalents) only required a valid JWT, so a
viewer account (files.write=false) could write and delete files. They now
require the files.write permission via permission_required(). Read
endpoints are unchanged. Adds regression tests covering both advisories.
Co-authored-by: AruvasgaChithan <AruvasgaChithan@users.noreply.github.com>
Juan Denis