Skip to content

Commit 4d62ad4

Browse files
committed
Address Codacy and SonarCloud findings on PR #56
- Tests: add tests/.pylintrc to scope pytest idioms (W0621, W0212, W0613, C1803, R0801, R1732) that are legitimate in fixtures. - Config: fix import ordering so stdlib imports precede first-party. - Notify manager: expose has_sinks() instead of reaching into _lock / _sinks from module-level helpers. - DAG executor: promote _mark_skipped to a _DagRun method so it no longer needs six positional arguments. - http_download: extract _reject_oversize, _make_reporter, _run_stream to keep download_file under the per-function limits. - __main__: split _build_parser into per-category subparser helpers. - sync_ops: make _process_source_entry's dry_run / summary keyword-only. - local_tab: declare widget attributes up front in __init__. - progress_tab: rename shadowed built-in (bar -> progress_bar). - tar_ops / __init__ / google_drive download_ops: clear W0706, C0414, C0325. - Bandit/Semgrep: narrow nosec / nosemgrep comments to the exact call sites in shell_ops, fast_find, ftp client / upload_ops, and package loader. - EmailSink / LocalOpsTab / metrics.record_action: justify the intentional pylint disables. - test_secrets: move NOSONAR to the flagged credential literal line.
1 parent ddd9101 commit 4d62ad4

File tree

23 files changed

+241
-107
lines changed

23 files changed

+241
-107
lines changed

automation_file/__init__.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -236,9 +236,7 @@
236236
from automation_file.utils.rotate import RotateException, rotate_backups
237237

238238
if TYPE_CHECKING:
239-
from automation_file.ui.launcher import (
240-
launch_ui as launch_ui, # pylint: disable=useless-import-alias
241-
)
239+
from automation_file.ui.launcher import launch_ui
242240

243241
# Shared callback executor + package loader wired to the shared registry.
244242
callback_executor: CallbackExecutor = CallbackExecutor(executor.registry)

automation_file/__main__.py

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -133,15 +133,7 @@ def _sleep_forever() -> None:
133133
time.sleep(3600)
134134

135135

136-
def _build_parser() -> argparse.ArgumentParser:
137-
parser = argparse.ArgumentParser(prog="automation_file")
138-
parser.add_argument("-e", "--execute_file", help="path to an action JSON file")
139-
parser.add_argument("-d", "--execute_dir", help="directory containing action JSON files")
140-
parser.add_argument("-c", "--create_project", help="scaffold a project at this path")
141-
parser.add_argument("--execute_str", help="JSON action list as a string")
142-
143-
subparsers = parser.add_subparsers(dest="command")
144-
136+
def _add_zip_commands(subparsers: argparse._SubParsersAction) -> None:
145137
zip_parser = subparsers.add_parser("zip", help="zip a file or directory")
146138
zip_parser.add_argument("source")
147139
zip_parser.add_argument("target")
@@ -159,6 +151,8 @@ def _build_parser() -> argparse.ArgumentParser:
159151
unzip_parser.add_argument("--password", default=None)
160152
unzip_parser.set_defaults(handler=_cmd_unzip)
161153

154+
155+
def _add_file_commands(subparsers: argparse._SubParsersAction) -> None:
162156
download_parser = subparsers.add_parser("download", help="SSRF-validated HTTP download")
163157
download_parser.add_argument("url")
164158
download_parser.add_argument("output")
@@ -169,6 +163,8 @@ def _build_parser() -> argparse.ArgumentParser:
169163
touch_parser.add_argument("--content", default="")
170164
touch_parser.set_defaults(handler=_cmd_create_file)
171165

166+
167+
def _add_server_commands(subparsers: argparse._SubParsersAction) -> None:
172168
server_parser = subparsers.add_parser("server", help="run the TCP action server")
173169
server_parser.add_argument("--host", default="localhost")
174170
server_parser.add_argument("--port", type=int, default=9943)
@@ -183,6 +179,8 @@ def _build_parser() -> argparse.ArgumentParser:
183179
http_parser.add_argument("--shared-secret", default=None)
184180
http_parser.set_defaults(handler=_cmd_http_server)
185181

182+
183+
def _add_integration_commands(subparsers: argparse._SubParsersAction) -> None:
186184
ui_parser = subparsers.add_parser("ui", help="launch the PySide6 GUI")
187185
ui_parser.set_defaults(handler=_cmd_ui)
188186

@@ -206,6 +204,19 @@ def _build_parser() -> argparse.ArgumentParser:
206204
drive_parser.add_argument("--name", default=None)
207205
drive_parser.set_defaults(handler=_cmd_drive_upload)
208206

207+
208+
def _build_parser() -> argparse.ArgumentParser:
209+
parser = argparse.ArgumentParser(prog="automation_file")
210+
parser.add_argument("-e", "--execute_file", help="path to an action JSON file")
211+
parser.add_argument("-d", "--execute_dir", help="directory containing action JSON files")
212+
parser.add_argument("-c", "--create_project", help="scaffold a project at this path")
213+
parser.add_argument("--execute_str", help="JSON action list as a string")
214+
215+
subparsers = parser.add_subparsers(dest="command")
216+
_add_zip_commands(subparsers)
217+
_add_file_commands(subparsers)
218+
_add_server_commands(subparsers)
219+
_add_integration_commands(subparsers)
209220
return parser
210221

211222

automation_file/core/config.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@
5353
from automation_file.notify.manager import NotificationManager
5454
from automation_file.notify.sinks import (
5555
EmailSink,
56-
NotificationException,
5756
NotificationSink,
5857
SlackSink,
5958
WebhookSink,
@@ -156,8 +155,6 @@ def _build_sink(entry: dict[str, Any]) -> NotificationSink:
156155
)
157156
try:
158157
return builder(entry)
159-
except NotificationException:
160-
raise
161158
except (TypeError, ValueError) as err:
162159
raise ConfigException(
163160
f"invalid config for sink {entry.get('name') or sink_type!r}: {err}"

automation_file/core/dag_executor.py

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,19 @@ def __init__(
5151
self.pool = pool
5252
self.fail_fast = fail_fast
5353

54+
def _mark_skipped(self, dependent: str, reason_id: str) -> None:
55+
with self.lock:
56+
if dependent in self.results:
57+
return
58+
self.results[dependent] = f"skipped: dep {reason_id!r} failed"
59+
for grandchild in self.graph.get(dependent, ()):
60+
self.indegree[grandchild] -= 1
61+
self._mark_skipped(grandchild, dependent)
62+
5463
def _skip_dependents(self, node_id: str) -> None:
5564
for dependent in self.graph.get(node_id, ()):
5665
self.indegree[dependent] -= 1
57-
_mark_skipped(dependent, node_id, self.graph, self.indegree, self.results, self.lock)
66+
self._mark_skipped(dependent, node_id)
5867

5968
def submit(self, node_id: str) -> None:
6069
action = self.node_map[node_id].get("action")
@@ -74,9 +83,7 @@ def _complete(self, node_id: str, value: Any, failed: bool) -> None:
7483
for dependent in self.graph.get(node_id, ()):
7584
self.indegree[dependent] -= 1
7685
if failed and self.fail_fast:
77-
_mark_skipped(
78-
dependent, node_id, self.graph, self.indegree, self.results, self.lock
79-
)
86+
self._mark_skipped(dependent, node_id)
8087
elif self.indegree[dependent] == 0 and dependent not in self.results:
8188
self.ready.append(dependent)
8289

@@ -179,20 +186,3 @@ def _detect_cycle(
179186
queue.append(dependent)
180187
if visited != len(ids):
181188
raise DagException("cycle detected in DAG")
182-
183-
184-
def _mark_skipped(
185-
dependent: str,
186-
reason_id: str,
187-
graph: dict[str, list[str]],
188-
indegree: dict[str, int],
189-
results: dict[str, Any],
190-
lock: threading.Lock,
191-
) -> None:
192-
with lock:
193-
if dependent in results:
194-
return
195-
results[dependent] = f"skipped: dep {reason_id!r} failed"
196-
for grandchild in graph.get(dependent, ()):
197-
indegree[grandchild] -= 1
198-
_mark_skipped(grandchild, dependent, graph, indegree, results, lock)

automation_file/core/metrics.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ def record_action(action: str, duration_seconds: float, ok: bool) -> None:
5656
try:
5757
ACTION_COUNT.labels(action=action, status=status).inc()
5858
ACTION_DURATION.labels(action=action).observe(max(0.0, float(duration_seconds)))
59-
except Exception as err: # pragma: no cover - defensive
59+
except Exception as err: # pylint: disable=broad-except # pragma: no cover - defensive
6060
file_automation_logger.error("metrics.record_action failed: %r", err)
6161

6262

automation_file/core/package_loader.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,10 @@ def load(self, package: str) -> ModuleType | None:
3232
file_automation_logger.error("PackageLoader: cannot find %s", package)
3333
return None
3434
try:
35-
# nosemgrep: python.lang.security.audit.non-literal-import.non-literal-import
3635
# `package` is a trusted caller-supplied name (see PackageLoader docstring and
3736
# the CLAUDE.md security note on plugin loading); it is not untrusted input.
38-
module = import_module(spec.name)
37+
name = spec.name
38+
module = import_module(name) # nosemgrep
3939
except ImportError as error:
4040
file_automation_logger.error("PackageLoader import error: %r", error)
4141
return None

automation_file/core/plugins.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,4 +83,4 @@ def _iter_entry_points() -> list[EntryPoint]:
8383
except TypeError:
8484
# importlib.metadata before 3.10 used a different API; the project
8585
# targets 3.10+, so this branch exists only as defensive padding.
86-
return list(entry_points().get(ENTRY_POINT_GROUP, []))
86+
return list(entry_points().get(ENTRY_POINT_GROUP, [])) # pylint: disable=no-member

automation_file/local/shell_ops.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
from __future__ import annotations
1313

14-
import subprocess
14+
import subprocess # nosec B404 — used only via _run_subprocess with argv list + shell=False
1515
from collections.abc import Mapping, Sequence
1616
from pathlib import Path
1717

@@ -44,7 +44,7 @@ def _run_subprocess(
4444
capture_output: bool,
4545
) -> subprocess.CompletedProcess[str]:
4646
try:
47-
return subprocess.run(
47+
return subprocess.run( # nosec B603 nosemgrep — argv_list validated; shell=False by default
4848
argv_list,
4949
timeout=timeout,
5050
cwd=cwd_path,

automation_file/local/sync_ops.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ def sync_dir(
6969

7070
src_entries = _walk_relative(source)
7171
for rel in src_entries:
72-
_process_source_entry(source, destination, rel, compare, dry_run, summary)
72+
_process_source_entry(source, destination, rel, compare, dry_run=dry_run, summary=summary)
7373

7474
if delete:
7575
_delete_extras(destination, src_entries, dry_run, summary)
@@ -103,6 +103,7 @@ def _process_source_entry(
103103
destination: Path,
104104
rel: Path,
105105
compare: str,
106+
*,
106107
dry_run: bool,
107108
summary: dict[str, Any],
108109
) -> None:

automation_file/local/tar_ops.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,6 @@ def extract_tar(source: str, target_dir: str) -> list[str]:
8787
else:
8888
archive.extract(member, str(dest))
8989
extracted.append(member.name)
90-
except PathTraversalException:
91-
raise
9290
except (OSError, tarfile.TarError) as err:
9391
raise TarException(f"extract_tar failed: {err}") from err
9492

0 commit comments

Comments
 (0)