Author SHA1 Message Date
sneak a89659d967 Enforce real timeouts on gpg subprocess calls (closes #62)
check / check (push) Waiting to run
Every gpg run now has a one-minute deadline (gpgTimeout) on top of its
caller's context and is killed when either ends. A timeout is reported
as "gpg timed out" under the failing operation instead of "signal:
killed". Only gpg itself is killed; WaitDelay (one second) stops the run
from waiting on a process gpg left behind that still holds its output,
such as a wrapper script that does not exec the real gpg.
Builder.Build and Checker.ExtractEmbeddedSigningKeyFP take a context, so
ToManifest's context now reaches signing and the contextcheck
suppression calling signing non-cancellable is gone. Manifest loading
takes no context, so its signature check is bounded by the timeout
alone.

Model: opus-5-5
2026-10-04 01:33:35 +00:00
clawbot 1adad7d3bc Remove TODO.md; track open work in the issues (closes #76)
check / check (push) Waiting to run
TODO.md is deleted and the README TODO section now only points to the
issue tracker, where open work and open design questions live from now
on. AGENTS.md says so too, and says this overrides the shared policy's
README todo list for this repo. The README Open Questions section
becomes Original Design Questions, each noting where it now stands.

Every open item in both lists was already done or already owned by an
issue, apart from one, now #121,
and the chunk checksum question, now on
#81.

Model: opus-5-5
2026-10-04 03:32:09 +02:00
clawbot c31796998f Pin CLI error messages by driving their real call sites (closes #87)
check / check (push) Successful in 58s
errmsg_test.go now calls the function that emits each user-visible CLI
error message and asserts on the full text it returns, instead of
rendering format strings copied from production, so rewording any pinned
message fails the suite. The unknown-command message is driven through
the root command's action.

The signer-mismatch case signs a manifest with a throwaway gpg key and
is skipped where gpg is absent, like the repo's other signing tests.

The freshen mtime-presence test gives the scanned file an epoch mtime,
so it fails if recordEntry reads an absent manifest mtime as the epoch.

Model: opus-4-8 (first implementation); opus-5-5 (completion, this message)
2026-10-03 18:24:30 +02:00
clawbot a3749e9e9b cibuild: build with --no-cache like script/docker (closes #89)
check / check (push) Successful in 1m1s
A bare `docker build .` served the Dockerfile's check steps from the
layer cache on an unchanged tree and exited 0 without running them.
script/cibuild now computes the version on its own line and runs the
same build command as script/docker and the shared policy, --no-cache
included, so every CI build runs the checks. README.md describes
script/cibuild accordingly.

Model: opus-5-5
2026-10-03 17:41:55 +02:00
clawbot fa97c4519c Never write fetch's temp file into an existing file (closes #115)
check / check (push) Successful in 53s
downloadFile opened the temp file with os.Create, which opens and
empties a file already at that name. If that file was a hard link to a
file outside the destination directory, fetch overwrote the outside
file. It now removes whatever is at the temp name, which removes only
that name, and creates the temp file with O_EXCL, so the create fails if
anything is still or again there. A leftover temp file from an
interrupted run is still replaced. The new test puts a hard link at the
temp name and checks that fetch succeeds and the outside file is
unchanged. The command name is now the constant cmdFetch, like the other
command names, because lint requires it once a third test uses it.

Model: opus-5-5
2026-10-03 16:58:35 +02:00
clawbot 7e601929c8 Refuse fetch writes through a symlink in the destination (closes #86)
check / check (push) Successful in 1m10s
sanitizePath checks manifest paths only as text, so a symlink already
inside the destination directory could send fetch's writes outside it.
checkNoSymlinks now looks at each existing part of a path with os.Lstat
and refuses the path if any part is a symlink, wherever it points.
fetch runs it immediately before each write: creating the parent
directories, creating the temp file, and renaming it into place. The new
test puts such a symlink at each of those three places, and once inside
a plain directory, and checks that the fetch fails and nothing outside
changes. The G304 comment now states what holds. A symlink swapped in
between a check and its write is not caught; os.Root closes that once
the Go version is raised.

Model: opus-5-5
2026-10-03 16:08:09 +02:00
clawbot c3b5fe651a Stamp the tag or short commit in a plain docker build (closes #112)
check / check (push) Successful in 48s
.dockerignore now sends .git but not .git/config, which can hold a
credential and which git describe does not need. The build stage stamps
main.Gitrev from the VERSION build argument when one is given, otherwise
from git describe --tags --always, and fails if .git is present and no
version comes out. git there trusts /src whoever owns it, since a
context sent as a tar archive keeps its files' owners. script/docker is
replaced by the canonical copy, which passes VERSION; bin/gitrev.sh uses
--tags too, so every entrypoint stamps the same value for a clean
commit.

Model: opus-5-5
2026-10-02 08:53:34 +02:00
clawbot 0deacfc7ed Validate manifest entry paths on deserialize (closes #61)
check / check (push) Failing after 1s
Untrusted .mf files were parsed with no path validation, so an entry like ../../etc/passwd reached filepath.Join against the checker's base path and mfer check could stat and read outside it. ValidatePath ran only when building a manifest. It now runs on every entry as the manifest loads, so every consumer is covered. The whole manifest is rejected on the first bad entry instead of dropping it, which could hide files from a check; the error wraps errInvalidManifestPath and names the path.

Disclosure: a path that is not valid UTF-8 is refused earlier, by the protobuf string decoder, so that error does not name the path.

Model: opus-4-8 (implementation); fable-5-1 (summary)
2026-09-22 00:47:26 +02:00
clawbot 7de4d6ec1c Rewrite script/test to the canonical race-enabled pattern (closes #67)
check / check (push) Successful in 51s
script/test now runs go test -timeout 30s -race -cover ./..., quiet on success and rerunning verbose on failure. -race exposed a data race on the process-global apex/log logger: Init reconfigured it on every CLI run while other goroutines logged through it. internal/log now mutates the global under the write lock and reads it under the read lock; WithError is dropped because its Entry logged outside that lock, and run() reports a failed command's error via log.Errorf instead (still shown under -q). The corruption fuzz test is scaled to 1500 files / 100 iterations to fit the budget under -race; it pins the same claims at reduced breadth. Independent review ran the Docker lint gate uncached.

Model: opus-4-8 (implementation, review)
model: claude-fable-5
2026-09-21 15:17:50 +02:00
29 changed files with 1030 additions and 670 deletions
+62 -2
View File
@@ -1,4 +1,64 @@
# .dockerignore does NOT use .gitignore semantics. Docker matches with
# moby/patternmatcher: filepath.Match plus `**`, so `*` does not cross
# `/` and an unprefixed pattern is anchored at the context root. Every
# depth-independent pattern therefore needs `**/`, or `config/.env` and
# `certs/server.key` still ship while this file reads as solved. Only
# genuinely root-anchored entries go unprefixed. Never transplant these
# into .gitignore, where `**/` is wrong.
#
# Matching is case-sensitive, so secrets use character ranges rather
# than an ALL-CAPS twin, which would still miss `Server.Key`.
#
# Extend with this repo's own host-built artifacts, written anchored:
# `/myapp`, never `**/myapp`, which also matches `cmd/myapp/` and
# deletes the package directory from the context.
# .git is sent without its config. Without a VERSION build argument the
# stage that compiles runs `git describe --tags --always` on .git, which
# does not need .git/config; that file can hold a credential, such as a
# password in a remote URL or the token the CI checkout step stores there.
.git/config
# Agent scratch: one full checkout of the repo per in-flight agent.
# Anchored because it occurs once where agents run at the repo root.
# KNOWN GAP: a repo running agents in subdirectories still ships
# `services/api/.claude/` and must add its own anchored entry.
.claude
# Environment files. `*.env` covers bare `.env` and the `prod.env`
# convention. Re-include a committed template with a negation if the
# build needs one: `!docs/example.env`.
**/*.[eE][nN][vV]
**/.[eE][nN][vV].*
**/.[eE][nN][vV][rR][cC]
# Private keys and the bundles carrying them. Public certificates
# (*.crt, *.cer) are deliberately absent: they are legitimate inputs.
**/*.[pP][eE][mM]
**/*.[kK][eE][yY]
**/*.[pP]12
**/*.[pP][fF][xX]
**/[iI][dD]_[rR][sS][aA]
**/[iI][dD]_[dD][sS][aA]
**/[iI][dD]_[eE][cC][dD][sS][aA]
**/[iI][dD]_[eE][dD]25519
# Dependencies: restored inside the image, never copied in.
**/node_modules
# OS metadata.
**/.DS_Store
**/Thumbs.db
# Editor state: never a build input, and it churns COPY.
**/*.swp
**/*.swo
**/*~
**/*.bak
**/.idea
**/.vscode
**/*.sublime-*
# This repo's own host-built archives (Makefile).
*.tmp *.tmp
*.dockerimage *.dockerimage
.git
node_modules
+5 -2
View File
@@ -27,5 +27,8 @@ source for coding standards, formatting, linting, and workflow rules.
- The proto definition is in `mfer/mf.proto`; generated `.pb.go` files are - The proto definition is in `mfer/mf.proto`; generated `.pb.go` files are
committed (required for `go get` compatibility). committed (required for `go get` compatibility).
- The format specification is in `FORMAT.md`. - The format specification is in `FORMAT.md`.
- See the TODO section in `README.md` for the 1.0 implementation plan and open - Open work, open design questions included, is tracked only in the repo's
design questions. issues: https://git.eeqj.de/sneak/mfer/issues. There is no `TODO.md` and no
TODO list in `README.md`; do not add either. For this repo this overrides the
`REPO_POLICIES.md` rule to put the todo list in the README, per sneak's
ruling: https://git.eeqj.de/sneak/mfer/issues/76#issuecomment-118130.
+20 -1
View File
@@ -48,7 +48,26 @@ COPY . .
RUN touch mfer/mf.pb.go RUN touch mfer/mf.pb.go
RUN make test RUN make test
RUN cd cmd/mfer && go build -tags urfave_cli_no_docs -o /mfer .
# A build context sent as a tar archive, as upaas sends it, keeps its files'
# owners, and git refuses to read a checkout owned by another user.
RUN git config --system --add safe.directory /src
# The revision `mfer version` prints, stamped into main.Gitrev: the VERSION
# build argument when one is given (script/docker passes one), otherwise
# `git describe --tags --always` of the .git the build context carries: the
# tag on a tagged commit, tag-N-gHASH on a commit after one, the short commit
# when no tag is reachable. git ships in this base image. A context that
# carries .git and still yields no version fails the build.
ARG VERSION
RUN version="${VERSION:-$(git describe --tags --always)}"; \
if [ -e .git ] && { [ -z "$version" ] || [ "$version" = dev ] || \
[ "$version" = unknown ]; }; then \
echo "no version could be derived although the build context carries .git" >&2; \
exit 1; \
fi; \
cd cmd/mfer && \
go build -tags urfave_cli_no_docs -ldflags "-X main.Gitrev=$version" -o /mfer .
FROM scratch FROM scratch
COPY --from=builder /mfer /mfer COPY --from=builder /mfer /mfer
+30 -274
View File
@@ -1,12 +1,12 @@
# mfer # mfer
[mfer](https://git.eeqj.de/sneak/mfer) is a [WTFPL](https://wtfpl.net)-licensed [mfer](https://git.eeqj.de/sneak/mfer) is a reference implementation library and
(public domain) [Go](https://golang.org) library and command-line tool by thin wrapper command-line utility written in [Go](https://golang.org) and first
[@sneak](https://sneak.berlin) that specifies and generates `.mf` manifest files published in 2022 under the [WTFPL](https://wtfpl.net) (public domain) license.
over a directory tree to encapsulate metadata about the files — such as It specifies and generates `.mf` manifest files over a directory tree of files
cryptographic checksums and signatures over same — to aid in archiving, to encapsulate metadata about them (such as cryptographic checksums or
downloading, streaming, and mirroring. It was first published in 2022. The signatures over same) to aid in archiving, downloading, and streaming, or
manifest files' data is serialized with Google's mirroring. The manifest files' data is serialized with Google's
[protobuf serialization format](https://developers.google.com/protocol-buffers). [protobuf serialization format](https://developers.google.com/protocol-buffers).
The structure of these files can be found The structure of these files can be found
[in the format specification](https://git.eeqj.de/sneak/mfer/src/branch/main/mfer/mf.proto) [in the format specification](https://git.eeqj.de/sneak/mfer/src/branch/main/mfer/mf.proto)
@@ -21,40 +21,11 @@ This project was started by [@sneak](https://sneak.berlin) to scratch an itch in
as a de-facto standard and be incorporated into other software. A compatible as a de-facto standard and be incorporated into other software. A compatible
javascript library is planned. javascript library is planned.
# Getting Started
`mfer` builds from source with a Go 1.23+ toolchain. The generated protobuf code
is committed, so no `protoc` toolchain is required:
```sh
git clone https://git.eeqj.de/sneak/mfer.git
cd mfer
go build -o bin/mfer ./cmd/mfer
```
Generate a manifest for a directory tree, verify it later, and fetch a published
tree by URL:
```sh
# Write .index.mf describing every file under the current directory.
bin/mfer gen .
# Verify the files on disk against the manifest. Exits nonzero if any file
# is missing or corrupted.
bin/mfer check .index.mf
# Download and cryptographically verify a tree published over HTTP: mfer
# fetches <url>/index.mf, then downloads every file it lists.
bin/mfer fetch https://example.com/tree/
```
Run `bin/mfer help` for the full command list, or `bin/mfer <command> --help`
for a single command's options.
# Build Status # Build Status
CI runs via `script/cibuild` (`docker build .`), which executes `make check` CI runs `script/cibuild`, which builds the Docker image with `--no-cache`, so
(formatting, linting, tests). The `main` branch must always be green. the formatting, lint and test steps in the `Dockerfile` run on every build. The
`main` branch must always be green.
# Entrypoints # Entrypoints
@@ -86,8 +57,8 @@ provide:
Docker lint stage, whose image has no node Docker lint stage, whose image has no node
- `script/check` — run `script/test`, `script/lint`, and `script/fmt-check` - `script/check` — run `script/test`, `script/lint`, and `script/fmt-check`
- `script/docker` — build the Docker image tagged with the project name - `script/docker` — build the Docker image tagged with the project name
- `script/cibuild` — CI entrypoint: `docker build .` (the Dockerfile runs the - `script/cibuild` — CI entrypoint: builds the image with the same command as
checks) `script/docker`, uncached, so the checks in the Dockerfile run every time
- `script/precommit` — pre-commit checks: `go mod tidy` verification, then - `script/precommit` — pre-commit checks: `go mod tidy` verification, then
`script/check` `script/check`
- `script/install-precommit` — install the git pre-commit hook that runs - `script/install-precommit` — install the git pre-commit hook that runs
@@ -111,9 +82,7 @@ Any changes submitted to this project must also be
See [`REPO_POLICIES.md`](REPO_POLICIES.md) for detailed coding standards, See [`REPO_POLICIES.md`](REPO_POLICIES.md) for detailed coding standards,
tooling requirements, and workflow conventions. tooling requirements, and workflow conventions.
# Rationale # Problem Statement
## The problem
Given a plain URL, there is no standard way to safely and programmatically Given a plain URL, there is no standard way to safely and programmatically
download everything "under" that URL path. `wget -r` can traverse directory download everything "under" that URL path. `wget -r` can traverse directory
@@ -137,7 +106,7 @@ Real issues I face:
- when I download a large file via HTTP, I have no way of knowing if the file - when I download a large file via HTTP, I have no way of knowing if the file
content is what it's supposed to be content is what it's supposed to be
## The solution # Proposed Solution
A standard, a manifest file format, and a tool for generating same. A standard, a manifest file format, and a tool for generating same.
@@ -169,27 +138,6 @@ The manifest file would do several important things:
- maybe a bittorrent chunklist for torrent client compatibility? perhaps a - maybe a bittorrent chunklist for torrent client compatibility? perhaps a
top-level infohash for the whole manifest? top-level infohash for the whole manifest?
# Design
The repository is split into a reusable library and a thin command-line wrapper
around it.
- `mfer/` is the reusable library and the heart of the project: it defines the
manifest format and implements building, scanning, checking, serialization,
and signing. The protobuf schema is `mfer/mf.proto`, and the generated code it
produces (`mfer/mf.pb.go`) is committed alongside it so the library builds
with `go get` and needs no `protoc` toolchain.
- `internal/cli/` holds the command implementations — `gen`, `check`, `freshen`,
`export`, `list`, and `fetch` — that wire the library to the command-line
interface.
- `internal/log/` provides the logging used across the commands.
- `internal/bork/` provides error-handling support.
- `cmd/mfer/` is the entrypoint: its `main` package assembles the pieces above
into the `mfer` binary.
Everything under `internal/` is private to this repository; only the `mfer/`
package is intended for import by other software.
# Design Goals # Design Goals
- Replace SHASUMS/SHASUMS.asc files - Replace SHASUMS/SHASUMS.asc files
@@ -209,18 +157,23 @@ package is intended for import by other software.
- metadata size should not be used as an excuse to sacrifice utility (such - metadata size should not be used as an excuse to sacrifice utility (such
as providing checksums over each chunk of a large file) as providing checksums over each chunk of a large file)
# Open Questions # Original Design Questions
These were the open questions when the project started; open design questions
are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues).
- Should the manifest file include checksums of individual file chunks, or just - Should the manifest file include checksums of individual file chunks, or just
for the whole assembled file? for the whole assembled file? If so, should the chunk size be fixed or
dynamic? Still open, on
- If so, should the chunksize be fixed or dynamic? [issue 81](https://git.eeqj.de/sneak/mfer/issues/81#issuecomment-118698).
- Should the manifest signature format be GnuPG signatures, or those from - Should the manifest signature format be GnuPG signatures, or those from
OpenBSD's signify (of which there is a good OpenBSD's signify (of which there is a good
[golang implementation](https://github.com/frankbraun/gosignify)? [golang implementation](https://github.com/frankbraun/gosignify))? Still open,
as question 10 on [issue 82](https://git.eeqj.de/sneak/mfer/issues/82).
- Should the on-disk serialization format be proto3 or json? - Should the on-disk serialization format be proto3 or json? Settled: it is
proto3, see `FORMAT.md` and `mfer/mf.proto`.
# Tool Examples # Tool Examples
@@ -298,207 +251,10 @@ regardless of filesystem format.
Please email [`sneak@sneak.berlin`](mailto:sneak@sneak.berlin) with your desired Please email [`sneak@sneak.berlin`](mailto:sneak@sneak.berlin) with your desired
username for an account on this Gitea instance. username for an account on this Gitea instance.
# TODO: Remaining Work for 1.0 # TODO
## Design Questions (Owner Decision Required) Open work, open design questions included, is tracked in this repo's issues:
[https://git.eeqj.de/sneak/mfer/issues](https://git.eeqj.de/sneak/mfer/issues).
These require @sneak's input before implementation. Answers should be added
inline below each question.
### Format Design
**1. Should `MFFileChecksum` be simplified?** Currently it's a separate message
wrapping a single `bytes multiHash` field. Since multihash already
self-describes the algorithm, `repeated bytes hashes` directly on `MFFilePath`
would be simpler and reduce per-file protobuf overhead. Is the extra message
layer intentional (e.g. planning to add per-hash metadata like `verified_at`)?
> _answer:_
**2. Should file permissions/mode be stored?** The format stores mtime/ctime but
not Unix file permissions. For archival use this may not matter, but for
software distribution or filesystem restoration it's a gap. Should we reserve a
field now (e.g. `optional uint32 mode = 305`) even if we don't populate it yet?
> _answer:_
**3. Should `atime` be removed from the schema?** Access time is volatile,
non-deterministic, and often disabled (`noatime`). Including it means two
manifests of the same directory at different times will differ, which conflicts
with the determinism goal. Remove it, or document it as "never set by default"?
> _answer:_
**4. What are the path normalization rules?** The proto has `string path` with
no specification about: always forward-slash? Must be relative? No `..`
components allowed? UTF-8 NFC vs NFD normalization (macOS vs Linux)? Max path
length? This is a security issue (path traversal) and a cross-platform
compatibility issue. What rules should the spec mandate?
> _answer:_
**5. Should we add a version byte after the magic?** Currently `ZNAVSRFG` is
followed immediately by protobuf. Adding a version byte (`ZNAVSRFG\x01`) would
allow future framing changes without requiring protobuf parsing to detect the
version. `MFFileOuter.Version` serves this purpose but requires successful
deserialization to read. Worth the extra byte?
> _answer:_
**6. Should we add a length-prefix after the magic?** Protobuf is not
self-delimiting. If we ever want to concatenate manifests or append data after
the protobuf, the current framing is insufficient. Add a varint or fixed-width
length-prefix?
> _answer:_
### Signature Design
**7. What does the outer SHA-256 hash cover — compressed or uncompressed data?**
The code currently hashes compressed data (good for verifying before
decompression), but this should be explicitly documented. Which is the intended
behavior?
> _answer:_
**8. Should `signatureString()` sign raw bytes instead of a hex-encoded
string?** Currently the canonical string is `MAGIC-UUID-MULTIHASH` with hex
encoding, which adds a transformation layer. Signing the raw `sha256` bytes (or
compressed `innerMessage` directly) would be simpler. Keep the string format or
switch to raw bytes?
> _answer:_
**9. Should we support detached signature files (`.mf.sig`)?** Embedded
signatures are better for single-file distribution. Detached `.mf.sig` files
follow the familiar `SHASUMS`/`SHASUMS.asc` pattern and are simpler for HTTP
serving. Support both modes?
> _answer:_
**10. GPG vs pure-Go crypto for signatures?** Shelling out to `gpg` is fragile
(may not be installed, version-dependent output).
`github.com/ProtonMail/go-crypto` provides pure-Go OpenPGP, or we could use
Ed25519/signify (simpler, no key management). Which direction?
> _answer:_
### Implementation Design
**11. Should manifests be deterministic by default?** This means: sort file
entries by path, omit `createdAt` timestamp (or make it opt-in), no `atime`.
Should determinism be the default, with a `--include-timestamps` flag to opt in?
> _answer:_
**12. Should we consolidate or keep both scanner/checker implementations?**
There are two parallel implementations: `mfer/scanner.go` + `mfer/checker.go`
(typed with `FileSize`, `RelFilePath`) and `internal/scanner/` +
`internal/checker/` (raw `int64`, `string`). The `mfer/` versions are superior.
Delete the `internal/` versions?
> _answer:_
**13. Should the `manifest` type be exported?** Currently unexported with
exported constructors (`NewManifestFromReader`, `NewManifestFromFile`).
Consumers can't declare `var m *mfer.manifest`. Export the type, or define an
interface?
> _answer:_
**14. What should the Go module path be for 1.0?** Currently
`sneak.berlin/go/mfer` in `go.mod` but `git.eeqj.de/sneak/mfer/mfer` in the
proto `go_package` option. Which is canonical?
> _answer:_
## Implementation Tasks
### Repo Infrastructure
- [ ] Add `.golangci.yml` (fetch from
`https://git.eeqj.de/sneak/prompts/raw/branch/main/.golangci.yml`)
- [ ] Add `.editorconfig`
- [ ] Add `.gitea/workflows/check.yml` that runs `docker build .`
### Format & Correctness
- [ ] Resolve proto `go_package` path inconsistency
(`git.eeqj.de/sneak/mfer/mfer` vs `sneak.berlin/go/mfer`)
- [ ] Specify path invariants — add proto comments requiring UTF-8,
forward-slash, relative paths, no `..`, no leading `/`; validate in
`Builder.AddFile` and `Builder.AddFileWithHash` (pending design question
answer)
- [ ] Remove or deprecate `atime` from proto (pending design question answer)
- [ ] Reserve `optional uint32 mode = 305` in `MFFilePath` for future file
permissions (pending design question answer)
- [ ] Add version byte after magic — `ZNAVSRFG\x01` for format version 1
(pending design question answer)
- [ ] Write format specification document — separate from README: magic, outer
structure, compression, inner structure, path invariants, signature
scheme, canonical serialization
### Library
- [ ] Delete `internal/scanner/` and `internal/checker/` — consolidate on
`mfer/` package versions; update CLI code (pending design question answer)
- [ ] Add deterministic file ordering — sort entries by path (lexicographic,
byte-order) in `Builder.Build()`; add test asserting byte-identical output
from two runs
- [ ] Add decompression size limit — `io.LimitReader` in `deserializeInner()`
with `m.pbOuter.Size` as bound
- [ ] Fix `errors.Is` dead code in checker — replace with `os.IsNotExist(err)`
or `errors.Is(err, fs.ErrNotExist)`
- [ ] Fix `AddFile` to verify size — check `totalRead == size` after reading,
return error on mismatch
- [ ] Export the `manifest` type or define a public interface (pending design
question answer) — currently consumers cannot hold a reference to a loaded
manifest in their own type declarations
- [ ] Replace GPG subprocess calls with pure-Go crypto (pending design question
answer) — current implementation shells out to `gpg` which may not be
installed
- [ ] Add timeout to any remaining subprocess calls
### CLI
- [ ] Fix flag naming — all CLI flags should use kebab-case as primary
(`--include-dotfiles`, `--follow-symlinks`)
- [ ] Fix URL construction in fetch — use `BaseURL.JoinPath()` or
`url.JoinPath()` instead of string concatenation
- [ ] Add progress rate-limiting to Checker — throttle to once per second,
matching Scanner
- [ ] Add `--deterministic` flag or make it default — omit `createdAt`, sort
files (pending design question answer)
- [ ] Wire `--version` flag properly (currently only a `version` subcommand
exists; top-level `--version` shows urfave/cli generic output)
- [ ] Add retry logic to `fetch` — currently no retries on transient HTTP
errors; needs exponential backoff
- [ ] `fetch` command uses bare `http.Get` with no timeout — needs `http.Client`
with configurable timeout
### Testing & Robustness
- [ ] Add fuzzing tests for `NewManifestFromReader` — protobuf deserialization
of untrusted input needs fuzz coverage
- [ ] Add integration test for `freshen` CLI command — current tests only verify
setup, not the actual freshen operation end-to-end
- [ ] Add test for `fetch` CLI command end-to-end (currently only `downloadFile`
is tested)
### Documentation
- [ ] Promote `FORMAT.md` as primary spec reference; README should link to it
more prominently
- [ ] Audit and update all error messages for consistency and helpfulness
- [ ] Document the signature scheme more thoroughly (canonical string format,
verification steps)
### Release
- [ ] Finalize Go module path
- [ ] Update version constant in `mfer/constants.go`
- [ ] Add `--version` output matching SemVer
- [ ] Tag `v1.0.0`
# See Also # See Also
@@ -515,9 +271,9 @@ proto `go_package` option. Which is canonical?
- Issues: - Issues:
[https://git.eeqj.de/sneak/mfer/issues](https://git.eeqj.de/sneak/mfer/issues) [https://git.eeqj.de/sneak/mfer/issues](https://git.eeqj.de/sneak/mfer/issues)
# Author # Authors
- [@sneak](https://sneak.berlin) - [@sneak &lt;sneak@sneak.berlin&gt;](mailto:sneak@sneak.berlin)
# License # License
-117
View File
@@ -1,117 +0,0 @@
# Workflow
- branch (from `main`)
- do the work in Next Step
- move Next Step to the top of Completed Steps
- move the top item of Future Steps into Next Step
- commit (`TODO.md` changes in the same commit as the work)
- merge to `main` if the branch is not protected, otherwise open a PR
- push
# Status
pre-1.0. No git tags. README section "TODO: Remaining Work for 1.0" lists open
design questions and implementation tasks; policy compliance work is in flight
and unmerged.
# Next Step
Work through the remaining compliance items folded from the 2026-07-02 audit
(the first group under Future Steps): `.editorconfig`, `.gitignore` coverage,
gofumpt-based `fmt-check`, README "Getting Started", and the rest.
`.golangci.yml` and `TODO.md` are tracked and committed as of 2026-08-07, so the
only thing left of the `chore/align-repo-policies` branch is the list below.
# Completed Steps
- 2026-09-21: added the required README sections — named author, license, and
category in the Description first line; added Getting Started (verified
install/usage block), Rationale (folding in Problem Statement and Proposed
Solution), and a Design section for the package layout; renamed Authors to
Author (#75)
- 2026-09-21: added the canonical `.editorconfig`, made `.gitignore` cover
secrets, OS, editor, and Go artifacts, and removed the dead Drone CI
references from `.gitignore` and `bin/gitrev.sh` (#72)
- 2026-08-09: added `.prettierrc`/`.prettierignore`, gave `script/fmt` and
`script/fmt-check` one shared prettier file set via `script/prettier`, dropped
the `|| true` that hid prettier failures, and added a node-based Dockerfile
stage so a markdown formatting violation fails `docker build .` (#69)
- 2026-08-07: updated golangci-lint to v2.12.2 everywhere it is pinned
(`Makefile`, `Dockerfile`), added the canonical `.golangci.yml`
(`default: all`), and fixed all resulting lint findings across the codebase
- 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints, Makefile
shims, README Entrypoints section
- 2026-07-03: aligned repo tooling, docs, and config with standardized policies
(7d9a138, on chore/align-repo-policies, unmerged)
- 2026-06-28: moved to standardized repo policies (#56, on main)
- 2026-04-07: added 1.0 roadmap as README TODO section, removed old TODO.md
(#54)
- 2026-03-20: added Gitea Actions CI workflow (#53)
- 2026-03-17: added REPO_POLICIES.md, renamed CLAUDE.md to AGENTS.md (#51);
removed committed .index.mf (#52)
- 2026-03-15: split Dockerfile with pre-built golangci-lint stage for faster CI
(#45)
- 2026-03-01: 1.0 quality polish: code review, tests, bug fixes, docs (#32)
- 2026-02-20: deterministic file ordering in Builder.Build() (#28); removed
committed vendor/modcache archives (#35)
- 2026-02-08: added --seed flag for deterministic manifest UUID
# Future Steps
- Compliance (fold of TODO.md audit 2026-07-02; verify which items the in-flight
branch already closes, then check off):
- Add .editorconfig (canonical copy from sneak/prompts)
- Make .gitignore cover secrets (.env, _.key, _.pem), OS files (.DS_Store),
and editor files (_.swp, _~)
- Make fmt-check/lint verify with gofumpt, not gofmt -l, so `make check`
matches what `make fmt` writes
- Add README "Getting Started" section with copy-pasteable install/usage
block
- Move FORMAT.md from repo root to docs/ and update the AGENTS.md reference
- Pin Makefile-installed Go tools (`protoc-gen-go@v1.28.1`,
`golangci-lint@v2.12.2`) by module hash, not mutable tag
- Set `make test` timeout to 30s (currently 10s)
- Add explicit README "Rationale" heading (content exists under other
names); name the author in the README Description first line
- Reconcile root-level AGENTS.md with directory-hygiene policy (keep or
relocate)
- Add a `make build` target
- Rewrite `make hooks` to use printf or a heredoc instead of non-portable
`echo '...\n...'`
- Answer the 14 owner design questions in the README 1.0 roadmap:
- Format: simplify MFFileChecksum; store file mode; drop atime; specify path
normalization rules; version byte after magic; length-prefix after magic
- Signatures: hash covers compressed or uncompressed data; sign raw bytes vs
hex canonical string; detached .mf.sig support; GPG subprocess vs pure-Go
crypto
- Implementation: deterministic manifests by default; consolidate duplicate
scanner/checker implementations; export the manifest type; canonical Go
module path for 1.0
- Format and correctness:
- Resolve proto go_package vs go.mod module path inconsistency
- Specify and validate path invariants (UTF-8, forward-slash, relative, no
.., no leading /)
- Remove or deprecate atime; reserve mode field; add version byte (all
pending design answers)
- Write a standalone format specification document
- Library:
- Delete internal/scanner and internal/checker; consolidate on the mfer/
package versions (pending design answer)
- Add decompression size limit via io.LimitReader in deserializeInner()
- Fix errors.Is dead code in checker; make AddFile verify totalRead == size
- Export manifest type or define a public interface (pending)
- Replace GPG subprocess with pure-Go crypto (pending); add timeouts to
remaining subprocess calls
- CLI:
- Kebab-case primary flag names; fix fetch URL construction with
url.JoinPath; add http.Client timeout and retry with backoff to fetch;
rate-limit Checker progress output; add --deterministic flag or default;
wire top-level --version properly
- Testing:
- Fuzz NewManifestFromReader; end-to-end tests for freshen and fetch
- Documentation:
- Promote docs/FORMAT.md as primary spec reference; audit error messages;
document the signature scheme fully
- Release:
- Finalize module path, bump version constant, SemVer --version output, tag
v1.0.0
+1 -1
View File
@@ -3,5 +3,5 @@
if [[ ! -z "$GITREV" ]]; then if [[ ! -z "$GITREV" ]]; then
echo $GITREV echo $GITREV
else else
git describe --always --dirty=-dirty git describe --tags --always --dirty=-dirty
fi fi
+6 -3
View File
@@ -2,6 +2,7 @@
package cli package cli
import ( import (
"context"
"encoding/hex" "encoding/hex"
"errors" "errors"
"fmt" "fmt"
@@ -126,7 +127,9 @@ func (mfa *CLIApp) fetchManifestToTemp(url string) (string, error) {
// verifyRequiredSigner enforces the --require-signature fingerprint // verifyRequiredSigner enforces the --require-signature fingerprint
// against the manifest's embedded signing key. // against the manifest's embedded signing key.
func verifyRequiredSigner(chk *mfer.Checker, requiredSigner string) error { func verifyRequiredSigner(
ctx context.Context, chk *mfer.Checker, requiredSigner string,
) error {
// Validate fingerprint format: must be exactly 40 hex characters // Validate fingerprint format: must be exactly 40 hex characters
if len(requiredSigner) != fingerprintHexLen { if len(requiredSigner) != fingerprintHexLen {
return fmt.Errorf("%w, got %d", errInvalidFingerprint, len(requiredSigner)) return fmt.Errorf("%w, got %d", errInvalidFingerprint, len(requiredSigner))
@@ -145,7 +148,7 @@ func verifyRequiredSigner(chk *mfer.Checker, requiredSigner string) error {
// Extract fingerprint from the embedded public key (not from the // Extract fingerprint from the embedded public key (not from the
// signer field). This validates the key is importable and gets its // signer field). This validates the key is importable and gets its
// actual fingerprint. // actual fingerprint.
embeddedFP, err := chk.ExtractEmbeddedSigningKeyFP() embeddedFP, err := chk.ExtractEmbeddedSigningKeyFP(ctx)
if err != nil { if err != nil {
return fmt.Errorf( return fmt.Errorf(
"failed to extract fingerprint from embedded signing key: %w", err) "failed to extract fingerprint from embedded signing key: %w", err)
@@ -303,7 +306,7 @@ func (mfa *CLIApp) checkManifestOperation(ctx *cli.Context) error {
// Check signature requirement // Check signature requirement
requiredSigner := ctx.String("require-signature") requiredSigner := ctx.String("require-signature")
if requiredSigner != "" { if requiredSigner != "" {
err = verifyRequiredSigner(chk, requiredSigner) err = verifyRequiredSigner(ctx.Context, chk, requiredSigner)
if err != nil { if err != nil {
return err return err
} }
+11 -9
View File
@@ -126,7 +126,7 @@ func TestHelpCommand(t *testing.T) {
stdout := testStdout(t, opts) stdout := testStdout(t, opts)
assert.Contains(t, stdout, cmdGenerate) assert.Contains(t, stdout, cmdGenerate)
assert.Contains(t, stdout, cmdCheck) assert.Contains(t, stdout, cmdCheck)
assert.Contains(t, stdout, "fetch") assert.Contains(t, stdout, cmdFetch)
} }
func TestGenerateCommand(t *testing.T) { func TestGenerateCommand(t *testing.T) {
@@ -679,11 +679,13 @@ func TestCheckDetectsManifestCorruption(t *testing.T) {
fs := afero.NewMemMapFs() fs := afero.NewMemMapFs()
rng := rand.New(rand.NewSource(42)) //nolint:gosec // deterministic test data rng := rand.New(rand.NewSource(42)) //nolint:gosec // deterministic test data
// Create many small files with random names to generate a ~1MB manifest // Create many small files with random names so the manifest has many
// Each manifest entry is roughly 50-60 bytes, so we need ~20000 files // entries and random single-byte flips land at varied offsets. Each
// manifest entry is roughly 50-60 bytes. Kept modest so the suite stays
// within its wall-clock budget under -race.
require.NoError(t, fs.MkdirAll(testDir, 0o755)) require.NoError(t, fs.MkdirAll(testDir, 0o755))
numFiles := 20000 numFiles := 1500
for range numFiles { for range numFiles {
// Generate random filename // Generate random filename
filename := fmt.Sprintf("/testdir/%08x%08x%08x.dat", filename := fmt.Sprintf("/testdir/%08x%08x%08x.dat",
@@ -699,11 +701,11 @@ func TestCheckDetectsManifestCorruption(t *testing.T) {
exitCode := runCLI(opts) exitCode := runCLI(opts)
require.Equal(t, 0, exitCode, "generate should succeed") require.Equal(t, 0, exitCode, "generate should succeed")
// Read the valid manifest and verify it's approximately 1MB // Read the valid manifest and verify it has real size.
validManifest, err := afero.ReadFile(fs, testManifest) validManifest, err := afero.ReadFile(fs, testManifest)
require.NoError(t, err) require.NoError(t, err)
require.GreaterOrEqual(t, len(validManifest), 1024*1024, require.GreaterOrEqual(t, len(validManifest), 64*1024,
"manifest should be at least 1MB, got %d bytes", len(validManifest)) "manifest should be at least 64KB, got %d bytes", len(validManifest))
t.Logf("manifest size: %d bytes (%d files)", len(validManifest), numFiles) t.Logf("manifest size: %d bytes (%d files)", len(validManifest), numFiles)
// First corruption: truncate the manifest // First corruption: truncate the manifest
@@ -726,8 +728,8 @@ func TestCheckDetectsManifestCorruption(t *testing.T) {
exitCode = runCLI(opts) exitCode = runCLI(opts)
require.Equal(t, 0, exitCode, "check should pass with valid manifest") require.Equal(t, 0, exitCode, "check should pass with valid manifest")
// Now do 500 random corruption iterations // Now do 100 random corruption iterations
for i := range 500 { for i := range 100 {
// Corrupt: write a random byte at a random offset // Corrupt: write a random byte at a random offset
corrupted := make([]byte, len(validManifest)) corrupted := make([]byte, len(validManifest))
copy(corrupted, validManifest) copy(corrupted, validManifest)
+322 -135
View File
@@ -2,167 +2,354 @@
package cli package cli
import ( import (
"fmt" "bytes"
"context"
"flag"
"net/http"
"net/http/httptest"
"os"
"os/exec"
"path/filepath"
"testing" "testing"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
urfcli "github.com/urfave/cli/v2"
"sneak.berlin/go/mfer/mfer"
) )
// errMsgCase is one pinned user-visible error message. // These tests pin the exact rendered text of the CLI's user-visible error
type errMsgCase struct { // messages. The messages are grepped for in CI pipelines and quoted in bug
name string // reports, so a reword is a deliberate change, never a refactoring side
err error // effect.
want string //
} // Every case drives the real function that emits the message and asserts on
// what it returns. No production format string is restated here: a test that
// only re-rendered a copied format string would keep passing after the real
// message changed, which is exactly the regression these tests exist to
// catch.
// Full 40-hex fingerprints used where a message embeds one.
const ( const (
msgFpA = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" msgFpA = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
msgFpB = "BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB" msgFpB = "BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"
) )
func checkErrMsgCases(t *testing.T, cases []errMsgCase) { // runLocked runs fn while holding runMu, so operations that write to the
// process-global logger do not race the other CLI runs.
func runLocked(fn func() error) error {
runMu.Lock()
defer runMu.Unlock()
return fn()
}
// unsignedChecker builds a Checker over a freshly scanned, unsigned manifest.
func unsignedChecker(t *testing.T) *mfer.Checker {
t.Helper() t.Helper()
for _, tc := range cases { fs := afero.NewMemMapFs()
t.Run(tc.name, func(t *testing.T) { require.NoError(t, fs.MkdirAll("/d", 0o755))
t.Parallel() require.NoError(t, afero.WriteFile(fs, "/d/f.txt", []byte("hi"), 0o644))
assert.Equal(t, tc.want, tc.err.Error())
}) s := mfer.NewScannerWithOptions(&mfer.ScannerOptions{Fs: fs})
} require.NoError(t, s.EnumeratePath("/d", nil))
var buf bytes.Buffer
require.NoError(t, s.ToManifest(context.Background(), &buf, nil))
require.NoError(t, afero.WriteFile(fs, "/d/index.mf", buf.Bytes(), 0o644))
chk, err := mfer.NewChecker("/d/index.mf", "/d", fs)
require.NoError(t, err)
require.False(t, chk.IsSigned())
return chk
} }
// TestErrorMessagesVerbatim pins the exact rendered text of the CLI's func TestNoManifestFoundMessage(t *testing.T) {
// user-visible error messages. t.Parallel()
_, err := findManifest(afero.NewMemMapFs(), "/tmp/x")
require.ErrorIs(t, err, errNoManifestFound)
assert.EqualError(t, err,
"no manifest found in /tmp/x (looked for index.mf and .index.mf)")
}
func TestVerifyRequiredSignerMessages(t *testing.T) {
t.Parallel()
t.Run("invalid fingerprint length", func(t *testing.T) {
t.Parallel()
err := verifyRequiredSigner(context.Background(),
unsignedChecker(t), "12345678")
require.ErrorIs(t, err, errInvalidFingerprint)
assert.EqualError(t, err,
"invalid fingerprint: must be exactly 40 hex characters, got 8")
})
t.Run("manifest not signed", func(t *testing.T) {
t.Parallel()
err := verifyRequiredSigner(context.Background(),
unsignedChecker(t), msgFpA)
require.ErrorIs(t, err, errManifestNotSigned)
assert.EqualError(t, err,
"manifest is not signed, but signature from "+msgFpA+" is required")
})
}
// TestSignerMismatchMessage drives verifyRequiredSigner against a real signed
// manifest. The embedded fingerprint is whatever the generated key produced,
// so it is read back from the checker and substituted into the expected
// string; the required signer is a fixed value that cannot match it. Requires
// gpg and is skipped where it is absent, as the other signing tests are.
// //
// These strings are an interface: they are grepped for in CI pipelines //nolint:paralleltest // signedChecker calls t.Setenv, which bars t.Parallel
// and quoted in bug reports. The messages are assembled by wrapping func TestSignerMismatchMessage(t *testing.T) {
// static sentinels, and it is easy to change what a user sees while chk := signedChecker(t)
// only meaning to make an error matchable with errors.Is - which is
// precisely what happened once already. Any change to a string below is embeddedFP, err := chk.ExtractEmbeddedSigningKeyFP(context.Background())
// therefore a deliberate, separately stated change, never a side effect require.NoError(t, err)
// of a refactor.
func TestErrorMessagesVerbatim(t *testing.T) { err = verifyRequiredSigner(context.Background(), chk, msgFpB)
require.ErrorIs(t, err, errSignerMismatch)
assert.EqualError(t, err,
"embedded signing key fingerprint "+embeddedFP+
" does not match required "+msgFpB)
}
// signedChecker builds a Checker over a manifest signed by a throwaway GPG
// key generated in a temporary GNUPGHOME.
func signedChecker(t *testing.T) *mfer.Checker {
t.Helper()
_, err := exec.LookPath("gpg")
if err != nil {
t.Skip("gpg not installed, skipping signing test")
}
gpgHome := t.TempDir()
params := "%no-protection\n" +
"Key-Type: RSA\nKey-Length: 2048\n" +
"Name-Real: MFER Test Key\nName-Email: test@mfer.test\n" +
"Expire-Date: 0\n%commit\n"
paramsFile := filepath.Join(gpgHome, "key-params")
require.NoError(t, os.WriteFile(paramsFile, []byte(params), 0o600))
//nolint:gosec // paramsFile is a test-controlled path inside t.TempDir()
cmd := exec.CommandContext(context.Background(), "gpg",
"--batch", "--gen-key", paramsFile)
cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHome)
out, err := cmd.CombinedOutput()
if err != nil {
t.Skipf("failed to generate test GPG key: %v: %s", err, out)
}
t.Setenv("GNUPGHOME", gpgHome)
b := mfer.NewBuilder()
b.SetSigningOptions(&mfer.SigningOptions{KeyID: mfer.GPGKeyID("test@mfer.test")})
content := []byte("signed file")
_, err = b.AddFile("f.txt", mfer.FileSize(len(content)), mfer.ModTime{},
bytes.NewReader(content), nil)
require.NoError(t, err)
var buf bytes.Buffer
require.NoError(t, b.Build(context.Background(), &buf))
fs := afero.NewMemMapFs()
require.NoError(t, afero.WriteFile(fs, "/index.mf", buf.Bytes(), 0o644))
chk, err := mfer.NewChecker("/index.mf", "/", fs)
require.NoError(t, err)
require.True(t, chk.IsSigned())
return chk
}
func TestPathDoesNotExistMessage(t *testing.T) {
t.Parallel() t.Parallel()
checkErrMsgCases(t, []errMsgCase{ set := flag.NewFlagSet("gen", flag.ContinueOnError)
{ require.NoError(t, set.Parse([]string{"nope"}))
name: "check: no manifest found",
err: fmt.Errorf("%w in %s (looked for index.mf and .index.mf)", mfa := &CLIApp{Fs: afero.NewMemMapFs()}
errNoManifestFound, "/tmp/x"), ctx := urfcli.NewContext(nil, set, nil)
want: "no manifest found in /tmp/x " +
"(looked for index.mf and .index.mf)", _, err := mfa.collectInputPaths(ctx.Args())
}, require.ErrorIs(t, err, errPathNotExist)
{ assert.EqualError(t, err, "path does not exist: nope")
name: "check: invalid fingerprint length", }
err: fmt.Errorf("%w, got %d", errInvalidFingerprint, 8),
want: "invalid fingerprint: must be exactly 40 hex characters, got 8", func TestOutputFileExistsMessage(t *testing.T) {
}, t.Parallel()
{
name: "check: manifest not signed", fs := afero.NewMemMapFs()
err: fmt.Errorf("%w, but signature from %s is required", require.NoError(t, fs.MkdirAll("/d", 0o755))
errManifestNotSigned, msgFpA), require.NoError(t, afero.WriteFile(fs, "/d/f.txt", []byte("hi"), 0o644))
want: "manifest is not signed, but signature from " + msgFpA + require.NoError(t, afero.WriteFile(fs, "/out.mf", []byte("old"), 0o644))
" is required",
}, set := flag.NewFlagSet("gen", flag.ContinueOnError)
{ set.String("output", "", "")
name: "check: signer mismatch", set.Bool("force", false, "")
err: fmt.Errorf("embedded signing key fingerprint %s %w %s", require.NoError(t, set.Parse([]string{"/d"}))
msgFpA, errSignerMismatch, msgFpB), require.NoError(t, set.Set("output", "/out.mf"))
want: "embedded signing key fingerprint " + msgFpA +
" does not match required " + msgFpB, mfa := &CLIApp{Fs: fs}
}, ctx := urfcli.NewContext(nil, set, nil)
{
name: "gen: path does not exist", // generateManifestOperation writes to the process-global logger during
err: fmt.Errorf("%w: %s", errPathNotExist, "nope"), // enumeration, so serialize with the other CLI runs.
want: "path does not exist: nope", err := runLocked(func() error { return mfa.generateManifestOperation(ctx) })
}, require.ErrorIs(t, err, errOutputExists)
{ assert.EqualError(t, err,
name: "gen: output file exists", "output file /out.mf already exists (use --force to overwrite)")
err: fmt.Errorf("output file %s %w", "index.mf", errOutputExists), }
want: "output file index.mf already exists " +
"(use --force to overwrite)", // TestUnknownCommandMessage drives the root command's action. run only logs
}, // the error that action returns, so the test lets run build the app with no
{ // command given and then runs that same app on an unknown command to get the
name: "mfer: unknown command", // error itself.
err: fmt.Errorf("%w %q", errUnknownCommand, "bogus"), func TestUnknownCommandMessage(t *testing.T) {
want: `unknown command "bogus"`, t.Parallel()
},
mfa := &CLIApp{
appname: testApp,
Stdout: &bytes.Buffer{},
Stderr: &bytes.Buffer{},
Fs: afero.NewMemMapFs(),
}
// run points the process-global logger at this app's output, so
// serialize with the other CLI runs.
err := runLocked(func() error {
mfa.run([]string{testApp})
return mfa.app.Run([]string{testApp, "bogus"})
})
require.ErrorIs(t, err, errUnknownCommand)
assert.EqualError(t, err, `unknown command "bogus"`)
}
func TestManifestLoaderHTTPStatusMessage(t *testing.T) {
t.Parallel()
server := httptest.NewServer(
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusNotFound)
}))
defer server.Close()
mfa := &CLIApp{Fs: afero.NewMemMapFs()}
_, err := mfa.openManifestReader(server.URL + "/foo.mf")
require.ErrorIs(t, err, errHTTPStatus)
assert.EqualError(t, err,
"failed to fetch "+server.URL+"/foo.mf: HTTP 404")
}
func TestFetchManifestHTTPStatusMessage(t *testing.T) {
t.Parallel()
server := httptest.NewServer(
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusNotFound)
}))
defer server.Close()
set := flag.NewFlagSet("fetch", flag.ContinueOnError)
require.NoError(t, set.Parse([]string{server.URL}))
mfa := &CLIApp{Fs: afero.NewMemMapFs()}
ctx := urfcli.NewContext(nil, set, nil)
// fetchManifestOperation logs to the process-global logger.
err := runLocked(func() error { return mfa.fetchManifestOperation(ctx) })
require.ErrorIs(t, err, errHTTPStatus)
assert.EqualError(t, err, "failed to fetch manifest: HTTP 404")
}
func TestFetchFileHTTPStatusMessage(t *testing.T) {
t.Parallel()
server := httptest.NewServer(
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusInternalServerError)
}))
defer server.Close()
err := downloadFile(context.Background(), server.URL+"/x", "x",
&mfer.MFFilePath{}, nil)
require.ErrorIs(t, err, errHTTPStatus)
assert.EqualError(t, err, "HTTP 500")
}
func TestURLRequiredMessage(t *testing.T) {
t.Parallel()
set := flag.NewFlagSet("fetch", flag.ContinueOnError)
require.NoError(t, set.Parse([]string{}))
mfa := &CLIApp{Fs: afero.NewMemMapFs()}
ctx := urfcli.NewContext(nil, set, nil)
// fetchManifestOperation logs to the process-global logger.
err := runLocked(func() error { return mfa.fetchManifestOperation(ctx) })
require.ErrorIs(t, err, errURLRequired)
assert.EqualError(t, err, "URL argument required")
}
func TestSanitizePathMessages(t *testing.T) {
t.Parallel()
t.Run("empty", func(t *testing.T) {
t.Parallel()
_, err := sanitizePath("")
require.ErrorIs(t, err, errEmptyPath)
assert.EqualError(t, err, "empty path")
})
t.Run("absolute", func(t *testing.T) {
t.Parallel()
_, err := sanitizePath("/etc/passwd")
require.ErrorIs(t, err, errAbsolutePath)
assert.EqualError(t, err, "absolute path not allowed: /etc/passwd")
})
t.Run("traversal", func(t *testing.T) {
t.Parallel()
_, err := sanitizePath("../x")
require.ErrorIs(t, err, errPathTraversal)
assert.EqualError(t, err, "path traversal not allowed: ../x")
}) })
} }
// TestFetchErrorMessagesVerbatim pins the fetch and manifest-loader func TestSizeMismatchMessage(t *testing.T) {
// messages; see TestErrorMessagesVerbatim for why.
func TestFetchErrorMessagesVerbatim(t *testing.T) {
t.Parallel() t.Parallel()
checkErrMsgCases(t, []errMsgCase{ // finishDownload returns the size-mismatch error before it touches the
{ // paths, digest, or entry, so those can be zero here.
name: "manifest_loader: http status", err := finishDownload("", "", 9, 10, nil, nil, nil, nil)
err: fmt.Errorf("failed to fetch %s: %w %d", require.ErrorIs(t, err, errSizeMismatch)
"https://example.com/index.mf", errHTTPStatus, 404), assert.EqualError(t, err, "size mismatch: expected 10 bytes, got 9")
want: "failed to fetch https://example.com/index.mf: HTTP 404",
},
{
name: "fetch: manifest http status",
err: fmt.Errorf("failed to fetch manifest: %w %d",
errHTTPStatus, 404),
want: "failed to fetch manifest: HTTP 404",
},
{
name: "fetch: file http status",
err: fmt.Errorf("%w %d", errHTTPStatus, 500),
want: "HTTP 500",
},
{
name: "fetch: empty path",
err: errEmptyPath,
want: "empty path",
},
{
name: "fetch: absolute path",
err: fmt.Errorf("%w: %s", errAbsolutePath, "/etc/passwd"),
want: "absolute path not allowed: /etc/passwd",
},
{
name: "fetch: path traversal",
err: fmt.Errorf("%w: %s", errPathTraversal, "../x"),
want: "path traversal not allowed: ../x",
},
{
name: "fetch: size mismatch",
err: fmt.Errorf("%w: expected %d bytes, got %d",
errSizeMismatch, 10, 9),
want: "size mismatch: expected 10 bytes, got 9",
},
{
name: "fetch: url required",
err: errURLRequired,
want: "URL argument required",
},
{
name: "fetch: hash mismatch",
err: errHashMismatch,
want: "hash mismatch",
},
})
} }
// TestSentinelsAreMatchable checks that the wrapped forms of the func TestHashMismatchMessage(t *testing.T) {
// messages above remain matchable with errors.Is, which is the reason
// the sentinels exist at all.
func TestSentinelsAreMatchable(t *testing.T) {
t.Parallel() t.Parallel()
wrapped := fmt.Errorf("embedded signing key fingerprint %s %w %s", // A 32-byte digest that matches none of the (empty) manifest hashes.
"a", errSignerMismatch, "b") err := verifyDownloadedHash(make([]byte, 32), &mfer.MFFilePath{})
require.ErrorIs(t, wrapped, errSignerMismatch) require.ErrorIs(t, err, errHashMismatch)
require.NotErrorIs(t, err, errSizeMismatch)
wrapped = fmt.Errorf("output file %s %w", "index.mf", errOutputExists) assert.EqualError(t, err, "hash mismatch")
require.ErrorIs(t, wrapped, errOutputExists)
wrapped = fmt.Errorf("failed to fetch manifest: %w %d", errHTTPStatus, 404)
require.ErrorIs(t, wrapped, errHTTPStatus)
assert.NotErrorIs(t, errHashMismatch, errSizeMismatch)
} }
+66 -9
View File
@@ -36,6 +36,11 @@ const (
// traversal bit for group and other must stay set. // traversal bit for group and other must stay set.
dirPerms os.FileMode = 0o755 dirPerms os.FileMode = 0o755
// filePerms is the permission mode, before the umask, for downloaded
// files. It is the mode os.Create uses; like dirPerms, it keeps group
// and other read access.
filePerms os.FileMode = 0o666
// Bitrate unit thresholds in bits per second. // Bitrate unit thresholds in bits per second.
bpsPerGbps = 1e9 bpsPerGbps = 1e9
bpsPerMbps = 1e6 bpsPerMbps = 1e6
@@ -53,6 +58,9 @@ var (
// errPathTraversal indicates a manifest path escaping the target // errPathTraversal indicates a manifest path escaping the target
// directory. // directory.
errPathTraversal = errors.New("path traversal not allowed") errPathTraversal = errors.New("path traversal not allowed")
// errSymlinkInPath indicates a manifest path running through a
// symlink that already exists in the target directory.
errSymlinkInPath = errors.New("symlink in path not allowed")
// errSizeMismatch indicates a downloaded file with an unexpected // errSizeMismatch indicates a downloaded file with an unexpected
// size. // size.
errSizeMismatch = errors.New("size mismatch") errSizeMismatch = errors.New("size mismatch")
@@ -274,6 +282,35 @@ func sanitizePath(p string) (string, error) {
return cleaned, nil return cleaned, nil
} }
// checkNoSymlinks returns an error if any part of the relative path p
// already exists as a symlink. sanitizePath checks p only as text, so
// without this a symlink inside the target directory could send a write
// to p outside of it. Parts that do not exist yet are fine: fetch creates
// them as plain directories and files. Call it immediately before each
// write: a symlink created after it returns is not caught.
func checkNoSymlinks(p string) error {
current := ""
for _, part := range strings.Split(p, string(filepath.Separator)) {
current = filepath.Join(current, part)
info, err := os.Lstat(current)
if errors.Is(err, os.ErrNotExist) {
return nil
}
if err != nil {
return fmt.Errorf("failed to check %s for a symlink: %w", current, err)
}
if info.Mode()&os.ModeSymlink != 0 {
return fmt.Errorf("%w: %s", errSymlinkInPath, current)
}
}
return nil
}
// resolveManifestURL takes a URL and returns the manifest URL. // resolveManifestURL takes a URL and returns the manifest URL.
// If the URL already ends with .mf, it's returned as-is. // If the URL already ends with .mf, it's returned as-is.
// Otherwise, index.mf is appended. // Otherwise, index.mf is appended.
@@ -419,7 +456,12 @@ func downloadFile(
// Create parent directories if needed // Create parent directories if needed
dir := filepath.Dir(localPath) dir := filepath.Dir(localPath)
if dir != "" && dir != "." { if dir != "" && dir != "." {
err := os.MkdirAll(dir, dirPerms) err = checkNoSymlinks(dir)
if err != nil {
return err
}
err = os.MkdirAll(dir, dirPerms)
if err != nil { if err != nil {
return fmt.Errorf("failed to create directory %s: %w", dir, err) return fmt.Errorf("failed to create directory %s: %w", dir, err)
} }
@@ -447,15 +489,25 @@ func downloadFile(
totalBytes = expectedSize totalBytes = expectedSize
} }
// Create temp file. err = checkNoSymlinks(tmpPath)
if err != nil {
return err
}
// Remove whatever is at tmpPath, such as a leftover from an
// interrupted run, rather than write into it: it may be a hard link
// to a file outside the target directory, and removing a hard link
// removes only this name. If the removal fails, O_EXCL below makes
// the create fail.
_ = os.Remove(tmpPath)
// Create the temp file only if nothing is at tmpPath (O_EXCL).
// //
// G304: tmpPath is derived from localPath, which sanitizePath above // G304: tmpPath is a relative path that sanitizePath keeps inside the
// constrains lexically to a relative path that does not escape the // target directory as text, and checkNoSymlinks just found no symlink
// destination directory. That is a purely lexical guarantee: it does // in it.
// not resolve symlinks, so a pre-existing symlink inside the out, err := os.OpenFile( //nolint:gosec // G304: see comment above
// destination tree can still redirect this write outside of it tmpPath, os.O_RDWR|os.O_CREATE|os.O_EXCL, filePerms)
// (tracked in issue #86).
out, err := os.Create(tmpPath) //nolint:gosec // G304: see comment above
if err != nil { if err != nil {
return fmt.Errorf("failed to create temp file: %w", err) return fmt.Errorf("failed to create temp file: %w", err)
} }
@@ -519,6 +571,11 @@ func finishDownload(
return err return err
} }
err = checkNoSymlinks(localPath)
if err != nil {
return err
}
// Rename temp file to final path // Rename temp file to final path
err = os.Rename(tmpPath, localPath) err = os.Rename(tmpPath, localPath)
if err != nil { if err != nil {
+83
View File
@@ -440,3 +440,86 @@ func TestFetchProgress(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, content, downloaded) assert.Equal(t, content, downloaded)
} }
// TestFetchRefusesSymlinks runs fetch into a destination directory that
// holds a symlink pointing outside it, in each of the three places fetch
// writes: a parent directory, the temp file, and the file itself, which
// the temp file is renamed onto; and once as a directory inside a plain
// directory. The fetch must fail and nothing outside may change.
//
//nolint:paralleltest // changes the process-global working directory
func TestFetchRefusesSymlinks(t *testing.T) {
tests := []struct {
name string
entry string // the manifest's only file
link string // symlink placed in the destination directory
target string // what link points to, relative to the outside directory
}{
{"parent directory", "sub/deeper/file.txt", "sub", "."},
{"directory inside a plain directory", "docs/data/passwd", "docs/data", "."},
{"temp file", testFileTxt, ".file.txt.tmp", "new.txt"},
{"file", testFileTxt, testFileTxt, "new.txt"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
content := []byte("fetched")
sourceFs := afero.NewMemMapFs()
require.NoError(t, sourceFs.MkdirAll(filepath.Dir("/"+tt.entry), 0o755))
require.NoError(t, afero.WriteFile(sourceFs, "/"+tt.entry, content, 0o644))
server := httptest.NewServer(fetchTestHandler(
scanToManifest(t, sourceFs), map[string][]byte{tt.entry: content}))
defer server.Close()
outside := t.TempDir()
chdirTemp(t)
require.NoError(t, os.MkdirAll(filepath.Dir(tt.link), 0o750))
require.NoError(t, os.Symlink(filepath.Join(outside, tt.target), tt.link))
opts := testOpts([]string{testApp, cmdFetch, "-q", server.URL}, afero.NewOsFs())
assert.Equal(t, 1, runCLI(opts))
assert.Contains(t, testStderr(t, opts), "failed to download "+tt.entry+
": symlink in path not allowed: "+tt.link)
written, err := os.ReadDir(outside)
require.NoError(t, err)
assert.Empty(t, written, "fetch wrote outside the destination")
})
}
}
// TestFetchReplacesHardLinkAtTempName runs fetch into a destination
// directory that holds, at the temp file's name, a hard link to a file
// outside it. To fetch that is an ordinary leftover from an interrupted
// earlier run: it must replace it and succeed, and the outside file must
// not change.
//
//nolint:paralleltest // changes the process-global working directory
func TestFetchReplacesHardLinkAtTempName(t *testing.T) {
content := []byte("fetched")
sourceFs := afero.NewMemMapFs()
require.NoError(t, afero.WriteFile(sourceFs, "/"+testFileTxt, content, 0o644))
server := httptest.NewServer(fetchTestHandler(
scanToManifest(t, sourceFs), map[string][]byte{testFileTxt: content}))
defer server.Close()
outsideFile := filepath.Join(t.TempDir(), "secret.txt")
require.NoError(t, os.WriteFile(outsideFile, []byte("outside"), 0o600))
chdirTemp(t)
require.NoError(t, os.Link(outsideFile, ".file.txt.tmp"))
opts := testOpts([]string{testApp, cmdFetch, "-q", server.URL}, afero.NewOsFs())
require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
fetched, err := os.ReadFile(testFileTxt)
require.NoError(t, err)
assert.Equal(t, content, fetched)
outside, err := os.ReadFile(outsideFile) //nolint:gosec // test-controlled path
require.NoError(t, err)
assert.Equal(t, "outside", string(outside), "fetch wrote outside the destination")
}
+4 -3
View File
@@ -1,6 +1,7 @@
package cli package cli
import ( import (
"context"
"crypto/sha256" "crypto/sha256"
"errors" "errors"
"fmt" "fmt"
@@ -304,7 +305,7 @@ func (h *freshenHasher) processEntry(e *freshenEntry) error {
// writeFreshenedManifest writes the manifest atomically (write to a // writeFreshenedManifest writes the manifest atomically (write to a
// temp file, then rename over the target). // temp file, then rename over the target).
func writeFreshenedManifest( func writeFreshenedManifest(
afs afero.Fs, builder *mfer.Builder, manifestPath string, ctx context.Context, afs afero.Fs, builder *mfer.Builder, manifestPath string,
) error { ) error {
tmpPath := manifestPath + ".tmp" tmpPath := manifestPath + ".tmp"
@@ -313,7 +314,7 @@ func writeFreshenedManifest(
return fmt.Errorf("failed to create temp file: %w", err) return fmt.Errorf("failed to create temp file: %w", err)
} }
err = builder.Build(outFile) err = builder.Build(ctx, outFile)
_ = outFile.Close() _ = outFile.Close()
if err != nil { if err != nil {
@@ -530,7 +531,7 @@ func (mfa *CLIApp) freshenManifestOperation(ctx *cli.Context) error {
} }
// Write updated manifest atomically (write to temp, then rename) // Write updated manifest atomically (write to temp, then rename)
err = writeFreshenedManifest(mfa.Fs, hasher.builder, manifestPath) err = writeFreshenedManifest(ctx.Context, mfa.Fs, hasher.builder, manifestPath)
if err != nil { if err != nil {
return err return err
} }
+4 -1
View File
@@ -110,7 +110,10 @@ func TestFreshenRecordEntryMtimePresence(t *testing.T) {
const relPath = "file1.txt" const relPath = "file1.txt"
mtime := time.Unix(1_700_000_000, 0) // The scanned file's mtime is the Unix epoch. If recordEntry ever misreads
// an absent manifest mtime as the epoch, the "absent" case below would
// compare equal to this and be classified unchanged, so the test fails.
mtime := time.Unix(0, 0)
info := stubFileInfo{size: 8, mtime: mtime} info := stubFileInfo{size: 8, mtime: mtime}
for _, tc := range []struct { for _, tc := range []struct {
+3 -2
View File
@@ -18,6 +18,7 @@ const (
cmdGenerate = "generate" cmdGenerate = "generate"
cmdCheck = "check" cmdCheck = "check"
cmdExport = "export" cmdExport = "export"
cmdFetch = "fetch"
flagProgress = "progress" flagProgress = "progress"
@@ -300,7 +301,7 @@ func (mfa *CLIApp) listCommand() *cli.Command {
func (mfa *CLIApp) fetchCommand() *cli.Command { func (mfa *CLIApp) fetchCommand() *cli.Command {
return &cli.Command{ return &cli.Command{
Name: "fetch", Name: cmdFetch,
Usage: "fetch manifest and referenced files", Usage: "fetch manifest and referenced files",
Action: func(c *cli.Context) error { Action: func(c *cli.Context) error {
mfa.setVerbosity(c) mfa.setVerbosity(c)
@@ -357,6 +358,6 @@ func (mfa *CLIApp) run(args []string) {
if err != nil { if err != nil {
mfa.exitCode = 1 mfa.exitCode = 1
log.WithError(err).Debugf("exiting") log.Errorf("%s", err)
} }
} }
+34 -41
View File
@@ -112,13 +112,16 @@ func DisableStyling() {
} }
// Init initializes the logger with the CLI handler and default log level. // Init initializes the logger with the CLI handler and default log level.
//
// It reconfigures the process-global apex/log logger under the write lock so
// the global is never mutated while another goroutine holds the read lock to
// read it in emit. Without this, parallel callers (e.g. the test suite) race
// Init's SetLevel/SetHandler against concurrent log calls.
func Init() { func Init() {
mu.RLock() mu.Lock()
defer mu.Unlock()
w := stderr log.SetHandler(acli.New(stderr))
mu.RUnlock()
log.SetHandler(acli.New(w))
log.SetLevel(log.DebugLevel) // Let apex/log pass everything; we filter ourselves log.SetLevel(log.DebugLevel) // Let apex/log pass everything; we filter ourselves
} }
@@ -130,74 +133,66 @@ func isEnabled(l Level) bool {
return l >= currentLevel return l >= currentLevel
} }
// emit calls fn while holding the read lock if messages at level l are
// enabled. Holding the read lock across the apex/log call keeps the global
// logger from being read while Init reconfigures it under the write lock.
func emit(l Level, fn func()) {
mu.RLock()
defer mu.RUnlock()
if l >= currentLevel {
fn()
}
}
// Fatalf logs a formatted message at fatal level. // Fatalf logs a formatted message at fatal level.
func Fatalf(format string, args ...any) { func Fatalf(format string, args ...any) {
if isEnabled(FatalLevel) { emit(FatalLevel, func() { log.Fatalf(format, args...) })
log.Fatalf(format, args...)
}
} }
// Fatal logs a message at fatal level. // Fatal logs a message at fatal level.
func Fatal(arg string) { func Fatal(arg string) {
if isEnabled(FatalLevel) { emit(FatalLevel, func() { log.Fatal(arg) })
log.Fatal(arg)
}
} }
// Errorf logs a formatted message at error level. // Errorf logs a formatted message at error level.
func Errorf(format string, args ...any) { func Errorf(format string, args ...any) {
if isEnabled(ErrorLevel) { emit(ErrorLevel, func() { log.Errorf(format, args...) })
log.Errorf(format, args...)
}
} }
// Error logs a message at error level. // Error logs a message at error level.
func Error(arg string) { func Error(arg string) {
if isEnabled(ErrorLevel) { emit(ErrorLevel, func() { log.Error(arg) })
log.Error(arg)
}
} }
// Warnf logs a formatted message at warn level. // Warnf logs a formatted message at warn level.
func Warnf(format string, args ...any) { func Warnf(format string, args ...any) {
if isEnabled(WarnLevel) { emit(WarnLevel, func() { log.Warnf(format, args...) })
log.Warnf(format, args...)
}
} }
// Warn logs a message at warn level. // Warn logs a message at warn level.
func Warn(arg string) { func Warn(arg string) {
if isEnabled(WarnLevel) { emit(WarnLevel, func() { log.Warn(arg) })
log.Warn(arg)
}
} }
// Infof logs a formatted message at info level. // Infof logs a formatted message at info level.
func Infof(format string, args ...any) { func Infof(format string, args ...any) {
if isEnabled(InfoLevel) { emit(InfoLevel, func() { log.Infof(format, args...) })
log.Infof(format, args...)
}
} }
// Info logs a message at info level. // Info logs a message at info level.
func Info(arg string) { func Info(arg string) {
if isEnabled(InfoLevel) { emit(InfoLevel, func() { log.Info(arg) })
log.Info(arg)
}
} }
// Verbosef logs a formatted message at verbose level. // Verbosef logs a formatted message at verbose level.
func Verbosef(format string, args ...any) { func Verbosef(format string, args ...any) {
if isEnabled(VerboseLevel) { emit(VerboseLevel, func() { log.Infof(format, args...) })
log.Infof(format, args...)
}
} }
// Verbose logs a message at verbose level. // Verbose logs a message at verbose level.
func Verbose(arg string) { func Verbose(arg string) {
if isEnabled(VerboseLevel) { emit(VerboseLevel, func() { log.Info(arg) })
log.Info(arg)
}
} }
// Debugf logs a formatted message at debug level with caller location. // Debugf logs a formatted message at debug level with caller location.
@@ -216,7 +211,10 @@ func Debug(arg string) {
// DebugReal logs at debug level with caller info from the specified stack depth. // DebugReal logs at debug level with caller info from the specified stack depth.
func DebugReal(arg string, cs int) { func DebugReal(arg string, cs int) {
if !isEnabled(DebugLevel) { mu.RLock()
defer mu.RUnlock()
if DebugLevel < currentLevel {
return return
} }
@@ -275,11 +273,6 @@ func GetLevel() Level {
return currentLevel return currentLevel
} }
// WithError returns a log entry with the error attached.
func WithError(e error) *log.Entry {
return log.Log.WithError(e)
}
// Progressf prints a progress message that overwrites the current line. // Progressf prints a progress message that overwrites the current line.
// Use ProgressDone() when progress is complete to move to the next line. // Use ProgressDone() when progress is complete to move to the next line.
func Progressf(format string, args ...any) { func Progressf(format string, args ...any) {
+6 -4
View File
@@ -3,6 +3,7 @@
package mfer package mfer
import ( import (
"context"
"crypto/sha256" "crypto/sha256"
"errors" "errors"
"fmt" "fmt"
@@ -281,8 +282,9 @@ func (b *Builder) SetSigningOptions(opts *SigningOptions) {
b.signingOptions = opts b.signingOptions = opts
} }
// Build finalizes the manifest and writes it to the writer. // Build finalizes the manifest and writes it to the writer. ctx bounds the
func (b *Builder) Build(w io.Writer) error { // gpg runs that sign the manifest when signing options are set.
func (b *Builder) Build(ctx context.Context, w io.Writer) error {
b.mu.Lock() b.mu.Lock()
defer b.mu.Unlock() defer b.mu.Unlock()
@@ -308,13 +310,13 @@ func (b *Builder) Build(w io.Writer) error {
} }
// Generate outer wrapper // Generate outer wrapper
err := m.generateOuter() err := m.generateOuter(ctx)
if err != nil { if err != nil {
return fmt.Errorf("build: generate outer: %w", err) return fmt.Errorf("build: generate outer: %w", err)
} }
// Generate final output // Generate final output
err = m.generate() err = m.generate(ctx)
if err != nil { if err != nil {
return fmt.Errorf("build: generate: %w", err) return fmt.Errorf("build: generate: %w", err)
} }
+9 -8
View File
@@ -3,6 +3,7 @@ package mfer
import ( import (
"bytes" "bytes"
"context"
"strings" "strings"
"testing" "testing"
"time" "time"
@@ -113,7 +114,7 @@ func TestBuilderBuild(t *testing.T) {
var buf bytes.Buffer var buf bytes.Buffer
err = b.Build(&buf) err = b.Build(context.Background(), &buf)
require.NoError(t, err) require.NoError(t, err)
// Should have magic bytes // Should have magic bytes
@@ -177,7 +178,7 @@ func TestBuilderDeterministicOutput(t *testing.T) {
var buf bytes.Buffer var buf bytes.Buffer
err := b.Build(&buf) err := b.Build(context.Background(), &buf)
require.NoError(t, err) require.NoError(t, err)
return buf.Bytes() return buf.Bytes()
@@ -325,7 +326,7 @@ func TestBuilderBuildRoundTrip(t *testing.T) {
} }
var buf bytes.Buffer var buf bytes.Buffer
require.NoError(t, b.Build(&buf)) require.NoError(t, b.Build(context.Background(), &buf))
m, err := NewManifestFromReader(&buf) m, err := NewManifestFromReader(&buf)
require.NoError(t, err) require.NoError(t, err)
@@ -383,7 +384,7 @@ func TestManifestString(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
var buf bytes.Buffer var buf bytes.Buffer
require.NoError(t, b.Build(&buf)) require.NoError(t, b.Build(context.Background(), &buf))
m, err := NewManifestFromReader(&buf) m, err := NewManifestFromReader(&buf)
require.NoError(t, err) require.NoError(t, err)
@@ -397,7 +398,7 @@ func TestBuilderBuildEmpty(t *testing.T) {
var buf bytes.Buffer var buf bytes.Buffer
err := b.Build(&buf) err := b.Build(context.Background(), &buf)
require.NoError(t, err) require.NoError(t, err)
// Should still produce valid manifest with 0 files // Should still produce valid manifest with 0 files
@@ -416,7 +417,7 @@ func TestBuilderOmitsCreatedAtByDefault(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
var buf bytes.Buffer var buf bytes.Buffer
require.NoError(t, b.Build(&buf)) require.NoError(t, b.Build(context.Background(), &buf))
m, err := NewManifestFromReader(&buf) m, err := NewManifestFromReader(&buf)
require.NoError(t, err) require.NoError(t, err)
@@ -438,7 +439,7 @@ func TestBuilderIncludesCreatedAtWhenRequested(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
var buf bytes.Buffer var buf bytes.Buffer
require.NoError(t, b.Build(&buf)) require.NoError(t, b.Build(context.Background(), &buf))
m, err := NewManifestFromReader(&buf) m, err := NewManifestFromReader(&buf)
require.NoError(t, err) require.NoError(t, err)
@@ -464,7 +465,7 @@ func TestBuilderDeterministicFileOrder(t *testing.T) {
} }
var buf bytes.Buffer var buf bytes.Buffer
require.NoError(t, b.Build(&buf)) require.NoError(t, b.Build(context.Background(), &buf))
m, err := NewManifestFromReader(&buf) m, err := NewManifestFromReader(&buf)
require.NoError(t, err) require.NoError(t, err)
+6 -2
View File
@@ -164,12 +164,12 @@ func (c *Checker) SigningPubKey() []byte {
// ExtractEmbeddedSigningKeyFP imports the manifest's embedded public key into a // ExtractEmbeddedSigningKeyFP imports the manifest's embedded public key into a
// temporary keyring and extracts its fingerprint. This validates the key and // temporary keyring and extracts its fingerprint. This validates the key and
// returns its actual fingerprint from the key material itself. // returns its actual fingerprint from the key material itself.
func (c *Checker) ExtractEmbeddedSigningKeyFP() (string, error) { func (c *Checker) ExtractEmbeddedSigningKeyFP(ctx context.Context) (string, error) {
if len(c.signingPubKey) == 0 { if len(c.signingPubKey) == 0 {
return "", errNoSigningPubKey return "", errNoSigningPubKey
} }
return gpgExtractPubKeyFingerprint(c.signingPubKey) return gpgExtractPubKeyFingerprint(ctx, c.signingPubKey)
} }
// Check verifies all files against the manifest. // Check verifies all files against the manifest.
@@ -312,6 +312,10 @@ func (c *Checker) FindExtraFiles(ctx context.Context, results chan<- Result) err
} }
func (c *Checker) checkFile(entry *MFFilePath, checkedBytes *FileSize) Result { func (c *Checker) checkFile(entry *MFFilePath, checkedBytes *FileSize) Result {
// entry.GetPath() is safe to join here: a manifest's entry paths are
// validated against the path invariants when it is loaded (see
// deserializeInner) or built (see Builder.AddFile), so a traversal or
// absolute path can never reach this point.
absPath := filepath.Join(string(c.basePath), entry.GetPath()) absPath := filepath.Join(string(c.basePath), entry.GetPath())
relPath := RelFilePath(entry.GetPath()) relPath := RelFilePath(entry.GetPath())
+1 -1
View File
@@ -61,7 +61,7 @@ func createTestManifest(
} }
var buf bytes.Buffer var buf bytes.Buffer
require.NoError(t, builder.Build(&buf)) require.NoError(t, builder.Build(context.Background(), &buf))
require.NoError(t, afero.WriteFile(fs, manifestPath, buf.Bytes(), 0o644)) require.NoError(t, afero.WriteFile(fs, manifestPath, buf.Bytes(), 0o644))
} }
+17
View File
@@ -2,6 +2,7 @@ package mfer
import ( import (
"bytes" "bytes"
"context"
"crypto/sha256" "crypto/sha256"
"errors" "errors"
"fmt" "fmt"
@@ -25,6 +26,7 @@ var (
errDecompressedTooLarge = errors.New("decompressed data exceeds maximum allowed size") errDecompressedTooLarge = errors.New("decompressed data exceeds maximum allowed size")
errUUIDMismatch = errors.New("outer and inner UUID mismatch") errUUIDMismatch = errors.New("outer and inner UUID mismatch")
errInvalidFileFormat = errors.New("invalid file format") errInvalidFileFormat = errors.New("invalid file format")
errInvalidManifestPath = errors.New("manifest contains invalid path")
) )
// validateUUID checks that the byte slice is a valid UUID (16 bytes, parseable). // validateUUID checks that the byte slice is a valid UUID (16 bytes, parseable).
@@ -91,7 +93,9 @@ func (m *manifest) verifyOuterIntegrity() error {
) )
} }
// Loading a manifest takes no context; gpgTimeout still bounds gpg.
err = gpgVerify( err = gpgVerify(
context.Background(),
[]byte(sigString), []byte(sigString),
m.pbOuter.GetSignature(), m.pbOuter.GetSignature(),
m.pbOuter.GetSigningPubKey(), m.pbOuter.GetSigningPubKey(),
@@ -181,6 +185,19 @@ func (m *manifest) deserializeInner() error {
return errUUIDMismatch return errUUIDMismatch
} }
// Enforce the manifest path invariants on every entry as it is loaded,
// so that no consumer of a manifest — Checker today, any restore or
// extract path tomorrow — acts on a traversal or absolute path from an
// untrusted .mf. Reject loudly on the first offender rather than
// dropping entries, which would let a hostile manifest hide files from a
// check.
for _, f := range m.pbInner.GetFiles() {
err = ValidatePath(f.GetPath())
if err != nil {
return fmt.Errorf("%w: %w", errInvalidManifestPath, err)
}
}
log.Infof("loaded manifest with %d files", len(m.pbInner.GetFiles())) log.Infof("loaded manifest with %d files", len(m.pbInner.GetFiles()))
return nil return nil
+146
View File
@@ -0,0 +1,146 @@
//nolint:testpackage // white-box tests exercise unexported internals
package mfer
import (
"bytes"
"context"
"crypto/sha256"
"fmt"
"testing"
"github.com/google/uuid"
"github.com/klauspost/compress/zstd"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/encoding/protowire"
"google.golang.org/protobuf/proto"
)
// craftInnerBytes builds the wire bytes of an inner MFFile holding a single
// file entry whose path is exactly pathBytes. It writes the wire form by hand
// so a hostile path — including one that is not valid UTF-8 — can be embedded
// without proto.Marshal's own UTF-8 enforcement rejecting it first.
func craftInnerBytes(id uuid.UUID, pathBytes string) []byte {
entry := protowire.AppendTag(nil, 1, protowire.BytesType) // MFFilePath.path
entry = protowire.AppendString(entry, pathBytes)
inner := protowire.AppendTag(nil, 100, protowire.VarintType) // MFFile.version
inner = protowire.AppendVarint(inner, uint64(MFFile_VERSION_ONE))
inner = protowire.AppendTag(inner, 101, protowire.BytesType) // MFFile.files
inner = protowire.AppendBytes(inner, entry)
inner = protowire.AppendTag(inner, 102, protowire.BytesType) // MFFile.uuid
inner = protowire.AppendBytes(inner, id[:])
return inner
}
// wrapInner wraps inner MFFile wire bytes in a complete, well-formed .mf
// envelope (magic prefix, zstd-compressed payload, matching hash and UUID) so
// that deserialization reaches path validation rather than failing earlier on
// an integrity check.
func wrapInner(t *testing.T, id uuid.UUID, innerData []byte) []byte {
t.Helper()
var cbuf bytes.Buffer
zw, err := zstd.NewWriter(&cbuf, zstd.WithEncoderLevel(zstd.SpeedBestCompression))
require.NoError(t, err)
_, err = zw.Write(innerData)
require.NoError(t, err)
require.NoError(t, zw.Close())
compressed := cbuf.Bytes()
sum := sha256.Sum256(compressed)
outer := &MFFileOuter{
InnerMessage: compressed,
Size: int64(len(innerData)),
Sha256: sum[:],
Uuid: id[:],
Version: MFFileOuter_VERSION_ONE,
CompressionType: MFFileOuter_COMPRESSION_ZSTD,
}
ob, err := proto.Marshal(outer)
require.NoError(t, err)
return append([]byte(MAGIC), ob...)
}
func TestDeserializeRejectsInvalidEntryPaths(t *testing.T) {
t.Parallel()
tests := []struct {
name string
path string
}{
{"parent traversal", "../escape"},
{"interior traversal", "a/../../escape"},
{"absolute path", "/etc/passwd"},
{"backslash path", `a\b`},
{"double slash", "a//b"},
{"empty path", ""},
{"invalid utf-8", "abc\xff"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
id := uuid.New()
data := wrapInner(t, id, craftInnerBytes(id, tt.path))
_, err := NewManifestFromReader(bytes.NewReader(data))
require.Error(t, err)
if tt.path == "abc\xff" {
// A path that is not valid UTF-8 cannot survive the proto3
// string decoder, which rejects it before path validation
// runs; the manifest is still refused at load time.
return
}
require.ErrorIs(t, err, errInvalidManifestPath)
if tt.path != "" {
// ValidatePath quotes the path with %q; assert against the
// same rendering so escaped characters (e.g. a backslash)
// still match.
assert.Contains(t, err.Error(), fmt.Sprintf("%q", tt.path),
"error must name the offending path")
}
})
}
}
func TestDeserializeValidManifestRoundTrips(t *testing.T) {
t.Parallel()
hash := make([]byte, 34) // multihash: 2-byte prefix + 32-byte SHA-256
b := NewBuilder()
require.NoError(t, b.AddFileWithHash("dir/file.txt", 123, ModTime{}, hash))
var buf bytes.Buffer
require.NoError(t, b.Build(context.Background(), &buf))
m, err := NewManifestFromReader(bytes.NewReader(buf.Bytes()))
require.NoError(t, err)
files := m.Files()
require.Len(t, files, 1)
assert.Equal(t, "dir/file.txt", files[0].GetPath())
assert.Equal(t, int64(123), files[0].GetSize())
}
// TestValidatePathRejectsInvalidUTF8 pins the ValidatePath rule that a manifest
// path must be valid UTF-8, independent of the proto decoder that also enforces
// it on the wire.
func TestValidatePathRejectsInvalidUTF8(t *testing.T) {
t.Parallel()
err := ValidatePath("abc\xff")
require.ErrorIs(t, err, errPathNotUTF8)
assert.Contains(t, err.Error(), "UTF-8")
}
+4 -2
View File
@@ -2,6 +2,7 @@
package mfer package mfer
import ( import (
"context"
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
@@ -80,6 +81,7 @@ func TestSerializeInternalErrorMessagesVerbatim(t *testing.T) {
t.Parallel() t.Parallel()
m := &manifest{} m := &manifest{}
require.EqualError(t, m.generate(), "internal error: pbInner not set") require.EqualError(t, m.generate(context.Background()),
require.EqualError(t, m.generateOuter(), "internal error") "internal error: pbInner not set")
require.EqualError(t, m.generateOuter(context.Background()), "internal error")
} }
+48 -15
View File
@@ -10,9 +10,21 @@ import (
"os/exec" "os/exec"
"path/filepath" "path/filepath"
"strings" "strings"
"time"
) )
const ( const (
// gpgTimeout bounds every gpg run, which can otherwise wait forever on
// a passphrase prompt or a stalled gpg-agent. A minute leaves a person
// time to type a passphrase or touch a smartcard.
gpgTimeout = time.Minute
// gpgWaitDelay is how long a gpg run keeps waiting for gpg's stdout
// and stderr to close once gpg has been killed or has exited. Reading
// what gpg itself wrote takes far less; only a process gpg left behind
// holds them open longer.
gpgWaitDelay = time.Second
// privateDirPerms is the permission mode for temporary GPG home // privateDirPerms is the permission mode for temporary GPG home
// directories. // directories.
privateDirPerms os.FileMode = 0o700 privateDirPerms os.FileMode = 0o700
@@ -66,8 +78,20 @@ func gpgArgs(opts []string, positional ...string) []string {
} }
// runGPG runs the gpg binary in batch mode with the given arguments and // runGPG runs the gpg binary in batch mode with the given arguments and
// optional stdin, returning captured stdout and stderr. // optional stdin, returning captured stdout and stderr. gpg is killed when
func runGPG(stdin io.Reader, args ...string) (*bytes.Buffer, *bytes.Buffer, error) { // ctx ends or gpgTimeout passes, whichever comes first.
func runGPG(
ctx context.Context, stdin io.Reader, args ...string,
) (*bytes.Buffer, *bytes.Buffer, error) {
// exec.CommandContext kills only gpg itself. A gpg-agent that gpg
// starts runs detached and holds none of gpg's output, but another
// process gpg leaves behind (a wrapper script that runs the real gpg
// without exec, for example) can keep gpg's stdout or stderr open, and
// Run would wait for it to exit. WaitDelay stops that wait
// gpgWaitDelay after the kill; that process is left running.
ctx, cancel := context.WithTimeout(ctx, gpgTimeout)
defer cancel()
fullArgs := append([]string{"--batch", "--no-tty"}, args...) fullArgs := append([]string{"--batch", "--no-tty"}, args...)
// G204: the executable name is a compile-time constant. The arguments // G204: the executable name is a compile-time constant. The arguments
@@ -76,7 +100,8 @@ func runGPG(stdin io.Reader, args ...string) (*bytes.Buffer, *bytes.Buffer, erro
// option or after the "--" end-of-options marker inserted by gpgArgs, // option or after the "--" end-of-options marker inserted by gpgArgs,
// and therefore cannot be reinterpreted by gpg as an option. // and therefore cannot be reinterpreted by gpg as an option.
cmd := exec.CommandContext( //nolint:gosec // G204: see comment above cmd := exec.CommandContext( //nolint:gosec // G204: see comment above
context.Background(), "gpg", fullArgs...) ctx, "gpg", fullArgs...)
cmd.WaitDelay = gpgWaitDelay
cmd.Stdin = stdin cmd.Stdin = stdin
var stdout, stderr bytes.Buffer var stdout, stderr bytes.Buffer
@@ -85,6 +110,14 @@ func runGPG(stdin io.Reader, args ...string) (*bytes.Buffer, *bytes.Buffer, erro
cmd.Stderr = &stderr cmd.Stderr = &stderr
err := cmd.Run() err := cmd.Run()
if err != nil && ctx.Err() != nil {
// gpg was killed because ctx ended, which Run reports only as
// "signal: killed"; return the reason instead.
err = ctx.Err()
if errors.Is(err, context.DeadlineExceeded) {
err = fmt.Errorf("gpg timed out: %w", err)
}
}
return &stdout, &stderr, err return &stdout, &stderr, err
} }
@@ -105,8 +138,8 @@ func parseFingerprint(colonOutput string) (string, bool) {
// gpgSign creates a detached signature of the data using the specified key. // gpgSign creates a detached signature of the data using the specified key.
// Returns the armored detached signature. // Returns the armored detached signature.
func gpgSign(data []byte, keyID GPGKeyID) ([]byte, error) { func gpgSign(ctx context.Context, data []byte, keyID GPGKeyID) ([]byte, error) {
stdout, stderr, err := runGPG(bytes.NewReader(data), stdout, stderr, err := runGPG(ctx, bytes.NewReader(data),
"--detach-sign", "--detach-sign",
gpgOptArmor, gpgOptArmor,
"--local-user", string(keyID), "--local-user", string(keyID),
@@ -120,8 +153,8 @@ func gpgSign(data []byte, keyID GPGKeyID) ([]byte, error) {
// gpgExportPublicKey exports the public key for the specified key ID. // gpgExportPublicKey exports the public key for the specified key ID.
// Returns the armored public key. // Returns the armored public key.
func gpgExportPublicKey(keyID GPGKeyID) ([]byte, error) { func gpgExportPublicKey(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
stdout, stderr, err := runGPG(nil, stdout, stderr, err := runGPG(ctx, nil,
gpgArgs([]string{"--export", gpgOptArmor}, string(keyID))..., gpgArgs([]string{"--export", gpgOptArmor}, string(keyID))...,
) )
if err != nil { if err != nil {
@@ -136,8 +169,8 @@ func gpgExportPublicKey(keyID GPGKeyID) ([]byte, error) {
} }
// gpgGetKeyFingerprint gets the full fingerprint for a key ID. // gpgGetKeyFingerprint gets the full fingerprint for a key ID.
func gpgGetKeyFingerprint(keyID GPGKeyID) ([]byte, error) { func gpgGetKeyFingerprint(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
stdout, stderr, err := runGPG(nil, stdout, stderr, err := runGPG(ctx, nil,
gpgArgs([]string{"--with-colons", "--fingerprint"}, string(keyID))..., gpgArgs([]string{"--with-colons", "--fingerprint"}, string(keyID))...,
) )
if err != nil { if err != nil {
@@ -157,7 +190,7 @@ func gpgGetKeyFingerprint(keyID GPGKeyID) ([]byte, error) {
// gpgExtractPubKeyFingerprint imports a public key into a temporary keyring // gpgExtractPubKeyFingerprint imports a public key into a temporary keyring
// and extracts its fingerprint. This verifies the key is valid and returns // and extracts its fingerprint. This verifies the key is valid and returns
// the actual fingerprint from the key material. // the actual fingerprint from the key material.
func gpgExtractPubKeyFingerprint(pubKey []byte) (string, error) { func gpgExtractPubKeyFingerprint(ctx context.Context, pubKey []byte) (string, error) {
// Create temporary directory for GPG operations // Create temporary directory for GPG operations
tmpDir, err := os.MkdirTemp("", "mfer-gpg-fingerprint-*") tmpDir, err := os.MkdirTemp("", "mfer-gpg-fingerprint-*")
if err != nil { if err != nil {
@@ -181,7 +214,7 @@ func gpgExtractPubKeyFingerprint(pubKey []byte) (string, error) {
} }
// Import the public key into the temporary keyring // Import the public key into the temporary keyring
_, importStderr, err := runGPG(nil, _, importStderr, err := runGPG(ctx, nil,
gpgArgs([]string{gpgOptHomedir, tmpDir, "--import"}, pubKeyFile)..., gpgArgs([]string{gpgOptHomedir, tmpDir, "--import"}, pubKeyFile)...,
) )
if err != nil { if err != nil {
@@ -191,7 +224,7 @@ func gpgExtractPubKeyFingerprint(pubKey []byte) (string, error) {
} }
// List keys to get fingerprint // List keys to get fingerprint
listStdout, listStderr, err := runGPG(nil, listStdout, listStderr, err := runGPG(ctx, nil,
"--homedir", tmpDir, "--homedir", tmpDir,
"--with-colons", "--with-colons",
"--fingerprint", "--fingerprint",
@@ -212,7 +245,7 @@ func gpgExtractPubKeyFingerprint(pubKey []byte) (string, error) {
// gpgVerify verifies a detached signature against data using the provided public key. // gpgVerify verifies a detached signature against data using the provided public key.
// It creates a temporary keyring to import the public key for verification. // It creates a temporary keyring to import the public key for verification.
func gpgVerify(data, signature, pubKey []byte) error { func gpgVerify(ctx context.Context, data, signature, pubKey []byte) error {
// Create temporary directory for GPG operations // Create temporary directory for GPG operations
tmpDir, err := os.MkdirTemp("", "mfer-gpg-verify-*") tmpDir, err := os.MkdirTemp("", "mfer-gpg-verify-*")
if err != nil { if err != nil {
@@ -252,7 +285,7 @@ func gpgVerify(data, signature, pubKey []byte) error {
} }
// Import the public key into the temporary keyring // Import the public key into the temporary keyring
_, importStderr, err := runGPG(nil, _, importStderr, err := runGPG(ctx, nil,
gpgArgs([]string{gpgOptHomedir, tmpDir, "--import"}, pubKeyFile)..., gpgArgs([]string{gpgOptHomedir, tmpDir, "--import"}, pubKeyFile)...,
) )
if err != nil { if err != nil {
@@ -262,7 +295,7 @@ func gpgVerify(data, signature, pubKey []byte) error {
} }
// Verify the signature // Verify the signature
_, verifyStderr, err := runGPG(nil, _, verifyStderr, err := runGPG(ctx, nil,
gpgArgs([]string{gpgOptHomedir, tmpDir, gpgOptVerify}, gpgArgs([]string{gpgOptHomedir, tmpDir, gpgOptVerify},
sigFile, dataFile)..., sigFile, dataFile)...,
) )
+100 -20
View File
@@ -4,11 +4,14 @@ package mfer
import ( import (
"bytes" "bytes"
"context" "context"
"io"
"os" "os"
"os/exec" "os/exec"
"path/filepath" "path/filepath"
"strconv"
"strings" "strings"
"testing" "testing"
"time"
"github.com/spf13/afero" "github.com/spf13/afero"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
@@ -43,8 +46,11 @@ Expire-Date: 0
paramsFile := filepath.Join(gpgHome, "key-params") paramsFile := filepath.Join(gpgHome, "key-params")
require.NoError(t, os.WriteFile(paramsFile, []byte(keyParams), 0o600)) require.NoError(t, os.WriteFile(paramsFile, []byte(keyParams), 0o600))
ctx, cancel := context.WithTimeout(context.Background(), gpgTimeout)
defer cancel()
//nolint:gosec // paramsFile is a test-controlled path inside t.TempDir() //nolint:gosec // paramsFile is a test-controlled path inside t.TempDir()
cmd := exec.CommandContext(context.Background(), "gpg", cmd := exec.CommandContext(ctx, "gpg",
"--batch", "--gen-key", paramsFile) "--batch", "--gen-key", paramsFile)
cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHome) cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHome)
@@ -55,7 +61,7 @@ Expire-Date: 0
} }
// Get the key fingerprint // Get the key fingerprint
cmd = exec.CommandContext(context.Background(), "gpg", cmd = exec.CommandContext(ctx, "gpg",
"--list-keys", "--with-colons", "test@mfer.test") "--list-keys", "--with-colons", "test@mfer.test")
cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHome) cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHome)
@@ -90,7 +96,7 @@ func TestGPGSign(t *testing.T) {
t.Setenv("GNUPGHOME", gpgHome) t.Setenv("GNUPGHOME", gpgHome)
data := []byte("test data to sign") data := []byte("test data to sign")
sig, err := gpgSign(data, keyID) sig, err := gpgSign(context.Background(), data, keyID)
require.NoError(t, err) require.NoError(t, err)
assert.NotEmpty(t, sig) assert.NotEmpty(t, sig)
assert.Contains(t, string(sig), "-----BEGIN PGP SIGNATURE-----") assert.Contains(t, string(sig), "-----BEGIN PGP SIGNATURE-----")
@@ -101,7 +107,7 @@ func TestGPGExportPublicKey(t *testing.T) {
keyID, gpgHome := testGPGEnv(t) keyID, gpgHome := testGPGEnv(t)
t.Setenv("GNUPGHOME", gpgHome) t.Setenv("GNUPGHOME", gpgHome)
pubKey, err := gpgExportPublicKey(keyID) pubKey, err := gpgExportPublicKey(context.Background(), keyID)
require.NoError(t, err) require.NoError(t, err)
assert.NotEmpty(t, pubKey) assert.NotEmpty(t, pubKey)
assert.Contains(t, string(pubKey), "-----BEGIN PGP PUBLIC KEY BLOCK-----") assert.Contains(t, string(pubKey), "-----BEGIN PGP PUBLIC KEY BLOCK-----")
@@ -112,7 +118,7 @@ func TestGPGGetKeyFingerprint(t *testing.T) {
keyID, gpgHome := testGPGEnv(t) keyID, gpgHome := testGPGEnv(t)
t.Setenv("GNUPGHOME", gpgHome) t.Setenv("GNUPGHOME", gpgHome)
fingerprint, err := gpgGetKeyFingerprint(keyID) fingerprint, err := gpgGetKeyFingerprint(context.Background(), keyID)
require.NoError(t, err) require.NoError(t, err)
assert.NotEmpty(t, fingerprint) assert.NotEmpty(t, fingerprint)
// The fingerprint should be 40 hex chars // The fingerprint should be 40 hex chars
@@ -146,12 +152,12 @@ func TestGPGOptionLikeKeyIDIsNotAnOption(t *testing.T) {
_, gpgHome := testGPGEnv(t) _, gpgHome := testGPGEnv(t)
t.Setenv("GNUPGHOME", gpgHome) t.Setenv("GNUPGHOME", gpgHome)
pubKey, err := gpgExportPublicKey(GPGKeyID("--version")) pubKey, err := gpgExportPublicKey(context.Background(), GPGKeyID("--version"))
require.Error(t, err) require.Error(t, err)
require.ErrorIs(t, err, errGPGKeyNotFound) require.ErrorIs(t, err, errGPGKeyNotFound)
assert.NotContains(t, string(pubKey), "gpg (GnuPG)") assert.NotContains(t, string(pubKey), "gpg (GnuPG)")
fpr, err := gpgGetKeyFingerprint(GPGKeyID("--version")) fpr, err := gpgGetKeyFingerprint(context.Background(), GPGKeyID("--version"))
require.Error(t, err) require.Error(t, err)
assert.NotContains(t, string(fpr), "gpg (GnuPG)") assert.NotContains(t, string(fpr), "gpg (GnuPG)")
} }
@@ -162,7 +168,8 @@ func TestGPGSignInvalidKey(t *testing.T) {
t.Setenv("GNUPGHOME", gpgHome) t.Setenv("GNUPGHOME", gpgHome)
data := []byte("test data") data := []byte("test data")
_, err := gpgSign(data, GPGKeyID("NONEXISTENT_KEY_ID_12345")) _, err := gpgSign(context.Background(), data,
GPGKeyID("NONEXISTENT_KEY_ID_12345"))
assert.Error(t, err) assert.Error(t, err)
} }
@@ -185,7 +192,7 @@ func TestBuilderWithSigning(t *testing.T) {
// Build the manifest // Build the manifest
var buf bytes.Buffer var buf bytes.Buffer
err = b.Build(&buf) err = b.Build(context.Background(), &buf)
require.NoError(t, err) require.NoError(t, err)
// Parse the manifest and verify signature fields are populated // Parse the manifest and verify signature fields are populated
@@ -251,14 +258,14 @@ func TestGPGVerify(t *testing.T) {
t.Setenv("GNUPGHOME", gpgHome) t.Setenv("GNUPGHOME", gpgHome)
data := []byte("test data to sign and verify") data := []byte("test data to sign and verify")
sig, err := gpgSign(data, keyID) sig, err := gpgSign(context.Background(), data, keyID)
require.NoError(t, err) require.NoError(t, err)
pubKey, err := gpgExportPublicKey(keyID) pubKey, err := gpgExportPublicKey(context.Background(), keyID)
require.NoError(t, err) require.NoError(t, err)
// Verify the signature // Verify the signature
err = gpgVerify(data, sig, pubKey) err = gpgVerify(context.Background(), data, sig, pubKey)
require.NoError(t, err) require.NoError(t, err)
} }
@@ -267,15 +274,15 @@ func TestGPGVerifyInvalidSignature(t *testing.T) {
t.Setenv("GNUPGHOME", gpgHome) t.Setenv("GNUPGHOME", gpgHome)
data := []byte("test data to sign") data := []byte("test data to sign")
sig, err := gpgSign(data, keyID) sig, err := gpgSign(context.Background(), data, keyID)
require.NoError(t, err) require.NoError(t, err)
pubKey, err := gpgExportPublicKey(keyID) pubKey, err := gpgExportPublicKey(context.Background(), keyID)
require.NoError(t, err) require.NoError(t, err)
// Try to verify with different data - should fail // Try to verify with different data - should fail
wrongData := []byte("different data") wrongData := []byte("different data")
err = gpgVerify(wrongData, sig, pubKey) err = gpgVerify(context.Background(), wrongData, sig, pubKey)
assert.Error(t, err) assert.Error(t, err)
} }
@@ -284,12 +291,12 @@ func TestGPGVerifyBadPublicKey(t *testing.T) {
t.Setenv("GNUPGHOME", gpgHome) t.Setenv("GNUPGHOME", gpgHome)
data := []byte("test data") data := []byte("test data")
sig, err := gpgSign(data, keyID) sig, err := gpgSign(context.Background(), data, keyID)
require.NoError(t, err) require.NoError(t, err)
// Try to verify with invalid public key - should fail // Try to verify with invalid public key - should fail
badPubKey := []byte("not a valid public key") badPubKey := []byte("not a valid public key")
err = gpgVerify(data, sig, badPubKey) err = gpgVerify(context.Background(), data, sig, badPubKey)
assert.Error(t, err) assert.Error(t, err)
} }
@@ -312,7 +319,7 @@ func TestManifestSignatureVerification(t *testing.T) {
// Build the manifest // Build the manifest
var buf bytes.Buffer var buf bytes.Buffer
err = b.Build(&buf) err = b.Build(context.Background(), &buf)
require.NoError(t, err) require.NoError(t, err)
// Parse the manifest - signature should be verified during load // Parse the manifest - signature should be verified during load
@@ -341,7 +348,7 @@ func TestManifestTamperedSignatureFails(t *testing.T) {
var buf bytes.Buffer var buf bytes.Buffer
err = b.Build(&buf) err = b.Build(context.Background(), &buf)
require.NoError(t, err) require.NoError(t, err)
// Tamper with the signature by replacing some bytes // Tamper with the signature by replacing some bytes
@@ -375,7 +382,7 @@ func TestBuilderWithoutSigning(t *testing.T) {
// Build the manifest // Build the manifest
var buf bytes.Buffer var buf bytes.Buffer
err = b.Build(&buf) err = b.Build(context.Background(), &buf)
require.NoError(t, err) require.NoError(t, err)
// Parse the manifest and verify signature fields are empty // Parse the manifest and verify signature fields are empty
@@ -390,3 +397,76 @@ func TestBuilderWithoutSigning(t *testing.T) {
assert.Empty(t, manifest.pbOuter.GetSigningPubKey(), assert.Empty(t, manifest.pbOuter.GetSigningPubKey(),
"signing public key should be empty when not signing") "signing public key should be empty when not signing")
} }
// fakeGPGPath writes script as an executable named gpg into a temporary
// directory and returns a PATH value with that directory first.
func fakeGPGPath(t *testing.T, script string) string {
t.Helper()
binDir := t.TempDir()
//nolint:gosec // G306: the fake gpg has to be executable
require.NoError(t, os.WriteFile(filepath.Join(binDir, "gpg"),
[]byte(script), 0o700))
return binDir + string(os.PathListSeparator) + os.Getenv("PATH")
}
// TestGPGTimeoutKillsGPG puts a fake gpg that never finishes first on
// PATH and checks that a run past its deadline is killed and reported as
// a timeout of the named operation, instead of hanging.
func TestGPGTimeoutKillsGPG(t *testing.T) {
t.Setenv("PATH", fakeGPGPath(t, "#!/bin/sh\nexec sleep 10\n"))
ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
defer cancel()
_, err := gpgSign(ctx, []byte("data"), GPGKeyID("any"))
require.ErrorIs(t, err, context.DeadlineExceeded)
assert.Contains(t, err.Error(), "gpg sign failed: gpg timed out")
}
// TestGPGTimeoutWhenChildHoldsOutput uses a fake gpg that runs sleep as a
// child instead of exec-ing it, the way a wrapper script around the real
// gpg might. Killing the fake gpg leaves sleep holding its stdout and
// stderr open; the call must still return shortly after the deadline
// instead of waiting for sleep to exit. The fake gpg writes the process ID
// of sleep to a file so that the test can kill it before returning.
func TestGPGTimeoutWhenChildHoldsOutput(t *testing.T) {
pidFile := filepath.Join(t.TempDir(), "sleep.pid")
t.Setenv("PATH", fakeGPGPath(t,
"#!/bin/sh\nsleep 3 &\necho $! >'"+pidFile+"'\nwait\n"))
t.Cleanup(func() {
pid, err := os.ReadFile(pidFile) //nolint:gosec // G304: path inside t.TempDir()
require.NoError(t, err)
n, err := strconv.Atoi(strings.TrimSpace(string(pid)))
require.NoError(t, err)
sleep, err := os.FindProcess(n)
require.NoError(t, err)
require.NoError(t, sleep.Kill())
})
ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
defer cancel()
start := time.Now()
_, err := gpgSign(ctx, []byte("data"), GPGKeyID("any"))
require.ErrorIs(t, err, context.DeadlineExceeded)
assert.Less(t, time.Since(start), 3*time.Second,
"the call waited for the child holding gpg's output to exit")
}
// TestBuildPassesContextToSigning checks that a caller can cancel the gpg
// runs that sign a manifest through the context given to Build.
func TestBuildPassesContextToSigning(t *testing.T) {
t.Parallel()
b := NewBuilder()
b.SetSigningOptions(&SigningOptions{KeyID: "any"})
ctx, cancel := context.WithCancel(context.Background())
cancel()
require.ErrorIs(t, b.Build(ctx, io.Discard), context.Canceled)
}
+1 -2
View File
@@ -283,8 +283,7 @@ func (s *Scanner) ToManifest(
} }
// Build and write manifest // Build and write manifest
//nolint:contextcheck // Build's GPG signing exec is not cancellable by design return builder.Build(ctx, w)
return builder.Build(w)
} }
// configureBuilder constructs a manifest builder configured from the // configureBuilder constructs a manifest builder configured from the
+9 -8
View File
@@ -2,6 +2,7 @@ package mfer
import ( import (
"bytes" "bytes"
"context"
"crypto/sha256" "crypto/sha256"
"errors" "errors"
"fmt" "fmt"
@@ -50,13 +51,13 @@ func newTimestampFromTime(t time.Time) *Timestamp {
} }
} }
func (m *manifest) generate() error { func (m *manifest) generate(ctx context.Context) error {
if m.pbInner == nil { if m.pbInner == nil {
return errInnerNotSet return errInnerNotSet
} }
if m.pbOuter == nil { if m.pbOuter == nil {
e := m.generateOuter() e := m.generateOuter(ctx)
if e != nil { if e != nil {
return e return e
} }
@@ -77,7 +78,7 @@ func (m *manifest) generate() error {
return nil return nil
} }
func (m *manifest) generateOuter() error { func (m *manifest) generateOuter(ctx context.Context) error {
if m.pbInner == nil { if m.pbInner == nil {
return errInternal return errInternal
} }
@@ -135,7 +136,7 @@ func (m *manifest) generateOuter() error {
// Sign the manifest if signing options are provided // Sign the manifest if signing options are provided
if m.signingOptions != nil && m.signingOptions.KeyID != "" { if m.signingOptions != nil && m.signingOptions.KeyID != "" {
return m.signOuter() return m.signOuter(ctx)
} }
return nil return nil
@@ -143,27 +144,27 @@ func (m *manifest) generateOuter() error {
// signOuter signs the outer message with the configured GPG key and // signOuter signs the outer message with the configured GPG key and
// embeds the signature, signer fingerprint, and public key. // embeds the signature, signer fingerprint, and public key.
func (m *manifest) signOuter() error { func (m *manifest) signOuter(ctx context.Context) error {
sigString, err := m.signatureString() sigString, err := m.signatureString()
if err != nil { if err != nil {
return fmt.Errorf("failed to generate signature string: %w", err) return fmt.Errorf("failed to generate signature string: %w", err)
} }
sig, err := gpgSign([]byte(sigString), m.signingOptions.KeyID) sig, err := gpgSign(ctx, []byte(sigString), m.signingOptions.KeyID)
if err != nil { if err != nil {
return fmt.Errorf("failed to sign manifest: %w", err) return fmt.Errorf("failed to sign manifest: %w", err)
} }
m.pbOuter.Signature = sig m.pbOuter.Signature = sig
fingerprint, err := gpgGetKeyFingerprint(m.signingOptions.KeyID) fingerprint, err := gpgGetKeyFingerprint(ctx, m.signingOptions.KeyID)
if err != nil { if err != nil {
return fmt.Errorf("failed to get key fingerprint: %w", err) return fmt.Errorf("failed to get key fingerprint: %w", err)
} }
m.pbOuter.Signer = fingerprint m.pbOuter.Signer = fingerprint
pubKey, err := gpgExportPublicKey(m.signingOptions.KeyID) pubKey, err := gpgExportPublicKey(ctx, m.signingOptions.KeyID)
if err != nil { if err != nil {
return fmt.Errorf("failed to export public key: %w", err) return fmt.Errorf("failed to export public key: %w", err)
} }
+15 -5
View File
@@ -1,14 +1,24 @@
#!/bin/sh #!/bin/sh
# script/cibuild: run the CI build. The Dockerfile runs script/check # script/cibuild: run the CI build; the Gitea workflow runs this on push.
# (via make check), so a successful build implies all checks pass. # It builds the image with the same command as script/docker. --no-cache
# Generic: needs no adaptation. The Gitea workflow runs this on push. # because the checks the final stage depends on are RUN steps, and a
# cached one is a check that did not run.
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
main() { main() {
cd "$ROOT" cd "$ROOT"
docker build . # Own line: a failing command substitution inside an argument does
# not trip `set -e`, so the inline form degrades silently to an
# empty constant. The VERSION build argument takes precedence over
# the version a build stage derives from the .git in the context.
version="$(git describe --tags --always --dirty 2>/dev/null || true)"
[ -n "$version" ] || version="unknown"
docker build --no-cache \
--build-arg VERSION="$version" \
-t "$("$SCRIPT_DIR/projectname")" .
} }
main "$@" main "$@"
+11 -2
View File
@@ -1,7 +1,8 @@
#!/bin/sh #!/bin/sh
# script/docker: build the Docker image tagged with the project name. # script/docker: build the Docker image tagged with the project name.
# Identical in all repos; the tag comes from script/projectname. # Identical in all repos; the tag comes from script/projectname.
# Generic: needs no adaptation. # --no-cache because the gate phases the final stage depends on are RUN
# steps, and a cached one is a check that did not run.
set -eu set -eu
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
@@ -9,7 +10,15 @@ ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
main() { main() {
cd "$ROOT" cd "$ROOT"
docker build -t "$("$SCRIPT_DIR/projectname")" . # Own line: a failing command substitution inside an argument does
# not trip `set -e`, so the inline form degrades silently to an
# empty constant. The VERSION build argument takes precedence over
# the version a build stage derives from the .git in the context.
version="$(git describe --tags --always --dirty 2>/dev/null || true)"
[ -n "$version" ] || version="unknown"
docker build --no-cache \
--build-arg VERSION="$version" \
-t "$("$SCRIPT_DIR/projectname")" .
} }
main "$@" main "$@"
+6 -1
View File
@@ -17,7 +17,12 @@ ensure_pb() {
main() { main() {
cd "$ROOT" cd "$ROOT"
ensure_pb ensure_pb
go test -v --timeout 10s ./... go test -timeout 30s -race -cover ./... ||
{
echo "--- Rerunning with -v for details ---"
go test -timeout 30s -race -v ./...
exit 1
}
} }
main "$@" main "$@"