diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 180e822..850cadf 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -16,6 +16,11 @@ permissions: # Build tools come from packaging/build-tools.lock and the build backend from it (or build-backend.lock) with # --require-hashes, and builds run without build isolation, so nothing unpinned is fetched for a build. jobs: + # The whole tests workflow (unit and end-to-end tests on every OS and Python, lint, packaging smoke tests); + # the release waits for it + tests: + uses: ./.github/workflows/tests.yml + version: runs-on: ubuntu-latest outputs: @@ -49,12 +54,8 @@ jobs: - name: Install the build tools (packaging/build-tools.lock, hash-checked) run: | python -m pip install --require-hashes --only-binary=:all: -r packaging/build-tools.lock + # twine check and the sdist's contents are checked in the tests job (tests.yml's packaging job) - run: python -m build --no-isolation - - run: twine check dist/* - - name: Check the sdist leaves out the tests, but has the dependency lock (install.sh uses it) - run: | - ! tar tzf dist/rnfs-*.tar.gz | grep -q '/tests/' - tar tzf dist/rnfs-*.tar.gz | grep -q '/packaging/requirements.lock$' - name: Smoke test the wheel (with the locked dependencies) run: | python -m venv /tmp/wheel-venv @@ -212,7 +213,7 @@ jobs: release: # Only for a pushed tag: a manual run builds the artifacts without publishing, even when started on a tag if: github.event_name == 'push' && github.ref_type == 'tag' - needs: [version, python, deb, flatpak, windows] + needs: [tests, version, python, deb, flatpak, windows] runs-on: ubuntu-latest permissions: contents: write # create the release diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 44157d4..4cd5ce6 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -2,7 +2,9 @@ name: tests on: push: + branches: ["**"] # not tags: the release workflow runs these jobs itself before publishing pull_request: + workflow_call: # from release.yml, which grants no more than contents: read # Read-only token: these jobs only check out and test the code permissions: @@ -16,7 +18,7 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest, macos-latest, windows-latest] - python: ["3.11", "3.12", "3.13"] + python: ["3.11", "3.12", "3.13", "3.14"] steps: - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: @@ -44,6 +46,25 @@ jobs: - name: Command works run: rnfs --version + # ruff with the rules in pyproject.toml (pyflakes, syntax errors, a few bugbear checks), and no merge-conflict + # markers left in any file + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + with: + persist-credentials: false + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 + with: + python-version: "3.12" + - name: Install the build tools (packaging/build-tools.lock, hash-checked) + run: | + python -m pip install --require-hashes --only-binary=:all: -r packaging/build-tools.lock + - name: ruff + run: ruff check . + - name: No merge-conflict markers + run: "! git grep -nE '^(<<<<<<<|>>>>>>>)( |$)|^=======$' -- . ':!*.lock'" + # The packages the release workflow builds, smoke-tested: wheel/sdist, .deb, install.sh, PyInstaller binary packaging: runs-on: ubuntu-latest diff --git a/CHANGELOG.md b/CHANGELOG.md index 87b27a1..51a0bb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,33 @@ [specification.md](specification.md) is the authority on behaviour. +## 0.1.2 + +Maintenance release from the fourth audit: fixes, release guardrails and internal consolidation. No format, +protocol, CLI or config changes. + +Fixes: +- `POST /api/add` refuses a `seal_name` that isn't `true` or `false` (a string like `"false"` used to seal the + name anyway). `/api/add/stream`, where `seal_name` is the name, is unchanged. +- `names.json`, name keys and the API token are written durably (file and directory synced, unique temp + file), like the other state files. +- The README's Tests and Known limitations sections are restored (0.1.1 shipped them inside a merge + conflict), and its download links point at the current release. + +Dependencies and install: +- rns is limited to <1.6 and lxmf to <1.3; a test checks every RNS/LXMF internal rnfs relies on, and the + daemon logs an error if incoming Resource advertisements no longer look as rnfs expects. +- install.sh installs the latest release instead of the main branch (`--main` for the old behaviour). +- requirements.txt is gone: install from source with `pip install -e .`, or hash-checked from + packaging/requirements.lock. +- Python 3.14 is tested in CI and covered by the build locks. + +Development: +- CI lints with ruff and rejects merge-conflict markers; releases run the full test matrix before + publishing. +- Shared test fakes live in tests/support.py; a contract test keeps the API tests' fake node in step with + RnfsNode. + ## 0.1.1 Fixes from the third audit. diff --git a/README.md b/README.md index d133b33..724f382 100644 --- a/README.md +++ b/README.md @@ -48,35 +48,35 @@ RNFS provides a number of benefits compared to other 'torrenting for RNS' or fil ### 1 - Install -Different package options availible at [latest release](https://github.com/afit21/rnfs/releases/latest) +Different package options available at [latest release](https://github.com/afit21/rnfs/releases/latest) **Windows:** -[`rnfs-0.1.0-windows-x64.exe`](https://github.com/afit21/rnfs/releases/download/v0.1.0/rnfs-0.1.0-windows-x64.exe), saved as `rnfs.exe` +[`rnfs-0.1.2-windows-x64.exe`](https://github.com/afit21/rnfs/releases/download/v0.1.2/rnfs-0.1.2-windows-x64.exe), saved as `rnfs.exe` somewhere on your PATH ([how](#windows)). **Linux:** One liner for install script: -```curl -fsSL https://raw.githubusercontent.com/afit21/rnfs/main/install.sh | sh``` +`curl -fsSL https://raw.githubusercontent.com/afit21/rnfs/main/install.sh | sh` Or installers for ([Debian & Ubuntu](#debian-and-ubuntu)). [Flatpak](#other-linux-flatpak) **macOS:** -`pipx install https://github.com/afit21/rnfs/releases/download/v0.1.0/rnfs-0.1.0-py3-none-any.whl` ([details](#macos)). +`pipx install https://github.com/afit21/rnfs/releases/download/v0.1.2/rnfs-0.1.2-py3-none-any.whl` ([details](#macos)). -### 2 - Start rnfs** and leave it running in its own terminal: +### 2 - Start rnfs and leave it running in its own terminal: ```sh rnfs daemon ``` -### 3 - Connect to other nodes.** Computers on the same local network or Wi-Fi find each other automatically. +### 3 - Connect to other nodes. Computers on the same local network or Wi-Fi find each other automatically. To reach nodes further away, see [Connect to a network](#2-connect-to-a-network). -### 4 - Download a file**, for example one of the [sample files](#sample-files): +### 4 - Download a file, for example one of the [sample files](#sample-files): ```sh rnfs get bagaibqabciqkq6rth6jhr2vkh7c2ip7p6bsmdxmslbia47rjbmwpcz6wwu6veya -o manifesto.pdf @@ -85,7 +85,7 @@ rnfs get bagaibqabciqkq6rth6jhr2vkh7c2ip7p6bsmdxmslbia47rjbmwpcz6wwu6veya -o man Or open it in your browser: -### 5 - Share your own file:** +### 5 - Share your own file: ```sh rnfs add holiday.jpg # prints its CID: give that to anyone who should have the file @@ -102,26 +102,26 @@ rnfs runs on Linux, macOS and Windows. The easiest way to install it is a file f | System | Download | Install | |---|---|---| -| Windows (64-bit) | [`rnfs-0.1.1-windows-x64.exe`](https://github.com/afit21/rnfs/releases/download/v0.1.1/rnfs-0.1.1-windows-x64.exe) | [Windows](#windows) | -| Debian 12+, Ubuntu 24.04+, Raspberry Pi OS (64-bit) | [`rnfs_0.1.1_amd64.deb`](https://github.com/afit21/rnfs/releases/download/v0.1.1/rnfs_0.1.1_amd64.deb) or [`rnfs_0.1.1_arm64.deb`](https://github.com/afit21/rnfs/releases/download/v0.1.1/rnfs_0.1.1_arm64.deb) | [Debian and Ubuntu](#debian-and-ubuntu) | -| Other Linux (x86_64) | [`rnfs-0.1.1-x86_64.flatpak`](https://github.com/afit21/rnfs/releases/download/v0.1.1/rnfs-0.1.1-x86_64.flatpak) | [Other Linux](#other-linux-flatpak) | -| macOS, or any system with Python 3.11+ | [`rnfs-0.1.1-py3-none-any.whl`](https://github.com/afit21/rnfs/releases/download/v0.1.1/rnfs-0.1.1-py3-none-any.whl) | [macOS](#macos) | +| Windows (64-bit) | [`rnfs-0.1.2-windows-x64.exe`](https://github.com/afit21/rnfs/releases/download/v0.1.2/rnfs-0.1.2-windows-x64.exe) | [Windows](#windows) | +| Debian 12+, Ubuntu 24.04+, Raspberry Pi OS (64-bit) | [`rnfs_0.1.2_amd64.deb`](https://github.com/afit21/rnfs/releases/download/v0.1.2/rnfs_0.1.2_amd64.deb) or [`rnfs_0.1.2_arm64.deb`](https://github.com/afit21/rnfs/releases/download/v0.1.2/rnfs_0.1.2_arm64.deb) | [Debian and Ubuntu](#debian-and-ubuntu) | +| Other Linux (x86_64) | [`rnfs-0.1.2-x86_64.flatpak`](https://github.com/afit21/rnfs/releases/download/v0.1.2/rnfs-0.1.2-x86_64.flatpak) | [Other Linux](#other-linux-flatpak) | +| macOS, or any system with Python 3.11+ | [`rnfs-0.1.2-py3-none-any.whl`](https://github.com/afit21/rnfs/releases/download/v0.1.2/rnfs-0.1.2-py3-none-any.whl) | [macOS](#macos) | Every package includes Reticulum and LXMF, gives you one `rnfs` command, and keeps your data in the same -places. The release also has the source archive (`rnfs-0.1.1.tar.gz`) and `SHA256SUMS`, to -[check your download](#checking-a-download). These links are for 0.1.1; for a newer version, take the +places. The release also has the source archive (`rnfs-0.1.2.tar.gz`) and `SHA256SUMS`, to +[check your download](#checking-a-download). These links are for 0.1.2; for a newer version, take the same files from the [latest release](https://github.com/afit21/rnfs/releases/latest). ### Windows -Download [`rnfs-0.1.1-windows-x64.exe`](https://github.com/afit21/rnfs/releases/download/v0.1.1/rnfs-0.1.1-windows-x64.exe). It is a single file that +Download [`rnfs-0.1.2-windows-x64.exe`](https://github.com/afit21/rnfs/releases/download/v0.1.2/rnfs-0.1.2-windows-x64.exe). It is a single file that includes Python, Reticulum and LXMF; nothing else needs installing. In PowerShell, from your download folder, put it on your PATH as `rnfs`: ```powershell $bin = "$env:USERPROFILE\.local\bin" New-Item -ItemType Directory -Force $bin | Out-Null -Move-Item .\rnfs-0.1.1-windows-x64.exe "$bin\rnfs.exe" -Force +Move-Item .\rnfs-0.1.2-windows-x64.exe "$bin\rnfs.exe" -Force Unblock-File "$bin\rnfs.exe" [Environment]::SetEnvironmentVariable("Path", [Environment]::GetEnvironmentVariable("Path", "User") + ";$bin", "User") # open a new terminal, then: @@ -152,8 +152,8 @@ or newer. Ubuntu 22.04 ships Python 3.10, so it isn't supported; use the [instal there instead. Pick `amd64` or `arm64` to match `dpkg --print-architecture`: ```sh -curl -LO https://github.com/afit21/rnfs/releases/download/v0.1.1/rnfs_0.1.1_amd64.deb -sudo apt install ./rnfs_0.1.1_amd64.deb +curl -LO https://github.com/afit21/rnfs/releases/download/v0.1.2/rnfs_0.1.2_amd64.deb +sudo apt install ./rnfs_0.1.2_amd64.deb rnfs daemon # or run it in the background: systemctl --user enable --now rnfs # a systemd user service; logs: journalctl --user -u rnfs -f ``` @@ -172,9 +172,9 @@ For x86_64 Linux distributions without `.deb` support. The Flatpak uses the Flat `org.freedesktop.Platform` runtime: ```sh -curl -LO https://github.com/afit21/rnfs/releases/download/v0.1.1/rnfs-0.1.1-x86_64.flatpak +curl -LO https://github.com/afit21/rnfs/releases/download/v0.1.2/rnfs-0.1.2-x86_64.flatpak flatpak remote-add --user --if-not-exists flathub https://dl.flathub.org/repo/flathub.flatpakrepo -flatpak install --user rnfs-0.1.1-x86_64.flatpak +flatpak install --user rnfs-0.1.2-x86_64.flatpak flatpak run io.github.afit21.rnfs daemon alias rnfs='flatpak run io.github.afit21.rnfs' # add to ~/.bashrc to type just "rnfs" ``` @@ -202,7 +202,7 @@ and puts the `rnfs` command on your PATH. Homebrew also installs a recent Python ```sh brew install pipx pipx ensurepath # then open a new terminal -pipx install https://github.com/afit21/rnfs/releases/download/v0.1.1/rnfs-0.1.1-py3-none-any.whl # includes Reticulum and LXMF +pipx install https://github.com/afit21/rnfs/releases/download/v0.1.2/rnfs-0.1.2-py3-none-any.whl # includes Reticulum and LXMF rnfs daemon ``` @@ -228,20 +228,21 @@ Each release's `SHA256SUMS` lists the checksum of every file. Download it next t ```sh sha256sum -c SHA256SUMS --ignore-missing # Linux -shasum -a 256 rnfs-0.1.1-py3-none-any.whl # macOS: compare with the line in SHA256SUMS +shasum -a 256 rnfs-0.1.2-py3-none-any.whl # macOS: compare with the line in SHA256SUMS ``` ```powershell -Get-FileHash .\rnfs-0.1.1-windows-x64.exe # Windows: compare with the line in SHA256SUMS +Get-FileHash .\rnfs-0.1.2-windows-x64.exe # Windows: compare with the line in SHA256SUMS ``` ### Other ways to install -These install the latest code from the main branch, which may be newer than the latest release. +The install script installs the latest release; pipx from GitHub and a source checkout install the latest +code from the main branch, which may be newer than the latest release. #### Install script -Installs from the main branch (newer than the latest release) on any Linux distribution. You need Python 3.11 or newer with venv: +Installs the latest release on any Linux distribution. You need Python 3.11 or newer with venv: - Debian/Ubuntu: `sudo apt install python3 python3-venv` - Fedora: `sudo dnf install python3` @@ -253,19 +254,23 @@ Git isn't needed. curl -fsSL https://raw.githubusercontent.com/afit21/rnfs/main/install.sh | sh ``` -This installs rnfs from the main branch, with Reticulum and LXMF, into its own virtual environment at +This installs the latest rnfs release, with Reticulum and LXMF, into its own virtual environment at `~/.local/share/rnfs/venv`. Dependencies are installed at the versions pinned in the source's `packaging/requirements.lock`, with their sha256 hashes checked. It links the `rnfs` command into `~/.local/bin` and tells you if that folder isn't on your PATH. Options go after `sh -s --`: ```sh curl -fsSL https://raw.githubusercontent.com/afit21/rnfs/main/install.sh | sh -s -- --service # also run the daemon as a systemd user service -curl -fsSL https://raw.githubusercontent.com/afit21/rnfs/main/install.sh | sh -s -- --update # update to the latest main branch +curl -fsSL https://raw.githubusercontent.com/afit21/rnfs/main/install.sh | sh -s -- --update # update to the latest release +curl -fsSL https://raw.githubusercontent.com/afit21/rnfs/main/install.sh | sh -s -- --main # install the main branch instead curl -fsSL https://raw.githubusercontent.com/afit21/rnfs/main/install.sh | sh -s -- --uninstall # remove it (keeps ~/.rnfs) ``` -- **Update:** run the script again, with or without `--update`. It re-installs rnfs from the latest main - branch, with the dependency versions pinned there. If you installed the service, it restarts. +- **Update:** run the script again, with or without `--update`. It re-installs rnfs from the latest + release (or with `--main`, the main branch), with the dependency versions pinned there. If you installed + the service, it restarts. +- **Main branch:** `--main` installs the latest code from the main branch, which may be newer than the + latest release. Give it again each time you update, or the next update goes back to the latest release. - **Service:** `--service` installs `~/.config/systemd/user/rnfs.service` and starts it. To see its logs: `journalctl --user -u rnfs -f`. To keep it running while you're logged out: `loginctl enable-linger $USER`. @@ -274,9 +279,9 @@ curl -fsSL https://raw.githubusercontent.com/afit21/rnfs/main/install.sh | sh -s This installs to `/opt/rnfs` and `/usr/local/bin/rnfs`. - `--python /usr/bin/python3.12` chooses the Python to use. - `--force` replaces an existing `~/.local/bin/rnfs`, for example one installed by pipx. -- `RNFS_SOURCE=` installs something other than the main branch. A - `.whl` or other pip requirement has no lock file, so its dependencies are installed unpinned (newest), - with a warning. +- `RNFS_SOURCE=` installs something other than the latest release (it + overrides `--main` too). A `.whl` or other pip requirement has no lock file, so its dependencies are + installed unpinned (newest), with a warning. #### pipx from GitHub @@ -291,13 +296,13 @@ Update with `pipx upgrade rnfs`; uninstall with `pipx uninstall rnfs`. Plain `pi ```sh git clone https://github.com/afit21/rnfs && cd rnfs -pip install -r requirements.txt # Reticulum and LXMF; or the pinned, hash-checked versions: - # pip install --require-hashes -r packaging/requirements.lock -python3 rnfs.py daemon # or python3 -m rnfs daemon; on Windows: py rnfs.py daemon +pip install -e . # the rnfs command, running your working copy, with Reticulum and LXMF +rnfs daemon ``` -`pip install -e .` installs the `rnfs` command and runs your working copy. Without it, use -`python3 rnfs.py` wherever this README says `rnfs`. +To get the tested, hash-checked versions of Reticulum and LXMF, run +`pip install --require-hashes -r packaging/requirements.lock` first. Without `pip install -e .`, install them +that way and use `python3 rnfs.py` (on Windows, `py rnfs.py`) wherever this README says `rnfs`. **Building the packages yourself.** `.github/workflows/release.yml` does this for releases. By hand: @@ -866,15 +871,13 @@ rnfs --dir /tmp/b --api-port 9002 get -o out ``` `tests/test_e2e.py` sets up exactly this, with short intervals in each node's `rnfs.toml`. -<<<<<<< HEAD -======= ## Tests ```sh -python3 -m unittest discover tests # unit tests, no network -RNFS_E2E=1 python3 -m unittest tests.test_e2e # real daemons over local TCP -python3 packaging/smoke_test.py dist/rnfs # smoke-test a built package +python3 -m unittest discover tests # unit tests, no network +RNFS_E2E=1 python3 -m unittest tests.test_e2e tests.test_client # real daemons over local TCP +python3 packaging/smoke_test.py dist/rnfs # smoke-test a built package ``` To debug a running daemon, `kill -USR1 ` prints every thread's stack. @@ -892,4 +895,3 @@ To debug a running daemon, `kill -USR1 ` prints every thread's stack - Windows support is new. The full list is in [design.md, Status](design.md#status). ->>>>>>> 3003517 (Add comprehensive tests for packaging, pinning, and sealing functionality) diff --git a/audit.md b/audit.md new file mode 100644 index 0000000..dd1e92c --- /dev/null +++ b/audit.md @@ -0,0 +1,483 @@ +# rnfs maintainability audit + +*Date: 2026-10-13 · Revision: `d0aa468` (branch `dev`) · Scope: the whole repository (`rnfs/`, `tests/`, packaging, CI, docs)* + +This audit looks at how hard the code is to change safely. It is not a bug hunt. Two incidental bugs found along the way are noted where relevant. + +## Summary + +rnfs is in good shape for a project of its size: about 40k lines, 1,020 unit tests plus a 28-test end-to-end suite, all passing in about 2 minutes. + +**Strengths:** +- Docstrings explain *why* code exists and cite the spec. +- Policy code is pure and takes an injected clock. +- Wire and DAG parsers are strict. +- `specification.md` and `design.md` match the code. +- Supply-chain discipline is above average: hash-locked deps, SHA-pinned actions, and a 3 OS × 3 Python CI matrix. + +The risks to future change are not in any single module. They come from six **cross-cutting patterns**: + +1. **RNS and LXMF internals are patched and read in many places.** Upstream versions are not capped, and nothing checks for these accesses at startup. +2. **Low-level helpers are written several times over.** Atomic file writes exist 7 times, hex-ID parsing about 8 times, path resolution 4 times, and the feature utility functions 3–5 times each. The copies have already drifted. +3. **Layer boundaries leak.** The API imports the CLI. Core modules and `Config` name specific features. Features use node private members. The content store knows feature pin-source prefixes. +4. **A few classes do too much.** `RnfsNode` (~840 lines), `features/lxmf.py` (1,885 lines), `RequestServer` (~540 lines) and `Fetch` (~650 lines). +5. **Lock ownership and ordering are implicit.** Several objects have fields guarded by two different locks or none, and slow work runs while locks are held. +6. **There are no automated hygiene checks.** No lint, type-check or coverage in CI. Merge-conflict markers shipped in the 0.1.1 README, and releases are not gated on the test suite. + +None of these is urgent on its own. Together they make each new feature or RNS upgrade more expensive than it needs to be. The [roadmap](#suggested-roadmap) orders the fixes by cost against payoff. + +### Severity overview + +| Area | High | Medium | Low | +|---|---|---|---| +| Cross-cutting | 3 | 3 | – | +| Core (node, api, cli, config, names, publisher) | 1 | 5 | 7 | +| Content layer, fetcher, gateway | 1 | 5 | 7 | +| Network and DHT | 1 | 4 | 6 | +| Features | 1 | 5 | 8 | +| Tests, CI, packaging, docs | 1 | 6 | 6 | + +The cross-cutting findings in the next section are counted once, there. The per-area sections only add what is specific to that area. + +--- + +## Method + +The review was split into five areas, each read in full by a separate reviewer: +- core node, API and CLI +- content layer +- network and DHT +- features +- tests, packaging and docs + +Every High finding, and a sample of the others, was then checked by hand against the source. Supporting metrics came from an AST scan of `rnfs/`: + +| Metric | Value | +|---|---| +| Functions/methods | 1,313 | +| Functions > 60 lines / > 100 lines | 8 / 2 (`cli.build_parser` 136, `Publisher.publish_due` 102) | +| Broad `except Exception` / bare `except` | 95 (most in `features/lxmf.py` 13, `node.py` 10, `network/server.py` 9, `names.py` 8) | +| Function-local imports | 125+ (most in `features/lxmf.py` 32, `features/jobs.py` 27, `cli.py` 16, `features/keepers.py` 15) | +| `TODO` / `FIXME` markers | 0 | +| Unit tests / runtime | 1,020 tests, 58 s (Python 3.12, rns 1.5.6) | +| E2E tests / runtime | 28 tests, 57 s | + +Most functions are short. The problems are at the module and class level, not inside individual functions. + +--- + +## 1. Cross-cutting themes (highest leverage) + +### X1 · High: RNS/LXMF internals are used throughout, with no upper version bound + +**Process-wide mutations:** +- `RNS.Resource.AUTO_COMPRESS_MAX_SIZE = 0` is set twice (`node.py:214`, `network/server.py:360`). +- `guards.install()` is called three times (`node.py:215`, `network/server.py:362`, `network/links.py:190`). +- `features/lxmf.py:1540-1552` temporarily replaces `signal.signal` for the whole process while `LXMRouter` is constructed. Any other thread calling `signal.signal` in that window is silently ignored. + +**Private RNS attributes read:** +- Resource advertisement fields `t/d/u/l/i/q/p` (`network/guards.py:46-56`) +- `Link.pending_requests` (`guards.py:66`) +- `Link.incoming_resources` (`links.py:502`) +- `Link.register_outgoing_resource`, which is wrapped per link (`server.py:684-705`) +- `RNS.Identity.known_destinations` entry layout (`node.py:396-411`, `names.py:175-179`, `features/peers.py:133`) +- `Transport.get_cached_packet` (`features/keepers.py:548`) +- `_no_use=True` private kwarg (`node_content.py:63-67`) + +**Private LXMF attributes and methods:** +- `LxmfNetwork.__init__` (`features/lxmf.py:1576-1633`) replaces three `LXMRouter` methods. +- It reads `propagation_transfer_state`, `outbound_propagation_link` and `delivery_destinations`. +- It sets an attribute on a foreign `Link`. + +**Version bounds:** `pyproject.toml:13` is `rns>=1.5.6`, `lxmf>=1.2.0`. Neither has an upper bound. + +**Why it matters:** an RNS point release that renames `adv.t` makes `guards.refusal()` reject **every** incoming Resource in the process, LXMF's included. Nothing fails at startup to say why. + +**Recommendation:** +- Create `rnfs/network/rns_compat.py` (plus an LXMF equivalent) to hold every patch and every private-attribute access, each with a comment naming the upstream version it was written against. +- Call it exactly once from node startup. It should check that the attributes it needs exist and log loudly, or refuse to start, when they don't. +- Add a unit test that imports RNS/LXMF and asserts the attributes exist, so a dependency bump fails in CI. +- Cap the versions: `rns>=1.5.6,<1.6`, `lxmf>=1.2,<1.3`. + +### X2 · High: core primitives are written several times and have drifted + +| Primitive | Copies | Drift | +|---|---|---| +| Atomic file write | `content/blockstore.py:59-89`, `content/storage.py:81-97` (`write_json`), `names.py:747-762`, `cli.py:56-69` (`write_token`), `network/identity.py:45-65`, `dht/providers.py:346-360`, `features/lxmf.py:424-444` | `storage.write_json` fsyncs the file and its directory. `names._write_atomic` never syncs the directory and uses a fixed temp name. `write_token` doesn't fsync at all. `design.md` promises "atomically and durably" for all state. | +| 32-hex ID / identity-hash parsing | `api.parse_hex_id` (`api.py:149`), `identity.parse_node_id` / `parse_identity_hash` (`network/identity.py:88-101`), `protocol.is_id` (`protocol.py:52`), `lookup._valid_id` (`dht/lookup.py:41`), `routing.check_id` (`dht/routing.py:20`) plus an inline copy (`routing.py:205`), `names.parse_name` (`names.py:78`), `config._HEX_ID` (`config.py:38`), `contacts.parse_hex`, `lxmf._HEX32`, `jobs._NODE_ID`, inline `.strip().strip("<>").lower()` (`node.py:264`, `server.py:356`) | Rules differ on letter case and on whether `<…>` brackets are allowed. | +| Feature utility helpers | `human_size` ×3 (`lxmf.py:165`, `jobs.py:710`, `peers.py:169`, plus `log.size`), `_printable` ×3 (`contacts.py:322`, `follow.py:339`, `peers.py:237`, plus `lxmf.py:111` and `fsutil.printable`), `_log` ×5, `_feature(handler)` ×5 | Three different size formats. Two `_log` level conventions (`"info"` vs `"LOG_INFO"`). `_feature` sometimes raises `NotFound` and sometimes doesn't. | +| Path resolution | `walker.resolve` (`walker.py:241-252`), `edit._directory_chain` (`edit.py:16-31`), `Fetch._resolve` (`fetcher.py:326-340`), `Storage._path_cids` (`storage.py:480-506`) | `_path_cids` re-implements the spec's "greatest `first_name` ≤ name" index rule separately from `walker.step`. | +| Strict msgpack reader | `dag._Reader` (`dag.py:145-224`), `archive._StreamReader` (`archive.py:56-142`) | Same tag tables. `archive_size` (`archive.py:43-53`) re-derives the encoding byte by byte. | +| Archive export | `node.export` (`node.py:993-1015`), `node_content.export_stream` (`node_content.py:194-208`) | `export_stream` omits `max_output_entries`. | +| "Read exactly n bytes" | `builder._read_full` (unused), `chunker.read_full`/`_read_limited`, `seal._read_segment`, `archive._take` | – | +| Constants | `PARTIAL_SUFFIX` (`node.py:100`, `jobs.py:60`), the `".partial-"` literal (`storage.py:84`, `lxmf.py:429`) instead of `blockstore.PARTIAL_PREFIX`, `RESPONSE_MAX_GRACE_TIME * 1.125` (`links.py:36`, `server.py:42`) | – | + +**Recommendation:** +- Add `fsutil.write_atomic(path, data, *, mode=0o644, durable=True)` and route all seven writers through it. +- Add one `rnfs/ids.py` for ID parsing. +- Add one `rnfs/features/_common.py` (or use `fsutil`/`log` directly) for the feature helpers. +- Add one `walker.walk_path()` generator used by `resolve`, `edit`, `fetcher` and `storage`. + +These are mostly mechanical changes, and they remove a whole class of "fixed it in one copy" bugs. + +### X3 · High: layer boundaries leak in both directions + +The intended layering in `design.md` is content/network/dht → node → api/cli, with features as plugins. These imports and accesses go against it: + +| Leak | Where | +|---|---| +| The API server imports the CLI | `api.py:75` imports `CHALLENGE_HEADER`, `PROOF_HEADER` and `api_proof` from `rnfs.cli`. | +| The gateway depends on the API module for its HTTP framework | `gateway.py:46`. `TimedHandler`, `BoundedHTTPServer`, `LocalServer` and `BodyReader` (~290 lines) live in `api.py:194-481`. | +| Core modules name specific features | `api.py:135` (`contacts.is_nick`), `api.py:816` (`keepers.parse_name_arg`), `node.py:631-633` (`contacts.expand_target`), `cli.py:444/462/511/661` (`jobs.run_transfer`, `jobs.StreamProgress`) | +| `Config` owns every feature's settings | Fields at `config.py:96-119`. `Config.validate` imports `features.lxmf` (`config.py:206`). This contradicts the comment at `config.py:19`. | +| The content store knows feature pin sources | `storage.check_source` hard-codes `"follow:"` and `lxmf:` (`storage.py:52-67`) | +| Features use node and name-manager private members | `node._lookup`, `node._is_known` (`keepers.py:637,646`), `node.names.keepers = self` (`keepers.py:382`), argparse's `_actions`/`_SubParsersAction` (`keepers.py:714-715`) | +| Content helpers use node private members | `node._note_fetched`, `_prune_fetched`, `_fetching` (`node_content.py:29-31,186,198`) | +| The network layer imports a top-level module | `network/client.py:16` and `network/server.py:32` import `rnfs.mesh` | +| Hidden import cycle | `gateway_formats` ↔ `gateway`, hidden by a function-local import (`gateway_formats.py:33`) | + +**Why it matters:** adding a feature currently means editing `config.py`, `api.py`, and sometimes `node.py` and `storage.py`. The `Feature` contract in `features/__init__.py` exists precisely to avoid that. + +**Recommendation:** +- Move the token and proof helpers to `rnfs/auth.py`. +- Move the HTTP framework to `rnfs/httpserver.py`. +- Add `Feature` hooks: `settings()`, `validate_config()`, `expand_target()`, `resolve_name_arg()` and `pin_source_prefixes()`. +- Make `_lookup`/`_is_known` public. +- Give `NameManager` a `set_keepers()` method. +- Move `StreamProgress`/`ProgressLine` out of `features/jobs.py`; `rnfs cat` uses them and has nothing to do with jobs. + +### X4 · Medium: a few classes carry too many responsibilities + +| Unit | Size | Responsibilities mixed | +|---|---|---| +| `RnfsNode` (`node.py:205-1044`) | ~840 lines | Startup wiring, DHT join and seeding, maintenance scheduling, fetched-target bookkeeping, a remote-pin work queue (`node.py:823-915`), and the public facade | +| `features/lxmf.py` | 1,885 lines | URI and field codec (175-414), `MessageLog` persistence (416-685), `LxmfFeature` (691-1323), API handlers, CLI printing, `LxmfNetwork` RNS adapter (1537-1885) | +| `RequestServer` (`network/server.py:330-870`) | ~540 lines | Handler dispatch, link admission, RNS Resource tracking and monkeypatching, budget settlement | +| `Fetch` (`fetcher.py:220-872`) | ~650 lines | Traversal policy, provider ranking, pacing, lookup threads, event loop | + +**Recommendation:** +- Extract `RemotePinQueue` and `FetchedSet` from `RnfsNode`. +- Turn `features/lxmf.py` into a package: `uri.py`, `messagelog.py`, `feature.py`, `network.py`, `cli.py`. +- Extract `ResponseTracker` from `RequestServer`. +- Extract a provider scheduler from `Fetch`, and replace its positional string-tagged event tuples with `NamedTuple`s. + +### X5 · Medium: lock ownership and ordering are implicit + +**Fields guarded by more than one lock, or by none:** +- **`Response`:** `charged` and `finished` are guarded by `RequestServer.lock`. `resource` (`server.py:712,836`) and `sent` (`:853`) are written without a lock, and `finished` is read without one (`:572,597,824`). +- **`Job`:** `state` is written under both `JobsFeature._lock` (`jobs.py:507,586`) and `Job._lock` (`:135`). `total_bytes` is written with no lock (`:109`). +- **`Exchange`:** fields are written on RNS threads and polled without a lock (`links.py:60-86`). This relies on the GIL and on write order, which is not documented. + +**Implicit lock order:** the order is names → publisher → node → storage, and it isn't written down anywhere: +- `NameManager.publish/remove/release_gone` pins and calls `publisher.refresh()` while holding `names._lock` (`names.py:288,315,365-367`). +- `Publisher.refresh` walks every pin's DAG under its lock and takes `node._lock` (`publisher.py:252-258`). +- RNS announce threads can therefore block behind a full provide-set recompute. + +**Slow work under locks:** +- `storage.evict/gc/verify` run full DAG walks with disk reads under the storage RLock (`storage.py:706,721,738`). The module docstring (`storage.py:16`) says long walks happen outside the lock. +- `_note_fetched` does an fsync'd write under `node._lock` (`node.py:513-519`). + +**Busy-wait:** `server.py:670-674` polls with `time.sleep(0.01)` for up to 5 s on an RNS request thread. + +**Recommendation:** +- Document the lock order in `design.md` § Threads. +- Give each mutable field exactly one owning lock, and say which in a comment. +- Move pin and refresh calls out of the names lock. +- Snapshot targets under the storage lock and walk them outside it. +- Wait on `response.done` instead of polling. + +### X6 · Medium: no automated hygiene checks in CI + +- **No lint, format, type-check or coverage.** Neither `pyproject.toml` nor `.github/workflows/tests.yml` configures any. The code is fully annotated and already has `# noqa` comments, so the groundwork is there. +- **Things a basic check would have caught:** + - merge-conflict markers in the released README (T-H1) + - a stray `#yo` comment (`rnfs.py:3`) + - unused code: `builder._read_full`, `announces.read_mode`, `PeerLink.mtu`, `guards.installed`, `constants.REQUEST_FRAMING`, and the unused `when` at `follow.py:178` +- **95 broad exception handlers.** Many are deliberate best-effort guards around RNS. Some swallow errors without logging (`fetcher.py:652`, `seal.py:387`). + +**Recommendation:** add a fast ubuntu-only `lint` job: +- `ruff check` with the pyflakes, isort and bugbear rule sets +- `ruff format --check`, if wanted +- `! git grep -nE '^(<<<<<<<|>>>>>>>) '` + +Optionally, add `mypy rnfs` with a permissive baseline that is tightened over time, and `coverage run` with a report. Coverage would also make the test gaps in T-L3 visible. + +--- + +## 2. Core: node, API, CLI, config, names, publisher + +**C-H1 · High: `RnfsNode` is used as a bag of components.** See X4 for its size. On top of that, callers drive its internals directly: +- Features call `node.publisher.refresh()` 7 times and `node.storage.pin/unpin` directly. +- The "pin, then `publisher.refresh()`" sequence is repeated 17 times inside `node.py`, and again in `_NamePins` (`node.py:145-154`). + +*Fix:* add public `node.pin(cid, source)`, `node.unpin(...)`, `node.fetching(...)` and `node.note_fetched(...)` methods that refresh internally, so components stop being reached through the node. + +**C-M1 · Medium: add options are encoded and decoded in two places, and `seal_name` changes type.** +- `cli.add_options` (`cli.py:348-365`) and `node_content.add_options` (`node_content.py:214-237`) are paired only by convention. +- `seal_name` is a `bool` in `add_path` (`node.py:566`) but a `str` in `add_stream` (`node.py:594`). +- *Incidental bug:* `node_content.py:233` applies `parse_bool` only to values that are *already* bools, so `POST /api/add {"seal_name": "false"}` passes the string `"false"` to `add_path`. A non-empty string is truthy, so the name is sealed anyway. That is the safe direction, but it shows the type confusion. + +*Fix:* split the parameter into `seal_name: bool` and `sealed_name: str | None`, and define one shared options schema. + +**C-M2 · Medium: the offline node is half-built, so the facade is probed with `getattr`.** +- `RnfsNode.offline()` uses `cls.__new__` (`node.py:251`) and never sets `identity`, `destination` or `paths`. +- Callers therefore probe with `getattr(node, "identity", None)` (`node_content.py:52,115,130`, `node.py:1036`, `gateway.py:541`). +- `node_content.py:63-67` catches `AttributeError` to detect the offline case. + +*Fix:* initialise every attribute (as `None`) in `_setup`, describe the facade as a `typing.Protocol`, and inject identity recall the same way `is_known` is already injected. + +**C-M3 · Medium: config is the extension point for every feature.** See X3. *Fix:* per-feature settings dataclasses, or `Feature.settings()`/`validate()` hooks. + +**C-M4 · Medium: `maintain()` schedules work with an ad-hoc dict.** The `last` dict (`node.py:461-484`) mixes `-inf` with `setdefault(now)` to mean "run on the first pass". *Fix:* a small `Periodic(interval, first_run=…)` helper. + +**C-M5 · Medium: the HTTP server framework lives in `api.py`.** See X3. Timeout defaults are also duplicated between `TimedHandler` and `LocalServer` (`api.py:289-291` vs `454-456`). + +**Low:** +- **Long functions:** `cli.build_parser` (136 lines, `cli.py:688`), `Publisher.publish_due` (102 lines, `publisher.py:318`), `cli.run_daemon` (80 lines, `cli.py:263`). +- **Mode strings vs constants:** the literals `"server"`/`"client"` (`node.py:166`, `announces.py:47`, `features/peers.py`) sit alongside the `MODE_SERVER`/`MODE_CLIENT` ints in `constants.py`. +- **Misleading type:** `Publisher._provided: frozenset[Cid]` also holds `DhtKey` (`publisher.py:129`). Use a type alias. +- **Re-exported helper:** `fsutil.printable` is re-exported as `api.loggable` (`api.py:114`) and `cli.printable` (`cli.py:38`), and `jobs` imports it from `cli`. +- **Repeated call:** `_make_private_dirs` is called twice (`node.py:212,265`). +- **Untyped hook:** `node.names.keepers = self` is an untyped attribute set from outside, with `None` checks scattered through `names.py`. +- **Stale doc:** `design.md`'s facade table says `pin_ls` returns `{cid, sources, complete}`. The code also returns `type` and `name` (`node.py:795`). + +## 3. Content layer, fetcher, gateway + +**N-H1 · High: path resolution is implemented four times.** See X2. Lookup rules are the most likely part of the format to change, and they live in two separate implementations. + +**N-M1 · Medium: the content layer loads all of RNS.** +- *Measured:* importing `rnfs.content.dag`, `builder` or `storage` loads 46 `RNS.*` modules, `RNS.Transport` among them. Only `content/cid.py` loads none. +- *Cause:* `from RNS.vendor import umsgpack` runs `RNS/__init__`, and `rnfs.log` does `import RNS`. +- The package docstring does acknowledge "bundled msgpack (and logging)". The practical effect, though, is that the lazy-import workarounds in `seal.py:25,71-75,172` and `walker.py:433` don't avoid loading RNS. + +*Fix:* depend on standalone `msgpack`/`umsgpack` and a logging shim with no RNS import. Add a test asserting `"RNS" not in sys.modules` after importing `rnfs.content.*`. + +**N-M2 · Medium: generic JSON persistence lives in `content/storage.py`.** +- `read_json`/`write_json` (`storage.py:81-139`) are imported by 12 modules across every layer, mostly through function-local imports. +- Partial files in `state_dir` are never cleaned up; only the blocks directory is (`blockstore.py:31-34`). + +*Fix:* add `rnfs/jsonstate.py` on top of `fsutil.write_atomic` (see X2), with startup cleanup. + +**N-M3 · Medium: the store knows pin-source prefixes, and drifts from the spec.** +- `storage.check_source` hard-codes `follow:` and `lxmf:` (`storage.py:52-67`). +- The module docstring (`storage.py:3`) lists four sources. +- `name:` accepts any non-empty string, but spec §8 requires 32 hex characters. + +*Fix:* a registry of prefixes and validators that features register into. + +**N-M4 · Medium: `Fetch` uses implicit protocols.** +- Events are positional tuples unpacked by index (`fetcher.py:284,604-613,719,760-763`). +- `InvalidDag.root` is monkey-patched onto exception objects (`187-188,292,303-304`). +- The context manager is driven by hand (`264-265,294`). +- `describe()` reads `fetch._providers` (`878`). +- `Fetch.get` duplicates `Storage.getter` (`fetcher.py:312-317` vs `storage.py:309-319`). + +*Fix:* `NamedTuple` events, a real `root` field on `InvalidDag`, and `contextlib.ExitStack`. + +**N-M5 · Medium: the chunker is chosen by ambient `ContextVar` state.** The output CID of `build_file` depends on hidden context (`chunker.py:56,67-78`, `builder.py:136-137`), and threads don't inherit the `ContextVar`. *Fix:* pass `chunker` explicitly. + +**N-M6 · Medium: `Session` reaches into storage privates.** It uses `storage._lock` and `storage._put` (`storage.py:160,166`). *Fix:* a narrow internal API. + +**Low:** +- **Confusing names:** `dag.MAX_SIZE` (2⁶⁴−1) and `chunker.MAX_SIZE` (128 KiB) share a name. +- **Four "type" vocabularies:** dag ints, walker strings, `kind_of` strings and `Built.kind` ints. +- **Mixed concerns in `builder.py`:** it holds the secure filesystem-walk code (`O_NOFOLLOW`, junctions, `FD_WALK`, `builder.py:211-337`) that `shared_folder` also uses. Move it to `fsutil` or `content/fswalk.py`. +- **Repeated validity check:** `len(block) <= MAX_BLOCK_SIZE and cid.verify(block)` appears 4 times (`blockstore.py:109`, `storage.py:261,288,437`). +- **Gateway duplication:** the 304 helpers are duplicated (`gateway.py:481-486`, `gateway_formats.py:129-134`). `gateway.decode_segment` (`gateway.py:453-461`) re-implements part of `dag.check_name` and skips the 255-byte limit. +- **Scattered normative constants:** `MAX_ROOTS`, `MAX_NAME_BYTES`, the CID prefixes, the FastCDC parameters and the seal format all live outside `constants.py`, which says it holds the protocol constants. +- **Silent broad excepts:** `fetcher.py:652` and `seal.py:387`. + +## 4. Network and DHT + +**W-H1 · High: RNS patching is scattered.** See X1. + +**W-M1 · Medium: request handlers repeat boilerplate, and adding a message type touches five places.** +- The `start_request` / `try` / `finally: finish_request(...)` pattern is copied in four handlers (`server.py:483-491, 499-509, 515-521, 529-535`). +- `_handle_blocks` (`server.py:537-626`, 90 lines) ends with a three-branch `finally` that includes a no-op `if handed_over: pass` (621-622). +- A new message type touches constants, protocol encode/parse, a server handler, a `Backend` method and a client method. +- `Backend` (`server.py:46`) omits `feature_requests`, which is instead duck-typed at `server.py:372`. + +*Fix:* a `with self.incoming.slot(link_id):` context manager, and add `feature_requests` to the `Backend` protocol. + +**W-M2 · Medium: the clock is injected only partly.** Classes take a `clock`, yet these paths call `time.monotonic()` and `time.sleep()` directly: +- `_previous_done` (`server.py:658,670-674`) +- `PeerLink.acquire` (`links.py:130,135`) +- `_recall` (`links.py:306-308`) +- `announces.read_mode` (`announces.py:134-139`) + +They can't be driven by a fake clock in tests. + +**W-M3 · Medium: timing constants are scattered.** +- There are six different poll intervals: 0.25, 0.5, 0.02, 0.1, 0.1 and 0.01 s. +- `path_timeout + 120` (`links.py:213`) is unnamed. +- The 5.625 s pacing cap is derived twice (see X2). + +*Fix:* name them in `protocol.py` or `rns_compat`. + +**W-M4 · Medium: `PeerClient` reaches into `LinkPool`.** It takes `links.lock` and reads `links.shared` (`client.py:59-60,173-174`). *Fix:* add a `LinkPool.peer(node_id)` accessor. + +**Low:** +- **Dead code:** `announces.read_mode`, `PeerLink.mtu`, `guards.installed` and `constants.REQUEST_FRAMING`. +- **Understated timeout:** the `LinkPool.get` docstring (`links.py:197`) gives a smaller worst case than the code: `await_path` plus `_recall` plus establishment plus 5 s. +- **Wire format split across modules:** the `/name` message is missing from the `protocol.py` table and is encoded in `features/keepers.py:45`. +- **Duplicate helper:** `_never` is defined in both `lookup.py:36` and `providers.py:36`. +- **Mode checked twice:** `server.py:496` and `node.py:177`. +- **Unbounded threads:** `Lookup._send` starts one daemon thread per query, and hung threads are abandoned (`lookup.py:268`). + +## 5. Features + +**F-H1 · High: `features/lxmf.py` is six modules in one file.** It also carries most of the LXMF monkeypatching. See X1 and X4. + +**F-M1 · Medium: features look each other up in four different ways.** +- The four patterns: + - `getattr(node, "features", {}).get("contacts")` plus `hasattr(..., "lookup")` (`lxmf.py:847,859,1145`, `jobs.py:532`) + - `node.features.get(...) if hasattr(node, "features")` (`keepers.py:723`) + - `node.features.get("keepers")` (`contacts.py:194`) + - `node.feature("keepers")` (`follow.py:73`) +- Feature start order is enforced only by list order in `FEATURE_MODULES` (`features/__init__.py:20`), with a comment as the only guard. Reordering the list causes a `KeyError` at startup. + +*Fix:* a declared `requires = ("keepers",)` that `feature_classes()` validates, plus a `Feature.optional(name)` helper. + +**F-M2 · Medium: persisted state has no shared format or versioning.** +- contacts, follow and keepers write `"version": 1` (`contacts.py:107`, `follow.py:98`, `keepers.py:282`), but no `_load` ever reads it. +- `jobs.json` and the lxmf inbox/outbox files have no version at all. + +*Fix:* one `load_versioned(path, version, migrate)` helper, and a small base class for the repeated "dirty flag + save lock + snapshot" pattern. + +**F-M3 · Medium: lock ownership in jobs is unclear.** See X5. `KeeperStore.save` also re-marks the store dirty without holding its lock (`keepers.py:291`). + +**F-M4 · Medium: shutdown behaviour differs per feature.** +- `jobs` joins its threads within `STOP_WAIT`. +- `lxmf` joins with a hard-coded 2.0 s per thread (`lxmf.py:765`). +- `keepers` (`keepers.py:399`) and `follow` (`follow.py:113`) never join their threads, and `follow` doesn't save on `stop()`. +- The background loops of keepers, follow and lxmf are nearly identical. + +*Fix:* a shared `BackgroundLoop` helper (stop event, wake event, join with a time budget). + +**F-M5 · Medium: the CLI progress widgets live in `features/jobs.py`.** `StreamProgress`/`ProgressLine` (`jobs.py:798-856`) are used by core `rnfs cat`. See X3. + +**Low:** +- **Bare state strings:** auto-fetch states such as `"queued"`, `"pinned"` and `"skipped"` are bare strings across `lxmf.py:639,984,1030,1159,1238`. `jobs.py:48` already shows the better pattern. +- **Constructor does real work:** `keepers.sync_own()` imports RNS and signs records (`keepers.py:390,512-519`), which strains the "constructors must be cheap" rule in `features/__init__.py:10`. +- **API handler skips the feature:** `_api_sync` calls `_require_network()` directly (`lxmf.py:1398`), bypassing the feature's own logging. +- **Magic number:** `len(name) == 16` at `follow.py:92`, where `ID_LENGTH` is used elsewhere. +- **Re-stated defaults:** `getattr(config, "max_jobs", None)` re-states defaults that `Config` already guarantees (`jobs.py:283,344`). +- **Docstring contradicts packaging:** the `lxmf.py:25` docstring says LXMF is optional, but `pyproject.toml` requires it. + +## 6. Tests, CI, packaging, docs + +**T-H1 · High: unresolved merge-conflict markers shipped in the 0.1.1 README.** +- *Where:* `README.md:869` (`<<<<<<< HEAD`), `:870` (`=======`), `:895` (`>>>>>>> 3003517 …`). +- *Effect:* the "Tests" and "Known limitations" sections render inside the conflict block on GitHub and PyPI. + +*Fix:* resolve the block, and add the marker check from X6 to CI. + +**T-M1 · Medium: test fakes are copied and imported across test modules.** +- *Copied fakes:* + - `Clock`/`FakeClock` ×5 + - `FakeDestination` ×3 + - `FakeLink`, `FakeNetwork`, `FakeClient`, `FakeNode` ×2 each + - `free_port()` ×4 +- *Cross-imports:* about 17 test modules import fakes from *other test modules*. For example, `test_names_airtime.py:31-34` pulls from four of them. +- *Dead fallback:* each of these imports sits behind a `try/except ImportError` fallback that can never work, because `from tests.support import …` runs unguarded first. + +*Fix:* move the shared fakes into `tests/support.py` or a `tests/fakes/` package, add `tests/__init__.py`, and delete the fallbacks. + +**T-M2 · Medium: the facade fake can drift silently.** +- `tests/test_api.py:24-376` re-implements the `RnfsNode` facade in about 350 lines, and test_cli, test_gateway and test_jobs reuse it. +- Nothing in the suite uses `spec=`/`autospec`, so a change to the real facade won't break the fake. +- Tests also patch many private attributes: + - test_lxmf: `_held` ×8, `_delivered` ×7 + - test_node: `_remote_reserved` ×5 + - elsewhere: `keepers._store`, `follow._due`, `storage._delete` + +*Fix:* a contract test that compares the fake's public method signatures with `RnfsNode` via `inspect`, plus small test hooks for the most-patched internals. + +**T-M3 · Medium: tests synchronise with fixed sleeps.** About 30 `time.sleep` calls stand in for synchronisation (e.g. `test_api.py:747,765,852`, `test_gateway.py:378`, `test_links.py:365,375`, `test_jobs.py:216`). They are likely to flake on loaded macOS and Windows runners; `d0aa468` already had to fix cross-platform failures. *Fix:* move `wait_for()` from `test_e2e.py:74` into `tests/support.py` and use it. + +**T-M4 · Medium: the end-to-end suite is one long test.** `test_e2e.test_network` runs about 220 lines of sequential scenario (`test_e2e.py:247-468`), so an early failure hides every later check. *Fix:* split it into test methods that share a class-level daemon fixture. + +**T-M5 · Medium: releases aren't gated on tests, and the installer installs unreleased code.** +- The `release` job in `.github/workflows/release.yml:215` depends only on the build jobs, so a tag can be released without the test suite having passed. +- `install.sh:35` installs `main.zip` by default, so the README's `curl | sh` one-liner installs unreleased code. +- The packaging checks are copied between `release.yml` and `tests.yml`. + +*Fix:* require the test matrix in `release.yml`, have `install.sh` default to the latest release tag, and move the shared packaging checks into one script. + +**T-M6 · Medium: the README is duplicated and partly stale.** +- **Mismatched versions:** the Quick start links to v0.1.0 (`README.md:55,68`), while the Install table links to v0.1.1. The version appears about 17 times. +- **Broken markup:** headings contain stray `**` (`:70,76,79,88`), and there is a typo, "availible" (`:50`). +- **Duplicate walkthroughs:** two parallel five-step walkthroughs, "Quick start" (`:47`) and "Getting started" (`:386`). +- **Duplicate test docs:** test instructions appear in both the README and `design.md:391`, and the README's e2e command omits `tests.test_client`, which CI runs. + +*Fix:* link to `releases/latest`, merge the walkthroughs, and keep test instructions in one place. + +**Low:** +- **T-L1: five copies of the dependency list.** It lives in `requirements.txt`, `pyproject.toml`, `design.md:498`, the lock header and `update-lock.py`. `__version__` is also parsed four separate ways. Drop `requirements.txt`, or test it like the lock. +- **T-L2: CI misses Python 3.14 and two packages.** The matrix is 3.11–3.13. The arm64 `.deb` and the Flatpak are never actually exercised; the Flatpak step is `continue-on-error`. +- **T-L3: test layout doesn't mirror the package.** + - `features/keepers.py` (775 lines) and `features/follow.py` have no dedicated test files. + - `node_content.py` has no direct tests. + - Several tiny files are split off by feature. +- **T-L4: no contributor docs.** There is no `CONTRIBUTING.md` covering dev setup, running a single test, updating the locks, or the CHANGELOG convention. +- **T-L5: CHANGELOG and companion material lag behind.** + - There's no "Unreleased" entry for `d0aa468`/`22688db`. + - `Planned-companion-applications.md` is only headings. + - `companion-nomadnet-sites/` is undocumented and untested, yet depends on the `pin ls --json` output format. +- **T-L6: stray comment.** `rnfs.py:3` contains `#yo`. + +**Spot-checks that passed:** +- All 37 commands and subcommands in `rnfs --help` match the README command tables. +- Every flag the README mentions exists in the source. +- Spec §12 constants match `rnfs/constants.py` exactly. +- `Config` defaults match the spec settings table. +- `design.md` "Layout" lists every module. + +--- + +## Strengths to preserve + +These are worth protecting during any refactor: + +1. **Pure, clock-injected policy objects.** `ResponsePools`, `IncomingLinks`, `UploadPacer`, the whole of `rnfs/dht/` and `mesh.py` don't import RNS and are easy to unit-test. The DHT has no circular imports and no network dependency. +2. **Strict parsers at every trust boundary.** + - Wire messages: `protocol.py` + - DAG nodes: `dag.decode` re-encodes to check the input is canonical. + - Signed records: `keepers.parse_signed` + - LXMF fields: `lxmf.decode_field`, `parse_uri` + + These reject booleans posing as ints and over-long lists, and do cheap checks before expensive ones. +3. **One error taxonomy.** `errors.py` is mapped to HTTP statuses in one table (`api.error_status`) and shared by the gateway. +4. **Builders take only a `put(cid, block)` callback.** This keeps tree building independent of storage; `NO_PUT` gives hash-only adds. +5. **A clear `Feature` contract.** `features/__init__.py` documents the plugin lifecycle; start and stop failures are isolated per feature, and duplicate routes are rejected. +6. **Docs that are accurate.** `specification.md` matches the code on every constant and format checked, and docstrings explain *why*, citing spec sections and the attacks they defend against. +7. **Supply-chain and platform discipline.** + - Hash-locked runtime and build dependencies come from a single generator. + - Actions are pinned to SHAs, and builds run without build isolation. + - CI runs a 3 OS × 3 Python matrix with real package smoke tests. + - `test_packaging.py` keeps the locks and workflow in sync. +8. **Careful persistence.** Damaged state files are moved aside, and a corrupt `names.json` is refused, which prevents rollback attacks. + +--- + +## Suggested roadmap + +### Quick wins (≤ 1 day total, low risk) +1. Resolve the README conflict block, fix the v0.1.0 links and stray `**`, and remove `#yo` (T-H1, T-M6, T-L6). +2. Add a CI `lint` job: `ruff check` plus the conflict-marker grep (X6). Fix or `noqa` what it finds, and delete the dead code it reports. +3. Add upper bounds to `rns` and `lxmf` in `pyproject.toml` and regenerate the locks (X1). +4. Make the `release` job require the test matrix (T-M5). +5. Fix the `seal_name` parsing in `node_content.add_options` (C-M1). + +### Consolidation (a few days, mechanical) +6. `fsutil.write_atomic` replaces the seven atomic-write copies; `jsonstate.py` moves out of `content/storage.py` (X2, N-M2). +7. `rnfs/ids.py` for ID parsing; `features/_common.py` for `human_size`/`_printable`/`_log`/`_feature` (X2). +8. `tests/fakes/` for the shared test doubles, plus a facade contract test (T-M1, T-M2). +9. `rns_compat.py`, which centralises every RNS/LXMF patch and check and is called once at startup with an attribute self-test (X1). + +### Structural (plan as separate pieces of work) +10. Split `api.py` into `httpserver.py`, `auth.py` and the routes, removing `api → cli` and `gateway → api` (X3). +11. Add `Feature` hooks for settings, validation, target expansion and pin-source registration, then remove feature names from `config.py`, `api.py`, `node.py` and `storage.py` (X3, F-M1). +12. Break up `features/lxmf.py` into a package; extract `RemotePinQueue` from `RnfsNode`, `ResponseTracker` from `RequestServer`, and a provider scheduler from `Fetch` (X4). +13. Write down lock ownership and ordering in `design.md`, then move slow work out from under the locks (X5). +14. Have one path-walk generator in `walker` used by all four resolvers (N-H1). diff --git a/companion-nomadnet-sites/display-mu-file/index.mu b/companion-nomadnet-sites/display-mu-file/index.mu new file mode 100755 index 0000000..6cd60ad --- /dev/null +++ b/companion-nomadnet-sites/display-mu-file/index.mu @@ -0,0 +1,100 @@ +#!/usr/bin/env python3 +"""NomadNet page that shows an RNFS file: pins it on this node and prints its content (e.g. a Micron page). + +Link to it with the CID as a link variable, which NomadNet passes in the environment as var_cid: +`[Example`:/page/display-mu-file.mu`cid=bafkreiailvfecuuvt6tctx3owax2jic7y5af2ndriwo6xymcq2uwyut34a] + + +`B333`!Find out more about 'Reticulum Network File System' at the project's Github:`!`b +`B333`F08f`_`[https://github.com/afit21/rnfs`https://github.com/afit21/rnfs]`_`f`b + +""" +import os +import re +import shutil +import subprocess + +# Larger files aren't pinned or shown. +MAX_SIZE = 200 * 1024 + +# How long to wait for each rnfs command; a fetch over a slow link can take a while. +TIMEOUT = 120 + +# NomadNet may run this without ~/.local/bin on PATH. +RNFS = shutil.which("rnfs") or os.path.expanduser("~/.local/bin/rnfs") + +# A bare CIDv1 in base32 (rnfs's form), so nothing else (a path, a nick, an option) reaches rnfs. +CID = re.compile(r"b[a-z2-7]{20,200}") + + +def escape(text): + """Escape Micron's formatting characters so text shows literally.""" + return text.replace("\\", "\\\\").replace("`", "\\`") + + +def rnfs(*args): + """Run an rnfs command; its result (returncode, stdout bytes, stderr text).""" + result = subprocess.run([RNFS, *args], capture_output=True, timeout=TIMEOUT) + return result.returncode, result.stdout, result.stderr.decode("utf-8", "replace").strip() + + +def fail(message, micron=""): + """Show message (escaped, so it shows literally) and then micron (as markup) instead of the file, and stop.""" + print("#!c=0") + print(escape(message)) + if micron: + print(micron) + raise SystemExit + + +def size_of(cid): + """The file's size in bytes from rnfs stat, or None if it isn't known.""" + code, out, err = rnfs("stat", cid) + if code != 0: + fail(f"Couldn't find {cid} ({err or 'rnfs stat failed'}).") + for line in out.decode("utf-8", "replace").splitlines(): + name, _, value = line.partition(": ") + if name == "size": + return int(value) if value.isdigit() else None + return None + + +def is_directory(cid): + """True if the CID is a directory (rnfs ls lists it; for a file it fails with "is not a directory").""" + code, _, err = rnfs("ls", cid) + if code != 0 and "is not a directory" not in err: + fail(f"Couldn't read {cid} ({err or 'rnfs ls failed'}).") + return code == 0 + + +cid = os.environ.get("var_cid", "").strip() +if not CID.fullmatch(cid): + fail("No valid RNFS CID was given. Link to this page with `cid=.", + "Example: `_`[Example Link`:/page/index.mu`cid=bafkreiailvfecuuvt6tctx3owax2jic7y5af2ndriwo6xymcq2uwyut34a]`_") + +try: + size = size_of(cid) + if size is None: + fail(f"The size of {cid} isn't known, so it isn't shown.") + if size > MAX_SIZE: + fail(f"{cid} is {size:,} bytes; only files up to {MAX_SIZE // 1024} KB are shown.") + if is_directory(cid): + fail(f"{cid} is a folder, not a file.") + + code, _, err = rnfs("pin", "add", cid) + if code != 0: + fail(f"Couldn't pin {cid} ({err or 'rnfs pin add failed'}).") + + code, content, err = rnfs("cat", cid) + if code != 0: + fail(f"Couldn't read {cid} ({err or 'rnfs cat failed'}).") +except subprocess.TimeoutExpired: + fail(f"Fetching {cid} took too long; try again later.") +except OSError as error: + fail(f"RNFS isn't available on this node ({error}).") + +try: + text = content.decode("utf-8") +except UnicodeDecodeError: + fail(f"{cid} isn't a text file, so it can't be shown here.") +print(text, end="" if text.endswith("\n") else "\n") diff --git a/design.md b/design.md index 15c43a8..5ee46a3 100644 --- a/design.md +++ b/design.md @@ -46,7 +46,7 @@ rnfs.py, rnfs/__main__.py entry points (python3 rnfs.py, python3 -m rnfs); the rnfs/ constants.py config.py errors.py log.py shared foundation fsutil.py cross-platform file operations: replace/remove retried on Windows, O_BINARY, IS_WINDOWS - content/ no RNS dependency + content/ no networking; uses RNS's bundled msgpack and RNS logging, so importing it loads RNS cid.py CIDs (section 4) dag.py DAG node encoding, strict decoding, names (section 5) chunker.py fixed and FastCDC chunkers; chunker.using(name) picks one per context (section 5) @@ -89,7 +89,8 @@ rnfs/ follow.py followed names (section 9) contacts.py address book (section 11) lxmf.py rnfs: URIs, LXMF send/inbox/outbox/auto-fetch (section 13) -install.sh Linux installer: venv, command, optional systemd user unit; --update, --uninstall +install.sh Linux installer of the latest release (or --main): venv, command, optional systemd user + unit; --update, --uninstall contrib/ rnfs.service (systemd user), io.github.afit21.rnfs.plist (launchd), install-windows-task.ps1 packaging/ requirements.lock (hash-pinned runtime dependencies), update-lock.py (regenerates it), build-deb.sh, flatpak/ (manifest, wheels generated from the lock), pyinstaller/ (one-file exe), @@ -204,7 +205,7 @@ out within `link_idle_timeout`. | `dir_set(target, name, child_cid, executable=False)`, `dir_rm(target)` | `{"cid"}`, pinned `user` | | `pin_add(target)` | `{"cid"}` once complete | | `pin_rm(cid)` | `None`; NotFound if no `user` source | -| `pin_ls(cid=None)` | `[{"cid", "sources": [str], "complete": bool}]` | +| `pin_ls(cid=None)` | `[{"cid", "sources": [str], "complete": bool, "type": "file" \| "directory" \| None, "name": str \| None}]`; `type` and `name` only when the root block is stored, `name` where the pin was added from (pin_names), its shared-folder entry, or a directory's single entry or entry count | | `pin_remote(node_id, cid, pin)` | `bool` accepted | | `gc()` / `verify()` | `{"removed_blocks", "freed_bytes"}` / `{"checked", "bad": [cid]}` | | `providers(cid)` | `[node_id bytes]` | @@ -371,6 +372,20 @@ name in the output's directory; the CLI sets it for `rnfs get ` withou The API and gateway run one thread per connection, capped (32 and 64). +Locks are taken in this order, never the reverse: `names._lock` → `publisher._lock` → `node._lock` → +`storage._lock`. `NameManager.publish`, `remove` and `release_gone` pin and unpin, and so call +`publisher.refresh()`, while holding the names lock; `Publisher.refresh` recomputes what is provided under its +own lock, reading the node's fetched targets (`node._lock`) and walking the pins' DAGs (`storage._lock`); the +node reads storage under its lock (`_prune_fetched`). This is how the code behaves today, not a design goal. +Slow work that runs while a lock is held: + +- `Publisher.refresh` walks every pin's DAG (memoised) under the publisher lock, behind the names lock when a + name changes, so an RNS thread delivering a name announce can wait behind a full recompute. +- `Storage.evict`, `gc` and `verify` check every pin and cached target for completeness (`dag_blocks`, + memoised, reading blocks from disk) under the storage lock; `evict` and `gc` also delete blocks under it. + `verify` re-reads the blocks themselves outside it. +- `RnfsNode._note_fetched` and `_prune_fetched` write `fetched.json`, synced to disk, under the node lock. + ## State files Under `/state/`, JSON written atomically from a temp file synced to disk: `pins.json`, `cache.json` @@ -390,12 +405,11 @@ and a resumed job removes one left by a crash. ## Tests -`python3 -m unittest discover tests` runs unit tests with no network. `RNFS_E2E=1 python3 -m unittest tests.test_e2e` -starts real daemons over local TCP. Debugging: `kill -USR1 ` prints every thread's stack. -Feature tests live in their own files (`test_jobs.py`, `test_mesh.py`, `test_names_keep.py`, `test_contacts.py`, -`test_seal.py`, `test_chunker.py`, `test_pin_false.py`, `test_gateway_formats.py`, `test_lxmf.py`), and -`test_e2e.py` has a marked block per feature. CI (`.github/workflows/tests.yml`) runs them on Linux, macOS and -Windows; `packaging/smoke_test.py` checks a built package (`--version`, `--help`, daemon and status). +How to run the tests is in [README.md, Tests](README.md#tests). Feature tests live in their own files +(`test_jobs.py`, `test_mesh.py`, `test_names_keep.py`, `test_contacts.py`, `test_seal.py`, `test_chunker.py`, +`test_pin_false.py`, `test_gateway_formats.py`, `test_lxmf.py`), and `test_e2e.py` has a marked block per +feature. CI (`.github/workflows/tests.yml`) runs them on Linux, macOS and Windows; `packaging/smoke_test.py` +checks a built package (`--version`, `--help`, daemon and status). ## Status @@ -492,11 +506,12 @@ Everything in the specification is implemented, with these simplifications and a propagation node is used unless configured (or asked for with `--nearest`). - **Packaging.** Release artifacts (`.deb` for amd64/arm64, x86_64 Flatpak, Windows x64 exe, wheel and sdist) are built by `.github/workflows/release.yml`. Only a pushed `vX.Y.Z` tag matching `rnfs/__init__.py`'s - `__version__` publishes them, with `SHA256SUMS` and signed build provenance (`gh attestation verify`), never - replacing files already attached; a manual run only builds. The Windows exe isn't code-signed, and unpacks - itself on each start. - - `lxmf>=1.2.0` is a regular dependency next to `rns>=1.5.6` (`pyproject.toml`, `requirements.txt`), so - every install includes LXMF; the PyInstaller spec fails the build without it. + `__version__` publishes them, once the whole of `tests.yml` (which release.yml calls) has passed, with + `SHA256SUMS` and signed build provenance (`gh attestation verify`), never replacing files already attached; + a manual run only builds. The Windows exe isn't code-signed, and unpacks itself on each start. + - `lxmf>=1.2.0,<1.3` is a regular dependency next to `rns>=1.5.6,<1.6` (`pyproject.toml`), so every + install includes LXMF; the PyInstaller spec fails the build without it. The upper bounds keep unpinned + installs off RNS and LXMF releases rnfs hasn't been tested with, since it relies on some of their internals. - `packaging/requirements.lock` pins the tested versions (rns 1.5.6, lxmf 1.2.0, cryptography 50.0.2, cffi 2.1.1, pycparser 3.0, pyserial 3.5) with the sha256 of every file PyPI has for each. `python3 -I packaging/update-lock.py [pkg==ver …]` regenerates it (no arguments: the newest versions @@ -506,12 +521,16 @@ Everything in the specification is implemented, with these simplifications and a release's flatpak-sources step fails if regenerating changes the committed files. The sdist includes the lock. - `update-lock.py --build pyinstaller==` writes `build-backend.lock` (setuptools, within - `pyproject.toml`'s `<85` bound) and `build-tools.lock` (build, twine, PyInstaller, setuptools and their - dependencies), hashed the same way. Every build installs these with `--require-hashes` and builds without - build isolation, so nothing unpinned is fetched for a build; the Flatpak's `build-sources.json` comes from + `pyproject.toml`'s `<85` bound) and `build-tools.lock` (build, twine, PyInstaller, setuptools, ruff and + their dependencies), hashed the same way. Every build installs these with `--require-hashes` and builds + without build isolation, so nothing unpinned is fetched for a build; the Flatpak's `build-sources.json` comes from `build-backend.lock`. `build-deb.sh` fails if a supported Python has no cffi backend, caps `python3` below 3.15, and writes a DEP-5 `copyright` and `md5sums`. - - `install.sh` unpacks an archive source itself (with Python, refusing absolute or `..` entries) to find its + - `install.sh` installs the latest release's source archive (`archive/refs/tags/.zip`, the tag taken + from where `releases/latest` redirects, so no GitHub API), the main branch's with `--main`, or + `RNFS_SOURCE`. Since the script on `main` installs older release trees, it must keep working with every + release's layout from 0.1.1 on (`packaging/requirements.lock`, `packaging/build-backend.lock`). + It unpacks an archive source itself (with Python, refusing absolute or `..` entries) to find its locks; a `.whl` or other requirement has none, so it warns and installs dependencies unpinned. It installs wheels only (`RNFS_ALLOW_SOURCE_BUILDS=1` builds missing ones from hashed sources with unpinned build tools), builds rnfs in a separate venv with the locked setuptools, sets `umask 022` in system mode, and diff --git a/install.sh b/install.sh index 1657449..11ad805 100755 --- a/install.sh +++ b/install.sh @@ -1,23 +1,26 @@ #!/bin/sh -# Install, update or uninstall rnfs from the main branch on GitHub, in its own Python virtual environment. +# Install, update or uninstall rnfs's latest release (or its main branch) from GitHub, in its own Python +# virtual environment. # # curl -fsSL https://raw.githubusercontent.com/afit21/rnfs/main/install.sh | sh # curl -fsSL https://raw.githubusercontent.com/afit21/rnfs/main/install.sh | sh -s -- --service -# sh install.sh [--system] [--service] [--update | --uninstall] +# sh install.sh [--system] [--service] [--main] [--update | --uninstall] # # Per user (the default): venv ~/.local/share/rnfs/venv, command ~/.local/bin/rnfs # --system (or as root): venv /opt/rnfs/venv, command /usr/local/bin/rnfs # # --service also install the systemd user unit and start it (system mode: install it for all users, # each then runs `systemctl --user enable --now rnfs`) -# --update re-install from the latest main branch (running the installer again does the same) +# --main install the main branch, which may be newer than the latest release, instead of the release +# --update re-install from the latest release, or the main branch with --main (running the installer +# again does the same) # --uninstall remove the venv, the command and the unit (keeps your data in ~/.rnfs) # --python P use this Python (3.11 or newer) for the venv # --force replace an existing rnfs command or systemd unit that this installer didn't make # (e.g. a pipx one) # -# Environment: RNFS_SOURCE what to install instead of the main branch: a local source directory, a -# .zip/.tar.gz/.whl path or URL, or another pip requirement +# Environment: RNFS_SOURCE what to install instead of the latest release (overrides --main too): a local +# source directory, a .zip/.tar.gz/.whl path or URL, or another pip requirement # RNFS_PYTHON same as --python # RNFS_ALLOW_SOURCE_BUILDS=1 build dependencies PyPI has no wheel of for this platform (e.g. # cffi on 32-bit ARM) from their pinned, hash-checked source; the tools that build @@ -32,8 +35,12 @@ # are never touched. set -eu -DEFAULT_SOURCE="https://github.com/afit21/rnfs/archive/refs/heads/main.zip" -SOURCE=${RNFS_SOURCE:-$DEFAULT_SOURCE} +REPO_URL="https://github.com/afit21/rnfs" +# Without RNFS_SOURCE, this script (as served from the main branch) installs the latest release's source tree, +# which may be older than the script. It must keep working with the layout of every release from 0.1.1 on: +# packaging/requirements.lock and packaging/build-backend.lock, which fetch_source looks for. +SOURCE=${RNFS_SOURCE:-} # empty: chosen by choose_source once Python is found +MAIN=no PYTHON=${RNFS_PYTHON:-} PYTHON_GIVEN=no MODE=user @@ -50,6 +57,7 @@ while [ $# -gt 0 ]; do --system) MODE=system ;; --service) SERVICE=yes ;; --update) ACTION=install ;; + --main) MAIN=yes ;; --uninstall) ACTION=uninstall ;; --force) FORCE=yes ;; --python) [ $# -ge 2 ] || die "--python needs a path"; PYTHON=$2; shift ;; @@ -253,6 +261,37 @@ WantedBy=default.target EOF } +latest_release() { # the tag of the latest release on GitHub: where releases/latest redirects (no API, no rate limit) + "$PYTHON" -I - "$REPO_URL" <<'EOF' +import re, sys, urllib.parse, urllib.request + +repo = sys.argv[1] +try: + with urllib.request.urlopen(repo + "/releases/latest", timeout=60) as response: + final = response.geturl() +except OSError as e: + sys.exit(f"couldn't reach {repo}/releases/latest: {e}") +prefix = repo + "/releases/tag/" +tag = urllib.parse.unquote(final[len(prefix):]) if final.startswith(prefix) else "" +if not re.fullmatch(r"[A-Za-z0-9][A-Za-z0-9._+-]*", tag): + sys.exit(f"{repo}/releases/latest doesn't lead to a release (it led to {final})") +print(tag) +EOF +} + +choose_source() { # without RNFS_SOURCE: the latest release's source archive, or the main branch's with --main + [ -z "$SOURCE" ] || return 0 + if [ "$MAIN" = yes ]; then + SOURCE=$REPO_URL/archive/refs/heads/main.zip + say "Installing the main branch, which may be newer than the latest release" + else + tag=$(latest_release) || die "couldn't find rnfs's latest release on GitHub. Check your connection, or install +the main branch instead with --main." + SOURCE=$REPO_URL/archive/refs/tags/$tag.zip + say "Installing rnfs $tag, the latest release" + fi +} + # The source: a local tree, or an archive downloaded and unpacked here (by Python, so no curl/unzip needed), # so its packaging/requirements.lock and packaging/build-backend.lock can be used. A wheel or other pip requirement goes to pip as it is. @@ -374,6 +413,7 @@ do_install() { find_python check_launcher check_unit + choose_source updating=no if venv_ok; then say "Updating rnfs in $VENV" @@ -391,7 +431,8 @@ do_install() { # pip isn't upgraded from PyPI (that would be an unpinned download); the venv's own pip does --require-hashes # Build the wheel once, then install it: the dependencies at the versions the source's lock pins, and rnfs - # itself with a forced reinstall, since the main branch can change without its version number changing + # itself with a forced reinstall, since the main branch (or a local source) can change without its version + # number changing wheels=$(mktemp -d "${TMPDIR:-/tmp}/rnfs-install.XXXXXX") trap 'rm -rf "$wheels"' EXIT INT TERM say "Fetching and building rnfs from $SOURCE" diff --git a/packaging/build-tools.lock b/packaging/build-tools.lock index a6afc2e..c379c6c 100644 --- a/packaging/build-tools.lock +++ b/packaging/build-tools.lock @@ -1,5 +1,5 @@ -# The tools the workflows build and check releases with (build, twine, PyInstaller, setuptools) and their -# dependencies, for CPython 3.11, 3.12, 3.13 on Linux x86_64 and Windows x64, with the sha256 of every +# The tools the workflows build, check and lint with (build, twine, PyInstaller, setuptools, ruff) and their +# dependencies, for CPython 3.11, 3.12, 3.13, 3.14 on Linux x86_64 and Windows x64, with the sha256 of every # file PyPI has for each. Generated by packaging/update-lock.py; don't edit by hand: # python3 -I packaging/update-lock.py --build pyinstaller==6.22.3 # Install with: pip install --require-hashes --only-binary=:all: -r packaging/build-tools.lock @@ -465,6 +465,25 @@ rfc3986==2.0.0 \ rich==15.0.0 \ --hash=sha256:33bd4ef74232fb73fe9279a257718407f169c09b78a87ad3d296f548e27de0bb \ --hash=sha256:edd07a4824c6b40189fb7ac9bc4c52536e9780fbbfbddf6f1e2502c31b068c36 +ruff==0.17.0 \ + --hash=sha256:0e271826af9a20d18c6cfae8c51e82959167c24859686ddd3eb9a7f0842ce81e \ + --hash=sha256:13ee90156522998c3037059d8f66885c8adeeaf7643bdce2caceee196ecd23e0 \ + --hash=sha256:3d8cc360e666d1914e47b0777c6906d70cf18891a55532bd0a16844195d70859 \ + --hash=sha256:3d8e4a002a94cd9d0dc48b51dc69d807a172b5b9bf2b668e656424dc5b55ead1 \ + --hash=sha256:5cd03240d8208a557c2a9655a5cb07ebe36aa6bb35065f97d48c1f6adef5a322 \ + --hash=sha256:5e50aa5b84decd9fe5b0bb0e6f71c3b592f1767ed09faa4b7207d933961e35cd \ + --hash=sha256:5f0ca4a40f81403689c04f12966e22f44e329ae362072d8f1587b7bda87f603b \ + --hash=sha256:7bb08489e234876fa2da67ae3ea938e9a2156da80293e0e4365abd6973d98329 \ + --hash=sha256:864b6c1acb6b0bccf94b5a3938a1531fd09aaca5e5659a2e7bf0f3cf2a685540 \ + --hash=sha256:8ab76bcda86dfd28e13776cb5de3c7bcdcf1ae3d37ed761113d1a5a415dc134c \ + --hash=sha256:a330178bdffc4205dbf3bda11d93e059e388fd6546f8cdd304501a9160363c0d \ + --hash=sha256:bc73e7c133e82d55b5f15897b2a442d72c0cb4a0c886c46801ce3c247150b60c \ + --hash=sha256:c0b8a60c06a218c337e1161638d34757f83449243e2db161483ddf948e53ad14 \ + --hash=sha256:c154c73ff43f9854395e24cac507af13078962e53d2b511605058d22af1fdb88 \ + --hash=sha256:c3f268baf004aea944f040623327119527ea231af15f7fb7890e82cea0679589 \ + --hash=sha256:cbf7149e0927dc3295d5d64679a4765576eef71b00782b2ae969ef82274d6bb9 \ + --hash=sha256:d66de796b726c4801e05fa99a2a8d7a780e107be222486c304ab61765561e866 \ + --hash=sha256:db4f74c533403ab70fe4007873f6ae0c9f94a8b03158cf48d78788e47cdbe399 secretstorage==3.5.0 ; sys_platform == "linux" \ --hash=sha256:0ce65888c0725fcb2c5bc0fdb8e5438eece02c523557ea40ce0703c266248137 \ --hash=sha256:f04b8e4689cbce351744d5537bf6b1329c6fc68f91fa666f60a380edddcd11be diff --git a/packaging/flatpak/python3-deps.json b/packaging/flatpak/python3-deps.json index c951440..74a59ee 100644 --- a/packaging/flatpak/python3-deps.json +++ b/packaging/flatpak/python3-deps.json @@ -2,7 +2,7 @@ "name": "python3-deps", "buildsystem": "simple", "build-commands": [ - "pip3 install --verbose --no-index --find-links=\"file://${PWD}\" --prefix=${FLATPAK_DEST} --no-build-isolation \"rns>=1.5.6\" \"lxmf>=1.2.0\"" + "pip3 install --verbose --no-index --find-links=\"file://${PWD}\" --prefix=${FLATPAK_DEST} --no-build-isolation \"rns>=1.5.6,<1.6\" \"lxmf>=1.2.0,<1.3\"" ], "sources": [ { diff --git a/packaging/update-lock.py b/packaging/update-lock.py index 7d73063..37bcc23 100644 --- a/packaging/update-lock.py +++ b/packaging/update-lock.py @@ -14,8 +14,8 @@ build-backend.lock just the build backend (setuptools), for `pip wheel --no-build-isolation` in install.sh, build-deb.sh and the tests. Its version must satisfy pyproject.toml's [build-system] requires (this script checks); raise that bound when updating it. - build-tools.lock what the workflows build and check releases with: build, twine, PyInstaller, setuptools - and their dependencies, for Linux and Windows (platform-only dependencies carry a + build-tools.lock what the workflows build, check and lint with: build, twine, PyInstaller, setuptools, + ruff and their dependencies, for Linux and Windows (platform-only dependencies carry a sys_platform marker). Packages also in requirements.lock are pinned to its versions, so both locks can go into one environment (the PyInstaller one). @@ -51,13 +51,13 @@ # The build tools, resolved on this machine (pip evaluates environment markers for the running Python, so run # it on Linux with Python 3.12, the release workflow's), then completed with the dependencies other # platforms' markers select. Every file must have a wheel for each of these environments. -BUILD_TOOLS = ["setuptools", "build", "twine", "pyinstaller"] +BUILD_TOOLS = ["setuptools", "build", "twine", "pyinstaller", "ruff"] BUILD_PLATFORMS = { # sys_platform: (marker values, wheel platform tags accepted) "linux": ({"os_name": "posix", "platform_system": "Linux", "platform_machine": "x86_64"}, ("manylinux", "linux_x86_64")), "win32": ({"os_name": "nt", "platform_system": "Windows", "platform_machine": "AMD64"}, ("win_amd64",)), } -BUILD_PYTHONS = ["3.11", "3.12", "3.13"] +BUILD_PYTHONS = ["3.11", "3.12", "3.13", "3.14"] BUILD_RESOLVE_ARGS = ["--only-binary=:all:", "--implementation", "cp", "--python-version", "3.12", "--platform", "manylinux_2_28_x86_64", "--platform", "manylinux_2_17_x86_64", "--platform", "manylinux2014_x86_64"] @@ -69,7 +69,7 @@ # then build with: pip wheel --no-build-isolation --no-deps . """ TOOLS_HEADER = """\ -# The tools the workflows build and check releases with (build, twine, PyInstaller, setuptools) and their +# The tools the workflows build, check and lint with (build, twine, PyInstaller, setuptools, ruff) and their # dependencies, for CPython {pythons} on Linux x86_64 and Windows x64, with the sha256 of every # file PyPI has for each. Generated by packaging/update-lock.py; don't edit by hand: # python3 -I packaging/update-lock.py --build {pins} diff --git a/pyproject.toml b/pyproject.toml index 1491929..4771e88 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,7 +10,7 @@ dynamic = ["version"] description = "Content-addressed file sharing over Reticulum, with the functional features of IPFS" readme = "README.md" requires-python = ">=3.11" -dependencies = ["rns>=1.5.6", "lxmf>=1.2.0"] +dependencies = ["rns>=1.5.6,<1.6", "lxmf>=1.2.0,<1.3"] keywords = ["reticulum", "rns", "ipfs", "mesh", "lora", "content-addressed", "file-sharing"] classifiers = [ "Programming Language :: Python :: 3", @@ -32,3 +32,10 @@ include = ["rnfs*"] [tool.setuptools.dynamic] version = {attr = "rnfs.__init__.__version__"} # not "rnfs": setuptools would find rnfs.py + +[tool.ruff] +target-version = "py311" +extend-exclude = ["packaging/flatpak"] + +[tool.ruff.lint] +select = ["F", "E9", "B006", "B008", "B904"] # pyflakes, syntax errors, mutable/call defaults, raise-from diff --git a/requirements.txt b/requirements.txt deleted file mode 100644 index a3cc8c5..0000000 --- a/requirements.txt +++ /dev/null @@ -1,2 +0,0 @@ -rns>=1.5.6 -lxmf>=1.2.0 diff --git a/rnfs.py b/rnfs.py index 2406d1b..52d4f36 100644 --- a/rnfs.py +++ b/rnfs.py @@ -1,5 +1,5 @@ """Run rnfs from a source checkout: `python3 rnfs.py `, the same as `python3 -m rnfs`.""" from rnfs.cli import main -#yo + if __name__ == "__main__": main() diff --git a/rnfs/__init__.py b/rnfs/__init__.py index 095215c..9173832 100644 --- a/rnfs/__init__.py +++ b/rnfs/__init__.py @@ -11,4 +11,4 @@ cli.py command line: run a daemon, or talk to one through its API """ -__version__ = "0.1.1" +__version__ = "0.1.2" diff --git a/rnfs/api.py b/rnfs/api.py index 87b2741..6930e9a 100644 --- a/rnfs/api.py +++ b/rnfs/api.py @@ -16,8 +16,8 @@ | GET | /api/ping | `node` | `{"rtt": seconds or null}` | | GET | /api/name/list | | `{"names": name_list()}` | | GET | /api/name/resolve | `name` | `name_resolve(name)` | -| POST | /api/add | JSON `path`, `wrap`, `only_hash`, `pin`, `to` (list), `to_self`, `seal_name`, `chunker` | `add_path(...)` | -| POST | /api/add/stream | query `name`, `only_hash`, `pin`, `to` (comma-separated), `to_self`, `seal_name`, `chunker`; body is the data | `add_stream(...)` | +| POST | /api/add | JSON `path`, `wrap`, `only_hash`, `pin`, `to` (list), `to_self`, `seal_name` (bool), `chunker` | `add_path(...)` | +| POST | /api/add/stream | query `name`, `only_hash`, `pin`, `to` (comma-separated), `to_self`, `seal_name` (the name), `chunker`; body is the data | `add_stream(...)` | | POST | /api/get | JSON `target`, `output`, `from`, `raw` | `get(...)` | | POST | /api/dir/set | JSON `target`, `name`, `cid`, `executable` | `dir_set(...)` | | POST | /api/dir/rm | JSON `target` | `dir_rm(target)` | @@ -831,7 +831,7 @@ def api_add_stream(self): size = None if reader.chunked else reader.remaining name = self.param("name", False) or None only_hash = parse_bool(self.param("only_hash", False) or "") - options = node_content.add_options(lambda key: self.param(key, False)) # pin, to, seal_name, chunker… + options = node_content.add_options(lambda key: self.param(key, False), stream=True) # pin, to, seal_name… self.send_json(200, self.node.add_stream(reader, size=size, name=name, only_hash=only_hash, **options)) def api_get(self): diff --git a/rnfs/cli.py b/rnfs/cli.py index 6efe276..4bab375 100644 --- a/rnfs/cli.py +++ b/rnfs/cli.py @@ -10,7 +10,6 @@ any local user could listen there. """ import argparse -import contextlib import errno import hashlib import hmac @@ -28,7 +27,6 @@ DEFAULT_DIR = Config.rnfs_dir COPY_SIZE = 64 * 1024 TOKEN_FILE = "api_token" # in the rnfs directory; written by the daemon, read by every other command -NOFOLLOW = getattr(os, "O_NOFOLLOW", 0) # Windows lacks it # Before sending the token, the CLI makes whatever listens on the API port prove it knows it (api_proof) CHALLENGE_HEADER = "X-Rnfs-Challenge" PROOF_HEADER = "X-Rnfs-Proof" @@ -55,18 +53,10 @@ def token_path(rnfs_dir: str) -> str: def write_token(path: str, token: str): """Write the API token to path atomically, created with mode 0600 so other users can't read it.""" - # A new file (O_EXCL) in the same directory, then a rename: the token is never readable with wider - # permissions, nor half-written, and an existing file or symlink at path is replaced, not written through - temp = os.path.join(os.path.dirname(path), f".{TOKEN_FILE}.{os.getpid()}.{secrets.token_hex(4)}") - fd = os.open(temp, os.O_WRONLY | os.O_CREAT | os.O_EXCL | NOFOLLOW | fsutil.O_BINARY, 0o600) - try: - with os.fdopen(fd, "w", encoding="ascii") as f: - f.write(token + "\n") - fsutil.replace(temp, path) # retried on Windows while a client command is reading the old token - except BaseException: - with contextlib.suppress(OSError): - fsutil.remove(temp) - raise + # A new file (O_EXCL, O_NOFOLLOW) in the same directory, then a rename: the token is never readable with + # wider permissions, nor half-written, and an existing file or symlink at path is replaced, not written + # through. Ended with os.linesep, as the text-mode write before 0.1.2 did, so the file is unchanged + fsutil.write_atomic(path, (token + os.linesep).encode("ascii"), mode=0o600) def remove_token(path: str, token: str): @@ -89,7 +79,7 @@ def api_proof(token: str, port: int, challenge: str) -> str: def read_token(path: str) -> str: """The daemon's API token from path; exits with an explanation if it can't be used.""" try: - fd = os.open(path, os.O_RDONLY | NOFOLLOW | fsutil.O_BINARY) + fd = os.open(path, os.O_RDONLY | fsutil.O_NOFOLLOW | fsutil.O_BINARY) except FileNotFoundError: fail(f"no API token at {path}: is the daemon running, with this --dir?") except PermissionError: diff --git a/rnfs/constants.py b/rnfs/constants.py index 1272d29..02564e5 100644 --- a/rnfs/constants.py +++ b/rnfs/constants.py @@ -23,7 +23,9 @@ ID_LENGTH = 16 # node IDs and DHT keys: RNS destination hashes, 128 bits BUCKET_COUNT = 128 RESPONSE_OVERHEAD = 1 * KIB # room left for msgpack framing in a /blocks response (section 7) -REQUEST_FRAMING = 28 # approximate bytes RNS adds around a request's data (section 3) + +# Local, not protocol: a file being written out (.rnfs-get-.partial, .rnfs-job-.partial), then renamed +PARTIAL_SUFFIX = ".partial" APP_NAME = "rnfs" DHT_ASPECT = "dht" # rnfs.dht: every node's request destination. A breaking change uses a new aspect diff --git a/rnfs/content/blockstore.py b/rnfs/content/blockstore.py index 590f627..3f37261 100644 --- a/rnfs/content/blockstore.py +++ b/rnfs/content/blockstore.py @@ -18,7 +18,7 @@ from rnfs.constants import MAX_BLOCK_SIZE from rnfs.content.cid import Cid -PARTIAL_PREFIX = ".partial-" +PARTIAL_PREFIX = fsutil.PARTIAL_PREFIX # also the prefix of every other atomic write's temporary file class BlockStore: diff --git a/rnfs/content/builder.py b/rnfs/content/builder.py index 27a057d..0afff0c 100644 --- a/rnfs/content/builder.py +++ b/rnfs/content/builder.py @@ -111,19 +111,6 @@ def _put_node(node: dag.Node, put: Put) -> Cid: return cid -def _read_full(stream: BinaryIO, want: int) -> bytes: - """Up to want bytes from stream, fewer only at end of stream (pipes and sockets return short reads).""" - parts = [] - got = 0 - while got < want: - data = stream.read(want - got) - if not data: - break - parts.append(data) - got += len(data) - return b"".join(parts) - - def build_file(stream: BinaryIO, put: Put, size: int | None = None, kind: int = dag.KIND_FILE) -> Built: """Build a file from everything read from stream, or exactly size bytes of it (ValueError if it ends early).""" diff --git a/rnfs/content/storage.py b/rnfs/content/storage.py index d023b24..38a41c9 100644 --- a/rnfs/content/storage.py +++ b/rnfs/content/storage.py @@ -19,7 +19,6 @@ import os import re import shutil -import tempfile import threading from collections import OrderedDict from contextlib import contextmanager @@ -78,23 +77,12 @@ def remote_source(identity_hash: bytes | str) -> str: return REMOTE_PREFIX + text -def write_json(path: str, value): - """Write value as JSON to path atomically and durably (temporary file synced to disk, then renamed).""" - directory = os.path.dirname(path) - fd, partial = tempfile.mkstemp(prefix=".partial-", dir=directory) - try: - with os.fdopen(fd, "w", encoding="utf-8") as f: - json.dump(value, f) - f.flush() - os.fsync(f.fileno()) # else a power loss can leave path empty once renamed - fsutil.replace(partial, path) # retried on Windows while read_json has path open - except BaseException: - try: - fsutil.remove(partial) - except OSError: - pass - raise - fsutil.sync_directory(directory or ".") +def write_json(path: str, value, ensure_ascii: bool = True): + """Write value as JSON to path atomically and durably (temporary file synced to disk, then renamed), readable + by its owner only. With ensure_ascii False, non-ASCII characters are written as UTF-8 rather than escaped.""" + # backslashreplace: a lone surrogate, which ensure_ascii False leaves as is, is written as its JSON escape + data = json.dumps(value, ensure_ascii=ensure_ascii).encode("utf-8", "backslashreplace") + fsutil.write_atomic(path, data, mode=0o600) # the rename is retried on Windows while read_json has path open def read_json(path: str, shape: Callable[[object], bool] | None = None): diff --git a/rnfs/dht/providers.py b/rnfs/dht/providers.py index d0a369e..15c8b6f 100644 --- a/rnfs/dht/providers.py +++ b/rnfs/dht/providers.py @@ -17,7 +17,6 @@ import json import os import random -import tempfile import threading import time from collections.abc import Callable, Iterable @@ -343,20 +342,11 @@ def save(self): self._dirty = False directory = os.path.dirname(os.path.abspath(self.state_path)) os.makedirs(directory, exist_ok=True) - fd, temp_path = tempfile.mkstemp(dir=directory, prefix=".providers-", suffix=".tmp") try: - with os.fdopen(fd, "w", encoding="utf-8") as f: - json.dump(state, f, separators=(",", ":")) - f.flush() - os.fsync(f.fileno()) - fsutil.replace(temp_path, self.state_path) + fsutil.write_atomic(self.state_path, json.dumps(state, separators=(",", ":")).encode(), mode=0o600) except BaseException: with self._lock: self._dirty = True # try again next time - try: - fsutil.remove(temp_path) - except OSError: - pass raise def _load(self): diff --git a/rnfs/features/__init__.py b/rnfs/features/__init__.py index cb005b2..51b45f2 100644 --- a/rnfs/features/__init__.py +++ b/rnfs/features/__init__.py @@ -9,7 +9,7 @@ its parser, and client commands must not pay for loading RNS. Import it inside functions. - The constructor must be cheap and must not need Reticulum, so tests can build an offline node. - API handlers take the ApiHandler and use its helpers (param, json_body, send_json); they reach the - feature through `handler.node.feature(NAME)`. Every feature route requires the API token. + feature through `_common.feature(handler, NAME)`. Every feature route requires the API token. - CLI commands are added with the `command(subparsers, name, run, help)` helper the CLI passes in, and run functions talk to the daemon with `rnfs.cli.client(args)` (import rnfs.cli inside the function). """ diff --git a/rnfs/features/_common.py b/rnfs/features/_common.py new file mode 100644 index 0000000..e866b3b --- /dev/null +++ b/rnfs/features/_common.py @@ -0,0 +1,19 @@ +"""Helpers shared by the feature modules. No RNS at module level (the CLI imports every feature module).""" +from rnfs.errors import NotFound + +# RNS's loglevels (RNS.LOG_ERROR ...), defined here so feature modules can name them without loading RNS +LOG_ERROR, LOG_WARNING, LOG_NOTICE, LOG_INFO, LOG_VERBOSE = 1, 2, 3, 4, 5 + + +def log(message: str, level: int = LOG_VERBOSE): + """Log through rnfs.log (imported here: it loads RNS).""" + from rnfs import log as rnfs_log + rnfs_log.log(message, level) + + +def feature(handler, name: str, missing: str | None = None): + """The feature called name of the handler's node. NotFound (with missing, if given) if the daemon has none.""" + try: + return handler.node.feature(name) + except (AttributeError, KeyError): + raise NotFound(missing or f"this daemon has no {name} feature") from None diff --git a/rnfs/features/contacts.py b/rnfs/features/contacts.py index e4a55b4..b03b49c 100644 --- a/rnfs/features/contacts.py +++ b/rnfs/features/contacts.py @@ -20,7 +20,9 @@ from rnfs.content.cid import is_valid as is_valid_cid from rnfs.errors import NotFound -from rnfs.features import Feature +from rnfs.features import Feature, _common +from rnfs.features._common import LOG_INFO, LOG_WARNING +from rnfs.fsutil import printable NAME = "contacts" CONTACTS_FILE = "contacts.json" @@ -97,7 +99,7 @@ def _load(self): check_nick(nick) self._contacts[nick] = (check_kind(entry["kind"]), parse_hex(entry["id"])) except (KeyError, TypeError, ValueError): - self._log(f"ignoring malformed contact {nick!r} in {self._path}", "LOG_WARNING") + self._log(f"ignoring malformed contact {nick!r} in {self._path}", LOG_WARNING) if len(self._contacts) >= MAX_CONTACTS: break @@ -109,10 +111,9 @@ def _save(self): for nick, (kind, ident) in sorted(self._contacts.items())}}) @staticmethod - def _log(message: str, level: str = "LOG_INFO"): - """Log through rnfs.log (imported here: it loads RNS).""" - from rnfs import log - log.log(f"contacts: {message}", getattr(log, level)) + def _log(message: str, level: int = LOG_INFO): + """Log with this feature's prefix.""" + _common.log(f"contacts: {message}", level) # The address book @@ -286,10 +287,7 @@ def cli_commands(cls, commands, command): def _feature(handler) -> ContactsFeature: """The node's address book. Raises NotFound if the daemon has none.""" - try: - return handler.node.feature(NAME) - except (AttributeError, KeyError): - raise NotFound("this daemon has no address book") from None + return _common.feature(handler, NAME, "this daemon has no address book") def _api_list(handler): @@ -319,16 +317,11 @@ def _api_rm(handler): # CLI (no RNS here) -def _printable(text) -> str: - """Escape control characters in text the daemon sent.""" - return "".join(c if c.isprintable() else repr(c)[1:-1] for c in str(text)) - - def run_add(args): """Add a contact; print it.""" import rnfs.cli contact = rnfs.cli.client(args).post("/api/contacts/add", nick=args.nick, id=args.id, kind=args.kind) - print(f"{_printable(contact['nick'])} {_printable(contact['kind'])} {_printable(contact['id'])}") + print(f"{printable(contact['nick'])} {printable(contact['kind'])} {printable(contact['id'])}") def run_rm(args): @@ -341,7 +334,7 @@ def run_ls(args): """Print every contact: nick, kind, hash.""" import rnfs.cli for contact in rnfs.cli.client(args).get("/api/contacts")["contacts"]: - print(f"{_printable(contact['nick'])} {_printable(contact['kind'])} {_printable(contact['id'])}") + print(f"{printable(contact['nick'])} {printable(contact['kind'])} {printable(contact['id'])}") def node_arg(args, text: str | None) -> str | None: @@ -354,5 +347,5 @@ def node_arg(args, text: str | None) -> str | None: import rnfs.cli contact = rnfs.cli.client(args).get("/api/contacts/get", nick=text) # exits with "no contact " if contact.get("kind") != "node": - rnfs.cli.fail(f"contact {text} is a {_printable(contact.get('kind'))}, not a node") + rnfs.cli.fail(f"contact {text} is a {printable(contact.get('kind'))}, not a node") return contact["id"] diff --git a/rnfs/features/follow.py b/rnfs/features/follow.py index 1234fc0..49a769b 100644 --- a/rnfs/features/follow.py +++ b/rnfs/features/follow.py @@ -21,10 +21,13 @@ import threading import time +from rnfs.constants import ID_LENGTH from rnfs.content.cid import Cid from rnfs.errors import NotFound, RnfsError -from rnfs.features import Feature +from rnfs.features import Feature, _common +from rnfs.features._common import LOG_ERROR, LOG_NOTICE, LOG_VERBOSE, LOG_WARNING from rnfs.features.keepers import FOLLOW, name_subcommands, parse_name_arg +from rnfs.fsutil import printable NAME = "follow" FOLLOW_FILE = "follow.json" @@ -46,10 +49,9 @@ def follow_source(name: bytes) -> str: return SOURCE_PREFIX + name.hex() -def _log(message: str, level: str = "LOG_VERBOSE"): - """Log through rnfs.log (imported here: it loads RNS).""" - from rnfs import log - log.log(f"follow: {message}", getattr(log, level)) +def _log(message: str, level: int = LOG_VERBOSE): + """Log with this feature's prefix.""" + _common.log(f"follow: {message}", level) class FollowFeature(Feature): @@ -89,7 +91,7 @@ def _load(self): pinned = Cid.parse(entry["pinned"]) if entry.get("pinned") is not None else None except (AttributeError, KeyError, TypeError, ValueError): continue - if len(name) == 16: + if len(name) == ID_LENGTH: self._pinned[name] = pinned def _save(self): @@ -129,7 +131,7 @@ def follow(self, name: bytes) -> dict: if name not in self._pinned: self._pinned[name] = None self._save() - _log(f"following name {name.hex()}", "LOG_NOTICE") + _log(f"following name {name.hex()}", LOG_NOTICE) self._failures.pop(name, None) # asked again by the user: start the backoff afresh self._due[name] = (time.monotonic(), True) self._wake.set() @@ -152,7 +154,7 @@ def unfollow(self, name: bytes): self._keepers.unkeep(name, FOLLOW) except NotFound: self.node.publisher.refresh() - _log(f"no longer following name {name.hex()}", "LOG_NOTICE") + _log(f"no longer following name {name.hex()}", LOG_NOTICE) def following(self) -> list[dict]: """Every followed name: the target pinned for it and what the last check did.""" @@ -175,7 +177,7 @@ def _changed(self, name: bytes): with self._lock: if name not in self._pinned: return - when, network = self._due.get(name, (FAR_FUTURE, False)) + _, network = self._due.get(name, (FAR_FUTURE, False)) self._due[name] = (time.monotonic(), network) self._wake.set() @@ -223,10 +225,10 @@ def check(self, name: bytes, network: bool = True) -> bool: try: self._save() except OSError as e: # the pins are right; follow.json catches up with the next save - _log(f"saving {FOLLOW_FILE} failed: {e}", "LOG_ERROR") + _log(f"saving {FOLLOW_FILE} failed: {e}", LOG_ERROR) except RnfsError as e: self._set_state(name, f"fetch of {target} failed: {e}") - _log(f"fetching {target} for name {name.hex()} failed: {e}", "LOG_WARNING") + _log(f"fetching {target} for name {name.hex()} failed: {e}", LOG_WARNING) return False if not followed: return True # unfollowed while fetching: nothing was pinned @@ -235,7 +237,7 @@ def check(self, name: bytes, network: bool = True) -> bool: self.node.storage.unpin(previous, source) self.node.publisher.refresh() self._set_state(name, "current") - _log(f"name {name.hex()} now pinned at {target}", "LOG_NOTICE") + _log(f"name {name.hex()} now pinned at {target}", LOG_NOTICE) return True def _set_state(self, name: bytes, state: str): @@ -259,7 +261,7 @@ def run_due(self, now: float | None = None) -> float: try: ok = self.check(name, network) except Exception as e: # noqa: BLE001 - e.g. disk full saving: a failed check, tried again later - _log(f"checking name {name.hex()} failed: {e!r}", "LOG_ERROR") + _log(f"checking name {name.hex()} failed: {e!r}", LOG_ERROR) self._set_state(name, f"check failed: {e}") ok = False with self._lock: @@ -283,7 +285,7 @@ def _loop(self): try: delay = self.run_due() except Exception as e: # noqa: BLE001 - keep following - _log(f"checking followed names failed: {e!r}", "LOG_ERROR") + _log(f"checking followed names failed: {e!r}", LOG_ERROR) delay = RETRY_INTERVAL self._wake.wait(min(delay, 60.0)) self._wake.clear() @@ -312,10 +314,7 @@ def cli_commands(cls, commands, command): def _feature(handler) -> FollowFeature: """The node's follow feature. NotFound if the daemon has none.""" - try: - return handler.node.feature(NAME) - except (AttributeError, KeyError): - raise NotFound("this daemon can't follow names") from None + return _common.feature(handler, NAME, "this daemon can't follow names") def _api_follow(handler): @@ -336,16 +335,11 @@ def _api_following(handler): handler.send_json(200, {"following": _feature(handler).following()}) -def _printable(text) -> str: - """Escape control characters in text the daemon sent.""" - return "".join(c if c.isprintable() else repr(c)[1:-1] for c in str(text)) - - def run_follow(args): """Follow a name.""" import rnfs.cli entry = rnfs.cli.client(args).post("/api/name/follow", name=args.name) - print(f"following {_printable(entry['name'])}; its target is fetched and pinned in the background") + print(f"following {printable(entry['name'])}; its target is fetched and pinned in the background") def run_unfollow(args): @@ -358,5 +352,5 @@ def run_following(args): """List followed names.""" import rnfs.cli for entry in rnfs.cli.client(args).get("/api/name/following")["following"]: - print(f"{_printable(entry['name'])} {_printable(entry['pinned'] or '-')} {_printable(entry['latest'] or '-')} " - f"{_printable(entry['state'])}") + print(f"{printable(entry['name'])} {printable(entry['pinned'] or '-')} {printable(entry['latest'] or '-')} " + f"{printable(entry['state'])}") diff --git a/rnfs/features/jobs.py b/rnfs/features/jobs.py index 5ca7e20..45fc94c 100644 --- a/rnfs/features/jobs.py +++ b/rnfs/features/jobs.py @@ -40,7 +40,9 @@ from collections import deque from contextlib import ExitStack -from rnfs.features import Feature +from rnfs.constants import PARTIAL_SUFFIX +from rnfs.features import Feature, _common +from rnfs.features._common import LOG_ERROR, LOG_INFO, LOG_NOTICE, LOG_WARNING NAME = "jobs" JOBS_FILE = "jobs.json" @@ -57,15 +59,13 @@ POLL_TTY = 1 # seconds between progress updates the CLI shows POLL_QUIET = 10 # seconds a CLI without a progress line waits per request (it returns early when the job ends) FINISHED_GRACE = 60.0 # seconds a finished job is kept whatever max_finished_jobs says -PARTIAL_SUFFIX = ".partial" # a get job's output while it is written: .rnfs-job-.partial _JOB_ID = re.compile(r"[0-9a-f]{8}") _NODE_ID = re.compile(r"[0-9a-f]{32}") -def _log(message: str, level: str = "LOG_INFO"): - """Log through rnfs.log, imported here since it loads RNS.""" - from rnfs import log - log.log(f"jobs: {message}", getattr(log, level)) +def _log(message: str, level: int = LOG_INFO): + """Log with this feature's prefix.""" + _common.log(f"jobs: {message}", level) class Job: @@ -293,7 +293,7 @@ def _load(self): try: job = Job.from_record(record) except (ValueError, TypeError, OverflowError) as e: - _log(f"ignoring a saved job ({e})", "LOG_WARNING") + _log(f"ignoring a saved job ({e})", LOG_WARNING) continue if job.id in self._jobs: continue @@ -333,7 +333,7 @@ def _save_quietly(self): try: self.save() except OSError as e: - _log(f"saving {self._path} failed: {e!r}", "LOG_ERROR") + _log(f"saving {self._path} failed: {e!r}", LOG_ERROR) def _cap_finished(self): """Forget the oldest finished jobs beyond max_finished_jobs (lock held, or before any thread runs). @@ -387,7 +387,7 @@ def stop(self): try: self.save() # only if something changed: offline test nodes with no jobs write nothing except OSError as e: - _log(f"saving {self._path} failed: {e!r}", "LOG_ERROR") + _log(f"saving {self._path} failed: {e!r}", LOG_ERROR) def status(self) -> dict: """Running and queued job counts for `rnfs status`.""" @@ -433,7 +433,7 @@ def _run(self, job: Job): error = str(e) or type(e).__name__ except Exception as e: # noqa: BLE001 - a job's failure mustn't take its thread's bookkeeping with it error = str(e) or type(e).__name__ - _log(f"job {job.id} ({job.kind} {job.target}) failed: {e!r}", "LOG_ERROR") + _log(f"job {job.id} ({job.kind} {job.target}) failed: {e!r}", LOG_ERROR) self._finish(job, state, error, result) def _execute(self, job: Job) -> dict: @@ -467,7 +467,7 @@ def _get(self, job: Job) -> dict: try: remove_partial(partial) # stored blocks stay, so a retry or resume writes it out again quickly except OSError as e: # the job's own error matters more; the next run tries again - _log(f"job {job.id}: removing {partial} failed: {e!r}", "LOG_WARNING") + _log(f"job {job.id}: removing {partial} failed: {e!r}", LOG_WARNING) raise with self._lock: job.output = output @@ -485,7 +485,7 @@ def _fetch_to(self, job: Job, path: str) -> dict: if from_node is None or not job.fallback_dht or isinstance(e, (fetcher.Cancelled, TooLarge, StorageFull)): raise _log(f"job {job.id}: getting {job.target} from {job.from_node} failed ({e}); trying the DHT", - "LOG_NOTICE") + LOG_NOTICE) remove_partial(path) return self.node.get(job.target, path) @@ -656,11 +656,7 @@ def remove_partial(path: str): def _feature(handler) -> JobsFeature: """The node's job manager. Raises NotFound if the node has none (clients then fall back).""" - try: - return handler.node.feature(NAME) - except (AttributeError, KeyError): - from rnfs.errors import NotFound - raise NotFound("this daemon has no job manager") from None + return _common.feature(handler, NAME, "this daemon has no job manager") def api_start(handler): diff --git a/rnfs/features/keepers.py b/rnfs/features/keepers.py index a361120..42d5d8c 100644 --- a/rnfs/features/keepers.py +++ b/rnfs/features/keepers.py @@ -39,7 +39,8 @@ from rnfs.constants import ID_LENGTH from rnfs.content.cid import Cid from rnfs.errors import NotFound -from rnfs.features import Feature +from rnfs.features import Feature, _common +from rnfs.features._common import LOG_ERROR, LOG_NOTICE, LOG_VERBOSE NAME = "keepers" NAME_PATH = "/name" @@ -69,10 +70,9 @@ def name_key(name: bytes) -> bytes: return hashlib.sha256(KEY_DOMAIN + name).digest()[:ID_LENGTH] -def _log(message: str, level: str = "LOG_VERBOSE"): - """Log through rnfs.log (imported here: it loads RNS).""" - from rnfs import log - log.log(f"keepers: {message}", getattr(log, level)) +def _log(message: str, level: int = LOG_VERBOSE): + """Log with this feature's prefix.""" + _common.log(f"keepers: {message}", level) # Signed records @@ -420,7 +420,7 @@ def _notify(self, name: bytes): try: listener(name) except Exception as e: # noqa: BLE001 - one listener mustn't break the others - _log(f"listener failed: {e!r}", "LOG_ERROR") + _log(f"listener failed: {e!r}", LOG_ERROR) # Keeping @@ -432,7 +432,7 @@ def keep(self, name: bytes, reason: str = USER) -> dict: """Keep name, and fetch its record in the background if none is held. ValueError if full.""" if self.store.keep(name, reason): self.store.save() - _log(f"keeping name {name.hex()} ({reason})", "LOG_NOTICE") + _log(f"keeping name {name.hex()} ({reason})", LOG_NOTICE) if self.store.get(name) is None: # NameManager may hold its app_data, but only a keeper's answer (or a cached announce) carries # the signature others need to check it @@ -445,7 +445,7 @@ def unkeep(self, name: bytes, reason: str = USER): raise NotFound(f"name {name.hex()} is not kept" + ("" if reason == USER else f" for {reason}")) self.store.save() self.node.publisher.refresh() - _log(f"no longer keeping name {name.hex()} for {reason}", "LOG_NOTICE") + _log(f"no longer keeping name {name.hex()} for {reason}", LOG_NOTICE) def kept(self) -> list[dict]: """Every kept name: reasons, and the record held if any.""" @@ -578,7 +578,7 @@ def _loop(self): self.sync_own() self.save() except Exception as e: # noqa: BLE001 - keep the thread alive - _log(f"background work failed: {e!r}", "LOG_ERROR") + _log(f"background work failed: {e!r}", LOG_ERROR) def retry_missing(self) -> list[bytes]: """Queue kept names still without a record, round robin from the cursor; the names queued.""" @@ -728,10 +728,7 @@ def parse_name_arg(node, text) -> bytes: def _feature(handler) -> KeepersFeature: """The node's keepers feature. NotFound if the daemon has none.""" - try: - return handler.node.feature(NAME) - except (AttributeError, KeyError): - raise NotFound("this daemon doesn't keep names") from None + return _common.feature(handler, NAME, "this daemon doesn't keep names") def _api_keep(handler): diff --git a/rnfs/features/lxmf.py b/rnfs/features/lxmf.py index ff0596b..43f3d85 100644 --- a/rnfs/features/lxmf.py +++ b/rnfs/features/lxmf.py @@ -32,7 +32,6 @@ import re import signal import sys -import tempfile import threading import time from collections import deque @@ -40,7 +39,9 @@ from dataclasses import dataclass from urllib.parse import quote, unquote_to_bytes -from rnfs import features +from rnfs import features, fsutil +from rnfs.features import _common +from rnfs.features._common import LOG_ERROR, LOG_INFO, LOG_NOTICE, LOG_VERBOSE, LOG_WARNING from rnfs.constants import ID_LENGTH, MAX_DEPTH from rnfs.content.cid import Cid @@ -100,18 +101,9 @@ METHOD_NAMES = {0x01: "opportunistic", 0x02: "direct", 0x03: "propagated", 0x05: "paper"} -def _log(message: str, level: str = "info"): - """Log through rnfs.log, imported here because it loads RNS.""" - from rnfs import log as rnfs_log - rnfs_log.log(f"lxmf: {message}", {"error": rnfs_log.LOG_ERROR, "warning": rnfs_log.LOG_WARNING, - "notice": rnfs_log.LOG_NOTICE, "info": rnfs_log.LOG_INFO, - "verbose": rnfs_log.LOG_VERBOSE}[level]) - - -def _printable(text) -> str: - """Untrusted text (names, errors) with non-printable characters escaped, for logs and terminals.""" - from rnfs import fsutil - return fsutil.printable(text) +def _log(message: str, level: int = LOG_INFO): + """Log with this feature's prefix.""" + _common.log(f"lxmf: {message}", level) _NO_PN_HELP = ("no LXMF propagation node is set: set lxmf_propagation_node, or use --nearest for the nearest " @@ -424,37 +416,8 @@ def _json_bytes(value) -> int: def _write_json(path: str, value): """Write value as UTF-8 JSON (non-ASCII as is, not as 6-byte escapes) to path atomically and durably: a power loss leaves the old or the new file whole (an emptied inbox would leave lxmf: pins nothing releases).""" - from rnfs import fsutil - directory = os.path.dirname(path) - fd, partial = tempfile.mkstemp(prefix=".partial-", dir=directory) - try: - # backslashreplace: a lone surrogate (e.g. in an error text) is written as its JSON escape - with os.fdopen(fd, "w", encoding="utf-8", errors="backslashreplace") as f: - json.dump(value, f, ensure_ascii=False) - f.flush() - os.fsync(f.fileno()) - fsutil.replace(partial, path) - except BaseException: - try: - fsutil.remove(partial) - except OSError: - pass - raise - _sync_directory(directory) - - -def _sync_directory(directory: str): - """fsync a directory so a rename in it is durable; best effort (Windows can't open a directory).""" - try: - fd = os.open(directory or ".", os.O_RDONLY) - except OSError: - return - try: - os.fsync(fd) - except OSError: - pass - finally: - os.close(fd) + from rnfs.content.storage import write_json + write_json(path, value, ensure_ascii=False) class MessageLog: @@ -490,7 +453,7 @@ def __init__(self, path: str, limit: int, max_bytes: int | None = None, protect= else: bad += 1 if bad: - _log(f"dropped {bad} malformed record(s) from {path}", "warning") + _log(f"dropped {bad} malformed record(s) from {path}", LOG_WARNING) self._entries = self._entries[-limit:] self._sizes = {e["id"]: _json_bytes(e) for e in self._entries} while self.max_bytes is not None and len(self._entries) > 1 and sum(self._sizes.values()) > max_bytes: @@ -730,18 +693,18 @@ def start(self): """Start LXMF (if enabled and installed), the announce/sync loop and the auto-fetch worker.""" if self.config.lxmf_enabled is False: self.off_reason = "LXMF is off (lxmf_enabled = false)" - _log("off: lxmf_enabled is false", "verbose") + _log("off: lxmf_enabled is false", LOG_VERBOSE) return try: network = LxmfNetwork(self, self.config, self.node.node_id) except ImportError as e: self.off_reason = "LXMF is off: the LXMF package is not installed (pip install lxmf)" _log(f"the LXMF package can't be imported ({e}), so LXMF messaging is off. Install it with " - f"`pip install lxmf`", "warning" if self.config.lxmf_enabled else "notice") + f"`pip install lxmf`", LOG_WARNING if self.config.lxmf_enabled else LOG_NOTICE) return except Exception as e: # noqa: BLE001 - LXMF failing must not stop the node self.off_reason = f"LXMF failed to start: {e}" - _log(f"failed to start: {e!r}", "error") + _log(f"failed to start: {e!r}", LOG_ERROR) return self.attach(network) self._requeue_auto_fetches() @@ -749,7 +712,7 @@ def start(self): thread = threading.Thread(target=target, name=name, daemon=True) thread.start() self._threads.append(thread) - _log(f"LXMF address {network.address.hex()} ({network.display_name})", "notice") + _log(f"LXMF address {network.address.hex()} ({network.display_name})", LOG_NOTICE) def attach(self, network): """Use network (LxmfNetwork, or a stand-in in tests) for sending and receiving.""" @@ -767,7 +730,7 @@ def stop(self): try: self.network.stop() except Exception as e: # noqa: BLE001 - _log(f"stopping LXMF failed: {e!r}", "error") + _log(f"stopping LXMF failed: {e!r}", LOG_ERROR) self.save() def save(self): @@ -810,7 +773,7 @@ def _run(self): for message_hash, state, progress in network.poll(): self.outbound_changed(message_hash, state, progress) except Exception as e: # noqa: BLE001 - keep the loop alive - _log(f"loop failed: {e!r}", "error") + _log(f"loop failed: {e!r}", LOG_ERROR) self._wake.wait(TICK) self._wake.clear() @@ -824,7 +787,7 @@ def _no_propagation_node(self, consequence: str): _log(f"no propagation node is set (lxmf_propagation_node), so {consequence} and failed direct messages " f"aren't sent through one.{heard} To use one, set lxmf_propagation_node = \"<32 hex digits>\" to a " f"node you trust to hold your messages, or use `rnfs lxmf sync --nearest` / " - f"`rnfs send --propagate --nearest` to use the nearest one heard, once", "notice") + f"`rnfs send --propagate --nearest` to use the nearest one heard, once", LOG_NOTICE) # Sending @@ -912,7 +875,7 @@ def send(self, to: str, target: str, text: str | None = None, propagate: bool = if not propagate: with self._lock: self._resend[message_hash.hex()] = (address, content, fields, nearest) - _log(f"queued {method} message {message_hash.hex()} to {address.hex()} with {link.uri()}", "info") + _log(f"queued {method} message {message_hash.hex()} to {address.hex()} with {link.uri()}", LOG_INFO) return {"id": entry_id, "hash": message_hash.hex(), "to": address.hex(), "method": method} def outbound_changed(self, message_hash: bytes, state: str, progress: float | None): @@ -937,10 +900,10 @@ def outbound_changed(self, message_hash: bytes, state: str, progress: float | No state="outbound", progress=0.0, direct_hash=message_hash.hex(), fallback=True) _log(f"direct delivery of {message_hash.hex()} failed; sending it through the propagation " - f"node as {new_hash.hex()}", "notice") + f"node as {new_hash.hex()}", LOG_NOTICE) return except Exception as e: # noqa: BLE001 - values["error"] = f"propagation fallback failed: {_printable(e)}" + values["error"] = f"propagation fallback failed: {fsutil.printable(e)}" if entry is not None: self.outbox.update(entry["id"], **values) @@ -953,7 +916,7 @@ def received(self, message: Incoming) -> int | None: if self.network is not None and message.source == self.network.address: return None if not self.accepts(message.source): - _log(f"ignored a message from {sender}: not in lxmf_accept_from", "verbose") + _log(f"ignored a message from {sender}: not in lxmf_accept_from", LOG_VERBOSE) return None message_hash = message.hash.hex() if self.inbox.find("hash", message_hash) is not None: @@ -962,14 +925,14 @@ def received(self, message: Incoming) -> int | None: try: links = decode_field(message.fields) except ValueError as e: - _log(f"ignored the malformed rnfs field of message {message_hash} from {sender}: {_printable(e)}", - "verbose") + _log(f"ignored the malformed rnfs field of message {message_hash} from {sender}: {fsutil.printable(e)}", + LOG_VERBOSE) links = None if links is None: # Links past the text kept count too: up to MAX_SCAN_CHARS are searched links = find_links(_text(message.content, MAX_SCAN_CHARS)) if not links: - _log(f"message {message_hash} from {sender} has no rnfs links; not kept", "verbose") + _log(f"message {message_hash} from {sender} has no rnfs links; not kept", LOG_VERBOSE) return None display_name = message.display_name if isinstance(message.display_name, str) else None entry = {"hash": message_hash, "from": sender, @@ -988,10 +951,10 @@ def received(self, message: Incoming) -> int | None: self._release(dropped) if entry_id is None: _log(f"message {message_hash} from {sender} not kept: the inbox is full of messages holding " - f"auto-fetched pins (rnfs inbox rm)", "notice") + f"auto-fetched pins (rnfs inbox rm)", LOG_NOTICE) return None _log(f"message {message_hash} from {sender} with {len(entry['links'])} rnfs link(s), inbox #{entry_id}", - "notice") + LOG_NOTICE) if auto: for index in range(len(entry["links"])): self._queue_auto_fetch(entry_id, index) @@ -1060,7 +1023,7 @@ def _resolve(self, link: Link, persist: bool = True) -> tuple[Cid, list[str]]: except NotFound: if link.cid is None: raise - _log(f"name {link.name.hex()} could not be resolved; using the CID it was sent with", "notice") + _log(f"name {link.name.hex()} could not be resolved; using the CID it was sent with", LOG_NOTICE) return link.cid, list(link.path) def _hints(self, link: Link) -> list[bytes | None]: @@ -1099,10 +1062,10 @@ def inbox_get(self, entry_id: int, index: int | None = None, output: str | None from rnfs import fsutil path = os.path.join(directory, fsutil.safe_default_name(names[-1] if names else None, str(root))) if os.path.lexists(path): - raise ValueError(f"{_printable(path)} already exists; remove it or choose another with -o") + raise ValueError(f"{fsutil.printable(path)} already exists; remove it or choose another with -o") same = next((i for i, (_, other, _) in enumerate(planned) if other == path), None) if same is not None: - raise ValueError(f"links {same + 1} and {number} would both be written to {_printable(path)}: " + raise ValueError(f"links {same + 1} and {number} would both be written to {fsutil.printable(path)}: " f"get them one at a time with --link and -o") planned.append((link, path, target)) results = [] @@ -1132,8 +1095,8 @@ def _get_link(self, link: Link, path: str, target: str) -> dict: except RnfsError as e: if attempt == len(hints) - 1 or os.path.lexists(path): raise - _log(f"{from_node.hex()} could not supply {_printable(target)} ({_printable(e)}); trying " - f"the DHT", "notice") + _log(f"{from_node.hex()} could not supply {fsutil.printable(target)} ({fsutil.printable(e)}); trying " + f"the DHT", LOG_NOTICE) return {"target": target, "output": path, "cid": result["cid"], "warnings": result.get("warnings", [])} def _start_job(self, target: str, output: str, link: Link): @@ -1227,22 +1190,23 @@ def auto_fetch(self, entry_id: int, index: int): except RnfsError as e: if attempt == len(hints) - 1: raise - _log(f"{from_node.hex()} could not supply {root} ({_printable(e)}); trying the DHT", "verbose") + _log(f"{from_node.hex()} could not supply {root} ({fsutil.printable(e)}); trying the DHT", + LOG_VERBOSE) if cid is not None: - _log(f"auto-fetched and pinned {cid} from inbox #{entry_id}", "notice") + _log(f"auto-fetched and pinned {cid} from inbox #{entry_id}", LOG_NOTICE) return # pinned and recorded by _fetch_and_pin, or the message was removed meanwhile except fetcher.Cancelled: - _log(f"auto-fetch of inbox #{entry_id} link {index + 1} stopped", "verbose") + _log(f"auto-fetch of inbox #{entry_id} link {index + 1} stopped", LOG_VERBOSE) return # still "queued" if the node is stopping: fetched at the next start except TooLarge as e: result = {"state": "refused", "error": str(e)} except (RnfsError, ValueError, KeyError, IndexError, TypeError) as e: result = {"state": "failed", "error": str(e) or type(e).__name__} except Exception as e: # noqa: BLE001 - _log(f"auto-fetch of inbox #{entry_id} failed: {e!r}", "error") + _log(f"auto-fetch of inbox #{entry_id} failed: {e!r}", LOG_ERROR) result = {"state": "failed", "error": repr(e)} - _log(f"auto-fetch of inbox #{entry_id} link {index + 1}: {result['state']}: {_printable(result['error'])}", - "notice") + _log(f"auto-fetch of inbox #{entry_id} link {index + 1}: {result['state']}: " + f"{fsutil.printable(result['error'])}", LOG_NOTICE) self._set_auto(entry_id, index, result) def _pin_recorded(self, entry_id: int, index: int, cid: Cid, source: str) -> bool: @@ -1330,8 +1294,8 @@ def cli_commands(cls, commands, command): def _feature(handler) -> LxmfFeature: - """The lxmf feature of the handler's node.""" - return handler.node.feature(NAME) + """The lxmf feature of the handler's node. NotFound if the daemon has none.""" + return _common.feature(handler, NAME) def _api_address(handler): @@ -1645,7 +1609,7 @@ def _announce_app_data(self, destination_hash): def announce(self): """Announce the delivery destination (with the display name).""" self.router.announce(self.destination.hash) - _log(f"announced {self.address.hex()}", "verbose") + _log(f"announced {self.address.hex()}", LOG_VERBOSE) def stop(self): """Persist LXMF's state and stop listening for propagation node announces.""" @@ -1700,7 +1664,7 @@ def _use_propagation_node(self, destination_hash: bytes): if destination_hash != self._current_pn: self.router.set_outbound_propagation_node(destination_hash) self._current_pn = destination_hash - _log(f"using propagation node {destination_hash.hex()}", "info") + _log(f"using propagation node {destination_hash.hex()}", LOG_INFO) def sync(self, nearest: bool = False) -> bool: """Start downloading waiting messages from the propagation node (propagation_node(nearest)); False if @@ -1786,7 +1750,7 @@ def _pn_resource(self, resource): sizes = (resource.get_data_size(), resource.get_transfer_size()) if resource.get_segments() != 1 or any(not isinstance(n, int) or n > MAX_PN_RESOURCE for n in sizes): _log(f"the propagation node sent a {sizes[0]}-byte response in {resource.get_segments()} segment(s), " - f"over the {MAX_PN_RESOURCE}-byte bound; download stopped", "warning") + f"over the {MAX_PN_RESOURCE}-byte bound; download stopped", LOG_WARNING) resource.cancel() resource.link.teardown() @@ -1815,7 +1779,7 @@ def _delivered(self, message): if not self.feature.accepts(source): # received() would ignore it: holding it would only let forged sources fill the queue _log(f"dropped a message from unknown sender {source.hex()}: not in lxmf_accept_from", - "verbose") + LOG_VERBOSE) return with self._lock: queued = len(self._held) < HOLD_LIMIT and all(p != packed for _, p, _ in self._held) @@ -1823,15 +1787,15 @@ def _delivered(self, message): self._held.append((time.monotonic() + HOLD_TIME, packed, message.method)) if not queued: _log(f"dropped a message from unknown sender {source.hex()}: already held, or " - f"{HOLD_LIMIT} are held", "verbose") + f"{HOLD_LIMIT} are held", LOG_VERBOSE) return self._request_path(source) - _log(f"holding a message from {source.hex()} until its identity is known", "verbose") + _log(f"holding a message from {source.hex()} until its identity is known", LOG_VERBOSE) else: _log(f"dropped a message with an invalid signature from {bytes(message.source_hash).hex()}", - "notice") + LOG_NOTICE) except Exception as e: # noqa: BLE001 - _log(f"handling a received message failed: {e!r}", "error") + _log(f"handling a received message failed: {e!r}", LOG_ERROR) def _request_path(self, destination: bytes) -> bool: """Request a path to a held message's sender, at most once per HOLD_TIME per sender and within the @@ -1870,16 +1834,16 @@ def _retry_held(self): continue try: if not known: - _log(f"dropped a message from {source.hex()}: its identity never became known", "notice") + _log(f"dropped a message from {source.hex()}: its identity never became known", LOG_NOTICE) continue message = self.LXMF.LXMessage.unpack_from_bytes(packed) message.method = method if message.signature_validated: self.feature.received(self._incoming(message)) else: - _log(f"dropped a message with an invalid signature from {source.hex()}", "notice") + _log(f"dropped a message with an invalid signature from {source.hex()}", LOG_NOTICE) except Exception as e: # noqa: BLE001 - _log(f"a held message could not be read: {e!r}", "verbose") + _log(f"a held message could not be read: {e!r}", LOG_VERBOSE) finally: with self._lock: self._held = [other for other in self._held if other is not item] diff --git a/rnfs/features/peers.py b/rnfs/features/peers.py index 7dd5f37..d629597 100644 --- a/rnfs/features/peers.py +++ b/rnfs/features/peers.py @@ -12,7 +12,8 @@ import time from rnfs import mesh -from rnfs.features import Feature +from rnfs.features import Feature, _common +from rnfs.fsutil import printable NAME = "peers" @@ -156,12 +157,12 @@ def cli_commands(cls, commands, command): def _api_peers(handler): """GET /api/peers: {"peers": [...]}.""" - handler.send_json(200, {"peers": handler.node.feature(NAME).peers()}) + handler.send_json(200, {"peers": _common.feature(handler, NAME).peers()}) def _api_stats(handler): """GET /api/stats: traffic totals, refusals and per-interface bytes.""" - handler.send_json(200, handler.node.feature(NAME).stats()) + handler.send_json(200, _common.feature(handler, NAME).stats()) # Output (no RNS here: the CLI must not load it) @@ -209,8 +210,8 @@ def format_peers(peers: list[dict], now: float) -> list[str]: rate = human_rate(peer["rate"]) if peer["link"] else "-" interface = (peer.get("interface") or "-")[:20] flags = (" banned" if peer["banned"] else "") + (f" bad:{peer['bad']}" if peer.get("bad") else "") - lines.append(f"{_printable(peer['node_id']):<32} {_printable(peer['mode'] or '?'):<7} {table:>6} " - f"{peer['hops'] if peer['hops'] is not None else '?':>4} {_printable(interface):<20} " + lines.append(f"{printable(peer['node_id']):<32} {printable(peer['mode'] or '?'):<7} {table:>6} " + f"{peer['hops'] if peer['hops'] is not None else '?':>4} {printable(interface):<20} " f"{rate:>10} {heard:>6} {human_size(peer['sent']):>9} {human_size(peer['received']):>9} " f"{peer['failed']:>5}{flags}") return lines @@ -229,16 +230,11 @@ def format_stats(stats: dict) -> list[str]: lines.append(f"upload budget: {human_size(stats.get('upload_budget_left'))} left of " f"{human_size(stats['upload_budget'])} per hour") for name, counters in sorted(stats["interfaces"].items()): - lines.append(f"interface {_printable(name)}: sent {human_size(counters['sent'])}, " + lines.append(f"interface {printable(name)}: sent {human_size(counters['sent'])}, " f"received {human_size(counters['received'])}") return lines -def _printable(text: str) -> str: - """Interface names come from config files and remote-chosen spawned names: escape control characters.""" - return "".join(c if c.isprintable() else repr(c)[1:-1] for c in str(text)) - - def run_peers(args): """Print the known nodes.""" import rnfs.cli diff --git a/rnfs/fsutil.py b/rnfs/fsutil.py index 4905dd7..d28ad76 100644 --- a/rnfs/fsutil.py +++ b/rnfs/fsutil.py @@ -6,6 +6,7 @@ """ import errno import os +import secrets import sys import time @@ -15,6 +16,11 @@ # Opening with os.open on Windows gives a text-mode descriptor (CRLF translation, Ctrl-Z as end of file) # unless O_BINARY is set; elsewhere the flag doesn't exist and isn't needed O_BINARY = getattr(os, "O_BINARY", 0) +O_NOFOLLOW = getattr(os, "O_NOFOLLOW", 0) # Windows lacks it + +# Temporary files of atomic writes start with this, so they can't be taken for what they replace (the +# blockstore deletes its own when opened) +PARTIAL_PREFIX = ".partial-" # Seconds slept between attempts while Windows reports a file in use: about 1 s in all RETRY_DELAYS = (0.01, 0.02, 0.05, 0.1, 0.15, 0.2, 0.2, 0.25) @@ -118,6 +124,44 @@ def sync_directory(directory: str): os.close(fd) +def _temp_path(directory: str) -> str: + """A new, unpredictable name for a temporary file in directory.""" + return os.path.join(directory, PARTIAL_PREFIX + secrets.token_hex(8)) + + +def write_atomic(path: str, data: bytes, *, mode: int = 0o644, durable: bool = True, exclusive: bool = False): + """Replace path with data, so a crash or power loss leaves the old or the new file whole. Raises OSError. + + data goes to a new temporary file in path's directory (O_EXCL and O_NOFOLLOW: never an existing file or + symlink, created with mode, which the umask can only narrow), synced to disk and renamed over path (retried + on Windows while path is in use); a symlink at path is replaced, not written through. If durable, the + directory is then synced so the rename survives a power loss too. With exclusive, an existing path is kept + and FileExistsError raised instead. On any failure before the rename the temporary file is removed and path + is left as it was. + """ + directory = os.path.dirname(path) or "." + temp = _temp_path(directory) + # Outside the try: if this fails, whatever is at temp isn't ours to remove + fd = os.open(temp, os.O_WRONLY | os.O_CREAT | os.O_EXCL | O_NOFOLLOW | O_BINARY, mode) + try: + with os.fdopen(fd, "wb") as f: + f.write(data) + f.flush() + os.fsync(f.fileno()) # else a power loss can leave path empty once renamed + if exclusive: + rename_new(temp, path) + else: + replace(temp, path) + except BaseException: + try: + remove(temp) + except OSError: + pass + raise + if durable: + sync_directory(directory) + + def remove(path: str) -> bool: """Delete the file at path (missing is fine); False if Windows still has it in use after retrying. diff --git a/rnfs/names.py b/rnfs/names.py index eeca22a..ef19c3f 100644 --- a/rnfs/names.py +++ b/rnfs/names.py @@ -242,6 +242,7 @@ def __init__(self, config: Config, pins, network=None, clock=time.time, monotoni # Pins of names whose key file was deleted by hand, released by release_gone() once the node is set up self._gone: list[tuple[str, Cid]] = [] + self._remove_old_temps() self._load_keys() self._load_state() @@ -665,9 +666,8 @@ def _add_key(self, alias: str, identity: RNS.Identity, published: _Published): """Save identity as alias's key, track it, and open its destination if running.""" os.makedirs(self.config.names_dir, mode=0o700, exist_ok=True) # Written atomically and private: RNS's Identity.to_file does neither. The temp name starts with "." - # so it can never be an alias - _write_atomic(self._key_path(alias), identity.get_private_key(), mode=0o600, - temp=os.path.join(self.config.names_dir, f".{alias}.tmp")) + # (fsutil.PARTIAL_PREFIX) so it can never be an alias + fsutil.write_atomic(self._key_path(alias), identity.get_private_key(), mode=0o600) name = name_for_identity(identity) self._keys[alias] = identity self._names[alias] = name @@ -677,6 +677,21 @@ def _add_key(self, alias: str, identity: RNS.Identity, published: _Published): if self._started: self._open_destination(alias) + def _remove_old_temps(self): + """Delete the temp files an interrupted write of rnfs 0.1.1 or earlier left (names.json.tmp and + ..tmp): it reused those names, but writes now take new ones each time.""" + try: + entries = os.listdir(self.config.names_dir) + except OSError: + entries = [] + old = [os.path.join(self.config.names_dir, entry) for entry in entries + if entry.startswith(".") and entry.endswith(".tmp") and _ALIAS.fullmatch(entry[1:-4])] + for path in (self._state_path + ".tmp", *old): + try: + fsutil.remove(path) + except OSError: + pass + def _load_keys(self): """Read every key file in names_dir.""" try: @@ -740,23 +755,5 @@ def _save(self): for name, r in sorted(self._held.items())}, } os.makedirs(self.config.state_dir, exist_ok=True) - _write_atomic(self._state_path, json.dumps(state, indent=1).encode()) + fsutil.write_atomic(self._state_path, json.dumps(state, indent=1).encode()) self._held_dirty = False - - -def _write_atomic(path: str, data: bytes, mode: int = 0o644, temp: str | None = None): - """Replace path with data via a synced temp file, so a crash leaves the old or the new file whole.""" - temp = temp or path + ".tmp" - fd = os.open(temp, os.O_WRONLY | os.O_CREAT | os.O_TRUNC | fsutil.O_BINARY, mode) - try: - with os.fdopen(fd, "wb") as f: - f.write(data) - f.flush() - os.fsync(f.fileno()) - fsutil.replace(temp, path) - except BaseException: - try: - fsutil.remove(temp) - except OSError: - pass - raise diff --git a/rnfs/network/guards.py b/rnfs/network/guards.py index f755cc3..fd5bdc4 100644 --- a/rnfs/network/guards.py +++ b/rnfs/network/guards.py @@ -17,8 +17,9 @@ import RNS +from rnfs import __version__ from rnfs.constants import APP_NAME, DHT_ASPECT, MAX_RESPONSE_SIZE -from rnfs.log import LOG_WARNING, log +from rnfs.log import LOG_ERROR, LOG_WARNING, log # Resource data is a 4-byte random hash plus the payload, encrypted as an RNS Token: 16-byte IV, 32-byte HMAC # and up to 16 bytes of PKCS7 padding. 128 bytes leaves room without letting the transfer outgrow the data. @@ -27,6 +28,8 @@ _lock = threading.Lock() _original_accept = None +_unsupported_reported = False # the error for an RNS whose advertisements lack t and d, logged once per process +_ABSENT = object() DHT_NAME = f"{APP_NAME}.{DHT_ASPECT}" @@ -41,9 +44,25 @@ def is_dht_link(link) -> bool: return isinstance(name, str) and (name == DHT_NAME or name.startswith(DHT_NAME + ".")) +def _report_unsupported(): + """Log, once per process, that RNS's advertisements no longer have the fields the guard reads.""" + global _unsupported_reported + with _lock: + if _unsupported_reported: + return + _unsupported_reported = True + log(f"this RNS version ({getattr(RNS, '__version__', 'unknown')}) isn't supported by rnfs {__version__}: " + f"incoming Resources are being refused", LOG_ERROR) + + def refusal(adv, link=None) -> str | None: """Why an unpacked ResourceAdvertisement arriving on link must be rejected, or None if it may be accepted.""" - transfer, data = getattr(adv, "t", None), getattr(adv, "d", None) + transfer, data = getattr(adv, "t", _ABSENT), getattr(adv, "d", _ABSENT) + if transfer is _ABSENT or data is _ABSENT: + # Not a malformed advertisement but an RNS that renamed the fields: every Resource is refused (failing + # open would undo the guard), so say why, loudly + _report_unsupported() + return "malformed sizes" if type(transfer) is not int or type(data) is not int or transfer < 0 or data < 0: return "malformed sizes" if getattr(adv, "u", False): diff --git a/rnfs/network/identity.py b/rnfs/network/identity.py index bb89e43..6ecfe1e 100644 --- a/rnfs/network/identity.py +++ b/rnfs/network/identity.py @@ -1,7 +1,6 @@ """Node identities: the RNS key pair a node keeps across restarts, and the node IDs derived from it.""" import os import stat -import tempfile import RNS @@ -38,31 +37,17 @@ def save_identity(identity: RNS.Identity, path: str, replace: bool = True) -> bo meanwhile. Not Identity.to_file, which creates the file with the process umask (often group- or world-readable) and could leave a truncated key behind if interrupted. """ - directory = os.path.dirname(os.path.abspath(path)) - # mkstemp creates the file with O_CREAT | O_EXCL and mode 0600 (IDENTITY_MODE), which a umask can only - # narrow, so no chmod is needed (and os.fchmod doesn't exist on Windows before Python 3.13). On Windows - # the file inherits the rnfs dir's ACL instead: under the user's profile, only them, admins and SYSTEM - fd, temp_path = tempfile.mkstemp(dir=directory, prefix=".identity-", suffix=".tmp") + # The file is created with O_CREAT | O_EXCL and mode 0600 (IDENTITY_MODE), which a umask can only narrow, + # so no chmod is needed (and os.fchmod doesn't exist on Windows before Python 3.13). On Windows the file + # inherits the rnfs dir's ACL instead: under the user's profile, only them, admins and SYSTEM. Without + # replace, rename_new keeps an existing file atomically where the platform allows (hard link, + # RENAME_NOREPLACE) try: - with os.fdopen(fd, "wb") as f: - f.write(identity.get_private_key()) - f.flush() - os.fsync(f.fileno()) + fsutil.write_atomic(path, identity.get_private_key(), mode=IDENTITY_MODE, exclusive=not replace) + except FileExistsError: if replace: - fsutil.replace(temp_path, path) - else: - try: - fsutil.rename_new(temp_path, path) # atomic where the platform allows (hard link, RENAME_NOREPLACE) - except FileExistsError: - fsutil.remove(temp_path) - return False - fsutil.sync_directory(directory) # the new name survives a power cut, not just the key's bytes - except BaseException: - try: - fsutil.remove(temp_path) - except OSError: - pass - raise + raise + return False return True diff --git a/rnfs/network/links.py b/rnfs/network/links.py index 6815fae..d487156 100644 --- a/rnfs/network/links.py +++ b/rnfs/network/links.py @@ -26,14 +26,14 @@ from rnfs.config import Config from rnfs.constants import APP_NAME, DHT_ASPECT, MAX_REQUESTS_PER_LINK from rnfs.log import LOG_VERBOSE, LOG_WARNING, log, node_name -from rnfs.network import guards, rates +from rnfs.network import guards, protocol, rates POLL_INTERVAL = 0.25 # how often a waiting request checks its link, progress and stall conditions CANCEL_GRACE = 2.0 # seconds a started response may be missing from the link's incoming Resources ESTABLISH_GRACE = 5 # seconds past RNS's own establishment timeout before giving up on a new link DEADLINE_FACTOR = 2 # a request other than /blocks may take this many times RNS's own request timeout # What RNS adds to rtt × traffic_timeout_factor for its request timeout (Link.request with timeout=None) -RESPONSE_GRACE = RNS.Resource.RESPONSE_MAX_GRACE_TIME * 1.125 +RESPONSE_GRACE = protocol.REQUEST_TIMEOUT_WITHOUT_RTT class Exchange: @@ -118,13 +118,6 @@ def mdu(self) -> int | None: except Exception: return None - def mtu(self) -> int | None: - """The link's MTU, which classifies it as slow before it has a rate estimate.""" - try: - return self.link.get_mtu() - except Exception: - return None - def acquire(self, blocks: bool, timeout: float | None = None) -> bool: """Wait for a request slot (and the /blocks slot if blocks). False if the link closed or timeout passed.""" deadline = None if timeout is None else time.monotonic() + timeout diff --git a/rnfs/network/protocol.py b/rnfs/network/protocol.py index 9cbe3a9..4d9be40 100644 --- a/rnfs/network/protocol.py +++ b/rnfs/network/protocol.py @@ -25,6 +25,9 @@ __all__ = ["FIND_PATH", "PROVIDE_PATH", "BLOCKS_PATH", "PIN_PATH"] DEFAULT_MDU = RNS.Link.MDU # packet MDU of a link at the default 500-byte MTU (431 bytes) +# RNS's request timeout without RTT (11.25 s): what it adds to rtt × traffic_timeout_factor (Link.request with +# timeout=None). Upload pacing holds a /blocks response back at most half of it, 5.625 s (section 3) +REQUEST_TIMEOUT_WITHOUT_RTT = RNS.Resource.RESPONSE_MAX_GRACE_TIME * 1.125 # Request sizes and batching diff --git a/rnfs/network/server.py b/rnfs/network/server.py index 55d55e4..d1bb006 100644 --- a/rnfs/network/server.py +++ b/rnfs/network/server.py @@ -39,7 +39,7 @@ MONITOR_TICK = 0.25 # Longest a /blocks response is held back for upload pacing: half of RNS's request timeout with no RTT, so the # requester doesn't give up meanwhile. Not RTT-based: the requester can inflate the link's RTT -MAX_PACING_WAIT = RNS.Resource.RESPONSE_MAX_GRACE_TIME * 1.125 / 2 +MAX_PACING_WAIT = protocol.REQUEST_TIMEOUT_WITHOUT_RTT / 2 CLOSED_MEMORY = 1024 # closed incoming links remembered, so a late request can't track them again @@ -618,12 +618,11 @@ def _handle_blocks(self, path, data, request_id, link_id, remote_identity): self.wake.set() return body finally: - if handed_over: - pass - elif response is not None: - self._finish(response) # frees the pool slot and the link's /blocks slot exactly once - else: - self.incoming.finish_request(link_id, blocks=True) + if not handed_over: + if response is not None: + self._finish(response) # frees the pool slot and the link's /blocks slot exactly once + else: + self.incoming.finish_request(link_id, blocks=True) @staticmethod def _pace(response: Response, wait: float): diff --git a/rnfs/node.py b/rnfs/node.py index 2ae0d58..1dfc922 100644 --- a/rnfs/node.py +++ b/rnfs/node.py @@ -23,7 +23,7 @@ from rnfs import __version__, features, fsutil from rnfs.config import Config -from rnfs.constants import APP_NAME, DHT_ASPECT, MODE_SERVER +from rnfs.constants import APP_NAME, DHT_ASPECT, MODE_SERVER, PARTIAL_SUFFIX from rnfs.content import archive, builder, dag, edit, walker from rnfs.content.cid import Cid from rnfs import node_content # --- content: add --no-pin/--to/--chunker, sealed files, gateway formats --- @@ -97,9 +97,6 @@ def _check_free_space(path: str, needed: int): raise TooLarge(f"{size(needed)} won't fit in the {size(free)} free in {directory}") -PARTIAL_SUFFIX = ".partial" # get writes out under .rnfs-get-.partial, then renames it - - def _remove_partial(path: str): """Delete a partial get output (file or directory tree) if there is one; a symlink is removed, not followed.""" try: diff --git a/rnfs/node_content.py b/rnfs/node_content.py index 9e1124b..4e7d4e8 100644 --- a/rnfs/node_content.py +++ b/rnfs/node_content.py @@ -211,9 +211,13 @@ def export_stream(node, target: str, start: Callable[[int], BinaryIO | None], fo # API parameters -def add_options(value: Callable[[str], object]) -> dict: +def add_options(value: Callable[[str], object], stream: bool = False) -> dict: """The add options this feature adds (pin, to, to_self, seal_name, chunker), from a JSON body's or a query - string's value(name) (None when absent); only those set, so the facade's defaults apply otherwise.""" + string's value(name) (None when absent); only those set, so the facade's defaults apply otherwise. + + seal_name means two things: for POST /api/add (JSON) whether to seal the file's name, which must be a JSON + boolean (ValueError otherwise); for POST /api/add/stream (stream, a query string) the name to seal inside. + """ from rnfs.api import parse_bool options: dict = {} @@ -228,9 +232,12 @@ def add_options(value: Callable[[str], object]) -> dict: options["to"] = to if value("to_self") is not None and parse_bool(value("to_self")): options["to_self"] = True - if value("seal_name") is not None: - seal_name = value("seal_name") - options["seal_name"] = parse_bool(seal_name) if isinstance(seal_name, bool) else seal_name + seal_name = value("seal_name") + if seal_name is not None: + # isinstance, not ==: True == 1, and a string like "false" would count as true + if not stream and not isinstance(seal_name, bool): + raise ValueError("seal_name must be true or false") + options["seal_name"] = seal_name if value("chunker"): from rnfs.content import chunker options["chunker"] = chunker.check(str(value("chunker"))) diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..7de784f --- /dev/null +++ b/tests/__init__.py @@ -0,0 +1 @@ +"""rnfs's tests: python3 -m unittest discover tests (support.py holds what they share).""" diff --git a/tests/support.py b/tests/support.py index 1c5fed7..efa63ab 100644 --- a/tests/support.py +++ b/tests/support.py @@ -1,6 +1,9 @@ -"""What the platform running the tests can do, for skipping tests that need POSIX features (not a test module).""" +"""Shared test helpers (not a test module): what the platform running the tests can do, for skipping tests that need +POSIX features, and small doubles and helpers several test modules use.""" import os +import socket import tempfile +import time # Unix permission bits (modes, umask, the execute bit): Windows has none, and its st_mode is synthesised POSIX_MODES = os.name != "nt" @@ -29,3 +32,40 @@ def real(path: str) -> str: 8.3 short name (RUNNER~1), so the same directory can be spelled differently by the CLI and the test. """ return os.path.normcase(os.path.realpath(path)) + + +class FakeClock: + """A settable clock tests move by hand (clock.now = ...), used for wall or monotonic time.""" + + def __init__(self, now: float = 1000.0): + """Start at now.""" + self.now = now + + def __call__(self) -> float: + """The current time.""" + return self.now + + +def free_port() -> int: + """A local TCP port that is free now (nothing is listening on it).""" + with socket.socket() as s: + s.bind(("127.0.0.1", 0)) + return s.getsockname()[1] + + +def wait_for(condition, timeout: float = 10.0, what: str | None = None, interval: float = 0.02): + """Poll condition until it returns something truthy and return that. + + After timeout seconds, fail with an AssertionError naming what if it's given; otherwise return condition's + last (falsy) value, for self.assertTrue(wait_for(...), message). + """ + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + result = condition() + if result: + return result + time.sleep(interval) + result = condition() + if not result and what is not None: + raise AssertionError(f"timed out waiting for {what}") + return result diff --git a/tests/test_api.py b/tests/test_api.py index 0d789c3..6814f2f 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -4,8 +4,10 @@ """ import hashlib import http.client +import inspect import json import os +import select import socket import tempfile import threading @@ -17,6 +19,7 @@ from rnfs.api import ApiHandler, ApiServer, BodyReader, LocalServer, loggable from rnfs.content.cid import DAG, RAW, Cid from rnfs.errors import Busy, InvalidDag, NotFound, RnfsError, StorageFull, TooLarge +from tests.support import wait_for LocalServer.poll_interval = 0.01 # so each test's server stops at once @@ -52,6 +55,11 @@ def called(self, method) -> list[tuple]: """The (args, kwargs) of every call to method.""" return [(args, kwargs) for name, args, kwargs in self.calls if name == method] + def _given(self, method: str, **options) -> dict: + """The options not at the facade method's defaults: recorded only when a caller sets them.""" + parameters = inspect.signature(getattr(FakeNode, method)).parameters + return {name: value for name, value in options.items() if value != parameters[name].default} + # Building content def put_file(self, data: bytes, executable=False, remote=False) -> str: @@ -143,8 +151,10 @@ def _add_tree(self, path: str) -> str: # Facade - def add_path(self, path, wrap=False, only_hash=False): - self.record("add_path", path, wrap=wrap, only_hash=only_hash) + def add_path(self, path, wrap=False, only_hash=False, pin=True, to=None, to_self=False, seal_name=True, + chunker=None): + self.record("add_path", path, wrap=wrap, only_hash=only_hash, + **self._given("add_path", pin=pin, to=to, to_self=to_self, seal_name=seal_name, chunker=chunker)) if not os.path.isabs(path): raise ValueError("path must be absolute") if not os.path.exists(path): @@ -152,24 +162,26 @@ def add_path(self, path, wrap=False, only_hash=False): cid = self._add_tree(path) if wrap: cid = self.put_dir({os.path.basename(path): cid}) - if not only_hash: + if pin and not only_hash: self.pins.setdefault(cid, set()).add("user") return {"cid": cid, "size": self._size(cid), "warnings": ["skipped a symlink"] if wrap else []} - def add_stream(self, stream, size=None, name=None, only_hash=False): + def add_stream(self, stream, size=None, name=None, only_hash=False, pin=True, to=None, to_self=False, + seal_name=None, chunker=None): data = stream.read() - self.record("add_stream", data, size=size, name=name, only_hash=only_hash) + self.record("add_stream", data, size=size, name=name, only_hash=only_hash, + **self._given("add_stream", pin=pin, to=to, to_self=to_self, seal_name=seal_name, chunker=chunker)) if size is not None and len(data) != size: raise ValueError("stream shorter than its size") cid = self.put_file(data) if name: cid = self.put_dir({name: cid}) - if not only_hash: + if pin and not only_hash: self.pins.setdefault(cid, set()).add("user") return {"cid": cid, "size": len(data), "warnings": []} - def get(self, target, output, from_node=None): - self.record("get", target, output, from_node=from_node) + def get(self, target, output, from_node=None, raw=False): # nothing is sealed here, so raw changes nothing + self.record("get", target, output, from_node=from_node, **self._given("get", raw=raw)) cid = self._resolve(target) def write(cid, dest): @@ -184,15 +196,16 @@ def write(cid, dest): write(cid, output) return {"cid": cid, "warnings": []} - def info(self, target, for_gateway=False, from_node=None): - self.record("info", target, for_gateway=for_gateway, from_node=from_node) + def info(self, target, for_gateway=False, from_node=None, raw=False): + self.record("info", target, for_gateway=for_gateway, from_node=from_node, **self._given("info", raw=raw)) cid = self._resolve(target) obj = self.objects[cid] return {"cid": cid, "type": "file" if obj[0] == "file" else "directory", "size": self._size(cid), "executable": obj[0] == "file" and obj[2]} - def read(self, target, offset=0, length=None, for_gateway=False, from_node=None): - self.record("read", target, offset=offset, length=length, for_gateway=for_gateway, from_node=from_node) + def read(self, target, offset=0, length=None, for_gateway=False, from_node=None, raw=False): + self.record("read", target, offset=offset, length=length, for_gateway=for_gateway, from_node=from_node, + **self._given("read", raw=raw)) cid = self._resolve(target) obj = self.objects[cid] if obj[0] != "file": @@ -506,6 +519,32 @@ def test_add_path(self): self.assertEqual(self.call("POST", "/api/add", b"not json")[0], 400) self.assertEqual(self.call("POST", "/api/add", b"[1]")[0], 400) + def test_add_seal_name_must_be_a_boolean(self): + """For /api/add seal_name says whether to seal the name: only JSON true and false are accepted, since a + string like "false" would count as true, and 1 == True in Python. Absent or null: the facade's default.""" + self.node.add_path = mock.Mock(return_value={"cid": "x", "size": 0, "warnings": []}) + path = os.path.join(tempfile.gettempdir(), "f.txt") + for value in (True, False): + self.node.add_path.reset_mock() + self.ok("POST", "/api/add", {"path": path, "to_self": True, "seal_name": value}) + self.assertIs(self.node.add_path.call_args.kwargs["seal_name"], value) + for body in ({"path": path, "to_self": True}, {"path": path, "to_self": True, "seal_name": None}): + self.node.add_path.reset_mock() + self.ok("POST", "/api/add", body) + self.assertNotIn("seal_name", self.node.add_path.call_args.kwargs) + self.node.add_path.reset_mock() + for value in ("false", 1, 0, "true", [], {}): + status, _, data = self.call("POST", "/api/add", {"path": path, "to_self": True, "seal_name": value}) + self.assertEqual(status, 400, value) + self.assertEqual(json.loads(data)["error"], "seal_name must be true or false") + self.node.add_path.assert_not_called() + + def test_add_stream_seal_name_is_the_name(self): + """For /api/add/stream seal_name is the name sealed inside, a string, as before.""" + self.node.add_stream = mock.Mock(return_value={"cid": "x", "size": 0, "warnings": []}) + self.ok("POST", "/api/add/stream?to_self=1&seal_name=false", b"hello") + self.assertEqual(self.node.add_stream.call_args.kwargs["seal_name"], "false") + def test_add_stream_content_length(self): result = self.ok("POST", "/api/add/stream?name=a.txt&only_hash=1", b"hello world") (data,), kwargs = self.node.called("add_stream")[0] @@ -684,6 +723,11 @@ def held_connections(port: int, count: int, data: bytes = b"GET /") -> list[sock return connections +def free_slots(server: LocalServer) -> int: + """How many more connections server would serve now (its slots semaphore's count).""" + return server.server.slots._value + + class ConnectionTest(ApiTestCase): """Time limits and the connection cap, with short limits so the tests are quick.""" @@ -762,7 +806,7 @@ def test_connection_cap(self): held = held_connections(self.port, 3) for sock in held: self.addCleanup(sock.close) - time.sleep(0.1) # let the server accept them + self.assertTrue(wait_for(lambda: free_slots(self.server) == 0)) # the server has accepted them status, headers, data = self.call("GET", "/api/status") self.assertEqual(status, 503) self.assertIn("too many", json.loads(data)["error"]) @@ -771,7 +815,7 @@ def test_connection_cap(self): # The held connections time out, freeing their slots for sock in held: read_until_closed(sock) - time.sleep(0.1) + self.assertTrue(wait_for(lambda: free_slots(self.server) == self.LIMITS["max_connections"])) self.assertEqual(self.call("GET", "/api/status")[0], 200) def test_log_escapes_the_target(self): @@ -849,7 +893,8 @@ def test_refusal_before_reading_the_body_is_received(self): self.addCleanup(sock.close) sock.sendall(f"POST /api/add/stream HTTP/1.1\r\nHost: 127.0.0.1:{self.port}\r\n" f"Content-Length: {len(body)}\r\n\r\n".encode() + body[:16384]) - time.sleep(0.3) # the server answers and closes its side meanwhile + # The server answers, and closes its side, before reading the body + self.assertTrue(select.select([sock], [], [], 10)[0], "no response") try: sock.sendall(body[16384:]) # the rest of the body arrives after the response was sent except OSError: diff --git a/tests/test_chunker.py b/tests/test_chunker.py index 90796aa..8025b5c 100644 --- a/tests/test_chunker.py +++ b/tests/test_chunker.py @@ -15,10 +15,7 @@ from rnfs.content.cid import Cid from rnfs.node import RnfsNode -try: - from tests.test_names import FakeNetwork -except ImportError: # run from inside tests/ - from test_names import FakeNetwork +from tests.test_names import FakeNetwork # Fixed by the specification text: every implementation must produce these VECTOR_LENGTHS = [68012, 67020, 76641, 76118, 65772, 99649, 18749, 68089, 81228, 74064, 66952, 86699, 75467, diff --git a/tests/test_cli.py b/tests/test_cli.py index 790646b..3cf2eac 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -3,7 +3,6 @@ import http.server import io import os -import socket import stat import subprocess import sys @@ -15,23 +14,12 @@ from rnfs import cli, fsutil from rnfs.api import ApiServer from rnfs.content.cid import RAW, Cid -from tests.support import CAN_SYMLINK, POSIX_MODES, real - -try: - from tests.test_api import FakeNode -except ImportError: # run from inside tests/ - from test_api import FakeNode +from tests.support import CAN_SYMLINK, POSIX_MODES, free_port, real +from tests.test_api import FakeNode ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) -def free_port() -> int: - """A local port nothing is listening on.""" - with socket.socket() as s: - s.bind(("127.0.0.1", 0)) - return s.getsockname()[1] - - class CliTestCase(unittest.TestCase): """Base: a FakeNode behind an ApiServer whose token is in /api_token, and run_cli() to call cli.main.""" diff --git a/tests/test_client.py b/tests/test_client.py index b474266..07cff8d 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -7,7 +7,6 @@ import json import os import random -import socket import subprocess import sys import tempfile @@ -26,6 +25,7 @@ from rnfs.network import client, links, protocol # noqa: E402 import tests.test_links as fakes # noqa: E402 (a module, so its test cases don't run twice) +from tests.support import free_port # noqa: E402 NODE = bytes(16) KEY = bytes(range(16)) @@ -316,13 +316,6 @@ def test_no_cids(self): E2E_MISSING = 1234 # seed of a block nobody serves -def free_port() -> int: - """A local TCP port that is currently free.""" - with socket.socket() as s: - s.bind(("127.0.0.1", 0)) - return s.getsockname()[1] - - def start_reticulum(configdir: str): """Start this process's Reticulum instance from configdir.""" import RNS diff --git a/tests/test_contacts.py b/tests/test_contacts.py index f130820..7dc5247 100644 --- a/tests/test_contacts.py +++ b/tests/test_contacts.py @@ -20,12 +20,8 @@ from rnfs.features.contacts import check_nick, is_nick, node_arg from rnfs.node import RnfsNode -try: - from tests.test_names import FakeNetwork - from tests.test_node import NetClient -except ImportError: # run from inside tests/ - from test_names import FakeNetwork - from test_node import NetClient +from tests.test_names import FakeNetwork +from tests.test_node import NetClient A_ID, B_ID = bytes([0xA] * 16), bytes([0xB] * 16) diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 65bda60..a141259 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -4,18 +4,20 @@ Each has its own --dir, API port and gateway port, and short intervals in its rnfs.toml. Takes about a minute. """ import filecmp +import functools import json import os import signal -import socket import subprocess import sys import tempfile -import time import unittest import urllib.error import urllib.request +from tests import support +from tests.support import free_port + ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) MAIN = os.path.join(ROOT, "rnfs.py") MISSING_CID = "bafkreidhtnhwyqkmbhxvp2mrery66f4kpdsfbxew3qrfqrbtwbs6zb5zce" # content no node has @@ -64,22 +66,8 @@ """ -def free_port() -> int: - """A local TCP port that is currently free.""" - with socket.socket() as s: - s.bind(("127.0.0.1", 0)) - return s.getsockname()[1] - - -def wait_for(condition, timeout: float, what: str): - """Poll condition until it returns something truthy and return that; fail after timeout seconds.""" - deadline = time.monotonic() + timeout - while time.monotonic() < deadline: - result = condition() - if result: - return result - time.sleep(0.5) - raise AssertionError(f"timed out waiting for {what}") +# Every poll here runs the CLI or calls a daemon's API, so poll every half second +wait_for = functools.partial(support.wait_for, interval=0.5) def same_tree(a: str, b: str) -> bool: diff --git a/tests/test_fake_contract.py b/tests/test_fake_contract.py new file mode 100644 index 0000000..9d2c670 --- /dev/null +++ b/tests/test_fake_contract.py @@ -0,0 +1,48 @@ +"""The facade fake the API, gateway, CLI and jobs tests run against (tests.test_api.FakeNode) must keep matching +the real facade (rnfs.node.RnfsNode), so a change to RnfsNode can't leave those tests passing against a stale fake.""" +import inspect +import unittest + +from rnfs.node import RnfsNode +from tests.test_api import FakeNode + +# FakeNode's own test helpers, not facade methods: building content, and inspecting the calls it recorded +FAKE_ONLY = {"record", "called", "put_file", "put_dir"} + + +def fake_methods() -> dict: + """FakeNode's public methods, by name.""" + return {name: value for name, value in vars(FakeNode).items() if not name.startswith("_") and callable(value)} + + +def parameters(function) -> list[tuple]: + """(name, kind, default) of each of function's parameters.""" + return [(p.name, p.kind, p.default) for p in inspect.signature(function).parameters.values()] + + +class FakeNodeContractTest(unittest.TestCase): + """Every facade method FakeNode has exists on RnfsNode, with the same parameter names, kinds and defaults.""" + + def test_methods_exist_on_the_facade(self): + for name in fake_methods().keys() - FAKE_ONLY: + with self.subTest(name): + self.assertTrue(callable(getattr(RnfsNode, name, None)), f"RnfsNode has no method {name}") + + def test_signatures_match(self): + for name, method in fake_methods().items(): + real = getattr(RnfsNode, name, None) + if name in FAKE_ONLY or real is None: # a missing method fails the test above + continue + with self.subTest(name): + self.assertEqual(parameters(method), parameters(real), + f"FakeNode.{name}{inspect.signature(method)} doesn't match " + f"RnfsNode.{name}{inspect.signature(real)}") + + def test_helpers_are_not_facade_methods(self): + # A helper named like a facade method would be skipped by the checks above + self.assertEqual(FAKE_ONLY & set(dir(RnfsNode)), set()) + self.assertLessEqual(FAKE_ONLY, fake_methods().keys()) # no stale entries + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_fsutil.py b/tests/test_fsutil.py index 779e1f9..4857e93 100644 --- a/tests/test_fsutil.py +++ b/tests/test_fsutil.py @@ -1,11 +1,15 @@ -"""Tests for fsutil: replace and remove retried while Windows reports a file in use, Windows name rules.""" +"""Tests for fsutil: replace and remove retried while Windows reports a file in use, atomic writes, Windows +name rules.""" +import errno import os +import stat import sys import tempfile import unittest from unittest import mock from rnfs import fsutil +from tests.support import CAN_SYMLINK, POSIX_MODES def in_use(failures: int, real): @@ -151,6 +155,119 @@ def test_atomic_on_linux(self): self.assertEqual(f.read(), b"old") +class WriteAtomicTest(unittest.TestCase): + """write_atomic replaces a file whole or not at all, never through a symlink, and leaves no temporary file.""" + + def setUp(self): + """A temporary directory with an existing file in it.""" + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + self.path = os.path.join(self.tmp.name, "file") + with open(self.path, "wb") as f: + f.write(b"old") + + def contents(self) -> bytes: + """What the file at self.path holds.""" + with open(self.path, "rb") as f: + return f.read() + + def assert_only_file(self, data: bytes): + """The directory holds the file with data, and no temporary file.""" + self.assertEqual(os.listdir(self.tmp.name), ["file"]) + self.assertEqual(self.contents(), data) + + def test_replaces_an_existing_file(self): + fsutil.write_atomic(self.path, b"new\r\n\x00") + self.assert_only_file(b"new\r\n\x00") # binary: no newline translation on Windows + os.remove(self.path) + fsutil.write_atomic(self.path, b"") + self.assert_only_file(b"") + + @unittest.skipUnless(POSIX_MODES, "Windows has no POSIX permission bits") + def test_mode(self): + umask = os.umask(0) + try: + for mode in (0o600, 0o644, 0o640): + with self.subTest(mode=oct(mode)): + fsutil.write_atomic(self.path, b"x", mode=mode) + self.assertEqual(stat.S_IMODE(os.stat(self.path).st_mode), mode) + os.umask(0o077) # the umask narrows the mode, as it does for any new file + fsutil.write_atomic(self.path, b"x") + self.assertEqual(stat.S_IMODE(os.stat(self.path).st_mode), 0o600) + finally: + os.umask(umask) + + @unittest.skipUnless(CAN_SYMLINK, "symlinks") + def test_symlink_at_path_is_replaced_not_followed(self): + elsewhere = os.path.join(self.tmp.name, "elsewhere") + os.remove(self.path) + os.symlink(elsewhere, self.path) + fsutil.write_atomic(self.path, b"new") + self.assertFalse(os.path.islink(self.path)) + self.assert_only_file(b"new") + + @unittest.skipUnless(CAN_SYMLINK, "symlinks") + def test_refuses_a_symlink_at_the_temporary_name(self): + # Someone able to guess the name plants a symlink there: it is neither followed nor removed + temp = os.path.join(self.tmp.name, fsutil.PARTIAL_PREFIX + "planted") + target = os.path.join(self.tmp.name, "target") + os.symlink(target, temp) + with mock.patch.object(fsutil, "_temp_path", return_value=temp): + with self.assertRaises(FileExistsError): + fsutil.write_atomic(self.path, b"new") + self.assertFalse(os.path.lexists(target)) + self.assertTrue(os.path.islink(temp)) + self.assertEqual(self.contents(), b"old") + + def test_temporary_file_is_in_the_same_directory_and_prefixed(self): + temps = [] + real_replace = fsutil.replace + with mock.patch.object(fsutil, "replace", lambda src, dst: (temps.append(src), real_replace(src, dst))): + fsutil.write_atomic(self.path, b"new") + self.assertEqual(os.path.dirname(temps[0]), self.tmp.name) + self.assertTrue(os.path.basename(temps[0]).startswith(fsutil.PARTIAL_PREFIX)) + self.assertNotEqual(fsutil._temp_path(self.tmp.name), fsutil._temp_path(self.tmp.name)) + + def test_failure_mid_write_removes_the_temporary_file(self): + failures = [ + ("fsync", mock.patch("os.fsync", side_effect=OSError(errno.EIO, "I/O error")), OSError), + ("interrupt", mock.patch("os.fsync", side_effect=KeyboardInterrupt), KeyboardInterrupt), + ("rename", mock.patch.object(fsutil, "replace", side_effect=PermissionError(13, "in use")), + PermissionError), + ] + for stage, patch, error in failures: + with self.subTest(stage): + with patch, self.assertRaises(error): + fsutil.write_atomic(self.path, b"new") + self.assert_only_file(b"old") # the original is left as it was + + def test_exclusive_keeps_an_existing_file(self): + with self.assertRaises(FileExistsError): + fsutil.write_atomic(self.path, b"new", exclusive=True) + self.assert_only_file(b"old") + os.remove(self.path) + fsutil.write_atomic(self.path, b"new", exclusive=True) + self.assert_only_file(b"new") + + def test_directory_synced_only_when_durable(self): + with mock.patch.object(fsutil, "sync_directory") as sync: + fsutil.write_atomic(self.path, b"new") + sync.assert_called_once_with(self.tmp.name) + with mock.patch.object(fsutil, "sync_directory") as sync: + fsutil.write_atomic(self.path, b"newer", durable=False) + sync.assert_not_called() + self.assert_only_file(b"newer") + + @unittest.skipIf(fsutil.IS_WINDOWS, "Windows can't sync a directory") + def test_synced_before_and_after_the_rename(self): + events = [] + real_fsync, real_replace = os.fsync, fsutil.replace + with mock.patch("os.fsync", side_effect=lambda fd: (events.append("fsync"), real_fsync(fd))[1]), \ + mock.patch.object(fsutil, "replace", lambda *a: (events.append("rename"), real_replace(*a))[1]): + fsutil.write_atomic(self.path, b"new") + self.assertEqual(events, ["fsync", "rename", "fsync"]) + + class WindowsNameTest(unittest.TestCase): """Names Windows can't take as file names, and the retry budget.""" diff --git a/tests/test_gateway.py b/tests/test_gateway.py index e1cbbf5..dc0629d 100644 --- a/tests/test_gateway.py +++ b/tests/test_gateway.py @@ -10,10 +10,8 @@ from rnfs.gateway import (CSP_NO_SCRIPTS, CSP_SCRIPTS, IMMUTABLE, GatewayServer, etag_matches, listing_html, parse_range, sniff) -try: - from tests.test_api import FakeNode, held_connections, raw_request, read_until_closed -except ImportError: # run from inside tests/ - from test_api import FakeNode, held_connections, raw_request, read_until_closed +from tests.support import wait_for +from tests.test_api import FakeNode, free_slots, held_connections, raw_request, read_until_closed SPEC_CSP = ("sandbox allow-scripts allow-forms allow-popups; default-src 'self' 'unsafe-inline' 'unsafe-eval' " "data: blob:; form-action 'self'") @@ -375,7 +373,7 @@ def hold(self, count: int) -> list[socket.socket]: held = held_connections(self.port, count, b"GET /rnfs/") for sock in held: self.addCleanup(sock.close) - time.sleep(0.1) # let the server accept them + self.assertTrue(wait_for(lambda: free_slots(self.server) == 0)) # the server has accepted them return held def test_idle_connection_is_closed(self): @@ -398,7 +396,7 @@ def test_connection_cap(self): self.assertEqual(self.node.called("info"), []) for sock in held: read_until_closed(sock) - time.sleep(0.1) + self.assertTrue(wait_for(lambda: free_slots(self.server) == 2)) self.assertEqual(self.get(f"/rnfs/{self.text}")[0], 200) def test_clients_that_stop_reading_are_dropped(self): @@ -410,7 +408,7 @@ def test_clients_that_stop_reading_are_dropped(self): sock.setsockopt(socket.SOL_SOCKET, socket.SO_RCVBUF, 4096) sock.connect(("127.0.0.1", self.port)) sock.sendall(f"GET /rnfs/{big} HTTP/1.1\r\nHost: 127.0.0.1:{self.port}\r\n\r\n".encode()) - time.sleep(0.2) + self.assertTrue(wait_for(lambda: free_slots(self.server) == 0)) self.assertEqual(self.get(f"/rnfs/{self.text}")[0], 503) # Writing to the stalled clients times out, which frees their slots deadline = time.monotonic() + 5 diff --git a/tests/test_gateway_formats.py b/tests/test_gateway_formats.py index 89ba24b..a32685c 100644 --- a/tests/test_gateway_formats.py +++ b/tests/test_gateway_formats.py @@ -17,12 +17,8 @@ from rnfs.gateway import CSP_SCRIPTS, GatewayServer from rnfs.node import RnfsNode -try: - from tests.test_api import raw_request, read_until_closed - from tests.test_names import FakeNetwork -except ImportError: # run from inside tests/ - from test_api import raw_request, read_until_closed - from test_names import FakeNetwork +from tests.test_api import raw_request, read_until_closed +from tests.test_names import FakeNetwork class FormatTest(unittest.TestCase): diff --git a/tests/test_identity.py b/tests/test_identity.py index ae1a41d..0c03432 100644 --- a/tests/test_identity.py +++ b/tests/test_identity.py @@ -38,7 +38,7 @@ def test_created_owner_only(self): self.assertEqual(os.listdir(self.directory), ["identity"]) # no temp file left behind def test_saved_without_fchmod(self): - # os.fchmod doesn't exist on Windows before Python 3.13; mkstemp's 0600 needs no chmod anyway + # os.fchmod doesn't exist on Windows before Python 3.13; creating the file 0600 needs no chmod anyway with mock.patch("os.fchmod", create=True, side_effect=AssertionError("fchmod called")): created = load_or_create_identity(self.path) self.assertEqual(load_or_create_identity(self.path).hash, created.hash) diff --git a/tests/test_jobs.py b/tests/test_jobs.py index 1166b80..f796bf1 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -21,16 +21,10 @@ from rnfs.content.storage import USER from rnfs.features import jobs from rnfs.node import RnfsNode -from tests.support import real - -try: - from tests.test_node import A_ID, B_ID, NetClient - from tests.test_names import FakeNetwork - from tests.test_api import FakeNode -except ImportError: # run from inside tests/ - from test_node import A_ID, B_ID, NetClient - from test_names import FakeNetwork - from test_api import FakeNode +from tests.support import real, wait_for +from tests.test_api import FakeNode +from tests.test_names import FakeNetwork +from tests.test_node import A_ID, B_ID, NetClient _saved_loglevel = RNS.loglevel @@ -45,16 +39,6 @@ def tearDownModule(): RNS.loglevel = _saved_loglevel -def wait_for(condition, timeout: float = 10.0) -> bool: - """Poll condition until true or timeout; its last value.""" - deadline = time.monotonic() + timeout - while time.monotonic() < deadline: - if condition(): - return True - time.sleep(0.02) - return bool(condition()) - - class GatedClient(NetClient): """A NetClient whose /blocks requests can be held at a gate, serving at most per_request blocks each.""" @@ -213,7 +197,7 @@ def test_cancel_queued_job_and_concurrency_limit(self): self.net.gate.clear() submitted = [self.jobs.submit("get", cid, self.out(f"q{i}"))["id"] for i, cid in enumerate(cids)] self.assertTrue(wait_for(lambda: self.net.waiting == 1)) - time.sleep(0.2) + time.sleep(0.2) # time for a second job to start, were the limit not kept self.assertEqual(self.jobs.status(), {"jobs_running": 1, "jobs_queued": 2}) self.assertEqual(self.net.waiting, 1) # only one job fetches self.assertEqual(self.jobs.cancel(submitted[2])["state"], "cancelled") diff --git a/tests/test_links.py b/tests/test_links.py index 39bcb43..d13bebd 100644 --- a/tests/test_links.py +++ b/tests/test_links.py @@ -7,6 +7,7 @@ from rnfs.config import Config from rnfs.network import links +from tests.support import wait_for NODE = bytes(16) OTHER = bytes([1]) * 16 @@ -361,8 +362,9 @@ def test_waiter_gets_slot_when_released(self): self.peer.acquire(True) got = [] thread = threading.Thread(target=lambda: got.append(self.peer.acquire(True, timeout=5))) - thread.start() - time.sleep(0.1) + with mock.patch.object(self.peer.cond, "wait", wraps=self.peer.cond.wait) as wait: + thread.start() + self.assertTrue(wait_for(lambda: wait.called)) # it waits for the slot self.peer.release(True) thread.join(5) self.assertEqual(got, [True]) @@ -371,8 +373,9 @@ def test_waiter_fails_when_link_closes(self): self.peer.acquire(True) got = [] thread = threading.Thread(target=lambda: got.append(self.peer.acquire(True))) - thread.start() - time.sleep(0.1) + with mock.patch.object(self.peer.cond, "wait", wraps=self.peer.cond.wait) as wait: + thread.start() + self.assertTrue(wait_for(lambda: wait.called)) # it waits for the slot self.peer.mark_closed() thread.join(5) self.assertEqual(got, [False]) diff --git a/tests/test_log.py b/tests/test_log.py index 842130d..057f44b 100644 --- a/tests/test_log.py +++ b/tests/test_log.py @@ -23,6 +23,12 @@ def test_helpers_share_one_implementation(self): self.assertIs(api.loggable, fsutil.printable) self.assertIs(cli.printable, fsutil.printable) + def test_feature_log_levels_are_rns_levels(self): + """rnfs.features._common names RNS's loglevels without importing RNS: they must stay the same.""" + from rnfs.features import _common + for level in ("LOG_ERROR", "LOG_WARNING", "LOG_NOTICE", "LOG_INFO", "LOG_VERBOSE"): + self.assertEqual(getattr(_common, level), getattr(log, level), level) + def test_cap_warnings(self): self.assertEqual(log.cap_warnings(["a", "b"]), {"warnings": ["a", "b"]}) capped = log.cap_warnings([str(i) for i in range(log.MAX_WARNINGS + 1)]) diff --git a/tests/test_lxmf.py b/tests/test_lxmf.py index e2b9af7..31ed1d2 100644 --- a/tests/test_lxmf.py +++ b/tests/test_lxmf.py @@ -22,12 +22,8 @@ from rnfs.features.lxmf import FIELD_CUSTOM_DATA, FIELD_CUSTOM_TYPE, Incoming, Link, MessageLog from rnfs.node import RnfsNode -try: - from tests.test_names import FakeNetwork - from tests.test_node import NetClient -except ImportError: # run from inside tests/ - from test_names import FakeNetwork - from test_node import NetClient +from tests.test_names import FakeNetwork +from tests.test_node import NetClient ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) A_ID, B_ID = bytes([0xA] * 16), bytes([0xB] * 16) diff --git a/tests/test_mesh.py b/tests/test_mesh.py index d36a23d..fe76c85 100644 --- a/tests/test_mesh.py +++ b/tests/test_mesh.py @@ -16,13 +16,10 @@ from rnfs.network import protocol, server from rnfs.network.client import BUSY, FAILED, OK, BlocksResult, PeerClient from rnfs.node import RnfsNode +from tests.support import FakeClock from tests.test_fetcher import OWN, FakeClient, FetcherTest -from tests.test_server import Clock, FakeBackend, FakeDestination, FakeLink, FakeResource, block_of - -try: - from tests.test_names import FakeNetwork -except ImportError: # run from inside tests/ - from test_names import FakeNetwork +from tests.test_names import FakeNetwork +from tests.test_server import FakeBackend, FakeDestination, FakeLink, FakeResource, block_of P1, P2, P3, P4 = (bytes([i]) * 16 for i in (1, 2, 3, 4)) @@ -121,7 +118,7 @@ class TokenBucketTest(unittest.TestCase): """Continuous refill; a response goes if the bucket holds its size or is full.""" def test_spend_and_refill(self): - clock = Clock() + clock = FakeClock() bucket = mesh.TokenBucket(3600, 3600, clock) # 1 byte/s self.assertTrue(bucket.take(3000)) self.assertFalse(bucket.take(1000)) # 600 left @@ -132,7 +129,7 @@ def test_spend_and_refill(self): self.assertEqual(bucket.available(), 3600) # never above capacity def test_large_response_allowed_when_full(self): - clock = Clock() + clock = FakeClock() bucket = mesh.TokenBucket(1000, 3600, clock) self.assertTrue(bucket.take(5000)) # bigger than the whole budget, but the bucket was full self.assertLess(bucket.available(), 0) @@ -143,7 +140,7 @@ def test_large_response_allowed_when_full(self): self.assertTrue(bucket.take(5000)) # full again def test_give_back(self): - clock = Clock() + clock = FakeClock() bucket = mesh.TokenBucket(1000, 3600, clock) bucket.take(800) bucket.give_back(800) @@ -238,7 +235,7 @@ def test_quiet_hours(self): self.assertIsNone(policy.refusal(["any"])) def test_budget_limit(self): - clock = Clock() + clock = FakeClock() policy = mesh.AirtimePolicy(Config(upload_budget=100 * KIB), clock) self.assertEqual(policy.limit(500 * KIB), 500 * KIB) # full: any one response may go self.assertTrue(policy.spend(60 * KIB)) @@ -269,7 +266,7 @@ def setUp(self): """A server over fake RNS objects serving four 100 KiB blocks.""" saved = RNS.Resource.AUTO_COMPRESS_MAX_SIZE self.addCleanup(setattr, RNS.Resource, "AUTO_COMPRESS_MAX_SIZE", saved) - self.clock = Clock() + self.clock = FakeClock() self.big = [block_of(100 * KIB, i) for i in range(4)] self.backend = FakeBackend(blocks=dict(self.big)) self.links = 0 diff --git a/tests/test_names.py b/tests/test_names.py index 85a57cf..198ca28 100644 --- a/tests/test_names.py +++ b/tests/test_names.py @@ -14,7 +14,7 @@ from rnfs.content.cid import DAG, RAW, Cid from rnfs.errors import NotFound from rnfs.names import NameManager, encode_record, parse_record -from tests.support import POSIX_MODES +from tests.support import POSIX_MODES, FakeClock _saved_loglevel = RNS.loglevel @@ -133,18 +133,6 @@ def announce(self, name, app_data, heard): handler.received_announce(destination_hash=name, announced_identity=None, app_data=app_data) -class Clock: - """A settable clock, used for both wall and monotonic time.""" - - def __init__(self, now=1_000_000.0): - """Start at now.""" - self.now = now - - def __call__(self) -> float: - """The current time.""" - return self.now - - def other_name(i: int) -> bytes: """A name that isn't local.""" return bytes([i]) * 16 @@ -190,7 +178,7 @@ def setUp(self): announce_interval=3600, startup_announce_delay=30) self.pins = FakePins() self.network = FakeNetwork() - self.clock = Clock() + self.clock = FakeClock(1_000_000.0) self.manager = self.make_manager() def make_manager(self) -> NameManager: @@ -289,6 +277,18 @@ def test_keys_survive_restart(self): name = self.manager.create("site") self.assertEqual(self.make_manager().name("site"), name) + def test_old_temp_files_removed_at_start(self): + """Temp files 0.1.1's interrupted writes left under fixed names are deleted; nothing else is.""" + self.manager.create("site") + old = [os.path.join(self.config.state_dir, "names.json.tmp"), os.path.join(self.config.names_dir, ".gone.tmp")] + kept = [os.path.join(self.config.names_dir, name) for name in (".notes", "x.tmp")] + for path in old + kept: + with open(path, "wb") as f: + f.write(b"partial") + self.assertEqual(self.make_manager().list()[0]["alias"], "site") + self.assertEqual([os.path.exists(path) for path in old + kept], [False, False, True, True]) + self.assertEqual(sorted(os.listdir(self.config.state_dir)), ["names.json"]) # no temp file of its own + def test_export_import_round_trip(self): """An exported key imported elsewhere gives the same name.""" name = self.manager.create("site") diff --git a/tests/test_names_airtime.py b/tests/test_names_airtime.py index 1d48268..442b54f 100644 --- a/tests/test_names_airtime.py +++ b/tests/test_names_airtime.py @@ -27,16 +27,10 @@ from rnfs.node import RnfsNode from rnfs.publisher import KEY_MIN_INTERVAL, SLOW_LOOKUP_PACE, SLOW_ROUND_LOOKUPS, DhtKey, Publisher, key_interval -try: - from tests.test_names import FakeNetwork - from tests.test_names_keep import KeeperNet - from tests.test_node import NetClient - from tests.test_publisher import Clock, FakeClient, FakeLookup -except ImportError: # run from inside tests/ - from test_names import FakeNetwork - from test_names_keep import KeeperNet - from test_node import NetClient - from test_publisher import Clock, FakeClient, FakeLookup +from tests.support import FakeClock +from tests.test_names import FakeNetwork +from tests.test_names_keep import KeeperNet +from tests.test_publisher import FakeClient, FakeLookup A_ID, B_ID = bytes([0xA] * 16), bytes([0xB] * 16) N1, N2 = bytes([1]) * 16, bytes([2]) * 16 @@ -184,7 +178,7 @@ def make_publisher(self, keys: set[bytes], slow: bool = False, **settings) -> Pu from rnfs.content.storage import Storage config = Config(rnfs_dir=self.tmp.name, **settings) storage = Storage(os.path.join(self.tmp.name, "blocks"), os.path.join(self.tmp.name, "state"), 10**9) - self.clock = Clock() + self.clock = FakeClock(1_000_000.0) self.client = FakeClient() publisher = Publisher(bytes(16), config, storage, ProviderStore(10_000, None), self.client, lambda key: FakeLookup([N1, N2]), slow_node=slow, clock=self.clock) diff --git a/tests/test_names_keep.py b/tests/test_names_keep.py index d8aee38..43d7995 100644 --- a/tests/test_names_keep.py +++ b/tests/test_names_keep.py @@ -23,14 +23,10 @@ from rnfs.node import RnfsNode from rnfs.publisher import DhtKey -try: - from tests.test_names import FakeNetwork - from tests.test_node import NetClient - from tests.test_server import Clock, FakeBackend, FakeDestination, FakeLink -except ImportError: # run from inside tests/ - from test_names import FakeNetwork - from test_node import NetClient - from test_server import Clock, FakeBackend, FakeDestination, FakeLink +from tests.support import FakeClock +from tests.test_names import FakeNetwork +from tests.test_node import NetClient +from tests.test_server import FakeBackend, FakeDestination, FakeLink A_ID, B_ID, C_ID = bytes([0xA] * 16), bytes([0xB] * 16), bytes([0xC] * 16) CID1 = Cid.for_block(RAW, b"one") @@ -475,7 +471,7 @@ def make_server(self, config, handler): backend = FakeBackend(members={b"m" * 16}) backend.feature_requests = lambda: {NAME_PATH: handler} destination = FakeDestination() - server.RequestServer(destination, config, backend, clock=Clock(), start_monitor=False) + server.RequestServer(destination, config, backend, clock=FakeClock(), start_monitor=False) link = FakeLink(b"l" * 16) destination.links.append(link) destination.established(link) diff --git a/tests/test_node.py b/tests/test_node.py index 9405a38..4a85129 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -14,7 +14,7 @@ from rnfs.config import Config from rnfs.constants import CHUNK_SIZE from rnfs.content.cid import RAW, Cid -from rnfs.content.storage import USER, remote_source +from rnfs.content.storage import remote_source from rnfs import fetcher from rnfs.errors import InvalidDag, NotFound, RnfsError, StorageFull, TooLarge from rnfs.features.keepers import name_key @@ -22,11 +22,7 @@ from rnfs import fsutil, node as node_module from rnfs.node import RnfsNode from tests.support import POSIX_MODES - -try: - from tests.test_names import FakeNetwork -except ImportError: # run from inside tests/ - from test_names import FakeNetwork +from tests.test_names import FakeNetwork A_ID, B_ID, N1 = bytes([0xA] * 16), bytes([0xB] * 16), bytes([1] * 16) PROVIDE_BATCH = 22 # keys per /provide at the default MDU (section 3) diff --git a/tests/test_pacing.py b/tests/test_pacing.py index a2afecc..004e538 100644 --- a/tests/test_pacing.py +++ b/tests/test_pacing.py @@ -15,7 +15,8 @@ from rnfs.fetcher import Fetcher from rnfs.network import protocol, rates, server from rnfs.network.client import BUSY, OK, BlocksResult -from tests.test_server import Clock, FakeBackend, FakeDestination, FakeLink, FakeResource +from tests.support import FakeClock +from tests.test_server import FakeBackend, FakeDestination, FakeLink, FakeResource OWN, PROVIDER = bytes(16), bytes([1]) * 16 _saved_loglevel = RNS.loglevel @@ -49,7 +50,7 @@ def fetch(self, rate: float, rtt: float, chunks: int, server_config: Config | No The responder measures server_measures × the rate (RNS's sender-side span includes a round trip). Returns (refusals, virtual seconds taken); fails the test if the provider is dropped. """ - clock = Clock() + clock = FakeClock() start = clock.now network = {} built = builder.build_bytes(os.urandom(chunks * CHUNK_SIZE), network.__setitem__) diff --git a/tests/test_packaging.py b/tests/test_packaging.py index dbab8ac..ab5d2f6 100644 --- a/tests/test_packaging.py +++ b/tests/test_packaging.py @@ -59,6 +59,16 @@ def test_leaves_the_callers_directory(self): self.assertRegex(text, r'(?m)^if \[ -e "\$SOURCE" \]; then SOURCE=\$\(abs_path "\$SOURCE"\)') self.assertLess(text.index("\ncd /\n"), text.index("\ndo_install() {")) + def test_installs_the_latest_release_by_default(self): + """Without RNFS_SOURCE it installs the latest release's tag, or the main branch with --main.""" + choose = shell_function("choose_source") + self.assertEqual(choose.splitlines()[1].strip(), '[ -z "$SOURCE" ] || return 0') # RNFS_SOURCE wins + self.assertIn("SOURCE=$REPO_URL/archive/refs/tags/$tag.zip", choose) + self.assertIn("SOURCE=$REPO_URL/archive/refs/heads/main.zip", choose) + self.assertIn('"/releases/latest"', shell_function("latest_release")) + self.assertIn(" choose_source\n", shell_function("do_install")) + self.assertIn("--main) MAIN=yes ;;", read(INSTALL_SH)) + @unittest.skipUnless(shutil.which("sh"), "needs a POSIX shell") def test_systemd_quote(self): """The launcher path is one double-quoted ExecStart word with % doubled (LP-L5).""" @@ -138,6 +148,15 @@ def test_release_publishes_only_on_a_pushed_tag(self): self.assertFalse([line for line in uploads if "--clobber" in line]) self.assertRegex(text, r"actions/attest-build-provenance@[0-9a-f]{40}") + def test_release_waits_for_the_tests(self): + """The release job needs the whole tests workflow, called from release.yml.""" + release = read(os.path.join(WORKFLOWS, "release.yml")) + self.assertRegex(release, r"(?m)^ tests:\n uses: \./\.github/workflows/tests\.yml$") + needs = re.search(r"(?m)^ release:\n(?: .*\n)*? needs: \[(.*)\]$", release) + self.assertTrue(needs) + self.assertIn("tests", [n.strip() for n in needs.group(1).split(",")]) + self.assertRegex(read(os.path.join(WORKFLOWS, "tests.yml")), r"(?m)^ workflow_call:") + class DependencyLockTest(unittest.TestCase): """packaging/requirements.lock pins every dependency, with hashes, and the Flatpak uses only those.""" @@ -159,7 +178,7 @@ def test_build_locks(self): """The build locks are hashed, agree with each other and with requirements.lock, and fit pyproject.toml.""" backend, tools, runtime = read_lock(BACKEND_LOCK), read_lock(TOOLS_LOCK), read_lock() self.assertEqual(list(backend), ["setuptools"]) - for name in ("setuptools", "build", "twine", "pyinstaller", "pyinstaller-hooks-contrib"): + for name in ("setuptools", "build", "twine", "pyinstaller", "pyinstaller-hooks-contrib", "ruff"): self.assertIn(name, tools) self.assertEqual(backend["setuptools"], tools["setuptools"]) for name, (version, hashes) in [*backend.items(), *tools.items()]: @@ -191,5 +210,20 @@ def test_flatpak_wheels_are_locked(self): self.assertIn(source["sha256"], lock[name][1], filename) +class ReadmeTest(unittest.TestCase): + """README.md's release links and file names are for the current version, so a version bump updates them.""" + + def test_release_links_match_version(self): + """Every releases/download/vX.Y.Z/ link and rnfs-X.Y.Z / rnfs_X.Y.Z file name is __version__.""" + from rnfs import __version__ + text = read(os.path.join(REPO, "README.md")) + version = r"(\d+\.\d+\.\d+[\w.]*?)" + found = (re.findall(r"releases/download/v%s/" % version, text) + + re.findall(r"\brnfs[-_]%s(?:[-_]|\.tar\.gz)" % version, text) + + re.findall(r"These links are for %s;" % version, text)) + self.assertGreaterEqual(len(found), 10) + self.assertEqual(sorted(set(found)), [__version__]) + + if __name__ == "__main__": sys.exit(unittest.main()) diff --git a/tests/test_pin_false.py b/tests/test_pin_false.py index b910809..368c226 100644 --- a/tests/test_pin_false.py +++ b/tests/test_pin_false.py @@ -11,10 +11,7 @@ from rnfs.content.storage import USER from rnfs.node import RnfsNode -try: - from tests.test_names import FakeNetwork -except ImportError: # run from inside tests/ - from test_names import FakeNetwork +from tests.test_names import FakeNetwork class PinFalseTest(unittest.TestCase): diff --git a/tests/test_providers.py b/tests/test_providers.py index 119395e..ffc6096 100644 --- a/tests/test_providers.py +++ b/tests/test_providers.py @@ -10,6 +10,7 @@ from rnfs import fsutil from rnfs.constants import DAY, ID_LENGTH, K, PROVIDER_TTL from rnfs.dht.providers import MIN_PER_PUBLISHER, ProviderStore +from tests.support import FakeClock def ident(n: int) -> bytes: @@ -20,18 +21,6 @@ def ident(n: int) -> bytes: KEY = ident(0xABC) -class FakeClock: - """A manually advanced wall clock.""" - - def __init__(self): - """Clock at an arbitrary start time; tests advance it by setting now.""" - self.now = 1_700_000_000.0 - - def __call__(self) -> float: - """The current fake time.""" - return self.now - - class StoreTestCase(unittest.TestCase): """Base: an in-memory store with a fake clock and seeded rng.""" @@ -39,7 +28,7 @@ class StoreTestCase(unittest.TestCase): def setUp(self): """Create the store.""" - self.clock = FakeClock() + self.clock = FakeClock(1_700_000_000.0) self.store = ProviderStore(self.max_records, None, clock=self.clock, rng=random.Random(1)) @@ -329,7 +318,7 @@ def setUp(self): """A temporary state directory.""" self.directory = tempfile.TemporaryDirectory() self.path = os.path.join(self.directory.name, "state", "providers.json") - self.clock = FakeClock() + self.clock = FakeClock(1_700_000_000.0) def tearDown(self): """Remove the state directory.""" diff --git a/tests/test_publisher.py b/tests/test_publisher.py index 7f9b068..f8306b1 100644 --- a/tests/test_publisher.py +++ b/tests/test_publisher.py @@ -13,6 +13,7 @@ from rnfs.content.storage import USER, Storage from rnfs.dht.providers import ProviderStore from rnfs.publisher import SLOW_LOOKUP_PACE, SLOW_ROUND_LOOKUPS, STATE_FILE, Publisher +from tests.support import FakeClock OWN = bytes(16) N1, N2, N3 = (bytes([i]) * 16 for i in (1, 2, 3)) @@ -59,18 +60,6 @@ def find_closest(self): return list(self.nodes) -class Clock: - """A settable wall clock.""" - - def __init__(self): - """Start at an arbitrary time.""" - self.now = 1_000_000.0 - - def __call__(self): - """The current time.""" - return self.now - - class PublisherTest(unittest.TestCase): """A Publisher over temporary storage with fakes for the network.""" @@ -83,7 +72,7 @@ def setUp(self): self.storage = Storage(os.path.join(self.tmp.name, "blocks"), self.state, 10**9) self.records = ProviderStore(1000, None) self.client = FakeClient() - self.clock = Clock() + self.clock = FakeClock(1_000_000.0) self.nodes = [N1, N2] self.eligible: set[Cid] = set() self.lookups: list[bytes] = [] diff --git a/tests/test_rns_compat.py b/tests/test_rns_compat.py new file mode 100644 index 0000000..3b5a085 --- /dev/null +++ b/tests/test_rns_compat.py @@ -0,0 +1,509 @@ +"""The installed RNS and LXMF still have every internal rnfs relies on. + +rnfs reads private attributes of RNS and LXMF objects, replaces some of their methods, passes undocumented +keywords and depends on data layouts neither documents, none of which their version numbers promise to keep. +INTERNALS lists each one with the rnfs module relying on it, so a dependency bump (Dependabot, a lock update, a +user's own environment) fails here, naming the module to update, rather than at runtime. Written against RNS +1.5.6 and LXMF 1.2.0. + +Nothing here starts Reticulum: objects are built offline (Transport.register_destination patched out), and +the process-wide state touched (Identity.known_destinations) is restored. +""" +import ast +import functools +import inspect +import os +import re +import tempfile +import textwrap +import threading +import time +import types +import unittest +from collections.abc import Callable +from typing import NamedTuple +from unittest import mock + +import LXMF +import RNS +from LXMF import LXMRouter +from RNS.vendor import umsgpack + +from rnfs import mesh, names +from rnfs.constants import ID_LENGTH +from rnfs.content import seal +from rnfs.content.cid import RAW, Cid +from rnfs.features import keepers, lxmf +from rnfs.network import guards, protocol, rates + + +class Internal(NamedTuple): + module: str # the rnfs modules that rely on it, under rnfs/ and separated by ", " + surface: str # what of RNS or LXMF it relies on + check: Callable[[], None] # raises if the installed RNS or LXMF doesn't provide it + + +INTERNALS = [ + # Resource advertisements and the guard on them + Internal("network/guards.py", "ResourceAdvertisement fields t d u l i q p, after a pack/unpack round trip", + lambda: _advertisement_round_trip()), + Internal("network/guards.py", "Resource.accept(advertisement_packet, callback, progress_callback, request_id) " + "and Resource.reject(advertisement_packet), called by Link as RNS.Resource.accept/reject", + lambda: (_params(_original_accept(), "advertisement_packet", "callback", "progress_callback", + "request_id"), + _params(RNS.Resource.reject, "advertisement_packet"), + _source(RNS.Link, "packet.plaintext = self.decrypt(packet.data)", + "RNS.Resource.accept(packet, callback=self.response_resource_concluded, " + "progress_callback=pending_request.response_resource_progress, request_id=request_id)", + "RNS.Resource.reject(packet)"))), + Internal("network/guards.py", "Resource data overhead (RANDOM_HASH_SIZE, Identity.TOKEN_OVERHEAD, one AES " + "block) within TRANSFER_OVERHEAD", + lambda: _true(RNS.Resource.RANDOM_HASH_SIZE + RNS.Identity.TOKEN_OVERHEAD + 16 + <= guards.TRANSFER_OVERHEAD, "overhead grew")), + Internal("network/guards.py", "Link.pending_requests of RequestReceipts with request_id and response_rejected()", + lambda: (_true(_incoming_link().pending_requests == [], "no pending_requests list"), + _assigns(RNS.RequestReceipt.__init__, "request_id"), + _has(RNS.RequestReceipt, "response_rejected"))), + Internal("network/guards.py", "Link.destination (outgoing), Link.owner (incoming) and Destination.name as " + "expand_name builds it", lambda: _dht_link_names()), + + # Responses: the server follows the Resources RNS sends them in + Internal("network/server.py, node.py", "Resource.AUTO_COMPRESS_MAX_SIZE, copied by every Resource at creation " + "into max_decompressed_size and auto_compress_limit", + lambda: (_has(RNS.Resource, "AUTO_COMPRESS_MAX_SIZE"), + _assigns(RNS.Resource.__init__, max_decompressed_size="Resource.AUTO_COMPRESS_MAX_SIZE", + auto_compress_limit="Resource.AUTO_COMPRESS_MAX_SIZE"), + _source(RNS.Resource.assemble, "max_length=self.max_decompressed_size"))), + Internal("network/server.py", "Link.register_outgoing_resource(resource), looked up on the link (and so " + "replaceable per link) when a Resource is advertised", + lambda: (_params(RNS.Link.register_outgoing_resource, "resource"), + _source(RNS.Resource, "self.link.register_outgoing_resource(self)"), + setattr(_incoming_link(), "register_outgoing_resource", None))), + Internal("network/server.py, network/links.py", "Link.outgoing_resources and Link.incoming_resources lists", + lambda: _true(_incoming_link().outgoing_resources == [] == _incoming_link().incoming_resources, + "no resource lists")), + Internal("network/server.py, network/links.py", "Resource request_id, status, callback (called as it " + "concludes) and link; get_progress() and cancel()", + lambda: (_assigns(RNS.Resource.__init__, "request_id", "status", "callback", "link"), + _source(RNS.Resource, "self.callback(self)"), + _has(RNS.Resource, "get_progress", "cancel"))), + Internal("network/server.py", "Resource status order: NONE..TRANSFERRING < AWAITING_PROOF < ASSEMBLING < " + "COMPLETE, and every other conclusion above COMPLETE", + lambda: _true(max(RNS.Resource.NONE, RNS.Resource.QUEUED, RNS.Resource.ADVERTISED, + RNS.Resource.TRANSFERRING) < RNS.Resource.AWAITING_PROOF < RNS.Resource.ASSEMBLING + < RNS.Resource.COMPLETE < min(RNS.Resource.FAILED, RNS.Resource.CORRUPT, + RNS.Resource.REJECTED), "statuses reordered")), + Internal("network/server.py", "request handlers with six parameters get (path, data, request_id, link_id, " + "remote_identity, requested_at)", + lambda: _source(RNS.Link.handle_request, + "len(inspect.signature(response_generator).parameters) == 6", + "response_generator(path, request_data, request_id, self.link_id, " + "self.__remote_identity, requested_at)")), + Internal("network/server.py", "Destination.links: the destination's incoming links", + lambda: _true(_destination("rnfs", "dht").links == [], "no links list")), + Internal("network/server.py, network/links.py", "Link status, rtt, traffic_timeout_factor, attached_interface " + "and link_id (set_link_id); Link.ACTIVE, CLOSED, TRAFFIC_TIMEOUT_FACTOR", + lambda: (_assigns(RNS.Link.__init__, "status", "rtt", "traffic_timeout_factor", "attached_interface"), + _has(RNS.Link, "ACTIVE", "CLOSED", "TRAFFIC_TIMEOUT_FACTOR", "set_link_id"))), + + # Requests + Internal("network/links.py, network/server.py", "Link.request's own timeout, rtt × traffic_timeout_factor + " + "Resource.RESPONSE_MAX_GRACE_TIME × 1.125 (RESPONSE_GRACE, MAX_PACING_WAIT)", + lambda: (_params(RNS.Link.request, "path", "data", "response_callback", "failed_callback", + "progress_callback", "timeout", "max_response_size"), + _source(RNS.Link.request, "timeout = self.rtt * self.traffic_timeout_factor + " + "RNS.Resource.RESPONSE_MAX_GRACE_TIME * 1.125"))), + Internal("network/links.py", "RequestReceipt link, response_size, get_response() and get_progress()", + lambda: (_assigns(RNS.RequestReceipt.__init__, "link", "response_size"), + _has(RNS.RequestReceipt, "get_response", "get_progress"))), + Internal("network/protocol.py", "request framing [time.time(), truncated path hash, data] (umsgpack packs a " + "float in 9 bytes), response framing [request_id, response] with truncated-hash request IDs", + lambda: (_source(RNS.Link.request, "unpacked_request = [time.time(), request_path_hash, data]", + "request_path_hash = RNS.Identity.truncated_hash(path.encode('utf-8'))"), + _source(RNS.Link.handle_request, "packed_response = umsgpack.packb([request_id, response])"), + _true(len(umsgpack.packb(0.0)) == 9, "a float isn't packed in 9 bytes"), + _true(protocol.REQUEST_ID_LENGTH == RNS.Identity.TRUNCATED_HASHLENGTH // 8 == ID_LENGTH, + "hash length changed"))), + + # Identities and announces RNS remembers + Internal("node.py, names.py, features/peers.py", "Identity.known_destinations entries: [time heard, packet " + "hash, public key, app_data, ...]", lambda: _known_destinations()), + Internal("node_content.py", "Identity.recall(hash, from_identity_hash=, _no_use=), raising AttributeError " + "without a Reticulum instance unless _no_use", lambda: _recall()), + Internal("features/keepers.py", "Transport.get_cached_packet(packet_hash, packet_type=\"announce\") and the " + "signed announce in it: public key, name hash, random hash (emission time in its last 5 bytes), " + "ratchet, signature, app_data", lambda: _cached_announce()), + Internal("names.py", "announce handlers with five parameters get announce_packet_hash and is_path_response", + lambda: _source(RNS.Transport, "len(inspect.signature(handler.received_announce).parameters) == 5", + "announce_packet_hash=packet.packet_hash", + "is_path_response=packet.context == RNS.Packet.PATH_RESPONSE")), + Internal("names.py", "an identity's private key is Identity.KEYSIZE // 8 (KEY_LENGTH) bytes for from_bytes", + lambda: _true(len(RNS.Identity().get_private_key()) == names.KEY_LENGTH, "key size changed")), + + # Paths and interfaces + Internal("mesh.py", "Transport.PATHFINDER_M (what hops_to returns for an unknown path); next_hop_interface, " + "Reticulum.get_next_hop_if_name, is_connected_to_shared_instance", + lambda: (_true(RNS.Transport.PATHFINDER_M == mesh.PATHFINDER_M, "PATHFINDER_M changed"), + _has(RNS.Transport, "hops_to", "next_hop_interface"), + _has(RNS.Reticulum, "get_next_hop_if_name", "get_instance", "configdir"), + _assigns(RNS.Reticulum.__init__, "is_connected_to_shared_instance"))), + Internal("mesh.py, network/rates.py", "Reticulum.get_interface_stats() {\"interfaces\": [{name, short_name, " + "type, status, bitrate}]}, with LocalServerInterface and LocalClientInterface types", + lambda: (_source(RNS.Reticulum.get_interface_stats, "ifstats['name'] = str(interface)", + "ifstats['short_name'] = str(interface.name)", + "ifstats['type'] = str(type(interface).__name__)", + "ifstats['status'] = interface.online", "ifstats['bitrate'] = interface.bitrate", + "stats['interfaces'] = interfaces"), + _has(RNS.Interfaces.LocalInterface, *rates.LOCAL_INTERFACE_TYPES))), + + # Sealed files + Internal("content/seal.py", "RNS.Cryptography.hkdf(length, derive_from, salt, context), " + "AES.AES_256_CBC.encrypt/decrypt(data, key, iv), and Identity.encrypt of a content key being " + "WRAPPED_KEY_SIZE bytes", lambda: _crypto()), + + # LXMF + Internal("features/lxmf.py", "LXMRouter(identity, storagepath, delivery_limit, autopeer), keeping its state " + "in storagepath/lxmf and setting signal handlers through signal.signal", + lambda: (_params(LXMRouter.__init__, "identity", "storagepath", "delivery_limit", "autopeer"), + _assigns(LXMRouter.__init__, storagepath="storagepath + '/lxmf'"), + _source(LXMRouter.__init__, "signal.signal(signal.SIGINT, self.sigint_handler)", + "signal.signal(signal.SIGTERM, self.sigterm_handler)"))), + Internal("features/lxmf.py", "LXMRouter.get_announce_app_data(destination_hash), looked up on the router " + "when announcing, and its layout [display name, stamp cost, supported functions]", + lambda: _announce_app_data()), + Internal("features/lxmf.py", "LXMRouter.delivery_resource_transfer_began(resource), looked up on the router " + "when a delivery link is established; Link.set_resource_started_callback", + lambda: (_params(LXMRouter.delivery_resource_transfer_began, "resource"), + _source(LXMRouter.delivery_link_established, + "link.set_resource_started_callback(self.delivery_resource_transfer_began)"), + _has(RNS.Link, "set_resource_started_callback"))), + Internal("features/lxmf.py", "LXMRouter.request_messages_from_propagation_node(identity, max_messages), " + "called again through the router once the propagation node link is up", + lambda: (_params(LXMRouter.request_messages_from_propagation_node, "identity", "max_messages"), + _source(LXMRouter.request_messages_from_propagation_node, + "self.request_messages_from_propagation_node(identity, " + "self.propagation_transfer_max_messages)"))), + Internal("features/lxmf.py", "LXMRouter propagation_transfer_state, outbound_propagation_link, " + "delivery_destinations; PR_* order: PR_IDLE < PR_PATH_REQUESTED..PR_RESPONSE_RECEIVED < PR_COMPLETE " + "< every failure", + lambda: (_assigns(LXMRouter.__init__, "propagation_transfer_state", "outbound_propagation_link", + delivery_destinations="{}"), + _true(LXMRouter.PR_IDLE < LXMRouter.PR_PATH_REQUESTED < LXMRouter.PR_LINK_ESTABLISHING + < LXMRouter.PR_LINK_ESTABLISHED < LXMRouter.PR_REQUEST_SENT < LXMRouter.PR_RECEIVING + < LXMRouter.PR_RESPONSE_RECEIVED < LXMRouter.PR_COMPLETE + < min(LXMRouter.PR_NO_PATH, LXMRouter.PR_LINK_FAILED, LXMRouter.PR_TRANSFER_FAILED, + LXMRouter.PR_NO_IDENTITY_RCVD, LXMRouter.PR_NO_ACCESS, LXMRouter.PR_FAILED), + "PR_* states reordered"))), + Internal("features/lxmf.py", "LXMRouter cancel_propagation_node_requests(), acknowledge_sync_completion(" + "reset_state=), set_outbound_propagation_node, register_delivery_identity(identity, display_name=), " + "register_delivery_callback, handle_outbound, announce, exit_handler", + lambda: (_params(LXMRouter.acknowledge_sync_completion, "reset_state"), + _params(LXMRouter.register_delivery_identity, "identity", "display_name"), + _has(LXMRouter, "cancel_propagation_node_requests", "set_outbound_propagation_node", + "register_delivery_callback", "handle_outbound", "announce", "exit_handler"))), + Internal("features/lxmf.py", "propagation node announce data: pn_announce_data_is_valid, enabled flag at " + "index 2", lambda: _propagation_announce()), + Internal("features/lxmf.py", "LXMessage(destination, source, content, title=, fields=, desired_method=); " + "packed as destination hash, source hash, ...; unpack_from_bytes; signature_validated, " + "unverified_reason SOURCE_UNKNOWN, packed, hash, timestamp, title and content (bytes), fields, " + "method, state, progress", lambda: _message_round_trip()), + Internal("features/lxmf.py", "LXMessage state and method values (STATE_NAMES, METHOD_NAMES); " + "FIELD_CUSTOM_TYPE, FIELD_CUSTOM_DATA", + lambda: (_true({getattr(LXMF.LXMessage, name.upper()): name for name in lxmf.STATE_NAMES.values()} + == lxmf.STATE_NAMES, "states renumbered"), + _true({getattr(LXMF.LXMessage, name.upper()): name for name in lxmf.METHOD_NAMES.values()} + == lxmf.METHOD_NAMES, "methods renumbered"), + _true((LXMF.FIELD_CUSTOM_TYPE, LXMF.FIELD_CUSTOM_DATA) + == (lxmf.FIELD_CUSTOM_TYPE, lxmf.FIELD_CUSTOM_DATA), "custom fields renumbered"))), + Internal("features/lxmf.py", "Resource get_data_size(), get_transfer_size(), get_segments(), cancel() and " + "link on the propagation node link's Resources; a foreign Link takes rnfs_bounded", + lambda: (_has(RNS.Resource, "get_data_size", "get_transfer_size", "get_segments", "cancel"), + setattr(_incoming_link(), "rnfs_bounded", True))), +] + + +class RnsCompatTest(unittest.TestCase): + """INTERNALS against the installed RNS and LXMF.""" + + def test_internals(self): + versions = f"RNS {RNS.__version__}, LXMF {LXMF.__version__}" + for internal in INTERNALS: + with self.subTest(internal.surface): + try: + internal.check() + except Exception as e: + modules = " and ".join(f"rnfs/{module}" for module in internal.module.split(", ")) + self.fail(f"{versions} doesn't provide what {modules} relies on (update it, or hold the " + f"dependency back): {internal.surface}\n {type(e).__name__}: {e}") + + def test_inventory_names_real_modules(self): + root = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "rnfs") + for internal in INTERNALS: + for module in internal.module.split(", "): + self.assertTrue(os.path.isfile(os.path.join(root, module)), module) + + +# Checks + + +def _true(condition, message: str): + """Raise AssertionError(message) unless condition.""" + if not condition: + raise AssertionError(message) + + +def _has(owner, *names: str): + """Every name is an attribute of owner.""" + missing = [name for name in names if not hasattr(owner, name)] + _true(not missing, f"{owner.__name__} has no {', '.join(missing)}") + + +def _params(function, *names: str): + """function takes parameters with every name.""" + parameters = inspect.signature(function).parameters + missing = [name for name in names if name not in parameters] + _true(not missing, f"{function.__qualname__} takes no {', '.join(missing)}") + + +def _normal(code: str) -> str: + """code without whitespace and with one kind of quotes, so source can be matched however it's laid out.""" + return re.sub(r"\s+", "", code).replace('"', "'") + + +@functools.lru_cache(maxsize=None) +def _tree(owner) -> ast.AST: + """The parsed source of a function, class or module.""" + return ast.parse(textwrap.dedent(inspect.getsource(owner))) + + +def _source(owner, *snippets: str): + """owner's source (a function, class or module) contains every snippet, ignoring layout.""" + code = _normal(ast.unparse(_tree(owner))) + missing = [snippet for snippet in snippets if _normal(snippet) not in code] + _true(not missing, f"{getattr(owner, '__qualname__', owner.__name__)} no longer does {missing}") + + +def _assigns(function, *names: str, **values: str): + """function assigns self. for every name, and self. = value for every value (as source).""" + assigned = {} + for node in ast.walk(_tree(function)): + if isinstance(node, ast.Assign): + for target in node.targets: + if isinstance(target, ast.Attribute) and isinstance(target.value, ast.Name) \ + and target.value.id == "self": + assigned.setdefault(target.attr, []).append(_normal(ast.unparse(node.value))) + missing = [name for name in (*names, *values) if name not in assigned] + _true(not missing, f"{function.__qualname__} sets no self.{', self.'.join(missing)}") + changed = [f"{key} = {value}" for key, value in values.items() if _normal(value) not in assigned[key]] + _true(not changed, f"{function.__qualname__} no longer sets {changed}") + + +# Offline RNS objects + + +def _destination(app: str, *aspects: str, identity=None, direction=RNS.Destination.IN): + """A Destination that isn't registered with Transport.""" + with mock.patch.object(RNS.Transport, "register_destination"): + return RNS.Destination(identity or RNS.Identity(), direction, RNS.Destination.SINGLE, app, *aspects) + + +def _incoming_link(): + """An incoming Link to an rnfs.dht destination, as RNS makes one for a link request (not established).""" + return RNS.Link(owner=_destination("rnfs", "dht")) + + +def _original_accept(): + """RNS's Resource.accept, even with the guard installed.""" + accept = RNS.Resource.__dict__["accept"].__func__ + return guards._original_accept if accept is guards.guarded_accept else accept + + +class _Remembered: + """Destination hashes added to Identity.known_destinations, removed on exit.""" + + def __init__(self): + self.hashes = [] + + def __enter__(self): + return self + + def remember(self, destination, app_data: bytes | None = None) -> bytes: + """Remember destination's announce as RNS does for a valid one; its packet hash.""" + packet_hash = os.urandom(32) + _true(destination.hash not in RNS.Identity.known_destinations, "already known") + self.hashes.append(destination.hash) + RNS.Identity.remember(packet_hash, destination.hash, destination.identity.get_public_key(), app_data) + return packet_hash + + def __exit__(self, *exc): + for destination_hash in self.hashes: + RNS.Identity.known_destinations.pop(destination_hash, None) + + +# Round trips + + +def _advertisement_round_trip(): + """Advertisements as Resource.advertise packs them and Link.receive unpacks them, through guards.refusal.""" + for is_response in (True, False): + resource = types.SimpleNamespace( + size=100_052, total_size=100_000, parts=[None], hash=os.urandom(32), random_hash=os.urandom(4), + original_hash=os.urandom(32), hashmap=os.urandom(4), compressed=False, encrypted=True, split=True, + has_metadata=False, segment_index=2, total_segments=3, request_id=os.urandom(16), is_response=is_response) + adv = RNS.ResourceAdvertisement.unpack(RNS.ResourceAdvertisement(resource).pack()) + fields = {name: getattr(adv, name, None) for name in "tdulipq"} + _true(fields == {"t": 100_052, "d": 100_000, "u": not is_response, "l": 3, "i": 2, "p": is_response, + "q": resource.request_id}, f"unpacked as {fields}") + _true(guards.refusal(adv) == (None if is_response else "requests are never sent as Resources"), + f"guards.refusal: {guards.refusal(adv)}") + + +def _dht_link_names(): + """guards.is_dht_link on real Destinations and an incoming Link.""" + identity = RNS.Identity() + dht = _destination("rnfs", "dht", identity=identity, direction=RNS.Destination.OUT) + _true(dht.name == RNS.Destination.expand_name(identity, "rnfs", "dht"), f"Destination.name is {dht.name}") + _true(guards.is_dht_link(types.SimpleNamespace(destination=dht)), "outgoing rnfs.dht link not recognised") + _true(guards.is_dht_link(_incoming_link()), "incoming rnfs.dht link not recognised") + delivery = _destination("lxmf", "delivery", identity=identity, direction=RNS.Destination.OUT) + _true(not guards.is_dht_link(types.SimpleNamespace(destination=delivery)), "lxmf.delivery taken for rnfs.dht") + _true(not guards.is_dht_link(RNS.Link(owner=_destination("lxmf", "delivery"))), "lxmf.delivery taken for rnfs.dht") + + +def _known_destinations(): + """An announce RNS remembers, read back as node._seed_routing, names.heard_time and peers do.""" + destination = _destination("rnfs", "dht") + with _Remembered() as known: + before = time.time() + packet_hash = known.remember(destination, b"app data") + entry = RNS.Identity.known_destinations.get(destination.hash) + _true(before <= entry[0] <= time.time(), f"entry[0] is {entry[0]!r}, not when it was heard") + _true(entry[1:4] == [packet_hash, destination.identity.get_public_key(), b"app data"], + f"entry is {entry!r}") + identity = RNS.Identity(create_keys=False) + identity.load_public_key(entry[2]) + _true(identity.pub is not None and RNS.Destination.hash(identity, "rnfs", "dht") == destination.hash, + "the public key doesn't make the destination hash") + + +def _recall(): + """Identity.recall with _no_use, as node_content does without a Reticulum instance.""" + destination = _destination("rnfs", "dht") + public_key = destination.identity.get_public_key() + with _Remembered() as known: + known.remember(destination) + for target, kwargs in ((destination.hash, {}), (destination.identity.hash, {"from_identity_hash": True})): + found = RNS.Identity.recall(target, _no_use=True, **kwargs) + _true(found is not None and found.get_public_key() == public_key, f"recall({kwargs}) found {found}") + if RNS.Reticulum.get_instance() is None: + try: + RNS.Identity.recall(target, **kwargs) + except AttributeError: + pass + else: + raise AssertionError("recall without _no_use and without Reticulum didn't raise AttributeError") + + +def _cached_announce(): + """An rnfs.name announce, cached as Transport caches it, read back and verified as keepers does.""" + _true((keepers.PUBLIC_KEY_LENGTH, keepers.NAME_HASH_LENGTH, keepers.RATCHET_LENGTH, keepers.SIGNATURE_LENGTH) + == (RNS.Identity.KEYSIZE // 8, RNS.Identity.NAME_HASH_LENGTH // 8, RNS.Identity.RATCHETSIZE // 8, + RNS.Identity.SIGLENGTH // 8), "announce field sizes changed") + destination = _destination("rnfs", "name") + _true(destination.name_hash == keepers.NAME_HASH, "name hash changed") + cid = Cid.for_block(RAW, b"kept") + app_data = names.encode_record(7, cid) + packet = destination.announce(app_data=app_data, send=False) + packet.pack() + with tempfile.TemporaryDirectory() as cache, mock.patch.object(RNS.Reticulum, "cachepath", cache): + os.mkdir(os.path.join(cache, "announces")) + RNS.Transport.cache(packet, force_cache=True, packet_type="announce") + cached = RNS.Transport.get_cached_packet(packet.get_hash(), packet_type="announce") + # As KeeperFeature._from_rns_cache reads it + _true(cached is not None and cached.unpack() is not False, "the cached packet doesn't unpack") + _true(cached.destination_hash == destination.hash and cached.packet_type == RNS.Packet.ANNOUNCE, + "the cached packet isn't the announce") + record = keepers.from_announce_data(destination.hash, bytes(cached.data), + cached.context_flag == RNS.Packet.FLAG_SET) + _true(record is not None and (record.sequence, record.cid) == (7, cid), f"keepers read {record}") + _true(abs(record.emitted - time.time()) < 60, f"emission time {record.emitted}") + + +def _crypto(): + """seal's primitives: HKDF, AES-256-CBC, and Identity.encrypt wrapping a content key.""" + from RNS.Cryptography import hkdf + from RNS.Cryptography.AES import AES_256_CBC + _true(len(hkdf(length=64, derive_from=bytes(32), salt=None, context=b"info")) == 64, "hkdf length") + key, iv, data = os.urandom(32), os.urandom(16), os.urandom(32) + _true(AES_256_CBC.decrypt(AES_256_CBC.encrypt(data, key, iv), key, iv) == data, "AES round trip") + identity = RNS.Identity() + wrapped = RNS.Identity(create_keys=False) + wrapped.load_public_key(identity.get_public_key()) + token = wrapped.encrypt(key) + _true(len(token) == seal.WRAPPED_KEY_SIZE, f"a wrapped key is {len(token)} bytes") + _true(identity.decrypt(token) == key, "Identity.decrypt round trip") + + +# LXMF round trips + + +def _announce_app_data(): + """rnfs's announce data reads back in LXMF as LXMF's own does, minus compression support.""" + for name in ("display_name_from_app_data", "stamp_cost_from_app_data", "compression_support_from_app_data"): + _has(LXMF, name) + _params(LXMRouter.get_announce_app_data, "destination_hash") + _source(LXMRouter.announce, "app_data=self.get_announce_app_data(destination_hash)") + _source(LXMRouter.register_delivery_identity, "self.get_announce_app_data(delivery_destination.hash)") + destination_hash = os.urandom(16) + router = types.SimpleNamespace(delivery_destinations={ + destination_hash: types.SimpleNamespace(display_name="rnfs node", stamp_cost=8)}) + ours = lxmf.LxmfNetwork._announce_app_data(types.SimpleNamespace(router=router, msgpack=umsgpack), + destination_hash) + theirs = LXMRouter.get_announce_app_data(router, destination_hash) + _true(umsgpack.unpackb(ours)[:2] == umsgpack.unpackb(theirs)[:2], f"{ours!r} isn't laid out as {theirs!r}") + _true((LXMF.display_name_from_app_data(ours), LXMF.stamp_cost_from_app_data(ours), + LXMF.compression_support_from_app_data(ours)) == ("rnfs node", 8, False), "LXMF reads it differently") + + +def _propagation_announce(): + """LXMF's propagation node announce data, enabled and not, as heard_propagation_node reads it.""" + node = types.SimpleNamespace(LXMF=LXMF, msgpack=umsgpack, _lock=threading.Lock(), _propagation_nodes={}) + destination_hash = os.urandom(16) + for enabled in (True, False): + router = types.SimpleNamespace( + propagation_node=enabled, from_static_only=False, propagation_stamp_cost=16, + propagation_stamp_cost_flexibility=3, peering_cost=18, propagation_per_transfer_limit=256, + propagation_per_sync_limit=10240, get_propagation_node_announce_metadata=lambda: {}) + app_data = LXMRouter.get_propagation_node_app_data(router) + _true(LXMF.pn_announce_data_is_valid(app_data), "LXMF's own announce data isn't valid") + lxmf.LxmfNetwork.heard_propagation_node(node, destination_hash, app_data) + _true((destination_hash in node._propagation_nodes) == enabled, f"enabled {enabled} not read as such") + + +def _message_round_trip(): + """An LXMessage packed and unpacked, its sender unknown and then known, as LxmfNetwork reads it.""" + sender = _destination("lxmf", "delivery") + recipient = _destination("lxmf", "delivery", direction=RNS.Destination.OUT) + fields = {lxmf.FIELD_CUSTOM_TYPE: lxmf.CUSTOM_TYPE} + message = LXMF.LXMessage(recipient, sender, "text", title="title", fields=fields, + desired_method=LXMF.LXMessage.DIRECT) + _true((message.state, message.progress) == (LXMF.LXMessage.GENERATING, 0.0), "a new message's state") + message.pack() + packed = bytes(message.packed) + _true(packed[ID_LENGTH:2 * ID_LENGTH] == sender.hash, "the source hash isn't at packed[16:32]") + instance = types.SimpleNamespace(is_blackholed=lambda identity: False, _used_destination_data=lambda h: None) + with mock.patch.object(RNS.Transport, "register_destination"), _Remembered() as known: + unknown = LXMF.LXMessage.unpack_from_bytes(packed) + _true(not unknown.signature_validated and unknown.unverified_reason == LXMF.LXMessage.SOURCE_UNKNOWN + and bytes(unknown.packed) == packed, "a message from an unknown sender") + known.remember(sender) + with mock.patch.object(RNS.Reticulum, "get_instance", return_value=instance): + received = LXMF.LXMessage.unpack_from_bytes(packed) + _true(received.signature_validated, "a signed message from a known sender doesn't validate") + _true((bytes(received.source_hash), bytes(received.hash), received.title, received.content, received.fields) + == (sender.hash, bytes(message.hash), b"title", b"text", fields), "the received message's fields") + _true(isinstance(received.timestamp, float) and hasattr(received, "method"), "timestamp and method") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_routing.py b/tests/test_routing.py index cc31b0b..49f905f 100644 --- a/tests/test_routing.py +++ b/tests/test_routing.py @@ -6,6 +6,7 @@ from rnfs.config import Config from rnfs.constants import BUCKET_COUNT, ID_LENGTH, K from rnfs.dht.routing import RoutingTable, distance +from tests.support import FakeClock OWN_ID = bytes(ID_LENGTH) ANNOUNCE = 6 * 3600 @@ -16,18 +17,6 @@ def node_id(n: int) -> bytes: return n.to_bytes(ID_LENGTH, "big") -class FakeClock: - """A manually advanced clock.""" - - def __init__(self): - """Clock at an arbitrary start time; tests advance it by setting now.""" - self.now = 1000.0 - - def __call__(self) -> float: - """The current fake time.""" - return self.now - - def bucket_ids(bucket: int, count: int) -> list[bytes]: """count distinct IDs that fall in the given bucket of OWN_ID's table.""" return [node_id((1 << bucket) + i) for i in range(count)] diff --git a/tests/test_seal.py b/tests/test_seal.py index 67fbcdf..c1b75d3 100644 --- a/tests/test_seal.py +++ b/tests/test_seal.py @@ -14,12 +14,8 @@ from rnfs.errors import TooLarge from rnfs.node import RnfsNode -try: - from tests.test_names import FakeNetwork - from tests.test_node import NetClient -except ImportError: # run from inside tests/ - from test_names import FakeNetwork - from test_node import NetClient +from tests.test_names import FakeNetwork +from tests.test_node import NetClient ALICE, BOB, CAROL = RNS.Identity(), RNS.Identity(), RNS.Identity() diff --git a/tests/test_server.py b/tests/test_server.py index 123e7a9..7b6b03e 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -16,22 +16,11 @@ from rnfs.content.cid import RAW, Cid from rnfs.network import guards, protocol, server from rnfs.network.identity import node_id_for_identity +from tests.support import FakeClock KEY = bytes(range(ID_LENGTH)) -class Clock: - """A clock tests move by hand.""" - - def __init__(self): - """Start at 1000 s.""" - self.now = 1000.0 - - def __call__(self) -> float: - """The current time.""" - return self.now - - def block_of(size: int, seed: int = 0) -> tuple[Cid, bytes]: """A raw block of size bytes and its CID.""" data = bytes([seed % 256]) * size @@ -77,7 +66,7 @@ class ResponsePoolsTest(unittest.TestCase): def setUp(self): """Pools of 4 slots (2 slow, 2 fast) with 30 s slack.""" - self.clock = Clock() + self.clock = FakeClock() self.pools = server.ResponsePools(4, 30, self.clock) def take(self, pool, name: bytes): @@ -133,7 +122,7 @@ class IncomingLinksTest(unittest.TestCase): def setUp(self): """Room for 3 links.""" - self.clock = Clock() + self.clock = FakeClock() self.links = server.IncomingLinks(3, self.clock, random.Random(1)) def test_per_link_limits(self): @@ -190,7 +179,7 @@ class UploadPacerTest(unittest.TestCase): """The next response on a link waits bytes × 8 / cap after the previous one started.""" def test_wait(self): - clock = Clock() + clock = FakeClock() pacer = server.UploadPacer(clock) pacer.started(b"a", 1000, 800) # 10 s at 800 bit/s clock.now += 4 @@ -199,7 +188,7 @@ def test_wait(self): self.assertEqual(pacer.wait_time(b"a"), 0) def test_no_cap(self): - pacer = server.UploadPacer(Clock()) + pacer = server.UploadPacer(FakeClock()) pacer.started(b"a", 1000, None) self.assertEqual(pacer.wait_time(b"a"), 0) @@ -361,7 +350,7 @@ def setUp(self): saved = RNS.Resource.AUTO_COMPRESS_MAX_SIZE self.addCleanup(setattr, RNS.Resource, "AUTO_COMPRESS_MAX_SIZE", saved) self.identity = RNS.Identity() - self.clock = Clock() + self.clock = FakeClock() self.big = [block_of(100 * KIB, i) for i in range(4)] self.backend = FakeBackend(blocks=dict(self.big)) self.make_server(Config(remote_pin_identities=[self.identity.hash.hex()])) @@ -881,6 +870,37 @@ def test_unreadable_advertisement_left_to_rns(self): RNS.Resource.accept(packet) self.assertEqual(self.accepted, [packet]) + def test_unsupported_rns_refused_with_one_error(self): + """An RNS whose unpacked advertisements have no t and d (renamed fields) is still refused, failing + closed, and says so once per process at LOG_ERROR rather than refusing every Resource silently.""" + import rnfs + lines = [] + for patcher in (mock.patch.object(guards, "_unsupported_reported", False), + mock.patch.object(guards, "log", lambda message, level: lines.append((message, level))), + mock.patch.object(RNS.ResourceAdvertisement, "unpack", + lambda data: types.SimpleNamespace(transfer_size=100, data_size=100))): + patcher.start() + self.addCleanup(patcher.stop) + packets = [self.advertisement(100, 100) for _ in range(3)] + for packet in packets: + self.assertIsNone(RNS.Resource.accept(packet)) + self.assertEqual(guards.refusal(types.SimpleNamespace(d=100)), "malformed sizes") + self.assertEqual((self.accepted, self.rejected), ([], packets)) + errors = [message for message, level in lines if level == RNS.LOG_ERROR] + self.assertEqual(errors, [f"this RNS version ({RNS.__version__}) isn't supported by rnfs " + f"{rnfs.__version__}: incoming Resources are being refused"]) + self.assertEqual(len(lines), 4) # and the usual warning per refused Resource + + def test_invalid_sizes_are_not_an_unsupported_rns(self): + lines = [] + for patcher in (mock.patch.object(guards, "_unsupported_reported", False), + mock.patch.object(guards, "log", lambda message, level: lines.append((message, level)))): + patcher.start() + self.addCleanup(patcher.stop) + for transfer, data in ((None, 100), ("100", 100), (-1, 100), (100, True)): + self.assertEqual(guards.refusal(types.SimpleNamespace(t=transfer, d=data)), "malformed sizes") + self.assertEqual(lines, []) + class CompressionSettingTest(unittest.TestCase): """RNS.Resource.AUTO_COMPRESS_MAX_SIZE = 0 refuses compressed Resources and stops compression.""" diff --git a/tests/test_storage.py b/tests/test_storage.py index 6baa940..402ca87 100644 --- a/tests/test_storage.py +++ b/tests/test_storage.py @@ -224,6 +224,19 @@ def test_state_is_synced_before_and_after_the_rename(self): storage_module.write_json(os.path.join(self.tmp.name, "state", "x.json"), {"a": 1}) self.assertEqual(events, ["fsync", "rename", "fsync"]) + def test_state_file_format(self): + path = os.path.join(self.tmp.name, "state", "x.json") + value = {"é": ["\ud800", 1]} # a lone surrogate is written as its escape either way + for ensure_ascii, expected in ((True, b'{"\\u00e9": ["\\ud800", 1]}'), + (False, b'{"\xc3\xa9": ["\\ud800", 1]}')): + with self.subTest(ensure_ascii=ensure_ascii): + storage_module.write_json(path, value, ensure_ascii=ensure_ascii) + with open(path, "rb") as f: + self.assertEqual(f.read(), expected) + if POSIX_MODES: + self.assertEqual(os.stat(path).st_mode & 0o777, 0o600) + self.assertEqual(sorted(os.listdir(os.path.dirname(path))), ["x.json"]) # no temporary file left + @unittest.skipUnless(POSIX_MODES, "Windows has no POSIX permission bits") def test_state_dir_is_private(self): self.assertEqual(os.stat(os.path.join(self.tmp.name, "state")).st_mode & 0o777, 0o700) diff --git a/v0.1.2proposal.md b/v0.1.2proposal.md new file mode 100644 index 0000000..20de167 --- /dev/null +++ b/v0.1.2proposal.md @@ -0,0 +1,444 @@ +# rnfs 0.1.2 proposal + +*Status: draft for review · Date: 2026-10-13 · Base: `dev` @ `22688db` · Source: [audit.md](audit.md)* + +## Goal + +0.1.2 is a **maintenance release**. It fixes what shipped broken in 0.1.1 and adds guardrails so the same problems can't ship again. It also takes the cheap, mechanical consolidation work from the audit while the test suite can still catch regressions. + +The larger refactors, such as splitting `api.py`, adding `Feature` hooks and breaking up `features/lxmf.py`, are deferred to 0.2.0 (see [Deferred to 0.2.0](#deferred-to-020)). + +## Ground rules for a patch release + +Every change below must keep these intact: + +- **Formats unchanged:** the wire protocol, DAG/CID encoding, archive format, sealed-file format, and on-disk state files (`*.json`, blocks, keys) are read and written exactly as in 0.1.1. +- **CLI and config unchanged:** no command, flag or config key is removed, renamed or reinterpreted. +- **HTTP API unchanged, with one exception:** the `seal_name` tightening in [F1](#f1--reject-non-boolean-seal_name-on-post-apiadd). It only affects requests that misbehave today. +- **Refactors are mechanical:** each one lands in its own PR and is covered by the existing tests. + +## Scope at a glance + +| ID | Change | Audit ref | Priority | Risk | Effort | +|---|---|---|---|---|---| +| **H1** | Resolve the README conflict block, fix stale links and broken markup | T-H1, T-M6 | Must | None | S | +| **H2** | Test that README release links match `__version__` | T-M6 | Must | None | S | +| **H3** | Remove the stray `#yo`; delete the stray `v1.0` and `test-0.1.0` tags | T-L6 | Must | None | XS | +| **H4** | Fix small inaccuracies in `design.md`; document the lock order | L7, N-M1, X5 | Should | None | S | +| **C1** | CI lint job: ruff (pyflakes rules) plus a conflict-marker check | X6 | Must | Low | S | +| **C2** | The release workflow requires the test matrix | T-M5 | Must | Low | S | +| **C3** | Add Python 3.14 to the CI matrix and build locks | T-L2 | Should | Low | S | +| **D1** | Cap upper versions of `rns` and `lxmf` | X1 | Must | Low | XS | +| **D2** | RNS/LXMF compatibility test for every private attribute rnfs relies on | X1 | Must | None | M | +| **D3** | A loud error when Resource advertisements stop matching rnfs's expectations | X1 | Should | Low | S | +| **D4** | Bump the lock to `rns==1.5.7` once D2 passes | – | Could | Low | XS | +| **F1** | Reject a non-boolean `seal_name` on `POST /api/add` | C-M1 | Must | Low | S | +| **F2** | `fsutil.write_atomic`, giving `names.json` and the API token durable writes | X2 | Must | Medium | M | +| **F3** | `install.sh` installs the latest release by default, not `main` | T-M5 | Should | Medium | S | +| **K1** | One `_printable`, `_log` and `_feature` helper for the features | X2 | Should | Low | S | +| **K2** | Each duplicated constant defined once | X2 | Should | Low | XS | +| **K3** | Remove dead code (driven by C1) | X6, W-Low | Should | Low | S | +| **T1** | Shared test fakes in `tests/support.py`; drop the dead import fallbacks | T-M1 | Should | Low | M | +| **T2** | A contract test that keeps `FakeNode` matching `RnfsNode` | T-M2 | Must | None | S | +| **T3** | Replace sleep-based synchronisation with `wait_for` in the worst offenders | T-M3 | Should | Low | S | + +Effort is per item: XS < 1 h, S ≈ half a day, M ≈ 1–2 days. **The Must items total about 3–4 days, and Must plus Should about 6–8 days.** + +--- + +## 1. Hygiene: what shipped broken + +### H1 · Resolve the README conflict and fix stale content + +`README.md:869-895` holds an unresolved conflict. The `HEAD` side is empty; the incoming side contains the "Tests" and "Known limitations" sections. Resolve it as follows: + +- **Conflict block:** keep the incoming side, removing the markers. In its Tests block, add `tests.test_client` to the E2E command so it matches what CI runs: + ```sh + RNFS_E2E=1 python3 -m unittest tests.test_e2e tests.test_client + ``` +- **Release links:** the Quick start still links to `v0.1.0` (`:55`, `:68`). Update them to the current release, and see H2 for keeping them in step. +- **Headings:** remove the stray `**` from the headings at `:70`, `:76`, `:79` and `:88`. +- **Code fence:** replace the inline code fence at `:61` with inline code. +- **Typo:** "availible" → "available" (`:50`). +- **Duplicate test docs:** remove the duplicate test instructions from `design.md:391` and link to the README section instead, so they live in one place. + +Merging the two parallel walkthroughs ("Quick start" `:47` and "Getting started" `:386`) is worthwhile, but it is an editorial rewrite. It is optional for 0.1.2. + +### H2 · Keep README release links in step with the version + +Add a test to `tests/test_packaging.py`. It finds every `releases/download/vX.Y.Z/` and `rnfs-X.Y.Z-` occurrence in `README.md` and asserts that each one equals `rnfs.__version__`. + +After that, bumping `__version__` without updating the README fails CI, so the v0.1.0/v0.1.1 drift can't recur. The existing `DependencyLockTest` already uses this pattern for the locks. + +### H3 · Remove the stray comment and tags + +- **Comment:** delete the `#yo` line in `rnfs.py:3`. +- **Tags:** the local tags `v1.0` and `test-0.1.0` both point at `7afb831`, the commit that introduced the README conflict. + - They were never released: the `version` job in `release.yml` rejects any tag that doesn't match `__version__`. + - They are still confusing, and F3 resolves "the latest release" from GitHub. + - Run `git ls-remote --tags origin` to check whether they were pushed. If so, delete them with `git push origin :refs/tags/v1.0 :refs/tags/test-0.1.0`, and delete them locally either way. + +### H4 · Fix small inaccuracies in design.md + +- **Facade table:** `pin_ls` also returns `type` and `name` (`node.py:795`). +- **Content layer:** replace "content/: no RNS dependency" with what is actually true. The layer uses RNS's bundled msgpack and RNS logging, so importing it loads RNS. +- **Threads section:** add the lock order as it actually is: `names._lock` → `publisher` lock → `node._lock` → `storage` lock. Also list the known places where slow work runs while a lock is held (audit X5). + + This is documentation only. It doesn't fix anything, but it makes the current behaviour reviewable, and it is the prerequisite for the 0.2.0 locking work. + +--- + +## 2. CI guardrails + +### C1 · Lint job + +The code already contains `# noqa: BLE001` and `# noqa: E402` comments, which are ruff/flake8 rule codes, so ruff is the natural fit. + +**Supply chain:** keep the existing hash-locking discipline: +- Add `ruff` to `BUILD_TOOLS` in `packaging/update-lock.py:54` and regenerate `build-tools.lock`. +- Add `ruff` to the names asserted by `test_build_locks` (`tests/test_packaging.py:162`). + +**Configuration** in `pyproject.toml`. Start with rules that only flag real defects, so the first pass is fixes, not reformatting: + +```toml +[tool.ruff] +target-version = "py311" +extend-exclude = ["packaging/flatpak"] + +[tool.ruff.lint] +select = ["F", "E9", "B006", "B008", "B904"] # pyflakes, syntax errors, mutable/call defaults, raise-from +``` + +**Job** in `.github/workflows/tests.yml`, running on `ubuntu-latest` with Python 3.12: + +```yaml +lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + with: { persist-credentials: false } + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 + with: { python-version: "3.12" } + - run: python -m pip install --require-hashes --only-binary=:all: -r packaging/build-tools.lock + - run: ruff check . + - name: No merge-conflict markers + run: "! git grep -nE '^(<<<<<<<|>>>>>>>)( |$)|^=======$' -- . ':!*.lock'" +``` + +Fix everything the first run reports in the same PR. K3 lists the dead code it is expected to find. Formatting (`ruff format`) and type-checking (mypy) stay out of 0.1.2: both would produce a large diff and belong in their own release. + +### C2 · Gate releases on the tests + +Today, `release.yml:215` makes the `release` job depend only on the build jobs, so a tag publishes even if the unit or E2E suite would fail. To fix this: + +1. Make `tests.yml` reusable by adding `workflow_call:` to its `on:` triggers. +2. In `release.yml`, add a job `tests: { uses: ./.github/workflows/tests.yml }` and add `tests` to the `release` job's `needs`. + +The packaging smoke tests then also run for every release, which means the duplicated sdist/wheel checks in `release.yml` can be removed (audit T-M5). + +### C3 · Python 3.14 + +- Add `"3.14"` to the matrix in `tests.yml` and to `BUILD_PYTHONS` in `update-lock.py:60`, then regenerate the locks. +- Add the `Programming Language :: Python :: 3.14` classifier if `pyproject.toml` lists versions. +- **Condition:** this depends on every locked dependency publishing 3.14 wheels (`cryptography` ships abi3 wheels, and `cffi` ≥ 2.0 has cp314). If the lock can't be regenerated cleanly, leave 3.14 for 0.1.3 rather than relaxing `--only-binary`. + +--- + +## 3. Dependency safety + +### D1 · Upper version bounds + +```toml +dependencies = ["rns>=1.5.6,<1.6", "lxmf>=1.2.0,<1.3"] +``` + +- **Why:** rnfs patches and reads RNS/LXMF internals in about 15 places (audit X1). An unbounded `pip install rnfs` should not pick up a release nobody has tested. +- **Caveat:** RNS does not follow strict semver; internals have changed in point releases. That is why the cap alone isn't enough and D2 is the real protection. +- **Other dependency copies:** apply the same bounds to `requirements.txt`, or delete it. It is the fifth copy of the dependency list (T-L1), and the README's "From source" can use `pip install -e .`. See Q3. +- **Lock test:** the lock test only parses package names (`tests/test_packaging.py:150`), so ranges need no test change. + +### D2 · Compatibility test for RNS and LXMF internals + +Add `tests/test_rns_compat.py`. It imports the *installed* RNS and LXMF and asserts that every private attribute, method and data layout rnfs depends on is present. Collect them into one documented list at the top of the test: + +| rnfs code | RNS/LXMF surface | +|---|---| +| `network/guards.py:46-66` | `ResourceAdvertisement` fields `t d u l i q p` (after `unpack`), `Link.pending_requests`, `Resource.accept`/`reject` | +| `network/server.py:684-705` | `Link.register_outgoing_resource`, `Link.outgoing_resources` | +| `network/links.py:502` | `Link.incoming_resources` | +| `node.py:214`, `server.py:360` | `Resource.AUTO_COMPRESS_MAX_SIZE` | +| `node.py:396-411`, `names.py:175-179`, `features/peers.py:133` | `Identity.known_destinations` entry layout | +| `features/keepers.py:548` | `Transport.get_cached_packet` | +| `node_content.py:63-67` | `Identity.recall(..., _no_use=...)` keyword | +| `features/lxmf.py:1576-1633,1712,1777,1637` | `LXMRouter.get_announce_app_data`, `delivery_resource_transfer_began`, `request_messages_from_propagation_node`, `propagation_transfer_state`, `PR_*` constants, `outbound_propagation_link`, `delivery_destinations` | + +**For the advertisement fields**, prefer a real round trip: pack an advertisement the way RNS does and `unpack` it. A simple `hasattr` on the class is not enough, because the fields are set per instance. + +Each failing assertion should name the rnfs module that needs updating. + +**Why:** a dependency bump, whether Dependabot, D4 or a user's own environment, then fails CI with a precise message rather than at runtime. The full `rns_compat.py` module that centralises the patches (audit X1) is deferred to 0.2.0. This test is the cheap half of that work, and it also produces the inventory the module will need. + +### D3 · Make an RNS incompatibility visible at runtime + +`guards.refusal()` returns `"malformed sizes"` when `adv.t`/`adv.d` are missing, so a renamed field in RNS would silently reject every incoming Resource, LXMF's included. + +**Proposed change:** +- If the attributes are *absent*, as opposed to present but invalid, log once at `LOG_ERROR`: + > `this RNS version (x.y.z) isn't supported by rnfs 0.1.2: incoming Resources are being refused` +- Keep failing closed. The guard exists to stop oversized allocations, and failing open would undo that. + +This is a change to logging only. Add a test that feeds an advertisement object without `t`/`d` and asserts on both the refusal and the single log line. + +### D4 · Bump the lock to rns 1.5.7 + +RNS 1.5.7 is on PyPI; the lock pins 1.5.6. Once D2 passes against 1.5.7, regenerate with `python3 -I packaging/update-lock.py rns==1.5.7 lxmf==1.2.0` and `python3 packaging/flatpak/generate-sources.py`, and run the full CI matrix. + +This is optional for 0.1.2. Pick it up only if the RNS 1.5.7 changelog includes fixes rnfs users need. + +--- + +## 4. Fixes + +### F1 · Reject a non-boolean `seal_name` on `POST /api/add` + +**Bug:** `node_content.add_options` (`node_content.py:231-233`) applies `parse_bool` only to values that are *already* booleans. Any other value passes through unchanged, so `POST /api/add {"seal_name": "false"}` reaches `add_path(seal_name="false")`, where the string counts as true. The failure is safe (the name is sealed anyway), but it is wrong. + +The parameter means two different things: + +| Endpoint | `seal_name` means | Type | +|---|---|---| +| `POST /api/add` (JSON body) | whether to seal the file's name | `bool` | +| `POST /api/add/stream` (query) | the name to seal inside | `str` | + +**Change:** +- Pass the endpoint kind into `add_options`, e.g. `add_options(value, stream=False)`. +- For the JSON endpoint, accept only a JSON boolean, and raise `ValueError("seal_name must be true or false")` otherwise; the API turns that into a 400. +- Leave the stream endpoint unchanged. +- The CLI (`cli.add_options`, `cli.py:362-364`) already sends `False` or a name string correctly, so it is unaffected. +- Add API tests for `true`, `false`, `"false"` (expect 400) and `1` (expect 400). + +Renaming the parameters to `seal_name: bool` and `sealed_name: str` is the clean fix, but it changes the API, so it is deferred to 0.2.0. + +### F2 · `fsutil.write_atomic` + +`design.md` promises that state is written "atomically and durably". Three of the seven copies of the atomic-write logic don't fully keep that promise: + +| Writer | fsync file | fsync dir | Unique temp | Notes | +|---|---|---|---|---| +| `content/storage.py:81-97` `write_json` | ✓ | ✓ | ✓ | the reference behaviour | +| `names.py:747-762` `_write_atomic` | ✓ | ✗ | ✗ (`path + ".tmp"`) | `names.json` and name keys | +| `cli.py:56-69` `write_token` | ✗ | ✗ | ✓ | `O_EXCL \| NOFOLLOW`, mode 0600 | +| `network/identity.py:45-65`, `dht/providers.py:346-360`, `features/lxmf.py:424-444` | ✓ | varies | ✓ | copies of `write_json` | + +**Proposed helper** in `rnfs/fsutil.py`: + +```python +def write_atomic(path: str, data: bytes, *, mode: int = 0o644, durable: bool = True) -> None: + """Replace path with data: a new temp file in the same directory (O_EXCL, O_NOFOLLOW where available, + created with mode), written and fsynced, renamed over path (retried on Windows), then the directory + synced if durable. On any failure the temp file is removed and path is left as it was.""" +``` + +**Migration**, one commit each: +1. `names._write_atomic` → `write_atomic`. It gains the directory sync and a unique temp name. +2. `cli.write_token` → `write_atomic(path, token, mode=0o600)`. It gains the fsync and keeps the 0600 and no-follow guarantees. +3. `storage.write_json` → `json.dumps` followed by `write_atomic`. Add an `ensure_ascii` parameter so `lxmf.MessageLog` can drop its own copy. +4. `identity` and `providers` → `write_atomic`. + +**Leave `blockstore.put` alone.** It is the hot path, has its own partial-file prefix and startup cleanup, and is already correct. + +**Tests:** unit tests for `write_atomic` covering mode, replacing an existing file, refusing to follow a symlinked temp, and cleanup after an exception mid-write. CI's three-OS matrix covers the Windows `replace` retry. + +**Risk:** Medium, because these paths write keys and tokens. Mitigations: one PR per call site, each with the existing tests for that file. File contents and names are unchanged, so no state migration is needed. + +### F3 · `install.sh` installs the latest release + +`install.sh:35` installs `archive/refs/heads/main.zip`, so the README's `curl … | sh` one-liner installs unreleased code, and `--update` "re-installs from the latest main branch". + +**Change:** +- **Default:** resolve the latest release by following the `https://github.com/afit21/rnfs/releases/latest` redirect with the script's existing Python downloader (`install.sh:261-267`). This uses no GitHub API and so has no rate limit. Take the tag from the final URL and install `archive/refs/tags/.zip`. +- **`--main`:** a new flag that keeps the old behaviour. +- **`RNFS_SOURCE`:** still overrides both. CI uses it, so the packaging smoke test is unaffected. +- **Docs:** update the usage text (`install.sh:4-13`) and the README install section. + +**Compatibility constraint:** the `install.sh` on `main` will now install trees from older tags. It must keep working with the 0.1.1+ layout (`packaging/requirements.lock`, `packaging/build-backend.lock`), which is already true. Add a comment saying so. + +**Risk:** Medium, because the installer is user-facing. The `--update` flow should be checked by hand on Linux and macOS before tagging. + +--- + +## 5. Low-risk consolidation + +### K1 · Shared feature helpers + +Create `rnfs/features/_common.py`. It replaces the copies of three helpers in five feature modules: + +- **`printable`:** delete the `_printable` copies in `contacts.py:322`, `follow.py:339`, `peers.py:237` and `lxmf.py:111`, and use `fsutil.printable`. Their output is identical for control characters (`repr(c)[1:-1]` versus `unicode_escape`). +- **`log(message, level=LOG_VERBOSE)`:** one helper taking RNS's level constants. It replaces the five `_log` definitions and removes lxmf's `"info"` string-level variant (`lxmf.py:103`). +- **`feature(handler, name)`:** one helper that always raises `NotFound` when the feature isn't loaded. It replaces the five `_feature` copies and the inlined lookup in `peers.py:159`. `lxmf`'s copy (`lxmf.py:1332`) lets the `KeyError` from `node.feature()` escape. In a real daemon this can't happen: a feature's routes exist only while it is loaded. It only matters for nodes without the feature, such as `RnfsNode.offline()` and test fakes, so it is a consistency fix, not a user-visible change. + +**Not in 0.1.2:** the three `human_size` implementations format sizes differently, so unifying them changes CLI output. That waits for 0.2.0. + +### K2 · Duplicated constants + +- **`PARTIAL_SUFFIX`:** defined in `node.py:100` and `features/jobs.py:60`. Define it once in `constants.py`. +- **`".partial-"` literals:** at `storage.py:84` and `lxmf.py:429`. Use `blockstore.PARTIAL_PREFIX`, or the helper from F2. +- **`RESPONSE_MAX_GRACE_TIME * 1.125`:** computed separately in `links.py:36` and `server.py:42`. Define it once in `network/protocol.py`, named for the 5.625 s pacing cap in spec §3. +- **ID length:** replace the literal `16` at `follow.py:92` with `ID_LENGTH`. + +### K3 · Dead code + +Remove the following after confirming that nothing, tests included, references them; C1 will flag most of them: + +- `builder._read_full` (`builder.py:114-124`) +- `announces.read_mode` (`announces.py:126`) +- `PeerLink.mtu` (`links.py:121`) +- `guards.installed` (`guards.py:102`) +- `constants.REQUEST_FRAMING` (`constants.py:26`) +- the unused `when` (`follow.py:178`) +- the no-op `if handed_over: pass` branch (`server.py:621-622`); invert the condition rather than removing the branch's meaning +- the second `_make_private_dirs` call (`node.py:212` / `:265`), if `_setup` always runs after the first + +--- + +## 6. Tests + +### T1 · Shared fakes in `tests/support.py` + +Move the duplicated test doubles into `tests/support.py`, or a `tests/fakes.py` if `support.py` gets too long: + +- `Clock` ×3 and `FakeClock` ×2 become one `FakeClock` +- `free_port` ×4, including `packaging/smoke_test.py`, which can import it or keep its own copy because it runs outside the test tree +- `FakeDestination` ×3 +- `FakeLink` ×2 +- `wait_for`, from `test_e2e.py:74` + +Then: +- Delete the `try/except ImportError` "run from inside tests/" fallbacks. They can't work, because `from tests.support import …` runs unguarded before them. +- Add `tests/__init__.py`. +- Check that both `python -m unittest discover tests` and the CI invocations still collect all 1,020 tests. Record the count in the PR. + +Moving `FakeNode` out of `test_api.py` is optional; the cross-module imports work, they are just untidy. + +### T2 · Facade contract test + +Add a test that compares the public methods of `tests.test_api.FakeNode` with `rnfs.node.RnfsNode` using `inspect.signature`: +- Every method the fake defines must exist on the real class. +- Parameter names and defaults must match. + +The fake is about 350 lines and is used by four test modules, and nothing currently stops it drifting from the real facade. This test is cheap insurance before the 0.2.0 refactors change the facade. + +### T3 · Replace sleeps used for synchronisation + +Using T1's `wait_for(condition, timeout)`, replace the fixed sleeps that wait for something to happen. The highest-value places: + +- `test_api.py:747`, `:765`, `:852` +- `test_gateway.py:378` +- `test_links.py:365`, `:375` +- `test_jobs.py:216` + +Sleeps that deliberately let time pass for a real clock can stay. `d0aa468` shows these timing-based tests already fail on the slower macOS and Windows CI runners. + +--- + +## Deferred to 0.2.0 + +These need design work, change interfaces, or move large amounts of code. They are worth doing, but not in a patch release. + +| Item | Audit ref | Why not now | +|---|---|---| +| `rns_compat.py` centralising every RNS/LXMF patch, called once at startup | X1 | Touches startup order in `node.py`, `server.py` and `links.py`. D2 builds the inventory first. | +| Split `api.py` into `httpserver.py`, `auth.py` and the routes | X3, C-M5 | Moves about 600 lines and changes imports in gateway, CLI and tests | +| `Feature` hooks (`settings`, `validate_config`, `expand_target`, `resolve_name_arg`, pin-source registration), and taking feature knowledge out of `config.py`, `api.py`, `node.py` and `storage.py` | X3, F-M1, N-M3 | Changes the plugin interface | +| Declared feature dependencies (`requires = (...)`) | F-M1 | Same | +| Make `features/lxmf.py` a package | X4, F-H1 | Large move; best after `rns_compat` | +| Extract `RemotePinQueue`/`FetchedSet` (node), `ResponseTracker` (server) and a provider scheduler (fetcher) | X4, C-H1, W-M1, N-M4 | Structural; needs lock-ownership decisions first | +| Lock ownership per field; move slow work out of locks | X5 | Behavioural concurrency change; H4 documents it first | +| One `walker.walk_path()` for the four path resolvers | N-H1 | Touches format-critical lookup code; wants new property tests | +| One strict msgpack reader for `dag` and `archive` | X2, N-M5 | Format-critical | +| A content layer free of RNS (standalone msgpack, logging shim) | N-M1 | Adds a dependency | +| Versioned state loader with migrations | F-M2 | Defines the state-file compatibility policy | +| `seal_name` / `sealed_name` split, one add-options schema | C-M1 | API change | +| Unify `human_size` | X2 | Changes CLI output | +| `ruff format`, mypy, coverage reporting | X6 | Large diffs; better in their own release | +| Split the E2E scenario into separate test methods | T-M4 | Larger test rewrite | +| Explicit `chunker` parameter instead of a `ContextVar` | N-M5 | Changes `build_file`'s signature | + +### Audit items examined and dropped + +Checking against the code showed that these need no change: + +- **"follow doesn't save on stop"** (audit F-M4): `follow` writes its state on every change (`follow.py:131,148,224`), so there is nothing unsaved at shutdown. +- **"`KeeperStore.save` re-marks dirty without the lock"** (F-M3): saves are serialised by `_save_lock`, and setting `dirty = True` after a failed write races only with other code setting it to `True`. It is benign. +- **"keepers `stop()` doesn't join its loop"**: the loop's `save()` and `stop()`'s `save()` are serialised by the same `_save_lock`, so there is no torn write. Joining would only tidy shutdown logging; it is left for the shared `BackgroundLoop` helper in 0.2.0. + +--- + +## Compatibility statement for 0.1.2 + +| Surface | Change | +|---|---| +| Wire protocol, DAG/CID, archive, sealed files | None | +| State files and keys | None. Same contents and names, now always written durably (F2). | +| CLI commands, flags and output | None | +| Config keys | None | +| HTTP API | `POST /api/add` returns 400 for a non-boolean `seal_name` (F1). No other change. | +| Dependencies | `rns<1.6`, `lxmf<1.3` (D1); optionally `rns==1.5.7` in the lock (D4) | +| Installer | Installs the latest release by default; `--main` gives the old behaviour (F3) | +| Python | 3.11–3.13, plus 3.14 if C3 lands | + +## Suggested PR sequence + +Each PR is independently reviewable and keeps CI green: + +1. **Hygiene:** H1, H2, H3, H4. +2. **Lint:** C1, plus the fixes ruff reports and K3. +3. **Release gating:** C2 (and C3 if the locks regenerate cleanly). +4. **Dependencies:** D1, D2, D3 (D4 optional). +5. **`seal_name`:** F1. +6. **Atomic writes:** F2, one commit per call site. +7. **Helpers and constants:** K1, K2. +8. **Tests:** T1, T2, T3. +9. **Installer:** F3, with a manual install and `--update` check on Linux and macOS. +10. **Release:** + - bump `__version__` to `0.1.2` + - update the README links (H2 enforces this) + - add the CHANGELOG entry + - regenerate the Flatpak sources if the lock changed + - tag `v0.1.2`; C2 runs the full matrix before publishing + +## Draft CHANGELOG entry + +```markdown +## 0.1.2 + +Maintenance release from the fourth audit (audit.md): fixes, release guardrails and internal consolidation. +No format, protocol, CLI or config changes. + +Fixes: +- `POST /api/add` refuses a `seal_name` that isn't `true` or `false` (a string like `"false"` used to seal the + name anyway). `/api/add/stream`, where `seal_name` is the name, is unchanged. +- `names.json`, name keys and the API token are written durably (file and directory synced, unique temp + file), like the other state files. +- The README's Tests and Known limitations sections are restored (0.1.1 shipped them inside a merge + conflict), and its download links point at the current release. + +Dependencies and install: +- rns is limited to <1.6 and lxmf to <1.3; a test checks every RNS/LXMF internal rnfs relies on, and the + daemon logs an error if incoming Resource advertisements no longer look as rnfs expects. +- install.sh installs the latest release instead of the main branch (`--main` for the old behaviour). + +Development: +- CI lints with ruff and rejects merge-conflict markers; releases run the full test matrix before + publishing. +- Shared test fakes live in tests/support.py; a contract test keeps the API tests' fake node in step with + RnfsNode. +``` + +## Open questions for the maintainer + +1. **RNS cap:** `<1.6`, or tighter (`<1.5.8`)? The tighter cap is safer, but blocks users from RNS patch fixes until rnfs is re-released. **Recommendation:** `<1.6` plus D2. +2. **Installer default (F3):** OK to change in a patch release? Users who relied on `install.sh` tracking `main` would need `--main`. **Recommendation:** yes. Today's behaviour contradicts the README. +3. **`requirements.txt`:** delete it, or keep it in sync with a test? **Recommendation:** delete it, and point "From source" at `pip install -e .`. +4. **Python 3.14 (C3):** include it if the locks regenerate cleanly, or hold it for a dedicated release? +5. **Stray tags (H3):** confirm that `v1.0` and `test-0.1.0` can be deleted from the remote, if they were pushed.