Add test suite, security hardening, bug fixes, and decorator migration by marloson · Pull Request #1435 · abrignoni/iLEAPP

marloson · 2026-02-20T23:17:38Z

Summary

pytest infrastructure: Add pytest.ini, requirements-dev.txt, and tests/conftest.py with Context singleton isolation fixture and --update-golden option
Unit tests (29 tests): Cover plugin_loader, context, ilapfuncs, and search_files
Artifact integration tests (76 parametrized tests): Run real artifact plugins against test data ZIPs with golden file regression detection
CI workflow: .github/workflows/run_tests.yml with separate jobs for unit and artifact tests on Python 3.12
Security: path traversal fix (search_files.py): Add _is_safe_path() containment check to FileSeekerTar and FileSeekerZip to prevent zip slip / tar slip attacks on malicious archives
Security: SQL injection fixes: Parameterize queries in does_column_exist_in_db, does_table_exist_in_db (ilapfuncs.py), lava_get_media_item, lava_get_media_references (lavafuncs.py), pingertextfree.py, googleChat.py, tikTok.py
Bug fixes: TSV export escapechar error ([Bug] systemVersionPlist.py crashes with TypeError due to incorrect variable assignment #1390), Chrome autofill column check ([Bug] sqlite3.ProgrammingError in mailprotect.py during iOS < 13 artifact parsing #1386), cashApp date format crash (Discord manifest script appends all found manifest files instead of only the Discord one #1340), Discord attachment column mismatch (bug: headers in ProcessedFilesLog.html #1264), Viber duplicate name error (Bug: get_data_list_with_media crashes on invalid media IDs #1237)
Plugin migration: Migrate all 75 legacy __artifacts__ (v1) plugins to __artifacts_v2__ format
Decorator migration: Migrate 45 plugins to @artifact_processor decorator, removing ~830 lines of boilerplate (229/281 = 81.5% decorated)
CLAUDE.md: Add pre-commit checklist and common pitfalls

Net result: 193 files changed, +22,296 / -2,898 lines

Commits

6af146f6 — Fix mypy type errors and add mypy configuration
6b009b4f — Add test suite and appleNotes artifact plugin
0cb8e3d1 — Add GitHub Actions CI workflow for unit and artifact tests
927e0d1a — Prevent path traversal in zip and tar file extraction
f29b6fa1 — Add pre-commit checklist and common pitfalls to CLAUDE.md
62431362 — Fix TSV export escapechar error and Chrome autofill column check
08890d7f — Parameterize SQL queries to prevent injection
dee456de — Migrate all 75 legacy plugins from __artifacts__ to __artifacts_v2__
8d25df39 — Resolve 5 open bugs ([Bug] systemVersionPlist.py crashes with TypeError due to incorrect variable assignment #1390, [Bug] sqlite3.ProgrammingError in mailprotect.py during iOS < 13 artifact parsing #1386, Discord manifest script appends all found manifest files instead of only the Discord one #1340, bug: headers in ProcessedFilesLog.html #1264, Bug: get_data_list_with_media crashes on invalid media IDs #1237)
c630d421 — Migrate 45 artifact plugins to @artifact_processor decorator

Test plan

PYTHONPATH=. pytest tests/unit/ -v -m unit — 29 passed
PYTHONPATH=. pytest tests/artifacts/ -v -m artifact — 74 passed, 2 skipped
PYTHONPATH=. pylint scripts/ --disable=C,R --exit-zero — no errors on migrated files
Path traversal: _is_safe_path() rejects ../ entries in both zip and tar seekers
SQL injection: All f-string interpolations replaced with ? params or whitelist validation
v1→v2 migration: grep -r '__artifacts__ =' scripts/artifacts/ returns 0 matches
Manual smoke test with a real iOS extraction to verify report output

Provides project-specific context for Claude Code including commands, plugin system architecture, execution flow, and key module descriptions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fix module naming collision by adding scripts/__init__.py. Fix real type errors in plugin_loader.py (None guard on spec/loader), ilapfuncs.py (dict annotations, lru_cache assignment, .lower() call), context.py (class attribute annotations), ileapp.py/ileappGUI.py (seeker type, datetime import, plugins.insert). Fix namedtuple name mismatch in applicationStateDB.py. Add mypy.ini to suppress missing stubs for third-party packages and exclude bundled libs and admin tooling.

- Add pytest-based unit tests for context, ilapfuncs, plugin_loader, and search_files (29 tests) - Add artifact integration test scaffolding under tests/artifacts/ - Add pytest.ini and requirements-dev.txt for test dependencies - Add appleNotes artifact plugin

Malicious archive entries with '../' sequences could escape the data_folder during extraction. Add _is_safe_path() containment check to both FileSeekerTar.search() and FileSeekerZip.search() that validates resolved paths stay within the extraction directory.

…heck Add escapechar='\\' to csv.writer in tsv() to fix _csv.Error when data contains special characters. Remove stale TODO comments from 4 plugins. Fix Chrome autofill to check for date_last_used column since the query depends on it.

Replace f-string interpolation with ? placeholders in does_column_exist_in_db, does_table_exist_in_db (ilapfuncs.py), lava_get_media_item, lava_get_media_references (lavafuncs.py), pingertextfree.py, and googleChat.py. Add regex whitelist validation for dynamic table names in tikTok.py.

…acts_v2__ Automated migration of all remaining v1 (__artifacts__) plugin metadata to v2 (__artifacts_v2__) format. Zero v1 plugins remain. The v2 format adds structured metadata fields (description, author, version, date, output_types, artifact_icon) for better plugin discovery and reporting.

, abrignoni#1264, abrignoni#1237) - systemVersionPlist.py: assign plist_file path instead of function object - mailprotect.py: remove redundant db.close() inside context manager - discordManifest.py: filter manifests to Discord app container only, fix paths tuple - ileapp.py: use <h2> headings in ProcessedFilesLog, rename "module" to "artifact" - ilapfuncs.py: defensive error handling for invalid media reference IDs - lavafuncs.py: parameterize SQL in lava_get_full_media_info()

marloson · 2026-02-22T17:27:33Z

Hi @abrignoni — wanted to flag a few things for easier review:

Bug fixes included — in addition to the test suite and plugin migration, the most recent commit (b3f5e4a) addresses 5 open issues:

[Bug] systemVersionPlist.py crashes with TypeError due to incorrect variable assignment #1390 — systemVersionPlist.py: was assigning the function object instead of calling it for the plist path
- [Bug] sqlite3.ProgrammingError in mailprotect.py during iOS < 13 artifact parsing #1386 — mailprotect.py: redundant db.close() inside context manager causing double-close
- Discord manifest script appends all found manifest files instead of only the Discord one #1340 — discordManifest.py: was pulling in all apps' manifests, not just Discord's container
- bug: headers in ProcessedFilesLog.html #1264 — ileapp.py: <h2> headings in ProcessedFilesLog, renamed "module" → "artifact"
- Bug: get_data_list_with_media crashes on invalid media IDs #1237 — ilapfuncs.py / lavafuncs.py: defensive error handling for invalid media reference IDs + parameterized SQL in lava_get_full_media_info()
  CI workflow — the run_tests.yml workflow is currently awaiting maintainer approval to run. If you're able to approve it, the unit tests (29 tests) and artifact integration tests (74 passing, 2 skipped) should give you a green signal on the test suite.

Scope note — I understand this is a large PR (+21k lines, mostly the v1→v2 plugin migration). Happy to split it into smaller focused PRs if that would make review easier — e.g., security fixes + bug fixes in one PR, test suite in another, and the v1→v2 migration separately. Just let me know what works best.

Migrate 45 single-report plugins from manual ArtifactHtmlReport usage to the @artifact_processor decorator pattern (217→229 decorated + 33 from prior session batches). This brings decorated coverage to 229/281 (81.5%). Changes per plugin: - Add @artifact_processor decorator - Remove manual HTML report, TSV, timeline, and KML generation code - Remove unused imports (ArtifactHtmlReport, logfunc, tsv, timeline, etc.) - Return (data_headers, data_list, source_path) tuple - Add output_types, artifact_icon, and html_columns to __artifacts_v2__ - Fix v2 key to match function name (plugin_loader requirement) - Fix paths tuples missing trailing commas - Remove deprecated "function" fields Notable fixes: - offlinePages: rename "Offline Pages" → "Offline Pages (MHTML)" (name conflict with chrome.py) - geodApplications/geodPDPlaceCache: rename both from "Location" to unique names - geodPDPlaceCache: fix logfunc() crash (no args) - iconsScreen: fix E0601/E0606 uninitialized variables (bbar, icon) - vippsContacts: fix E0606 in relative_paths helper (variable shadowing) - kikBplistmeta: convert tabs to 4-space indent, rename builtin 'id' variable - Oops: remove unused timestamp conversion imports - Multiple files: fix W0631 undefined-loop-variable on return Remaining 52 plugins are all complex (multi-report, per-file report generators, direct LAVA callers, or seeker/version-dependent) and require structural refactoring rather than simple migration.

…rocessor Remove manual ArtifactHtmlReport/tsv/timeline/kml calls and let the decorator handle all output generation. Fix __artifacts_v2__ keys to match function names. Add html_columns for notes thumbnail column. Fix early returns in weatherAppLocations to return proper tuples.

…r functions Split simInfo (UUID + Label Store), kikMessages (Messages + Users), interactionCcontacts (Contacts + Attachments), mobileInstallb (History + Reboots), imoHD_Chat (Messages + Contacts) — each had 2 manual ArtifactHtmlReport blocks. Now 10 separate decorated functions with proper __artifacts_v2__ entries. Fix indentation and add missing imports.

stark4n6 · 2026-02-24T13:40:08Z

@marloson it might be good to chunk these PRs out to separate ones just from a size perspective for reviewing

marloson · 2026-02-25T00:50:17Z

Thanks @stark4n6 — totally agree, this grew larger than ideal. Happy to split it out. Here's what I'm thinking for the breakdown:

Security + bug fixes — path traversal fix, SQL injection hardening, and the 5 open bug resolutions ([Bug] systemVersionPlist.py crashes with TypeError due to incorrect variable assignment #1390, [Bug] sqlite3.ProgrammingError in mailprotect.py during iOS < 13 artifact parsing #1386, Discord manifest script appends all found manifest files instead of only the Discord one #1340, bug: headers in ProcessedFilesLog.html #1264, Bug: get_data_list_with_media crashes on invalid media IDs #1237). Small, self-contained, high value.
1. Test suite + CI — pytest infrastructure, unit tests, artifact integration tests, and the GitHub Actions workflow.
1. v1→v2 plugin migration — the 75 legacy __artifacts__ → __artifacts_v2__ conversions. Mechanical but large.
1. Decorator migration — the 45 plugins migrated to @artifact_processor.
  Let me know if you'd prefer a different grouping, or if there's a particular chunk you'd want to see first. I'll close this PR and open the focused ones once I hear back.

stark4n6 · 2026-02-25T15:15:39Z

@marloson I think these 4 steps work, if you can do them in 4 PRs. We can take them in whatever order is easiest

Store sorted data in the actual snapshot and compare actual["data"] == sorted(expected["data"]) so tests are order-independent regardless of dict insertion order. Mirrors the fix already applied to main branch in commit 0ff8ef7.

JamesHabben · 2026-03-05T23:25:03Z

Overall this is way too much. Even many of the commits are going after too many things. Impossible to review this.

We would love to get the artifact modules up to the v2, but we have found that many are not as simple to update. They've needed some careful handling and adjustment. Plus there are quite a few PR currently open that are addressing this. I hope you've reviewed those and not stepped on top.

Also, there is a doc that describes the general flow of updates we are looking for. For one thing, we are deprecating the multi-parameter function and moving all to the single parameter with context. I saw many here updating to the multi.

The test suite needs to be reviewed further. I've already built and documented most of the artifact integration tests and this looks to be reinventing that. I ran into lots of bumps on that road and had to adjust it in a bunch of ways. I saw that you are using the test data being generated but didn't consider the test code already written.

The golden image is not yet built into that, but it's a goal once we have comfort in the tests. That said, we need to build these gold images carefully. I have found parsing errors in modules when building those result files that are currently in place, and I've had to modify the module to fix issues. We do not want to assume that current state of any module is ready for gold images.

marloson · 2026-03-06T05:52:11Z

Thanks @JamesHabben and @stark4n6 for the thorough feedback — completely agree this PR grew too large. Closing this and will reopen as 4 focused PRs in the order that makes most sense for review:

Security + bug fixes — path traversal fix, SQL injection hardening, and the 5 open bug resolutions ([Bug] systemVersionPlist.py crashes with TypeError due to incorrect variable assignment #1390, [Bug] sqlite3.ProgrammingError in mailprotect.py during iOS < 13 artifact parsing #1386, Discord manifest script appends all found manifest files instead of only the Discord one #1340, bug: headers in ProcessedFilesLog.html #1264, Bug: get_data_list_with_media crashes on invalid media IDs #1237)
1. Test suite + CI — pytest infrastructure, unit tests, and GitHub Actions workflow
1. v1→v2 plugin migration — the 75 legacy __artifacts__ → __artifacts_v2__ conversions
1. Decorator migration — the 45 plugins migrated to @artifact_processor
  I'll review the existing test code and open PRs before opening any of these to avoid stepping on in-progress work. Thanks again for the guidance.

marloson and others added 9 commits February 17, 2026 06:06

Add CLAUDE.md for Claude Code guidance

32f7c36

Provides project-specific context for Claude Code including commands, plugin system architecture, execution flow, and key module descriptions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add GitHub Actions CI workflow for unit and artifact tests

0cb8e3d

docs: add pre-commit checklist and common pitfalls to CLAUDE.md

f29b6fa

marloson changed the title ~~Add formal pytest test suite with unit and artifact integration tests~~ Add test suite, security hardening, SQL injection fixes, and v1→v2 plugin migration Feb 22, 2026

marloson changed the title ~~Add test suite, security hardening, SQL injection fixes, and v1→v2 plugin migration~~ Add test suite, security hardening, bug fixes, and decorator migration Feb 22, 2026

marloson force-pushed the pr/pytest-suite branch from 89b4382 to c630d42 Compare February 22, 2026 17:49

marloson added 4 commits February 22, 2026 12:16

chore: trigger CLA rescan

46103e5

fix: make artifact integration tests order-independent

d2bfe72

fix: make artifact test comparison order-independent

b6adfd3

Store sorted data in the actual snapshot and compare actual["data"] == sorted(expected["data"]) so tests are order-independent regardless of dict insertion order. Mirrors the fix already applied to main branch in commit 0ff8ef7.

marloson closed this Mar 6, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add test suite, security hardening, bug fixes, and decorator migration#1435

Add test suite, security hardening, bug fixes, and decorator migration#1435
marloson wants to merge 16 commits intoabrignoni:mainfrom
marloson:pr/pytest-suite

marloson commented Feb 20, 2026 •

edited

Loading

Uh oh!

marloson commented Feb 22, 2026

Uh oh!

stark4n6 commented Feb 24, 2026

Uh oh!

marloson commented Feb 25, 2026

Uh oh!

stark4n6 commented Feb 25, 2026

Uh oh!

JamesHabben commented Mar 5, 2026

Uh oh!

marloson commented Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

marloson commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Commits

Test plan

Uh oh!

marloson commented Feb 22, 2026

Uh oh!

stark4n6 commented Feb 24, 2026

Uh oh!

marloson commented Feb 25, 2026

Uh oh!

stark4n6 commented Feb 25, 2026

Uh oh!

JamesHabben commented Mar 5, 2026

Uh oh!

marloson commented Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

marloson commented Feb 20, 2026 •

edited

Loading