Skip to content

Commit cb97bd4

Browse files
committed
Audit pass + pytest suite + CI
Multi-tool audit (ruff, mypy, bandit, pylint, vulture) with all medium+ findings fixed, plus 403 pytest tests at 71% line coverage. Notable bug fixes: - utils/api.py: delete_permanently() built items payload but delete() ignored it, so backup-file purges silently no-opped (data leak). - __init__.py: missing opening triple-quote -> SyntaxError on import. - config/config.py: duplicate 'NETWORK_URL' dict key silently overrode the first value. - services/drive.py: create_folder_recursive defined twice (104 LOC of dead code); utils/api.py: move_file/move_folder defined twice. - services/webdav_server.py: active_server could be referenced unbound when server_choice was unknown (UnboundLocalError); host hardcoded to 0.0.0.0 instead of using config (now defaults to 127.0.0.1 loopback). - services/webdav_provider.py:end_write: finally clause crashed if _upload_buffer was never assigned. - services/crypto.py:generate_filename_encryption_iv: hashlib.sha512 was passed as msg instead of digestmod (TypeError at runtime). - services/webdav_provider.py:set_property: imported nonexistent rfc_1123_to_timestamp/rfc_3339_to_timestamp from wsgidav.util (every PROPPATCH crashed); also called the missing drive_service.set_folder_timestamps method (now added). - cli.py config: crashed on every run due to commented-out DRIVE_WEB_URL. - install.py: subprocess shell=True (B602) -> argv form. Security: - All Bandit medium+ findings (was 14, now 0) fixed or annotated. - network_utils.test_webdav_connection: verify=False restricted to loopback only. - crypto.py SHA1/MD5 annotated as protocol-required (Internxt server compat); MD5 marked usedforsecurity=False; checkpoint hash switched from MD5 to SHA-256. Test infra: - pyproject.toml (pytest, coverage, ruff config) - requirements-dev.txt (pytest, pytest-cov, ruff, mypy, bandit) - .github/workflows/ci.yml (lint -> type -> security -> test on Py 3.10/3.11/3.12) - tests/ with 403 tests covering crypto round-trips, path resolution, upload/download conflict handling, WebDAV provider, server lifecycle, CLI commands (Click CliRunner), SSL cert lifecycle, range parsing, credential persistence, batch mv with wildcards, memory-gated upload concurrency, and a real upload->encrypt->wire->decrypt->download cycle test. Coverage by module: config/config.py 85% services/auth.py 100% services/crypto.py 85% services/drive.py 76% services/network_utils.py 90% services/webdav_provider.py 57% services/webdav_server.py 47% utils/api.py 74% TOTAL 71% See CHANGELOG.md for full details.
1 parent cdaa7d7 commit cb97bd4

55 files changed

Lines changed: 5977 additions & 538 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/ci.yml

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
name: CI
2+
3+
on:
4+
push:
5+
branches: [main]
6+
pull_request:
7+
branches: [main]
8+
9+
jobs:
10+
test:
11+
runs-on: ubuntu-latest
12+
strategy:
13+
fail-fast: false
14+
matrix:
15+
python-version: ["3.10", "3.11", "3.12"]
16+
17+
steps:
18+
- uses: actions/checkout@v4
19+
20+
- name: Set up Python ${{ matrix.python-version }}
21+
uses: actions/setup-python@v5
22+
with:
23+
python-version: ${{ matrix.python-version }}
24+
cache: pip
25+
cache-dependency-path: |
26+
requirements.txt
27+
requirements-dev.txt
28+
29+
- name: Install dependencies
30+
run: |
31+
python -m pip install --upgrade pip
32+
pip install -r requirements-dev.txt
33+
34+
- name: Lint (ruff)
35+
run: ruff check .
36+
37+
- name: Type check (mypy)
38+
run: mypy --no-incremental cli.py services
39+
40+
- name: Security scan (bandit, medium+)
41+
run: bandit -r . -x ./.mypy_cache,./__pycache__,./.git,./tests -ll
42+
43+
- name: Run tests with coverage
44+
run: |
45+
pytest tests/ \
46+
--cov=services --cov=utils --cov=config \
47+
--cov-report=term --cov-report=xml
48+
49+
- name: Upload coverage XML
50+
if: matrix.python-version == '3.11'
51+
uses: actions/upload-artifact@v4
52+
with:
53+
name: coverage-xml
54+
path: coverage.xml

.gitignore

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,25 @@
11
.git/
2-
**pycache**/
3-
internxt_python_cli.egg-info/
2+
**/__pycache__/
3+
*.pyc
4+
internxt_python_cli.egg-info/
5+
6+
# Test / coverage / cache artifacts
7+
.coverage
8+
.coverage.*
9+
.pytest_cache/
10+
.mypy_cache/
11+
.ruff_cache/
12+
htmlcov/
13+
coverage.xml
14+
15+
# Virtualenvs
16+
venv/
17+
.venv/
18+
env/
19+
20+
# OS
21+
.DS_Store
22+
23+
# Editor
24+
.vscode/
25+
.idea/

CHANGELOG.md

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
# Changelog
2+
3+
## Unreleased — Audit & Test Infrastructure
4+
5+
A multi-tool security/correctness audit was run (ruff, mypy, bandit, pylint,
6+
vulture, pyflakes), every medium+ finding was fixed, and a pytest suite was
7+
built up alongside CI to lock in those fixes.
8+
9+
### Bug fixes (high impact)
10+
11+
- **`utils/api.py`**`delete_permanently()` built `data={'items':[…]}` but
12+
the inner `delete()` method never accepted a body, so the items list was
13+
silently dropped on the wire. Backup-file purges after `safety_pattern`
14+
uploads were no-ops, leaking trash storage. **Fixed:** extended `delete()`
15+
to accept and forward `data=`, and updated `delete_permanently()` to
16+
pass it through.
17+
- **`__init__.py`** — file was missing the opening `"""` of its docstring,
18+
so importing the package raised `SyntaxError`. **Fixed.**
19+
- **`config/config.py`**`'NETWORK_URL'` was defined twice in the config
20+
dict; the second definition (`https://api.internxt.com`) silently
21+
overrode the first. The dead key has been removed; the active value is
22+
unchanged.
23+
- **`services/drive.py`**`create_folder_recursive` was defined twice
24+
(104 lines of dead code). **Fixed:** removed the dead first definition.
25+
- **`utils/api.py`**`move_file` and `move_folder` were each defined twice
26+
(the second silently overriding the first). **Fixed:** removed the
27+
duplicates.
28+
- **`services/webdav_server.py`**`start()` referenced `active_server`
29+
unbound when `server_choice` was anything other than
30+
`auto`/`waitress`/`cheroot`, raising `UnboundLocalError`. **Fixed:**
31+
initialize upfront and raise an explicit `ValueError` for unknown choices.
32+
- **`services/webdav_server.py`**`start()` hardcoded `host="0.0.0.0"`,
33+
ignoring the `host` setting in webdav config (which itself was being
34+
silently dropped by `read_webdav_config`). **Fixed:** default to
35+
`127.0.0.1` (loopback only) and pass through the user-set `host` from
36+
config so `0.0.0.0` is opt-in.
37+
- **`services/webdav_provider.py`** — the `finally:` clause in `end_write`
38+
called `self._upload_buffer.cleanup()` without checking `hasattr`, so an
39+
exception before `begin_write` had set the buffer would crash with
40+
`AttributeError` masking the real error. **Fixed.**
41+
- **`services/crypto.py`**`generate_filename_encryption_iv` called
42+
`hmac.new(key, hashlib.sha512)``hashlib.sha512` was being passed as
43+
`msg` instead of `digestmod`. The function would `TypeError` at
44+
runtime. (Currently dead code, but a real latent bug.) **Fixed.**
45+
- **`services/webdav_provider.py`**`set_property()` (the PROPPATCH
46+
handler used by macOS Finder & Windows Explorer to set folder
47+
timestamps) imported two functions from `wsgidav.util` that don't exist
48+
in any released wsgidav version (`rfc_1123_to_timestamp`,
49+
`rfc_3339_to_timestamp`). Any timestamp PROPPATCH would crash with
50+
`ImportError`. **Fixed:** use `parse_time_string` for RFC 1123 with a
51+
stdlib `datetime.fromisoformat` fallback for ISO 8601 / RFC 3339.
52+
- **`services/webdav_provider.py` / `services/drive.py`**`set_property()`
53+
called `drive_service.set_folder_timestamps(...)`, but that method was
54+
never defined. **Fixed:** added the method, which calls
55+
`api.update_folder_metadata` and invalidates the parent-folder cache.
56+
- **`cli.py config` command** — referenced `DRIVE_WEB_URL` which had been
57+
commented out of the config dict. The `config` command crashed with
58+
"Config key DRIVE_WEB_URL was not found in process.env" on every run.
59+
**Fixed:** tolerate missing keys with `(not configured)` placeholder.
60+
- **`install.py`** — used `subprocess.run(..., shell=True)` with a
61+
string-concatenated `pip install` command. **Fixed:** switched to argv
62+
form using `sys.executable`.
63+
64+
### Security hardening (Bandit medium+ findings)
65+
66+
All Bandit medium+ findings (was 14, now 0) are resolved or annotated:
67+
68+
- **`services/network_utils.py:test_webdav_connection`**`verify=False`
69+
was unconditional. Restricted to localhost loopback; remote URLs now
70+
use real TLS verification.
71+
- **`services/webdav_server.py`** — multiple `0.0.0.0` binds replaced
72+
with the configurable `host` from webdav config (defaults to loopback).
73+
- **`services/crypto.py:206`** (SHA1 in PBKDF2) and
74+
**`services/crypto.py:257`** (MD5 in OpenSSL EVP-style key derivation)
75+
— both required for protocol compatibility with the Internxt server.
76+
Annotated with `# nosec` and rationale comments; MD5 marked
77+
`usedforsecurity=False`.
78+
- **`services/drive.py:739`** — MD5 used as a non-cryptographic
79+
cache-key hash. Switched to SHA-256.
80+
81+
### Hygiene
82+
83+
- All bare `except:` clauses replaced with `except Exception:` or
84+
specific exception types.
85+
- All unused imports removed (cli.py, services/*, utils/api.py, debug/*).
86+
- All ~180 empty f-strings (`f"static text"`) replaced with regular strings.
87+
- `raise ... from e` chains added where pylint suggested (proper
88+
exception causality preservation).
89+
- mypy implicit-Optional defaults annotated explicitly.
90+
- `cli.py test` smoke command's two stale URL assertions updated to
91+
match the actual configured endpoints.
92+
93+
### Test infrastructure (new)
94+
95+
- **`tests/`** — 403 pytest tests covering crypto round-trips, path
96+
resolution, upload/download conflict handling (skip/overwrite/safety
97+
pattern), WebDAV provider (resource + collection + isolated session),
98+
WebDAV server lifecycle, all major CLI commands (Click `CliRunner`),
99+
SSL cert lifecycle, range parsing, credential persistence, batch `mv`
100+
with wildcards, memory-gated upload concurrency. **71% line coverage**
101+
across the codebase.
102+
- **End-to-end cycle test** (`test_upload_download_e2e.py`) — writes a
103+
real local file, runs `upload_file_to_folder` (real crypto, mocked
104+
network), captures the encrypted bytes "on the wire", then runs
105+
`download_file` against them and asserts byte-for-byte recovery. This
106+
catches any drift in the Internxt encryption protocol.
107+
- **`pyproject.toml`** — pytest, coverage, ruff config.
108+
- **`requirements-dev.txt`** — pinned floor for pytest, pytest-cov, ruff,
109+
mypy, bandit.
110+
- **`.github/workflows/ci.yml`** — runs ruff → mypy → bandit (medium+) →
111+
pytest with coverage on Python 3.10 / 3.11 / 3.12.
112+
113+
### Test coverage by module
114+
115+
| Module | Coverage |
116+
|---------------------------------|---------:|
117+
| `config/config.py` | 85% |
118+
| `services/auth.py` | 100% |
119+
| `services/crypto.py` | 85% |
120+
| `services/drive.py` | 76% |
121+
| `services/network_utils.py` | 90% |
122+
| `services/webdav_provider.py` | 57% |
123+
| `services/webdav_server.py` | 47% |
124+
| `utils/api.py` | 74% |
125+
| **Total** | **71%** |

__init__.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1+
"""
12
Internxt Python CLI
23
A Python implementation of the Internxt CLI for encrypted cloud storage
34
"""
45

56
__version__ = "1.0.1"
67
__author__ = "Internxt"
7-
__description__ = "Encrypted cloud storage CLI"
8+
__description__ = "Encrypted cloud storage CLI"

0 commit comments

Comments
 (0)