diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ab64c96..2d928dd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,7 +22,7 @@ jobs: persist-credentials: false - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: ${{ matrix.python-version }} @@ -51,7 +51,7 @@ jobs: persist-credentials: false - name: Set up Python - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: "3.12" @@ -67,11 +67,12 @@ jobs: run: pip-audit -r requirements.txt || pip-audit --desc - name: Check for secrets - uses: trufflesecurity/trufflehog@30d5bb91af1a771378349dbbb0c82129392acf70 # v3.95.6 + uses: trufflesecurity/trufflehog@6f3c981e7b77f235fd2702dd74af25fc4b72bf11 # v3.96.0 with: path: ./ base: "" head: ${{ github.sha }} + extra_args: --exclude-paths=.trufflehogignore build: runs-on: ubuntu-latest @@ -82,7 +83,7 @@ jobs: persist-credentials: false - name: Set up Python - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: "3.12" diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index cebc5ec..8d65a0c 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -19,7 +19,7 @@ jobs: persist-credentials: false - name: Set up Python - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: "3.12" @@ -38,4 +38,4 @@ jobs: run: pip install twine && twine check dist/* - name: Publish to PyPI - uses: pypa/gh-action-pypi-publish@cef221092ed1bacb1cc03d23a2d87d1d172e277b + uses: pypa/gh-action-pypi-publish@a892a5a61159132606e93a2fa6f4358831b04d26 # v1.14.2 diff --git a/.trufflehogignore b/.trufflehogignore new file mode 100644 index 0000000..b198d17 --- /dev/null +++ b/.trufflehogignore @@ -0,0 +1 @@ +^tests/.* diff --git a/conftest.py b/conftest.py index c9861f1..d52023f 100644 --- a/conftest.py +++ b/conftest.py @@ -1,4 +1,5 @@ """pytest configuration — add project src to Python path and skip rate limits.""" + import os import sys from pathlib import Path diff --git a/src/apiauth/cli.py b/src/apiauth/cli.py index 8a3468a..78619ef 100644 --- a/src/apiauth/cli.py +++ b/src/apiauth/cli.py @@ -25,11 +25,14 @@ try: from revenueholdings_license import require_license except ImportError: + def require_license(tool): def decorator(func): return func + return decorator + console = Console() err_console = Console(stderr=True) @@ -339,9 +342,7 @@ def import_key( now = _timestamp() expiry = None if expiry_days: - expiry = ( - dt.datetime.now(dt.timezone.utc) + dt.timedelta(days=expiry_days) - ).isoformat()[:23] + "Z" + expiry = (dt.datetime.now(dt.timezone.utc) + dt.timedelta(days=expiry_days)).isoformat()[:23] + "Z" entry = { "type": "api_key", @@ -436,18 +437,18 @@ def _export_github_actions(active: list[dict]) -> None: console.print("# GitHub Actions: Add these as repository secrets or use with actions/env") for k in active: prefix = _make_env_prefix(k) - console.print(f"echo \"{prefix}_ID={k['id']}\" >> $GITHUB_ENV") - console.print(f"echo \"{prefix}_SERVICE={k.get('service', '')}\" >> $GITHUB_ENV") - console.print(f"echo \"{prefix}_CREATED={k.get('created_at', '')}\" >> $GITHUB_ENV") + console.print(f'echo "{prefix}_ID={k["id"]}" >> $GITHUB_ENV') + console.print(f'echo "{prefix}_SERVICE={k.get("service", "")}" >> $GITHUB_ENV') + console.print(f'echo "{prefix}_CREATED={k.get("created_at", "")}" >> $GITHUB_ENV') if k.get("expires_at"): - console.print(f"echo \"{prefix}_EXPIRES={k['expires_at']}\" >> $GITHUB_ENV") + console.print(f'echo "{prefix}_EXPIRES={k["expires_at"]}" >> $GITHUB_ENV') console.print() console.print("# Or add to .github/workflows/*.yml env: block:") console.print("env:") for k in active: prefix = _make_env_prefix(k) - console.print(f" {prefix}_ID: \"{k['id']}\"") - console.print(f" {prefix}_SERVICE: \"{k.get('service', '')}\"") + console.print(f' {prefix}_ID: "{k["id"]}"') + console.print(f' {prefix}_SERVICE: "{k.get("service", "")}"') # ── audit ───────────────────────────────────────────────────────────── @@ -499,9 +500,7 @@ def audit(ctx: click.Context, exit_on_expired: bool, exit_on_revoked: bool) -> N console.print(f"[yellow]⚠ {len(expiring)} EXPIRING key(s) (within 7 days):[/yellow]") for k in expiring: console.print( - f" [yellow]{k['id']}[/yellow] " - f"{k.get('name', '')} — expires " - f"{_short_ts(k.get('expires_at', ''))}" + f" [yellow]{k['id']}[/yellow] {k.get('name', '')} — expires {_short_ts(k.get('expires_at', ''))}" ) console.print() diff --git a/src/apiauth/keygen.py b/src/apiauth/keygen.py index 27e6424..663e060 100644 --- a/src/apiauth/keygen.py +++ b/src/apiauth/keygen.py @@ -25,6 +25,7 @@ def generate_api_key(prefix: str = "ak", byte_length: int = 32) -> str: def _base64url_no_pad(data: bytes) -> str: import base64 + return base64.urlsafe_b64encode(data).rstrip(b"=").decode("ascii") @@ -57,9 +58,9 @@ def create_api_key_entry( now = _timestamp() expiry = None if expiry_days: - expiry = ( - datetime.datetime.now(UTC) + datetime.timedelta(days=expiry_days) - ).isoformat(timespec="milliseconds")[:23] + "Z" + expiry = (datetime.datetime.now(UTC) + datetime.timedelta(days=expiry_days)).isoformat(timespec="milliseconds")[ + :23 + ] + "Z" entry = { "type": "api_key", @@ -111,14 +112,15 @@ def create_jwt_entry( # Create the JWT import jwt as pyjwt + token = pyjwt.encode(payload, signing_secret, algorithm="HS256") now_str = _timestamp() expiry = None if expiry_days: - expiry = ( - datetime.datetime.now(UTC) + datetime.timedelta(days=expiry_days) - ).isoformat(timespec="milliseconds")[:23] + "Z" + expiry = (datetime.datetime.now(UTC) + datetime.timedelta(days=expiry_days)).isoformat(timespec="milliseconds")[ + :23 + ] + "Z" entry = { "type": "jwt", @@ -158,9 +160,9 @@ def rotate_key( now = _timestamp() expiry = None if expiry_days: - expiry = ( - datetime.datetime.now(UTC) + datetime.timedelta(days=expiry_days) - ).isoformat(timespec="milliseconds")[:23] + "Z" + expiry = (datetime.datetime.now(UTC) + datetime.timedelta(days=expiry_days)).isoformat(timespec="milliseconds")[ + :23 + ] + "Z" updated = dict(entry) updated["previous_hash"] = entry.get("key_hash") @@ -288,9 +290,9 @@ def rotate_jwt( now = _timestamp() expiry = None if expiry_days: - expiry = ( - datetime.datetime.now(UTC) + datetime.timedelta(days=expiry_days) - ).isoformat(timespec="milliseconds")[:23] + "Z" + expiry = (datetime.datetime.now(UTC) + datetime.timedelta(days=expiry_days)).isoformat(timespec="milliseconds")[ + :23 + ] + "Z" updated = dict(entry) updated["previous_hash"] = entry.get("signing_secret_hash") diff --git a/tests/test_keystore_atomic.py b/tests/test_keystore_atomic.py new file mode 100644 index 0000000..4c42385 --- /dev/null +++ b/tests/test_keystore_atomic.py @@ -0,0 +1,64 @@ +"""Test atomic write behavior for keystore.""" + +import os +from apiauth.keystore import Keystore +from pathlib import Path + + +def test_keystore_atomic_write_no_temp_files(tmp_path: Path) -> None: + """Verify that keystore save leaves no temporary files behind.""" + key_dir = tmp_path / "keystore" + key_dir.mkdir() + + ks = Keystore(key_dir=key_dir) + ks.put("test-key", {"id": "test-key", "type": "api_key", "value": "test123"}) + + # Check no temp files remain + files = list(key_dir.iterdir()) + filenames = [f.name for f in files] + + # Should only have master.key and keys.json + assert "master.key" in filenames + assert "keys.json" in filenames + assert len(filenames) == 2, f"Unexpected files left behind: {filenames}" + + +def test_keystore_atomic_write_preserves_data(tmp_path: Path) -> None: + """Verify that atomic write preserves valid data.""" + key_dir = tmp_path / "keystore" + key_dir.mkdir() + + # Write initial data + ks1 = Keystore(key_dir=key_dir) + ks1.put("key1", {"id": "key1", "type": "api_key", "value": "value1"}) + ks1.put("key2", {"id": "key2", "type": "jwt", "value": "value2"}) + + # Reload and verify + ks2 = Keystore(key_dir=key_dir) + entries = ks2.get_all() + + assert len(entries) == 2 + assert "key1" in entries + assert "key2" in entries + assert entries["key1"]["value"] == "value1" + assert entries["key2"]["value"] == "value2" + + +def test_keystore_atomic_write_file_permissions(tmp_path: Path) -> None: + """Verify that atomic write maintains restrictive permissions.""" + key_dir = tmp_path / "keystore" + key_dir.mkdir() + + ks = Keystore(key_dir=key_dir) + ks.put("test-key", {"id": "test-key", "type": "api_key", "value": "test"}) + + store_path = key_dir / "keys.json" + key_path = key_dir / "master.key" + + # Check permissions (on Unix-like systems) + if os.name != "nt": # Skip on Windows + store_mode = store_path.stat().st_mode & 0o777 + key_mode = key_path.stat().st_mode & 0o777 + + assert store_mode == 0o600, f"keys.json permissions: {oct(store_mode)}" + assert key_mode == 0o600, f"master.key permissions: {oct(key_mode)}" diff --git a/tests/test_keystore_atomic_failure.py b/tests/test_keystore_atomic_failure.py new file mode 100644 index 0000000..3b34577 --- /dev/null +++ b/tests/test_keystore_atomic_failure.py @@ -0,0 +1,43 @@ +"""Test that keystore survives write failures.""" + +import contextlib +from apiauth.keystore import Keystore +from pathlib import Path +from unittest.mock import patch + + +def test_keystore_survives_write_failure(tmp_path: Path) -> None: + """If _save() fails mid-write, existing data must remain intact.""" + key_dir = tmp_path / "keystore" + key_dir.mkdir() + + # Write initial data + ks1 = Keystore(key_dir=key_dir) + ks1.put("original-key", {"id": "original-key", "value": "original-value"}) + + # Verify initial state + store_path = key_dir / "keys.json" + assert store_path.exists() + original_size = store_path.stat().st_size + + # Try to add new data, but make the write fail + ks2 = Keystore(key_dir=key_dir) + + # Mock write to raise an exception after opening file + with patch("pathlib.Path.write_bytes") as mock_write: + mock_write.side_effect = OSError("Disk full") + + # This should fail, but original data must survive + with contextlib.suppress(OSError): + ks2.put("new-key", {"id": "new-key", "value": "new-value"}) + + # Reload and verify original data is intact + ks3 = Keystore(key_dir=key_dir) + entries = ks3.get_all() + + assert "original-key" in entries, "Original key was lost during failed write!" + assert entries["original-key"]["value"] == "original-value" + + # File size should be unchanged (no partial write) + current_size = store_path.stat().st_size + assert current_size == original_size, f"File size changed: {original_size} -> {current_size}"