feat(#93): filesystem permission pre-flight for credential configs (0600/0700)
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 13s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 13s
CI / Lint (ruff) (pull_request) Successful in 8s
CI / Tests (py3.12 / windows-latest) (pull_request) Failing after 24s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 12s
CI / Catalog signature (pull_request) Successful in 8s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 13s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 13s
CI / Lint (ruff) (pull_request) Successful in 8s
CI / Tests (py3.12 / windows-latest) (pull_request) Failing after 24s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 12s
CI / Catalog signature (pull_request) Successful in 8s
ssh-mcp refuses to start if its config is group/world-readable (mode & 0o077 → throws, requiring dir 0700 / file 0600). A GUI user has no idea what chmod 600 means — they just get a dead server. This checks it and offers a one-click fix. Core (pure, POSIX-only, injectable stat/chmod so tests never touch a real file): - permission_status(path): the ssh-mcp rule — any group/other bit set (mode & 0o077) is not-ok; file must be 0600, its dir 0700. Returns None on Windows (modes don't apply) or when the file is absent (nothing to pre-flight). Plain-language problems naming the offending octal mode. - fix_permissions(path): chmod file → 0600, dir → 0700. No-op on Windows; reports an OSError instead of raising. - sidecar_permission_warnings() / sidecar_permission_fix_target(): tie the check to #91's sidecar path resolution so it knows WHICH file to inspect. Mirror the sidecar-warnings shape. GUI: a warning label + "Fix permissions" button in the stdio editor (mirrors the removed-flag surface), shown only when the sidecar exists and is too open. Hidden on Windows and for non-sidecar servers. Smoke-tested headlessly. pytest green (529 passed), ruff + format clean. Closes #93. Part of epic #94. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
2e5d0351b4
commit
0b2827e6b8
+146
@@ -2777,6 +2777,152 @@ def sidecar_warnings(
|
||||
return out
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Filesystem permission pre-flight (issue #93, epic #94)
|
||||
#
|
||||
# ssh-mcp refuses to start if its config is group/world-readable: it throws when
|
||||
# `mode & 0o077` is set, requiring dir 0700 / file 0600. A GUI user has no idea
|
||||
# what `chmod 600` means — they just get a dead server. Generalise: any config
|
||||
# file that carries credentials should be permission-checked. POSIX only — modes
|
||||
# don't apply on Windows, where this degrades to a clean no-op.
|
||||
#
|
||||
# Depends on #91's sidecar path resolution to know WHICH file to check.
|
||||
# --------------------------------------------------------------------------- #
|
||||
def _is_windows_platform(platform: str | None) -> bool:
|
||||
p = platform if platform is not None else sys.platform
|
||||
return p.startswith("win")
|
||||
|
||||
|
||||
def _permission_mode(path, stat_mode) -> int | None:
|
||||
"""The 0o777 permission bits of `path`, or None if it can't be stat-ed."""
|
||||
if stat_mode is not None:
|
||||
return stat_mode(path)
|
||||
try:
|
||||
return os.stat(path).st_mode & 0o777
|
||||
except OSError:
|
||||
return None
|
||||
|
||||
|
||||
def permission_status(
|
||||
path: str | os.PathLike,
|
||||
*,
|
||||
platform: str | None = None,
|
||||
stat_mode=None,
|
||||
) -> dict | None:
|
||||
"""Check a credential-bearing config file's mode (and its directory's). Pure.
|
||||
|
||||
Returns None on Windows (POSIX modes don't apply) or when the file doesn't
|
||||
exist (nothing to check). Otherwise a dict:
|
||||
``{path, mode, file_ok, dir, dir_mode, dir_ok, ok, problems}``.
|
||||
|
||||
The rule mirrors ssh-mcp's own guard: any group/other bit set (``mode &
|
||||
0o077``) is not-ok — the file must be 0600, its directory 0700. ``stat_mode``
|
||||
is an injectable ``path -> int|None`` (the 0o777 bits) so tests never chmod a
|
||||
real file; it defaults to a real ``os.stat``.
|
||||
"""
|
||||
if _is_windows_platform(platform):
|
||||
return None
|
||||
p = Path(path)
|
||||
fmode = _permission_mode(p, stat_mode)
|
||||
if fmode is None:
|
||||
return None # file absent / unreadable -> nothing to pre-flight
|
||||
dmode = _permission_mode(p.parent, stat_mode)
|
||||
problems: list[str] = []
|
||||
file_ok = not (fmode & 0o077)
|
||||
if not file_ok:
|
||||
problems.append(
|
||||
f"the config file is readable by other users (mode {fmode:04o}); "
|
||||
f"ssh-mcp requires 0600 and refuses to start otherwise"
|
||||
)
|
||||
# The directory is only judged when we could read its mode.
|
||||
dir_ok = dmode is None or not (dmode & 0o077)
|
||||
if dmode is not None and not dir_ok:
|
||||
problems.append(
|
||||
f"the containing directory is accessible to other users (mode {dmode:04o}); "
|
||||
f"ssh-mcp requires 0700"
|
||||
)
|
||||
return {
|
||||
"path": p,
|
||||
"mode": fmode,
|
||||
"file_ok": file_ok,
|
||||
"dir": p.parent,
|
||||
"dir_mode": dmode,
|
||||
"dir_ok": dir_ok,
|
||||
"ok": file_ok and dir_ok,
|
||||
"problems": problems,
|
||||
}
|
||||
|
||||
|
||||
def fix_permissions(
|
||||
path: str | os.PathLike,
|
||||
*,
|
||||
platform: str | None = None,
|
||||
chmod=None,
|
||||
) -> tuple[bool, str | None]:
|
||||
"""Tighten a config file to 0600 and its directory to 0700. POSIX only.
|
||||
|
||||
Returns ``(changed, note)``. On Windows: ``(False, None)`` — nothing to do.
|
||||
``chmod`` is an injectable ``(path, mode) -> None`` so tests don't touch real
|
||||
files; it defaults to ``os.chmod``. Only the bits that are currently wrong are
|
||||
reported, but both file and dir are set unconditionally (cheap and idempotent).
|
||||
"""
|
||||
if _is_windows_platform(platform):
|
||||
return False, None
|
||||
p = Path(path)
|
||||
chmod = chmod if chmod is not None else os.chmod
|
||||
try:
|
||||
chmod(p, 0o600)
|
||||
chmod(p.parent, 0o700)
|
||||
except OSError as e:
|
||||
return False, f"could not change permissions: {e}"
|
||||
return True, "set the config file to 0600 and its directory to 0700"
|
||||
|
||||
|
||||
def sidecar_permission_warnings(
|
||||
data: dict,
|
||||
*,
|
||||
platform: str | None = None,
|
||||
environ: dict | None = None,
|
||||
home: str | os.PathLike | None = None,
|
||||
stat_mode=None,
|
||||
) -> list[str]:
|
||||
"""Permission advisories for a server's sidecar config (#93 over #91's path).
|
||||
|
||||
Resolves the ServerSpec sidecar path, and — when that file exists and is
|
||||
group/other-accessible — returns a plain-language warning per problem. Empty
|
||||
list == fine, no sidecar, or Windows. Mirrors ``sidecar_warnings``' shape so
|
||||
the GUI wiring is identical.
|
||||
"""
|
||||
if _is_windows_platform(platform):
|
||||
return []
|
||||
spec = resolve_server_spec(data)
|
||||
if spec is None:
|
||||
return []
|
||||
path = sidecar_path(spec, platform=platform, environ=environ, home=home)
|
||||
if path is None:
|
||||
return []
|
||||
status = permission_status(path, platform=platform, stat_mode=stat_mode)
|
||||
if status is None or status["ok"]:
|
||||
return []
|
||||
return [f"{spec.package}: {prob}" for prob in status["problems"]]
|
||||
|
||||
|
||||
def sidecar_permission_fix_target(
|
||||
data: dict,
|
||||
*,
|
||||
platform: str | None = None,
|
||||
environ: dict | None = None,
|
||||
home: str | os.PathLike | None = None,
|
||||
) -> Path | None:
|
||||
"""The sidecar path a "fix permissions" action should chmod, or None."""
|
||||
if _is_windows_platform(platform):
|
||||
return None
|
||||
spec = resolve_server_spec(data)
|
||||
if spec is None:
|
||||
return None
|
||||
return sidecar_path(spec, platform=platform, environ=environ, home=home)
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Validation
|
||||
# --------------------------------------------------------------------------- #
|
||||
|
||||
Reference in New Issue
Block a user