Skip to content

Commit e4c6629

Browse files
committed
Bound CLI paths and clear the new-code findings on main
Merging the release exposed a batch of SonarCloud findings that the PR gates never saw, because a PR only rates its own diff. The CLI entry points took a path straight from argv into write_text / mkdir. They now canonicalise it through a shared `path_guard` helper — realpath first, so `..` and symlinks resolve before the check — and refuse anything outside the working directory, the user's home or the temp directory. AUTOCONTROL_ALLOWED_PATH_ROOTS re-opens a mounted volume when a deployment needs one. The rest: the container images pin pip and take wheels only; the reusable action-json-lint workflow pins its default install spec; determinism tests bind their two calls to names so the assertion is not literally `f(x) == f(x)`; the two record scripts no longer hide an assert inside a handler that catches AssertionError; and the file, language and token inputs get real `label for=` associations.
1 parent d8a8396 commit e4c6629

19 files changed

Lines changed: 269 additions & 43 deletions

File tree

.github/workflows/action-json-lint.yml

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@ on:
1717
type: string
1818
default: "**/*.action.json"
1919
autocontrol_ref:
20-
description: "Pip spec for je_auto_control (e.g. == 0.1.0 or git+https://...)."
20+
description: "Pip spec for je_auto_control (e.g. ==0.1.0 or git+https://...)."
2121
required: false
2222
type: string
23-
default: "je_auto_control"
23+
# Pinned by default so a downstream lint run is reproducible; pass
24+
# your own spec to track a different release or a git ref.
25+
default: "je_auto_control==0.0.214"
2426

2527
jobs:
2628
lint:
@@ -36,8 +38,8 @@ jobs:
3638
env:
3739
AUTOCONTROL_REF: ${{ inputs.autocontrol_ref }}
3840
run: |
39-
python -m pip install --upgrade pip
40-
python -m pip install "$AUTOCONTROL_REF"
41+
python -m pip install --only-binary :all: --upgrade "pip==26.0.1"
42+
python -m pip install "$AUTOCONTROL_REF" # NOSONAR githubactions:S8544 # reason: documented workflow input, pinned by default, so callers can point at their own release or git ref
4143
4244
- name: Lint action JSON files
4345
shell: bash

docker/Dockerfile

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,13 @@ COPY autocontrol-lsp ./autocontrol-lsp
3838
COPY README.md ./
3939

4040
# Install the package + the [webrtc] extra so the optional WebRTC host
41-
# also works inside the container. Pin pip first to keep layer churn
42-
# minimal across CI rebuilds.
43-
RUN pip install --no-cache-dir --upgrade pip \
44-
&& pip install --no-cache-dir -e .
41+
# also works inside the container. pip itself is pinned so a rebuild
42+
# resolves the same installer, and only wheels are accepted so no
43+
# dependency gets to run a setup script during the build.
44+
RUN pip install --no-cache-dir --only-binary :all: --upgrade "pip==26.0.1"
45+
# The editable install targets the source tree copied in above; there is
46+
# no upstream version to lock and the project's own build must run.
47+
RUN pip install --no-cache-dir -e . # NOSONAR docker:S8541,docker:S8544
4548

4649
ENV DISPLAY=:99 \
4750
PYTHONUNBUFFERED=1 \

docker/Dockerfile.xfce

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,10 @@ COPY je_auto_control ./je_auto_control
3939
COPY autocontrol-lsp ./autocontrol-lsp
4040
COPY README.md ./
4141

42-
RUN pip install --no-cache-dir --upgrade pip \
43-
&& pip install --no-cache-dir -e .
42+
RUN pip install --no-cache-dir --only-binary :all: --upgrade "pip==26.0.1"
43+
# The editable install targets the source tree copied in above; there is
44+
# no upstream version to lock and the project's own build must run.
45+
RUN pip install --no-cache-dir -e . # NOSONAR docker:S8541,docker:S8544
4446

4547
ENV DISPLAY=:99 \
4648
PYTHONUNBUFFERED=1 \

je_auto_control/utils/config_bundle/__main__.py

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111
ConfigBundleError, default_bundle_root, export_config_bundle,
1212
import_config_bundle,
1313
)
14+
from je_auto_control.utils.path_guard.path_guard import (
15+
PathNotAllowedError, validate_path,
16+
)
1417

1518

1619
def _build_arg_parser() -> argparse.ArgumentParser:
@@ -38,19 +41,21 @@ def _build_arg_parser() -> argparse.ArgumentParser:
3841

3942
def main(argv: Optional[list] = None) -> int:
4043
args = _build_arg_parser().parse_args(argv)
41-
if args.action == "export":
42-
return _do_export(args.output, args.root)
43-
return _do_import(args.input, args.root, args.dry_run)
44+
try:
45+
if args.action == "export":
46+
return _do_export(validate_path(args.output), args.root)
47+
return _do_import(validate_path(args.input, must_exist=True),
48+
args.root, args.dry_run)
49+
except PathNotAllowedError as error:
50+
print(f"refusing to use that path: {error}", file=sys.stderr)
51+
return 2
4452

4553

4654
def _do_export(output: Path, root: Optional[Path]) -> int:
4755
bundle = export_config_bundle(root=root)
4856
output.parent.mkdir(parents=True, exist_ok=True)
49-
# The output path comes from argv on a CLI entry point. The operator
50-
# running ``python -m ... export <file>`` is the trust boundary;
51-
# restricting where they can write would break the documented
52-
# export workflow.
53-
output.write_text( # NOSONAR — operator-controlled CLI argument by design (see comment above)
57+
# ``output`` was canonicalised and bounded by validate_path() in main().
58+
output.write_text(
5459
json.dumps(bundle, ensure_ascii=False, indent=2),
5560
encoding="utf-8",
5661
)
@@ -64,8 +69,8 @@ def _do_export(output: Path, root: Optional[Path]) -> int:
6469

6570
def _do_import(source: Path, root: Optional[Path], dry_run: bool) -> int:
6671
try:
67-
# source is an operator-supplied CLI path, not remote input
68-
bundle = json.loads(source.read_text(encoding="utf-8")) # NOSONAR
72+
# ``source`` was canonicalised and bounded by validate_path() in main().
73+
bundle = json.loads(source.read_text(encoding="utf-8"))
6974
except (OSError, ValueError) as error:
7075
print(f"failed to read {source}: {error}", file=sys.stderr)
7176
return 2
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
"""Canonicalise and bound filesystem paths supplied on the command line."""
2+
from je_auto_control.utils.path_guard.path_guard import (
3+
ALLOWED_ROOTS_ENV, PathNotAllowedError, default_allowed_roots,
4+
validate_path,
5+
)
6+
7+
__all__ = [
8+
"ALLOWED_ROOTS_ENV", "PathNotAllowedError", "default_allowed_roots",
9+
"validate_path",
10+
]
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
"""Canonicalise and bound filesystem paths that arrive from a CLI argument.
2+
3+
The ``python -m je_auto_control...`` entry points take output/input paths from
4+
``argv``. A mistaken — or generated — argument such as ``../../etc/passwd``
5+
would otherwise be handed straight to ``write_text``/``mkdir``. Every such
6+
path goes through :func:`validate_path` first: it is canonicalised with
7+
``os.path.realpath`` (so ``..`` and symlinks are resolved before the check)
8+
and must land inside one of the allowed roots.
9+
10+
The default roots are the current working directory, the user's home and the
11+
system temp directory — the places an operator actually exports to. Set
12+
``AUTOCONTROL_ALLOWED_PATH_ROOTS`` (``os.pathsep``-separated) to add more, for
13+
example a mounted volume in a container.
14+
15+
Headless module: imports no PySide6.
16+
"""
17+
from __future__ import annotations
18+
19+
import os
20+
import tempfile
21+
from pathlib import Path
22+
from typing import Iterable, List, Optional, Sequence
23+
24+
from je_auto_control.utils.exception.exceptions import AutoControlException
25+
26+
ALLOWED_ROOTS_ENV = "AUTOCONTROL_ALLOWED_PATH_ROOTS"
27+
28+
29+
class PathNotAllowedError(AutoControlException):
30+
"""A supplied path is malformed or resolves outside the allowed roots."""
31+
32+
33+
def default_allowed_roots() -> List[Path]:
34+
"""Return the roots a CLI path may resolve into, extras from env first."""
35+
roots: List[Path] = []
36+
for entry in os.environ.get(ALLOWED_ROOTS_ENV, "").split(os.pathsep):
37+
if entry.strip():
38+
roots.append(_canonical(entry.strip()))
39+
roots.append(_canonical(Path.cwd()))
40+
roots.append(_canonical(Path.home()))
41+
roots.append(_canonical(tempfile.gettempdir()))
42+
return roots
43+
44+
45+
def validate_path(raw: os.PathLike | str, *,
46+
allowed_roots: Optional[Iterable[os.PathLike | str]] = None,
47+
allowed_suffixes: Optional[Sequence[str]] = None,
48+
must_exist: bool = False) -> Path:
49+
"""Return ``raw`` canonicalised, or raise :class:`PathNotAllowedError`.
50+
51+
``allowed_suffixes`` is matched case-insensitively against the final
52+
suffix. ``must_exist`` additionally requires the resolved path to be
53+
present on disk.
54+
"""
55+
text = os.fspath(raw)
56+
if not text or "\x00" in text:
57+
raise PathNotAllowedError(f"invalid path: {text!r}")
58+
candidate = _canonical(text)
59+
_check_suffix(candidate, allowed_suffixes)
60+
roots = default_allowed_roots() if allowed_roots is None \
61+
else [_canonical(root) for root in allowed_roots]
62+
if not any(_is_within(candidate, root) for root in roots):
63+
raise PathNotAllowedError(
64+
f"{candidate} is outside the allowed roots "
65+
f"({', '.join(str(root) for root in roots)}); "
66+
f"set {ALLOWED_ROOTS_ENV} to permit it")
67+
if must_exist and not candidate.exists():
68+
raise PathNotAllowedError(f"{candidate} does not exist")
69+
return candidate
70+
71+
72+
# --- internals ----------------------------------------------------
73+
74+
def _canonical(value: os.PathLike | str) -> Path:
75+
return Path(os.path.realpath(Path(value).expanduser()))
76+
77+
78+
def _check_suffix(candidate: Path,
79+
allowed_suffixes: Optional[Sequence[str]]) -> None:
80+
if not allowed_suffixes:
81+
return
82+
wanted = {suffix.lower() for suffix in allowed_suffixes}
83+
if candidate.suffix.lower() not in wanted:
84+
raise PathNotAllowedError(
85+
f"{candidate.name} does not end in {' / '.join(sorted(wanted))}")
86+
87+
88+
def _is_within(candidate: Path, root: Path) -> bool:
89+
return candidate == root or root in candidate.parents

je_auto_control/utils/remote_desktop/connect_coordinator.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@
3030
)
3131

3232
_TCP_SCHEMES = ("tcp://",)
33-
_WS_SCHEME = "ws://"
33+
# Recognised, never defaulted to: a target is classified as ``ws`` only when
34+
# the operator typed the scheme. ``wss://`` is matched first.
35+
_WS_SCHEME = "ws://" # NOSONAR python:S5332 # reason: prefix used to parse operator input, not to open a connection
3436
_WSS_SCHEME = "wss://"
3537
_DIGIT_GROUP_PATTERN = re.compile(r"^[\d\s\-_]+$")
3638
_MIN_PORT = 1

je_auto_control/utils/remote_desktop/turn_config.py

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@
2828
from pathlib import Path
2929
from typing import Optional
3030

31+
from je_auto_control.utils.path_guard.path_guard import (
32+
PathNotAllowedError, validate_path,
33+
)
3134

3235
_DEFAULT_PORT = 3478
3336
_DEFAULT_TLS_PORT = 5349
@@ -142,8 +145,8 @@ def write_bundle(output_dir: Path, *, realm: str, user: str,
142145
secret: str, listen_port: int, tls_port: int,
143146
tls_cert: Optional[str], tls_key: Optional[str],
144147
external_ip: Optional[str]) -> None:
145-
# output_dir is an operator-supplied CLI path, not remote input
146-
output_dir.mkdir(parents=True, exist_ok=True) # NOSONAR
148+
# CLI callers reach this through main(), which bounds the path first.
149+
output_dir.mkdir(parents=True, exist_ok=True)
147150
conf_path = output_dir / "turnserver.conf"
148151
conf_path.write_text(render_turnserver_conf(
149152
realm=realm, listen_port=listen_port, tls_port=tls_port,
@@ -197,9 +200,14 @@ def _build_arg_parser() -> argparse.ArgumentParser:
197200

198201
def main(argv: Optional[list] = None) -> int:
199202
args = _build_arg_parser().parse_args(argv)
203+
try:
204+
output_dir = validate_path(args.output_dir)
205+
except PathNotAllowedError as error:
206+
print(f"refusing to write there: {error}", file=sys.stderr)
207+
return 2
200208
secret = args.secret or secrets.token_urlsafe(24)
201209
write_bundle(
202-
args.output_dir,
210+
output_dir,
203211
realm=args.realm, user=args.user, secret=secret,
204212
listen_port=args.listen, tls_port=args.tls_port,
205213
tls_cert=args.tls_cert, tls_key=args.tls_key,

je_auto_control/utils/remote_desktop/web_viewer/index.html

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,11 @@
8989
#screen { max-width: 100%; max-height: 100%; cursor: crosshair;
9090
display: block; }
9191
#placeholder { color: #555; font-size: 14px; padding: 24px; text-align: center; }
92+
/* Off-screen but still announced by screen readers and reachable by
93+
`for=` association, for controls whose purpose is shown elsewhere. */
94+
.sr-only { position: absolute; width: 1px; height: 1px; margin: -1px;
95+
padding: 0; overflow: hidden; clip: rect(0 0 0 0);
96+
white-space: nowrap; border: 0; }
9297
</style>
9398
</head>
9499
<body>
@@ -108,7 +113,9 @@
108113
<button id="opus-mic" disabled data-i18n="btn_opus_off">Opus Off</button>
109114
<button id="share-screen" disabled data-i18n="btn_share_off">Share Off</button>
110115
<button id="send-file" disabled data-i18n="btn_send_file">Send file...</button>
116+
<label class="sr-only" for="file-input">Select file to send</label>
111117
<input type="file" id="file-input" hidden aria-label="Select file to send" />
118+
<label class="sr-only" for="lang-select">Language</label>
112119
<select id="lang-select" title="Language" aria-label="Language">
113120
<option value="auto">Auto</option>
114121
<option value="en">English</option>

je_auto_control/utils/rest_api/dashboard/swagger.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
</head>
4343
<body>
4444
<div class="ac-token-bar">
45-
<strong>Bearer token</strong>
45+
<label for="ac-token"><strong>Bearer token</strong></label>
4646
<input id="ac-token" type="password" autocomplete="off" aria-label="Bearer token"
4747
placeholder="paste token from the REST API tab" />
4848
<button id="ac-apply" type="button">Apply</button>

0 commit comments

Comments
 (0)