Skip to content

Commit 2e9b3f5

Browse files
authored
Skip invalid payloads in _scan_payload_dir instead of crashing (#1810)
## Summary `Director._scan_payload_dir` (`garak/payloads.py`) catches `JSONDecodeError`/`KeyError` for a payload file and logs `"Invalid payload, skipping"`, but it does **not** `continue`. Execution falls through to the code that reads `payload_types` and records the file: ```python except (json.JSONDecodeError, KeyError) as exc: msg = f"payload scan: Invalid payload, skipping: {payload_path}" logging.debug(msg, exc_info=exc) # raise garak.exception.PayloadFailure(msg) from exc payload_name = payload_path.stem payloads_found[payload_name] = { "path": payload_path, "types": payload_types, # <- read even when the except fired } ``` Consequences: - **Crash:** if the first/only scanned file is invalid, `payload_types` was never bound → `UnboundLocalError`, which aborts the whole payload inventory (`_refresh_payloads`). - **Silent data corruption:** if a valid file was processed earlier in the loop, the stale `payload_types` is reused, registering the invalid file under the **previous** file's types. This is reachable in normal use — `PAYLOAD_DIR` includes user / `XDG_DATA_DIR` custom payload directories, so a single stray or malformed `.json` there breaks payload loading. ## Fix Add `continue` in the `except` block so invalid payloads are skipped, matching the log message ("skipping") and the commented-out `raise`. One line; no other behavior changes. ## Why this is not a duplicate Checked open PRs (`_scan_payload_dir`, `payload scan`, `payloads` in title) and open issues (`invalid payload`) on NVIDIA/garak — none address this. ## Testing - Added `tests/test_payloads.py::test_scan_payload_dir_skips_invalid`, which scans a temp dir containing a malformed JSON, a JSON missing `payload_types`, and one valid payload; asserts only the valid payload is registered and no stale types leak. It **fails on `main`** (`UnboundLocalError`) and **passes** with this change. - Commands run: - `python3 -m pytest tests/test_payloads.py::test_scan_payload_dir_skips_invalid tests/test_payloads.py::test_non_json_direct_load tests/test_payloads.py::test_payload_Director` → 3 passed - `python3 -m black --check garak/payloads.py tests/test_payloads.py` → clean ## AI assistance AI assistance (Claude) was used to help locate the bug, implement the fix, and write the test. I reviewed every changed line and ran the tests above.
2 parents 2b12c25 + 4facc4f commit 2e9b3f5

2 files changed

Lines changed: 18 additions & 0 deletions

File tree

garak/payloads.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ def _scan_payload_dir(self, dir) -> dict:
153153
msg = f"payload scan: Invalid payload, skipping: {payload_path}"
154154
logging.debug(msg, exc_info=exc)
155155
# raise garak.exception.PayloadFailure(msg) from exc
156+
continue
156157

157158
payload_name = payload_path.stem
158159

tests/test_payloads.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
# SPDX-License-Identifier: Apache-2.0
33

44
import csv
5+
import json
56
import tempfile
67
import types
78

@@ -81,6 +82,22 @@ def test_non_json_direct_load():
8182
garak.payloads.Director._load_payload("jkasfohgi", t.name)
8283

8384

85+
def test_scan_payload_dir_skips_invalid(tmp_path):
86+
# An invalid payload (malformed JSON or missing payload_types) must be
87+
# skipped, not crash the scan or leak a previous file's types.
88+
(tmp_path / "bad.json").write_text("{not valid json", encoding="utf-8")
89+
(tmp_path / "nokey.json").write_text(json.dumps({"foo": "bar"}), encoding="utf-8")
90+
(tmp_path / "good.json").write_text(
91+
json.dumps({"payload_types": ["Security circumvention instructions"]}),
92+
encoding="utf-8",
93+
)
94+
95+
found = garak.payloads.Director()._scan_payload_dir(tmp_path)
96+
97+
assert set(found.keys()) == {"good"}
98+
assert found["good"]["types"] == ["Security circumvention instructions"]
99+
100+
84101
OK_PAYLOADS = [
85102
{"garak_payload_name": "test", "payloads": ["pay", "load"], "payload_types": []},
86103
{

0 commit comments

Comments
 (0)