docs: record tested paid-download PRs for next release
This commit is contained in:
@@ -14,6 +14,17 @@ doc. See [`ROADMAP.md`](ROADMAP.md) for the curated, public-facing direction.
|
||||
|
||||
## Next release after 1.8.21 — reported 2026-09-30
|
||||
|
||||
- [x] Review and repair open paid-download PRs #161 and #162, refresh both
|
||||
branches from main, run independent and combined isolated suites, and verify
|
||||
rootless file permissions in disposable scratch storage. Combined result:
|
||||
1,585 passed, zero failed, four existing tests ignored. See the
|
||||
[review evidence and remaining acceptance work](pr-review-20260930.md).
|
||||
- [ ] Integrate the reviewed PR branches into the next release and run funded
|
||||
candidate acceptance, including Tor-only transport and payments with change.
|
||||
PRs remain open; the reviewed code has not been deployed to live wallets.
|
||||
- [ ] Design durable recovery for an accepted payment whose response is lost.
|
||||
Preserve the truthful unconfirmed-refund warning and prevent automatic
|
||||
duplicate payment while that recovery work is outstanding.
|
||||
- [ ] **ThinkPad X250 kiosk: Bitcoin installation version selector is unreadable
|
||||
and appears underneath the pruning information.** Operator reports white
|
||||
styling with invisible text on the actual kiosk; the same flow works in remote
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
# Paid-download PR review — 2026-09-30
|
||||
|
||||
## Scope and result
|
||||
|
||||
Reviewed both open PRs from the repository pull-request list: [#161](https://source.archipelago-foundation.org/lfg2025/archy/pulls/161)
|
||||
and [#162](https://source.archipelago-foundation.org/lfg2025/archy/pulls/162).
|
||||
Both branches were updated from main, repaired and tested independently and
|
||||
together. Their existing remote branches were advanced without rewriting the
|
||||
contributors' history. They remain open for integration into the release after
|
||||
1.8.21; no reviewed code was merged into main or deployed to a live wallet.
|
||||
The signed 1.8.21 artifacts are unchanged.
|
||||
|
||||
| Candidate | Tested commit | Isolated backend result |
|
||||
| --- | --- | --- |
|
||||
| PR #161 | `971d4777` | 1,576 passed, 0 failed, 4 existing tests ignored |
|
||||
| PR #162 | `0677924a` | 1,568 passed, 0 failed, 4 existing tests ignored |
|
||||
| Both together | `4bf4bf1a` | 1,585 passed, 0 failed, 4 existing tests ignored |
|
||||
|
||||
Both individual branches also passed production `cargo check`, with the
|
||||
repository's existing 16 warnings. The combined merge required no conflict
|
||||
resolution. Backend tests ran through `scripts/test-backend-isolated.sh` so they
|
||||
could not access host wallets, native services or production container storage.
|
||||
|
||||
## Findings and repairs
|
||||
|
||||
### #161 — payment delivery and file readability
|
||||
|
||||
- The branch conflicted with newer mint-fee, keyset-ID and truthful refund
|
||||
reporting fixes. Preserve those implementations from main; do not reintroduce
|
||||
its older unconditional “refunded” messages or duplicate keyset resolution.
|
||||
- Opening a file before charging, then reopening/reading it afterward, still
|
||||
permits a read failure after payment. Prepare the complete requested bytes
|
||||
before redemption, including ranged reads. Tests delete or alter the backing
|
||||
file during payment verification and still receive the prepared original data.
|
||||
- Empty/out-of-bounds/reversed ranges could fail after redemption, and empty
|
||||
files could underflow the range calculation. Validate ranges before charging
|
||||
and return HTTP 416 when unsatisfiable.
|
||||
- `chmod a+r` unnecessarily changed the permissions of shared paid/private
|
||||
files. Read restricted FileBrowser files through the rootless namespace while
|
||||
retaining their mode. Scope the fallback to regular files canonically inside
|
||||
FileBrowser storage, and reject unauthorized peers before reading.
|
||||
- A single-delivery flag must also prevent redirects and ambiguous transport
|
||||
retries. Payment-bearing GET and POST requests now retain the first HTTP
|
||||
response and do not retry after timeouts or disconnects that might follow
|
||||
delivery. Refused connections and normal nonpayment browsing retain the
|
||||
appropriate retry behavior.
|
||||
- Interrupted response bodies now report the outcome using the actual local
|
||||
refund result. Seller explanations are bounded, stripped of control
|
||||
characters and explicitly identified as peer text.
|
||||
- Original permission tests silently returned when run as root. Replacement
|
||||
tests inject read/payment boundary failures, exercise them under the isolated
|
||||
runner, and assert that read failures never invoke redemption.
|
||||
|
||||
### #162 — saving purchases in Files
|
||||
|
||||
- Its host-permission repair overlapped 1.8.21's authenticated Files API path.
|
||||
Review of [FileBrowser v2.63.23's resource handler](https://github.com/filebrowser/filebrowser/blob/v2.63.23/http/resource.go)
|
||||
showed that `override=false` checks for existence separately from opening
|
||||
with truncation. It does not guarantee no overwrites under concurrent saves.
|
||||
- The proposed direct path exposed the final filename before the write
|
||||
completed. Both direct and namespace paths now finish a private temporary
|
||||
file and publish it through a no-clobber hard link, retrying numbered names.
|
||||
- Plain `ln` could place a temporary file inside an existing directory instead
|
||||
of treating the destination as a collision. Use `ln -T`; existing directories
|
||||
and dangling symlinks are conflicts, never replacement targets.
|
||||
- Add filename and destination checks, unique temporary names, bounded name
|
||||
retries, synchronization before publication, and exact input-length checks.
|
||||
Truncated pipe input cannot become a completed purchased file.
|
||||
- Files storage remains optional. An unavailable copy destination does not
|
||||
undo the purchase or create a fake FileBrowser installation; the durable
|
||||
purchased-content cache remains primary.
|
||||
|
||||
## Additional verification on the development node
|
||||
|
||||
Used disposable scratch directories only, then removed them:
|
||||
|
||||
- Reproduced a FileBrowser-style rootless-owned 0640 upload. The host backend
|
||||
UID could not read it. `podman unshare cat` returned identical bytes without
|
||||
changing its 0640 mode.
|
||||
- Ran the exact namespace writer script with four concurrent writers against
|
||||
a directory owned by the container UID range. Every file had unique naming,
|
||||
exact bytes, the expected owner and mode, and no remaining temporary file.
|
||||
- Sent truncated input to the namespace writer and verified refusal, no final
|
||||
file and temporary-file cleanup.
|
||||
|
||||
The isolated tests additionally exercised 24 simultaneous direct writes,
|
||||
existing-file preservation, symlink/directory conflicts, collision exhaustion,
|
||||
root-independent permission failures, read-before-redemption ordering,
|
||||
authorization, redirects and peer disconnects.
|
||||
|
||||
Logs on the development box:
|
||||
`/tmp/archy-pr161-tests.log`, `/tmp/archy-pr162-tests.log`,
|
||||
`/tmp/archy-pr-integration-tests.log`, `/tmp/archy-pr161-check.log`,
|
||||
`/tmp/archy-pr162-check.log`, `/tmp/archy-pr-userns-scratch-test.log`.
|
||||
|
||||
## Next-release acceptance and limits
|
||||
|
||||
- Integrate the reviewed branches and repeat the release gates against the
|
||||
final release commit if additional code changes land.
|
||||
- Perform funded peer-to-peer acceptance on the candidate build, including a
|
||||
Tor-only purchase and a purchase requiring change, before the next release.
|
||||
The new review branches were not deployed to funded live wallets here.
|
||||
- These PRs do not implement durable payment receipts. If a seller redeems a
|
||||
payment and the connection subsequently loses the response, the buyer may
|
||||
receive an unconfirmed-refund warning. Do not represent that warning as proof
|
||||
of a refund or automatically charge the buyer again. Receipt-based recovery
|
||||
remains separate follow-up work.
|
||||
- Abrupt process termination can leave a hidden namespace temporary file;
|
||||
ordinary write failures and truncated input are tested to clean up. The final
|
||||
filename is published only after complete input, and existing files remain
|
||||
protected.
|
||||
- The separately reported X250 kiosk version-selector rendering issue remains
|
||||
open in `TODO.md` and requires validation on the actual kiosk.
|
||||
Reference in New Issue
Block a user