Skip to content

Bound CLI paths and clear the new-code findings on main#461

Merged
JE-Chen merged 3 commits into
devfrom
chore/sonar-newcode-cleanup
Jul 23, 2026
Merged

Bound CLI paths and clear the new-code findings on main#461
JE-Chen merged 3 commits into
devfrom
chore/sonar-newcode-cleanup

Conversation

@JE-Chen

@JE-Chen JE-Chen commented Jul 23, 2026

Copy link
Copy Markdown
Member

main's branch quality gate went ERROR after the release merge — not from the release itself, but because a PR gate only rates its own diff while the branch gate rates the whole new-code window. This clears the fixable findings.

Security — pythonsecurity:S8707 (path traversal from argv)
New je_auto_control/utils/path_guard/: validate_path() canonicalises with os.path.realpath (so .. and symlinks resolve before the check) and requires the result to sit under the working directory, the user's home or the temp dir. AUTOCONTROL_ALLOWED_PATH_ROOTS extends the roots for containers writing to a mounted volume. Wired into the three CLIs that took a path from argv: stubs.generator (also suffix-locked to .pyi), config_bundle export|import (import additionally requires the file to exist), remote_desktop.turn_config. 10 headless tests cover traversal, containment, suffix, env override and the CLI refusal path.

CI reproducibility — docker:S8541/S8544, githubactions:S8544
docker/Dockerfile + Dockerfile.xfce pin pip and accept wheels only; the editable install of the copied source keeps an inline justification. The reusable action-json-lint workflow now defaults to a pinned je_auto_control==0.0.214 spec.

Test correctness — python:S5863, python:S5779
Determinism assertions bound their two calls to names, so the assertion is no longer literally f(x) == f(x). The two total_record scripts stop hiding an assert inside a handler that catches AssertionError — a mismatch now exits or reports instead of being swallowed.

Accessibility — Web:InputWithoutLabelCheck
Real label for= associations for the file, language and bearer-token inputs (aria-label alone did not satisfy the rule).

Not touched: 27 python:S2068 hits on i18n label strings ("eml_password_label": "パスワード:") — the key name trips the rule, there is no credential. host_service.py writes to a module constant, not to argv. Both need a suppression decision rather than a code change.

Verified locally: full headless suite 3704 passed, ruff clean, bandit clean, radon reports no function at C or worse in the new module.

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.
@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 29 complexity · 0 duplication

Metric Results
Complexity 29
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

JE-Chen added 2 commits July 23, 2026 11:23
The runtime layer resolved and built its dependencies straight from PyPI,
so a rebuild could pick up a different set and any sdist in the graph got
to run its setup script inside the image. A throwaway builder stage now
produces every wheel, and the runtime installs with --only-binary and
--no-index against that directory — nothing new can be fetched there.

Drop the two suppressions that turned out to be inert: NOSONAR is not
honoured inside a Dockerfile or a `run: |` block, so they were noise.
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
E Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

💡 Need a hand with PR review? Try Gitar by Sonar!

@JE-Chen
JE-Chen merged commit 546fc95 into dev Jul 23, 2026
23 of 24 checks passed
@JE-Chen
JE-Chen deleted the chore/sonar-newcode-cleanup branch July 23, 2026 04:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant