Repository navigation
ci: release pipeline staging checks - #293
Conversation
Signed-off-by: Alexander Brichkin (Agonist Development AB) <git@ariada.org>
📝 WalkthroughWalkthroughAdds the Changestox-nox-ariada helper
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ariada-toxnox
participant scan_target
participant Ariada CLI
participant Report filesystem
User->>ariada-toxnox: Run scan command
ariada-toxnox->>scan_target: Pass target and options
scan_target->>Ariada CLI: Execute Ariada scan
Ariada CLI->>Report filesystem: Write scan report
scan_target->>Report filesystem: Read and count findings
scan_target-->>ariada-toxnox: Return AriadaScanResult
ariada-toxnox-->>User: Print JSON or summary
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
integrations/tox-nox-ariada/tox_nox_ariada/__main__.py (1)
3-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGuard the entry point with
__name__.Line 5 runs the CLI during module import. Any import of
tox_nox_ariada.__main__— for example test collection or tooling that walks the package — executes a scan and raisesSystemExit. Add the standard guard.♻️ Proposed refactor
from .cli import main -raise SystemExit(main()) +if __name__ == "__main__": + raise SystemExit(main())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integrations/tox-nox-ariada/tox_nox_ariada/__main__.py` around lines 3 - 5, Update the module-level entry point around cli.main so SystemExit(main()) runs only when __name__ equals "__main__"; keep importing main unchanged and prevent CLI execution during module imports.integrations/tox-nox-ariada/tox_nox_ariada/scanner.py (1)
96-113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
normalize_targethas a test caller; extracting a shared validation helper is still recommended.
normalize_target(lines 96–102) has a direct test reference attest_scanner.py:75. The duplication concern remains valid: bothnormalize_targetandnormalized_targetrepeat the sameis_http_url()check, the same suffix set{".html", ".htm"}, and the identical error message. These will drift as the codebase evolves.Extract a shared validation helper to reduce duplication:
♻️ Proposed refactor
+HTML_SUFFIXES = {".html", ".htm"} + + +def _validate_local_html(target: str) -> Path: + """Validate and resolve a local HTML file path.""" + path = Path(target) + if not path.exists() or path.suffix.lower() not in HTML_SUFFIXES: + raise ValueError( + f"tox/nox Ariada target must be http(s) or an existing HTML file: {target}" + ) + return path.resolve() + + def normalize_target(target: str) -> str: if is_http_url(target): return target - path = Path(target) - if path.exists() and path.suffix.lower() in {".html", ".htm"}: - return path.resolve().as_uri() - raise ValueError(f"tox/nox Ariada target must be http(s) or an existing HTML file: {target}") + return _validate_local_html(target).as_uri() `@contextmanager` def normalized_target(target: str) -> Iterator[str]: if is_http_url(target): yield target return - path = Path(target) - if not path.exists() or path.suffix.lower() not in {".html", ".htm"}: - raise ValueError(f"tox/nox Ariada target must be http(s) or an existing HTML file: {target}") + path = _validate_local_html(target)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integrations/tox-nox-ariada/tox_nox_ariada/scanner.py` around lines 96 - 113, Extract the shared target validation logic from normalize_target and normalized_target into a private helper that handles HTTP URLs, existing .html/.htm files, and the common ValueError message. Update both functions to reuse this helper while preserving normalize_target’s resolved file URI return and normalized_target’s context-manager behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@integrations/tox-nox-ariada/pyproject.toml`:
- Around line 1-11: Update the [build-system] requires entry to require
setuptools>=77.0.0, keeping wheel and the existing PEP 639 license expression in
the [project] metadata unchanged.
In `@integrations/tox-nox-ariada/tests/test_scanner.py`:
- Around line 78-85: Update test_cli_returns_zero_with_no_fail_and_json_output
to capture stdout, parse the CLI output as JSON, and record the
AriadaScanOptions received by fake_scan_target. Assert the captured output is
valid JSON and verify the recorded options have no_fail set to True, while
retaining the zero exit-code assertion.
In `@integrations/tox-nox-ariada/tox_nox_ariada/cli.py`:
- Around line 36-47: Update the CLI flow around scan_target and
AriadaScanOptions to catch setup-time ValueError from target normalization and
OSError from launching ariada, then print the error to stderr and return exit
code 2 instead of letting a traceback escape. Keep the existing scan execution
path unchanged for valid inputs, and add regression coverage for both a missing
HTML target and an unavailable executable case.
In `@integrations/tox-nox-ariada/tox_nox_ariada/scanner.py`:
- Around line 145-147: Update the summary-total handling in the scanner to read
summary["total"] via a single value variable using summary.get("total"), then
validate that value is an int before returning it. Ensure a missing total falls
through without indexing summary["total"] and raising KeyError.
- Around line 78-84: Update the CLI invocation in the scanner flow around runner
to pass a wall-clock timeout derived from options.timeout_ms, and catch
subprocess.TimeoutExpired to terminate the flow with the existing
runtime-failure exit code. Ensure timeout handling still allows cleanup of the
server managed by normalized_target.
- Around line 133-139: Update read_report_summary and scan_target to prevent
stale reports from being accepted: add or invoke a clear_stale_reports helper
before the CLI runs, after creating options.output_dir, to remove or record
existing candidate reports. Catch json.JSONDecodeError while reading candidate
reports and treat malformed files as no usable report, preserving the existing
(None, 0) fallback when no valid report remains.
In `@integrations/tox-nox-ariada/tox_nox_ariada/snippets.py`:
- Line 10: Update the snippet generation around the ariada-toxnox scan recipe so
target is safely escaped before being embedded into tox and nox source. In the
snippet builder that emits the ariada-toxnox command, reject any target
containing CR or LF, use tox-compatible shlex quoting for the tox token, and use
repr(target) for the nox session.run argument. Add regression coverage for
quoted targets and line-break inputs to ensure the generated recipes stay
executable and non-injectable.
---
Nitpick comments:
In `@integrations/tox-nox-ariada/tox_nox_ariada/__main__.py`:
- Around line 3-5: Update the module-level entry point around cli.main so
SystemExit(main()) runs only when __name__ equals "__main__"; keep importing
main unchanged and prevent CLI execution during module imports.
In `@integrations/tox-nox-ariada/tox_nox_ariada/scanner.py`:
- Around line 96-113: Extract the shared target validation logic from
normalize_target and normalized_target into a private helper that handles HTTP
URLs, existing .html/.htm files, and the common ValueError message. Update both
functions to reuse this helper while preserving normalize_target’s resolved file
URI return and normalized_target’s context-manager behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6a3dee24-8405-47d6-b208-b94e28d278c3
📒 Files selected for processing (9)
integrations/tox-nox-ariada/README.mdintegrations/tox-nox-ariada/examples/site/index.htmlintegrations/tox-nox-ariada/pyproject.tomlintegrations/tox-nox-ariada/tests/test_scanner.pyintegrations/tox-nox-ariada/tox_nox_ariada/__init__.pyintegrations/tox-nox-ariada/tox_nox_ariada/__main__.pyintegrations/tox-nox-ariada/tox_nox_ariada/cli.pyintegrations/tox-nox-ariada/tox_nox_ariada/scanner.pyintegrations/tox-nox-ariada/tox_nox_ariada/snippets.py
| [build-system] | ||
| requires = ["setuptools>=69", "wheel"] | ||
| build-backend = "setuptools.build_meta" | ||
|
|
||
| [project] | ||
| name = "tox-nox-ariada" | ||
| version = "0.1.0" | ||
| description = "tox and nox helper that scans generated HTML with the shared Ariada CLI." | ||
| readme = "README.md" | ||
| requires-python = ">=3.9" | ||
| license = "EUPL-1.2" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Which setuptools version added PEP 639 SPDX license expression string support in pyproject.toml?
💡 Result:
Support for PEP 639 SPDX license expression strings in pyproject.toml was introduced in setuptools version 77.0.0 [1][2][3]. Starting with this version, the project.license field in pyproject.toml can be defined as a string containing an SPDX license expression, and the project.license-files field is used to declare license files [1]. While some documentation or discussions may reference version 77.0.3 as a recommended minimum due to early maintenance releases [2], the feature was first implemented and shipped in v77.0.0 [4][5][6].
Citations:
- 1: https://setuptools.pypa.io/en/latest/userguide/license_migration.html
- 2: Minimal version of setuptools supporting license-files pypa/packaging.python.org#1870
- 3: https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html
- 4: https://github.com/pypa/setuptools/blob/v77.0.0/NEWS.rst
- 5: Add initial support for license expression (PEP 639) pypa/setuptools#4706
- 6: Implement PEP 639 pypa/setuptools#4829
Require setuptools>=77 to support PEP 639 SPDX license expressions.
Line 11 uses license = "EUPL-1.2", which is the PEP 639 license expression string form. Versions of setuptools earlier than 77.0.0 reject this form and accept only the table syntax license = { text = "..." }. The current requirement setuptools>=69 permits versions 69–76 that cannot validate this metadata. Update the build requirement to enforce setuptools 77.0.0 or later.
Proposed fix
[build-system]
-requires = ["setuptools>=69", "wheel"]
+requires = ["setuptools>=77", "wheel"]
build-backend = "setuptools.build_meta"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [build-system] | |
| requires = ["setuptools>=69", "wheel"] | |
| build-backend = "setuptools.build_meta" | |
| [project] | |
| name = "tox-nox-ariada" | |
| version = "0.1.0" | |
| description = "tox and nox helper that scans generated HTML with the shared Ariada CLI." | |
| readme = "README.md" | |
| requires-python = ">=3.9" | |
| license = "EUPL-1.2" | |
| [build-system] | |
| requires = ["setuptools>=77", "wheel"] | |
| build-backend = "setuptools.build_meta" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@integrations/tox-nox-ariada/pyproject.toml` around lines 1 - 11, Update the
[build-system] requires entry to require setuptools>=77.0.0, keeping wheel and
the existing PEP 639 license expression in the [project] metadata unchanged.
| def test_cli_returns_zero_with_no_fail_and_json_output(tmp_path: Path, monkeypatch) -> None: # type: ignore[no-untyped-def] | ||
| def fake_scan_target(target, options): # type: ignore[no-untyped-def] | ||
| from tox_nox_ariada.scanner import AriadaScanResult | ||
|
|
||
| return AriadaScanResult(target, 0, "", "", tmp_path / "report.json", 3) | ||
|
|
||
| monkeypatch.setattr("tox_nox_ariada.cli.scan_target", fake_scan_target) | ||
| assert main(["scan", "https://example.test", "--json", "--no-fail"]) == 0 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assert the JSON output and the forwarded no_fail option.
The fake scanner always returns exit code 0. This test passes if main ignores --json or stops forwarding --no-fail. Capture stdout, parse it as JSON, and record the received AriadaScanOptions before asserting options.no_fail is True.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@integrations/tox-nox-ariada/tests/test_scanner.py` around lines 78 - 85,
Update test_cli_returns_zero_with_no_fail_and_json_output to capture stdout,
parse the CLI output as JSON, and record the AriadaScanOptions received by
fake_scan_target. Assert the captured output is valid JSON and verify the
recorded options have no_fail set to True, while retaining the zero exit-code
assertion.
| result = scan_target( | ||
| args.target, | ||
| AriadaScanOptions( | ||
| output_dir=Path(args.output_dir), | ||
| cli_command=args.cli, | ||
| browser=args.browser, | ||
| format=args.format, | ||
| severity_threshold=args.severity_threshold, | ||
| timeout_ms=args.timeout_ms, | ||
| no_fail=args.no_fail, | ||
| ), | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Handle expected scan setup errors.
normalize_target can raise ValueError for an invalid target. subprocess.run can raise OSError when ariada is unavailable or not executable. The CLI does not catch either error, so a normal user error produces a traceback instead of a diagnostic and a stable exit code.
Return exit code 2 after printing the error to stderr. Add regression tests for a missing HTML file and an unavailable executable.
Proposed fix
import argparse
import json
+import sys
from pathlib import Path
...
- result = scan_target(
- args.target,
- AriadaScanOptions(
- output_dir=Path(args.output_dir),
- cli_command=args.cli,
- browser=args.browser,
- format=args.format,
- severity_threshold=args.severity_threshold,
- timeout_ms=args.timeout_ms,
- no_fail=args.no_fail,
- ),
- )
+ try:
+ result = scan_target(
+ args.target,
+ AriadaScanOptions(
+ output_dir=Path(args.output_dir),
+ cli_command=args.cli,
+ browser=args.browser,
+ format=args.format,
+ severity_threshold=args.severity_threshold,
+ timeout_ms=args.timeout_ms,
+ no_fail=args.no_fail,
+ ),
+ )
+ except (OSError, ValueError) as error:
+ print(f"ariada-toxnox: {error}", file=sys.stderr)
+ return 2📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| result = scan_target( | |
| args.target, | |
| AriadaScanOptions( | |
| output_dir=Path(args.output_dir), | |
| cli_command=args.cli, | |
| browser=args.browser, | |
| format=args.format, | |
| severity_threshold=args.severity_threshold, | |
| timeout_ms=args.timeout_ms, | |
| no_fail=args.no_fail, | |
| ), | |
| ) | |
| try: | |
| result = scan_target( | |
| args.target, | |
| AriadaScanOptions( | |
| output_dir=Path(args.output_dir), | |
| cli_command=args.cli, | |
| browser=args.browser, | |
| format=args.format, | |
| severity_threshold=args.severity_threshold, | |
| timeout_ms=args.timeout_ms, | |
| no_fail=args.no_fail, | |
| ), | |
| ) | |
| except (OSError, ValueError) as error: | |
| print(f"ariada-toxnox: {error}", file=sys.stderr) | |
| return 2 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@integrations/tox-nox-ariada/tox_nox_ariada/cli.py` around lines 36 - 47,
Update the CLI flow around scan_target and AriadaScanOptions to catch setup-time
ValueError from target normalization and OSError from launching ariada, then
print the error to stderr and return exit code 2 instead of letting a traceback
escape. Keep the existing scan execution path unchanged for valid inputs, and
add regression coverage for both a missing HTML target and an unavailable
executable case.
| "--timeout-ms", | ||
| str(options.timeout_ms), | ||
| ] | ||
| completed = runner(command, text=True, capture_output=True, check=False) | ||
| report_path, total = read_report_summary(options.output_dir) | ||
| exit_code = completed.returncode | ||
| if options.no_fail and exit_code == 1 and not looks_like_runtime_failure(completed.stderr or ""): |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add a wall-clock timeout to the CLI call.
options.timeout_ms is passed to the Ariada CLI as a per-target budget only. The runner call on line 81 has no timeout. If the CLI hangs — for example a browser process that never exits — the tox or nox session blocks forever, and the local HTTP server in normalized_target stays open.
Pass an explicit timeout and convert subprocess.TimeoutExpired into a runtime failure exit code.
🔧 Proposed fix
- completed = runner(command, text=True, capture_output=True, check=False)
+ deadline_s = max(options.timeout_ms / 1000.0, 1.0) + 60.0
+ try:
+ completed = runner(
+ command, text=True, capture_output=True, check=False, timeout=deadline_s
+ )
+ except subprocess.TimeoutExpired as exc:
+ return AriadaScanResult(
+ target=normalized,
+ exit_code=2,
+ stdout=exc.stdout or "",
+ stderr=f"ariada scan timed out after {deadline_s:.0f}s",
+ report_path=None,
+ total_findings=0,
+ )Note: the test double fake_run in integrations/tox-nox-ariada/tests/test_scanner.py accepts **_kwargs, so it stays compatible with the extra argument.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "--timeout-ms", | |
| str(options.timeout_ms), | |
| ] | |
| completed = runner(command, text=True, capture_output=True, check=False) | |
| report_path, total = read_report_summary(options.output_dir) | |
| exit_code = completed.returncode | |
| if options.no_fail and exit_code == 1 and not looks_like_runtime_failure(completed.stderr or ""): | |
| "--timeout-ms", | |
| str(options.timeout_ms), | |
| ] | |
| deadline_s = max(options.timeout_ms / 1000.0, 1.0) + 60.0 | |
| try: | |
| completed = runner( | |
| command, text=True, capture_output=True, check=False, timeout=deadline_s | |
| ) | |
| except subprocess.TimeoutExpired as exc: | |
| return AriadaScanResult( | |
| target=normalized, | |
| exit_code=2, | |
| stdout=exc.stdout or "", | |
| stderr=f"ariada scan timed out after {deadline_s:.0f}s", | |
| report_path=None, | |
| total_findings=0, | |
| ) | |
| report_path, total = read_report_summary(options.output_dir) | |
| exit_code = completed.returncode | |
| if options.no_fail and exit_code == 1 and not looks_like_runtime_failure(completed.stderr or ""): |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@integrations/tox-nox-ariada/tox_nox_ariada/scanner.py` around lines 78 - 84,
Update the CLI invocation in the scanner flow around runner to pass a wall-clock
timeout derived from options.timeout_ms, and catch subprocess.TimeoutExpired to
terminate the flow with the existing runtime-failure exit code. Ensure timeout
handling still allows cleanup of the server managed by normalized_target.
| def read_report_summary(output_dir: Path) -> tuple[Path | None, int]: | ||
| for name in ("multi-domain-report.json", "scan.json"): | ||
| path = output_dir / name | ||
| if path.exists(): | ||
| data = json.loads(path.read_text(encoding="utf-8")) | ||
| return path, count_findings(data) | ||
| return None, 0 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject stale reports and malformed JSON.
Two problems exist in this read path.
scan_targetcallsoutput_dir.mkdir(parents=True, exist_ok=True)and never clears the directory. If the CLI exits before it writes a report, this function returns the report from a previous run. The result then reports finding counts that do not belong to the current scan.json.loadsis unguarded. A truncated or non-JSON report raisesjson.JSONDecodeErrorand the scan crashes instead of reporting the CLI failure.
Record the pre-run state of the candidate files, or remove them before the run, and handle a decode error as "no usable report".
🔧 Proposed fix
def read_report_summary(output_dir: Path) -> tuple[Path | None, int]:
for name in ("multi-domain-report.json", "scan.json"):
path = output_dir / name
if path.exists():
- data = json.loads(path.read_text(encoding="utf-8"))
- return path, count_findings(data)
+ try:
+ data = json.loads(path.read_text(encoding="utf-8"))
+ except (json.JSONDecodeError, OSError):
+ return path, 0
+ return path, count_findings(data)
return None, 0
+
+
+REPORT_NAMES = ("multi-domain-report.json", "scan.json")
+
+
+def clear_stale_reports(output_dir: Path) -> None:
+ for name in REPORT_NAMES:
+ (output_dir / name).unlink(missing_ok=True)Then call the helper in scan_target before the CLI runs:
options.output_dir.mkdir(parents=True, exist_ok=True)
clear_stale_reports(options.output_dir)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def read_report_summary(output_dir: Path) -> tuple[Path | None, int]: | |
| for name in ("multi-domain-report.json", "scan.json"): | |
| path = output_dir / name | |
| if path.exists(): | |
| data = json.loads(path.read_text(encoding="utf-8")) | |
| return path, count_findings(data) | |
| return None, 0 | |
| def read_report_summary(output_dir: Path) -> tuple[Path | None, int]: | |
| for name in ("multi-domain-report.json", "scan.json"): | |
| path = output_dir / name | |
| if path.exists(): | |
| try: | |
| data = json.loads(path.read_text(encoding="utf-8")) | |
| except (json.JSONDecodeError, OSError): | |
| return path, 0 | |
| return path, count_findings(data) | |
| return None, 0 | |
| REPORT_NAMES = ("multi-domain-report.json", "scan.json") | |
| def clear_stale_reports(output_dir: Path) -> None: | |
| for name in REPORT_NAMES: | |
| (output_dir / name).unlink(missing_ok=True) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@integrations/tox-nox-ariada/tox_nox_ariada/scanner.py` around lines 133 -
139, Update read_report_summary and scan_target to prevent stale reports from
being accepted: add or invoke a clear_stale_reports helper before the CLI runs,
after creating options.output_dir, to remove or record existing candidate
reports. Catch json.JSONDecodeError while reading candidate reports and treat
malformed files as no usable report, preserving the existing (None, 0) fallback
when no valid report remains.
| summary = data.get("summary") | ||
| if isinstance(summary, dict) and isinstance(summary.get("total", 0), int): | ||
| return int(summary["total"]) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Fix the KeyError when summary has no total key.
summary.get("total", 0) returns the default 0, which is an int, so the condition is true even when the key is absent. Line 147 then indexes summary["total"] and raises KeyError. A report that contains a summary object without total crashes the scan after the CLI has already run.
Read the value once and test that value.
🐛 Proposed fix
summary = data.get("summary")
- if isinstance(summary, dict) and isinstance(summary.get("total", 0), int):
- return int(summary["total"])
+ if isinstance(summary, dict):
+ total = summary.get("total")
+ if isinstance(total, int) and not isinstance(total, bool):
+ return total📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| summary = data.get("summary") | |
| if isinstance(summary, dict) and isinstance(summary.get("total", 0), int): | |
| return int(summary["total"]) | |
| summary = data.get("summary") | |
| if isinstance(summary, dict): | |
| total = summary.get("total") | |
| if isinstance(total, int) and not isinstance(total, bool): | |
| return total |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@integrations/tox-nox-ariada/tox_nox_ariada/scanner.py` around lines 145 -
147, Update the summary-total handling in the scanner to read summary["total"]
via a single value variable using summary.get("total"), then validate that value
is an int before returning it. Ensure a missing total falls through without
indexing summary["total"] and raising KeyError.
| "[testenv:a11y]", | ||
| "deps = tox-nox-ariada", | ||
| "commands =", | ||
| f" ariada-toxnox scan {target} --no-fail", |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/sh
set -eu
printf '%s\n' '--- snippets.py ---'
sed -n '1,80p' integrations/tox-nox-ariada/tox_nox_ariada/snippets.py
printf '%s\n' '--- CLI and package metadata ---'
sed -n '1,70p' integrations/tox-nox-ariada/tox_nox_ariada/cli.py
find integrations/tox-nox-ariada -maxdepth 2 -type f \
\( -name 'pyproject.toml' -o -name 'setup.cfg' -o -name 'tox.ini' -o -name 'noxfile.py' \) \
-print
printf '%s\n' '--- focused tests ---'
sed -n '80,110p' integrations/tox-nox-ariada/tests/test_scanner.py
printf '%s\n' '--- deterministic rendering probe (without importing repository code) ---'
python3 - <<'PY'
def tox_env_snippet(target):
return "\n".join([
"[testenv:a11y]",
"deps = tox-nox-ariada",
"commands =",
f" ariada-toxnox scan {target} --no-fail",
])
def nox_session_snippet(target):
return "\n".join([
"import nox", "", "",
"`@nox.session`",
"def a11y(session):",
' session.install("tox-nox-ariada")',
f' session.run("ariada-toxnox", "scan", "{target}", "--no-fail")',
])
for target in ['site/index.html', 'x"\\nprint("INJECTED")#', 'x\n[testenv:injected]\ncommands = echo injected']:
print('TARGET:', repr(target))
print('TOX:')
print(tox_env_snippet(target))
print('NOX:')
print(nox_session_snippet(target))
print('---')
PYRepository: ariada-org/ariada
Length of output: 4574
🏁 Script executed:
#!/bin/sh
set -eu
printf '%s\n' '--- declared dependencies and tox configuration ---'
sed -n '1,180p' integrations/tox-nox-ariada/pyproject.toml
rg -n -i 'tox|nox|commands\s*=' integrations/tox-nox-ariada README.md .github 2>/dev/null | head -120Repository: ariada-org/ariada
Length of output: 5512
🌐 Web query:
tox documentation commands list quoting command arguments tox 4
💡 Result:
In tox 4, command line arguments and commands defined in tox.ini (or tox.toml) are parsed using shlex POSIX-mode quoting rules on all supported platforms (as of tox 4.4.0) [1][2][3]. Key points for quoting and command arguments in tox 4: 1. Shlex Quoting Rules: The backslash \ character acts as an escape character for quotes, whitespace, and itself [1][3]. However, on Windows, backslashes in paths are not interpreted as escape characters [1][3]. An unescaped single quote disables backslash escaping until it is closed by another unescaped single quote [1][3]. 2. Passing Positional Arguments: To pass extra arguments from the command line to the commands defined in your configuration, you must use the -- separator [4][2][5]. Any arguments provided after -- are available via the {posargs} substitution [2]. Example: $ tox run -e envname -- --my-option value This makes --my-option value appear wherever {posargs} is specified in your commands definition [2]. 3. Breaking Changes from Tox 3: Unlike tox 3, which sometimes accepted positional arguments without the -- separator, tox 4 explicitly requires it [6][5]. If you omit the --, tox 4 will report an error for unrecognized arguments [6][7]. For additional details on escaping specific characters or complex command structures, refer to the Python shlex parsing rules, which now govern tox command execution [1][3].
Citations:
- 1: https://tox.wiki/en/4.24.2/config.html
- 2: https://tox.wiki/en/4.32.0/config.html
- 3: https://tox.wiki/en/4.56.0/reference/config.html
- 4: https://tox.wiki/en/4.1.3/faq.html
- 5: https://tox.wiki/en/4.14.1/upgrading.html
- 6: tox4 requires "--" separator for posargs, while tox 3 didn't tox-dev/tox#2183
- 7: https://stackoverflow.com/questions/75056352/poetry-tox-how-to-pass-arguments-to-commands-via-tox-when-running-in-poetry
Injection (CWE-94): Improper Control of Generation of Code ('Code Injection')
Reachability: External
Reachability path
● Entry
integrations/tox-nox-ariada/tox_nox_ariada/__main__.py:3
main
│
▼
● Hop
integrations/tox-nox-ariada/tox_nox_ariada/cli.py:11
main
│
▼
● Sink
integrations/tox-nox-ariada/tox_nox_ariada/snippets.py
Escape target before generating executable recipes.
target is inserted directly into generated tox and nox source. A line break can add tox directives. A quote or line break can inject Python statements into session.run(...).
Reject CR/LF. Use tox-compatible shlex quoting for the tox token. Use repr(target) for the nox argument. Add regression tests for quotes and line breaks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@integrations/tox-nox-ariada/tox_nox_ariada/snippets.py` at line 10, Update
the snippet generation around the ariada-toxnox scan recipe so target is safely
escaped before being embedded into tox and nox source. In the snippet builder
that emits the ariada-toxnox command, reject any target containing CR or LF, use
tox-compatible shlex quoting for the tox token, and use repr(target) for the nox
session.run argument. Add regression coverage for quoted targets and line-break
inputs to ensure the generated recipes stay executable and non-injectable.
Temporary release-pipeline listener PR for staging check-runs. The release pipeline advances main by fast-forward after required checks are attached to the staging SHA.
Summary by CodeRabbit
New Features
Documentation
Tests