Author SHA1 Message Date
sneak 080e84dfc0 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 00:41:59 +00:00
17 changed files with 93 additions and 533 deletions
+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
committed (required for `go get` compatibility).
- The format specification is in `FORMAT.md`.
- See the TODO section in `README.md` for the 1.0 implementation plan and open
design questions.
- Open work, open design questions included, is tracked only in the repo's
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.
+14 -206
View File
@@ -157,18 +157,23 @@ The manifest file would do several important things:
- metadata size should not be used as an excuse to sacrifice utility (such
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
for the whole assembled file?
- If so, should the chunksize be fixed or dynamic?
for the whole assembled file? If so, should the chunk size be fixed or
dynamic? Still open, on
[issue 81](https://git.eeqj.de/sneak/mfer/issues/81#issuecomment-118698).
- Should the manifest signature format be GnuPG signatures, or those from
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
@@ -246,207 +251,10 @@ regardless of filesystem format.
Please email [`sneak@sneak.berlin`](mailto:sneak@sneak.berlin) with your desired
username for an account on this Gitea instance.
# TODO: Remaining Work for 1.0
# TODO
## Design Questions (Owner Decision Required)
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`
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).
# See Also
-138
View File
@@ -1,138 +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-10-03: every gpg run is killed after one minute or when its caller's
context ends, and a timeout reads as "gpg timed out" under the failing
operation; `Builder.Build` and `Checker.ExtractEmbeddedSigningKeyFP` take a
context, which reaches gpg (#62)
- 2026-10-03: pinned the CLI error messages by driving the functions that emit
them in `internal/cli/errmsg_test.go`, and made the freshen mtime-presence
test distinguish an absent mtime from the epoch (#87)
- 2026-10-03: `script/cibuild` builds the image with the same command as
`script/docker`, `--no-cache` included, so the checks in the Dockerfile run on
every build, also on an unchanged tree (#89)
- 2026-10-03: `fetch` removes whatever sits at a file's temp name and then
creates the temp file only if that name is free, so a hard link left there
cannot make it write into a file outside the destination directory (#115)
- 2026-10-03: `fetch` refuses any manifest path that runs through a symlink
already in the destination directory, checked before each of its writes
(directories, temp file, rename), so such a symlink cannot send a write
outside it (#86)
- 2026-10-02: a plain `docker build .` of a clone now stamps the tag or short
commit into `mfer version` instead of nothing: `.dockerignore` sends `.git`
(not `.git/config`), and the build stage takes the `VERSION` build argument,
otherwise `git describe --tags --always`, failing if `.git` is present and no
version comes out. `script/docker` is the canonical copy, which passes
`VERSION`; `bin/gitrev.sh` uses `--tags` too (#112)
- 2026-09-21: validate manifest entry paths on deserialize so untrusted `.mf`
files cannot make `Checker` stat or read outside `basePath` (#61)
- 2026-09-21: rewrote `script/test` to the canonical pattern (30s timeout,
`-race -cover`, quiet-first with verbose-on-failure rerun) and fixed the
process-global logger data race it surfaced (#67)
- 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
- 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)
- 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
+3 -6
View File
@@ -2,7 +2,6 @@
package cli
import (
"context"
"encoding/hex"
"errors"
"fmt"
@@ -127,9 +126,7 @@ func (mfa *CLIApp) fetchManifestToTemp(url string) (string, error) {
// verifyRequiredSigner enforces the --require-signature fingerprint
// against the manifest's embedded signing key.
func verifyRequiredSigner(
ctx context.Context, chk *mfer.Checker, requiredSigner string,
) error {
func verifyRequiredSigner(chk *mfer.Checker, requiredSigner string) error {
// Validate fingerprint format: must be exactly 40 hex characters
if len(requiredSigner) != fingerprintHexLen {
return fmt.Errorf("%w, got %d", errInvalidFingerprint, len(requiredSigner))
@@ -148,7 +145,7 @@ func verifyRequiredSigner(
// Extract fingerprint from the embedded public key (not from the
// signer field). This validates the key is importable and gets its
// actual fingerprint.
embeddedFP, err := chk.ExtractEmbeddedSigningKeyFP(ctx)
embeddedFP, err := chk.ExtractEmbeddedSigningKeyFP()
if err != nil {
return fmt.Errorf(
"failed to extract fingerprint from embedded signing key: %w", err)
@@ -306,7 +303,7 @@ func (mfa *CLIApp) checkManifestOperation(ctx *cli.Context) error {
// Check signature requirement
requiredSigner := ctx.String("require-signature")
if requiredSigner != "" {
err = verifyRequiredSigner(ctx.Context, chk, requiredSigner)
err = verifyRequiredSigner(chk, requiredSigner)
if err != nil {
return err
}
+5 -7
View File
@@ -83,8 +83,7 @@ func TestVerifyRequiredSignerMessages(t *testing.T) {
t.Run("invalid fingerprint length", func(t *testing.T) {
t.Parallel()
err := verifyRequiredSigner(context.Background(),
unsignedChecker(t), "12345678")
err := verifyRequiredSigner(unsignedChecker(t), "12345678")
require.ErrorIs(t, err, errInvalidFingerprint)
assert.EqualError(t, err,
"invalid fingerprint: must be exactly 40 hex characters, got 8")
@@ -93,8 +92,7 @@ func TestVerifyRequiredSignerMessages(t *testing.T) {
t.Run("manifest not signed", func(t *testing.T) {
t.Parallel()
err := verifyRequiredSigner(context.Background(),
unsignedChecker(t), msgFpA)
err := verifyRequiredSigner(unsignedChecker(t), msgFpA)
require.ErrorIs(t, err, errManifestNotSigned)
assert.EqualError(t, err,
"manifest is not signed, but signature from "+msgFpA+" is required")
@@ -111,10 +109,10 @@ func TestVerifyRequiredSignerMessages(t *testing.T) {
func TestSignerMismatchMessage(t *testing.T) {
chk := signedChecker(t)
embeddedFP, err := chk.ExtractEmbeddedSigningKeyFP(context.Background())
embeddedFP, err := chk.ExtractEmbeddedSigningKeyFP()
require.NoError(t, err)
err = verifyRequiredSigner(context.Background(), chk, msgFpB)
err = verifyRequiredSigner(chk, msgFpB)
require.ErrorIs(t, err, errSignerMismatch)
assert.EqualError(t, err,
"embedded signing key fingerprint "+embeddedFP+
@@ -162,7 +160,7 @@ func signedChecker(t *testing.T) *mfer.Checker {
var buf bytes.Buffer
require.NoError(t, b.Build(context.Background(), &buf))
require.NoError(t, b.Build(&buf))
fs := afero.NewMemMapFs()
require.NoError(t, afero.WriteFile(fs, "/index.mf", buf.Bytes(), 0o644))
+3 -4
View File
@@ -1,7 +1,6 @@
package cli
import (
"context"
"crypto/sha256"
"errors"
"fmt"
@@ -305,7 +304,7 @@ func (h *freshenHasher) processEntry(e *freshenEntry) error {
// writeFreshenedManifest writes the manifest atomically (write to a
// temp file, then rename over the target).
func writeFreshenedManifest(
ctx context.Context, afs afero.Fs, builder *mfer.Builder, manifestPath string,
afs afero.Fs, builder *mfer.Builder, manifestPath string,
) error {
tmpPath := manifestPath + ".tmp"
@@ -314,7 +313,7 @@ func writeFreshenedManifest(
return fmt.Errorf("failed to create temp file: %w", err)
}
err = builder.Build(ctx, outFile)
err = builder.Build(outFile)
_ = outFile.Close()
if err != nil {
@@ -531,7 +530,7 @@ func (mfa *CLIApp) freshenManifestOperation(ctx *cli.Context) error {
}
// Write updated manifest atomically (write to temp, then rename)
err = writeFreshenedManifest(ctx.Context, mfa.Fs, hasher.builder, manifestPath)
err = writeFreshenedManifest(mfa.Fs, hasher.builder, manifestPath)
if err != nil {
return err
}
+4 -6
View File
@@ -3,7 +3,6 @@
package mfer
import (
"context"
"crypto/sha256"
"errors"
"fmt"
@@ -282,9 +281,8 @@ func (b *Builder) SetSigningOptions(opts *SigningOptions) {
b.signingOptions = opts
}
// Build finalizes the manifest and writes it to the writer. ctx bounds the
// gpg runs that sign the manifest when signing options are set.
func (b *Builder) Build(ctx context.Context, w io.Writer) error {
// Build finalizes the manifest and writes it to the writer.
func (b *Builder) Build(w io.Writer) error {
b.mu.Lock()
defer b.mu.Unlock()
@@ -310,13 +308,13 @@ func (b *Builder) Build(ctx context.Context, w io.Writer) error {
}
// Generate outer wrapper
err := m.generateOuter(ctx)
err := m.generateOuter()
if err != nil {
return fmt.Errorf("build: generate outer: %w", err)
}
// Generate final output
err = m.generate(ctx)
err = m.generate()
if err != nil {
return fmt.Errorf("build: generate: %w", err)
}
+8 -9
View File
@@ -3,7 +3,6 @@ package mfer
import (
"bytes"
"context"
"strings"
"testing"
"time"
@@ -114,7 +113,7 @@ func TestBuilderBuild(t *testing.T) {
var buf bytes.Buffer
err = b.Build(context.Background(), &buf)
err = b.Build(&buf)
require.NoError(t, err)
// Should have magic bytes
@@ -178,7 +177,7 @@ func TestBuilderDeterministicOutput(t *testing.T) {
var buf bytes.Buffer
err := b.Build(context.Background(), &buf)
err := b.Build(&buf)
require.NoError(t, err)
return buf.Bytes()
@@ -326,7 +325,7 @@ func TestBuilderBuildRoundTrip(t *testing.T) {
}
var buf bytes.Buffer
require.NoError(t, b.Build(context.Background(), &buf))
require.NoError(t, b.Build(&buf))
m, err := NewManifestFromReader(&buf)
require.NoError(t, err)
@@ -384,7 +383,7 @@ func TestManifestString(t *testing.T) {
require.NoError(t, err)
var buf bytes.Buffer
require.NoError(t, b.Build(context.Background(), &buf))
require.NoError(t, b.Build(&buf))
m, err := NewManifestFromReader(&buf)
require.NoError(t, err)
@@ -398,7 +397,7 @@ func TestBuilderBuildEmpty(t *testing.T) {
var buf bytes.Buffer
err := b.Build(context.Background(), &buf)
err := b.Build(&buf)
require.NoError(t, err)
// Should still produce valid manifest with 0 files
@@ -417,7 +416,7 @@ func TestBuilderOmitsCreatedAtByDefault(t *testing.T) {
require.NoError(t, err)
var buf bytes.Buffer
require.NoError(t, b.Build(context.Background(), &buf))
require.NoError(t, b.Build(&buf))
m, err := NewManifestFromReader(&buf)
require.NoError(t, err)
@@ -439,7 +438,7 @@ func TestBuilderIncludesCreatedAtWhenRequested(t *testing.T) {
require.NoError(t, err)
var buf bytes.Buffer
require.NoError(t, b.Build(context.Background(), &buf))
require.NoError(t, b.Build(&buf))
m, err := NewManifestFromReader(&buf)
require.NoError(t, err)
@@ -465,7 +464,7 @@ func TestBuilderDeterministicFileOrder(t *testing.T) {
}
var buf bytes.Buffer
require.NoError(t, b.Build(context.Background(), &buf))
require.NoError(t, b.Build(&buf))
m, err := NewManifestFromReader(&buf)
require.NoError(t, err)
+2 -2
View File
@@ -164,12 +164,12 @@ func (c *Checker) SigningPubKey() []byte {
// ExtractEmbeddedSigningKeyFP imports the manifest's embedded public key into a
// temporary keyring and extracts its fingerprint. This validates the key and
// returns its actual fingerprint from the key material itself.
func (c *Checker) ExtractEmbeddedSigningKeyFP(ctx context.Context) (string, error) {
func (c *Checker) ExtractEmbeddedSigningKeyFP() (string, error) {
if len(c.signingPubKey) == 0 {
return "", errNoSigningPubKey
}
return gpgExtractPubKeyFingerprint(ctx, c.signingPubKey)
return gpgExtractPubKeyFingerprint(c.signingPubKey)
}
// Check verifies all files against the manifest.
+1 -1
View File
@@ -61,7 +61,7 @@ func createTestManifest(
}
var buf bytes.Buffer
require.NoError(t, builder.Build(context.Background(), &buf))
require.NoError(t, builder.Build(&buf))
require.NoError(t, afero.WriteFile(fs, manifestPath, buf.Bytes(), 0o644))
}
-3
View File
@@ -2,7 +2,6 @@ package mfer
import (
"bytes"
"context"
"crypto/sha256"
"errors"
"fmt"
@@ -93,9 +92,7 @@ func (m *manifest) verifyOuterIntegrity() error {
)
}
// Loading a manifest takes no context; gpgTimeout still bounds gpg.
err = gpgVerify(
context.Background(),
[]byte(sigString),
m.pbOuter.GetSignature(),
m.pbOuter.GetSigningPubKey(),
+1 -2
View File
@@ -3,7 +3,6 @@ package mfer
import (
"bytes"
"context"
"crypto/sha256"
"fmt"
"testing"
@@ -123,7 +122,7 @@ func TestDeserializeValidManifestRoundTrips(t *testing.T) {
require.NoError(t, b.AddFileWithHash("dir/file.txt", 123, ModTime{}, hash))
var buf bytes.Buffer
require.NoError(t, b.Build(context.Background(), &buf))
require.NoError(t, b.Build(&buf))
m, err := NewManifestFromReader(bytes.NewReader(buf.Bytes()))
require.NoError(t, err)
+2 -4
View File
@@ -2,7 +2,6 @@
package mfer
import (
"context"
"testing"
"github.com/stretchr/testify/assert"
@@ -81,7 +80,6 @@ func TestSerializeInternalErrorMessagesVerbatim(t *testing.T) {
t.Parallel()
m := &manifest{}
require.EqualError(t, m.generate(context.Background()),
"internal error: pbInner not set")
require.EqualError(t, m.generateOuter(context.Background()), "internal error")
require.EqualError(t, m.generate(), "internal error: pbInner not set")
require.EqualError(t, m.generateOuter(), "internal error")
}
+15 -48
View File
@@ -10,21 +10,9 @@ import (
"os/exec"
"path/filepath"
"strings"
"time"
)
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
// directories.
privateDirPerms os.FileMode = 0o700
@@ -78,20 +66,8 @@ func gpgArgs(opts []string, positional ...string) []string {
}
// runGPG runs the gpg binary in batch mode with the given arguments and
// optional stdin, returning captured stdout and stderr. gpg is killed when
// 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()
// optional stdin, returning captured stdout and stderr.
func runGPG(stdin io.Reader, args ...string) (*bytes.Buffer, *bytes.Buffer, error) {
fullArgs := append([]string{"--batch", "--no-tty"}, args...)
// G204: the executable name is a compile-time constant. The arguments
@@ -100,8 +76,7 @@ func runGPG(
// option or after the "--" end-of-options marker inserted by gpgArgs,
// and therefore cannot be reinterpreted by gpg as an option.
cmd := exec.CommandContext( //nolint:gosec // G204: see comment above
ctx, "gpg", fullArgs...)
cmd.WaitDelay = gpgWaitDelay
context.Background(), "gpg", fullArgs...)
cmd.Stdin = stdin
var stdout, stderr bytes.Buffer
@@ -110,14 +85,6 @@ func runGPG(
cmd.Stderr = &stderr
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
}
@@ -138,8 +105,8 @@ func parseFingerprint(colonOutput string) (string, bool) {
// gpgSign creates a detached signature of the data using the specified key.
// Returns the armored detached signature.
func gpgSign(ctx context.Context, data []byte, keyID GPGKeyID) ([]byte, error) {
stdout, stderr, err := runGPG(ctx, bytes.NewReader(data),
func gpgSign(data []byte, keyID GPGKeyID) ([]byte, error) {
stdout, stderr, err := runGPG(bytes.NewReader(data),
"--detach-sign",
gpgOptArmor,
"--local-user", string(keyID),
@@ -153,8 +120,8 @@ func gpgSign(ctx context.Context, data []byte, keyID GPGKeyID) ([]byte, error) {
// gpgExportPublicKey exports the public key for the specified key ID.
// Returns the armored public key.
func gpgExportPublicKey(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
stdout, stderr, err := runGPG(ctx, nil,
func gpgExportPublicKey(keyID GPGKeyID) ([]byte, error) {
stdout, stderr, err := runGPG(nil,
gpgArgs([]string{"--export", gpgOptArmor}, string(keyID))...,
)
if err != nil {
@@ -169,8 +136,8 @@ func gpgExportPublicKey(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
}
// gpgGetKeyFingerprint gets the full fingerprint for a key ID.
func gpgGetKeyFingerprint(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
stdout, stderr, err := runGPG(ctx, nil,
func gpgGetKeyFingerprint(keyID GPGKeyID) ([]byte, error) {
stdout, stderr, err := runGPG(nil,
gpgArgs([]string{"--with-colons", "--fingerprint"}, string(keyID))...,
)
if err != nil {
@@ -190,7 +157,7 @@ func gpgGetKeyFingerprint(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
// gpgExtractPubKeyFingerprint imports a public key into a temporary keyring
// and extracts its fingerprint. This verifies the key is valid and returns
// the actual fingerprint from the key material.
func gpgExtractPubKeyFingerprint(ctx context.Context, pubKey []byte) (string, error) {
func gpgExtractPubKeyFingerprint(pubKey []byte) (string, error) {
// Create temporary directory for GPG operations
tmpDir, err := os.MkdirTemp("", "mfer-gpg-fingerprint-*")
if err != nil {
@@ -214,7 +181,7 @@ func gpgExtractPubKeyFingerprint(ctx context.Context, pubKey []byte) (string, er
}
// Import the public key into the temporary keyring
_, importStderr, err := runGPG(ctx, nil,
_, importStderr, err := runGPG(nil,
gpgArgs([]string{gpgOptHomedir, tmpDir, "--import"}, pubKeyFile)...,
)
if err != nil {
@@ -224,7 +191,7 @@ func gpgExtractPubKeyFingerprint(ctx context.Context, pubKey []byte) (string, er
}
// List keys to get fingerprint
listStdout, listStderr, err := runGPG(ctx, nil,
listStdout, listStderr, err := runGPG(nil,
"--homedir", tmpDir,
"--with-colons",
"--fingerprint",
@@ -245,7 +212,7 @@ func gpgExtractPubKeyFingerprint(ctx context.Context, pubKey []byte) (string, er
// gpgVerify verifies a detached signature against data using the provided public key.
// It creates a temporary keyring to import the public key for verification.
func gpgVerify(ctx context.Context, data, signature, pubKey []byte) error {
func gpgVerify(data, signature, pubKey []byte) error {
// Create temporary directory for GPG operations
tmpDir, err := os.MkdirTemp("", "mfer-gpg-verify-*")
if err != nil {
@@ -285,7 +252,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) error {
}
// Import the public key into the temporary keyring
_, importStderr, err := runGPG(ctx, nil,
_, importStderr, err := runGPG(nil,
gpgArgs([]string{gpgOptHomedir, tmpDir, "--import"}, pubKeyFile)...,
)
if err != nil {
@@ -295,7 +262,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) error {
}
// Verify the signature
_, verifyStderr, err := runGPG(ctx, nil,
_, verifyStderr, err := runGPG(nil,
gpgArgs([]string{gpgOptHomedir, tmpDir, gpgOptVerify},
sigFile, dataFile)...,
)
+20 -85
View File
@@ -4,13 +4,11 @@ package mfer
import (
"bytes"
"context"
"io"
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
"time"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
@@ -45,11 +43,8 @@ Expire-Date: 0
paramsFile := filepath.Join(gpgHome, "key-params")
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()
cmd := exec.CommandContext(ctx, "gpg",
cmd := exec.CommandContext(context.Background(), "gpg",
"--batch", "--gen-key", paramsFile)
cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHome)
@@ -60,7 +55,7 @@ Expire-Date: 0
}
// Get the key fingerprint
cmd = exec.CommandContext(ctx, "gpg",
cmd = exec.CommandContext(context.Background(), "gpg",
"--list-keys", "--with-colons", "test@mfer.test")
cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHome)
@@ -95,7 +90,7 @@ func TestGPGSign(t *testing.T) {
t.Setenv("GNUPGHOME", gpgHome)
data := []byte("test data to sign")
sig, err := gpgSign(context.Background(), data, keyID)
sig, err := gpgSign(data, keyID)
require.NoError(t, err)
assert.NotEmpty(t, sig)
assert.Contains(t, string(sig), "-----BEGIN PGP SIGNATURE-----")
@@ -106,7 +101,7 @@ func TestGPGExportPublicKey(t *testing.T) {
keyID, gpgHome := testGPGEnv(t)
t.Setenv("GNUPGHOME", gpgHome)
pubKey, err := gpgExportPublicKey(context.Background(), keyID)
pubKey, err := gpgExportPublicKey(keyID)
require.NoError(t, err)
assert.NotEmpty(t, pubKey)
assert.Contains(t, string(pubKey), "-----BEGIN PGP PUBLIC KEY BLOCK-----")
@@ -117,7 +112,7 @@ func TestGPGGetKeyFingerprint(t *testing.T) {
keyID, gpgHome := testGPGEnv(t)
t.Setenv("GNUPGHOME", gpgHome)
fingerprint, err := gpgGetKeyFingerprint(context.Background(), keyID)
fingerprint, err := gpgGetKeyFingerprint(keyID)
require.NoError(t, err)
assert.NotEmpty(t, fingerprint)
// The fingerprint should be 40 hex chars
@@ -151,12 +146,12 @@ func TestGPGOptionLikeKeyIDIsNotAnOption(t *testing.T) {
_, gpgHome := testGPGEnv(t)
t.Setenv("GNUPGHOME", gpgHome)
pubKey, err := gpgExportPublicKey(context.Background(), GPGKeyID("--version"))
pubKey, err := gpgExportPublicKey(GPGKeyID("--version"))
require.Error(t, err)
require.ErrorIs(t, err, errGPGKeyNotFound)
assert.NotContains(t, string(pubKey), "gpg (GnuPG)")
fpr, err := gpgGetKeyFingerprint(context.Background(), GPGKeyID("--version"))
fpr, err := gpgGetKeyFingerprint(GPGKeyID("--version"))
require.Error(t, err)
assert.NotContains(t, string(fpr), "gpg (GnuPG)")
}
@@ -167,8 +162,7 @@ func TestGPGSignInvalidKey(t *testing.T) {
t.Setenv("GNUPGHOME", gpgHome)
data := []byte("test data")
_, err := gpgSign(context.Background(), data,
GPGKeyID("NONEXISTENT_KEY_ID_12345"))
_, err := gpgSign(data, GPGKeyID("NONEXISTENT_KEY_ID_12345"))
assert.Error(t, err)
}
@@ -191,7 +185,7 @@ func TestBuilderWithSigning(t *testing.T) {
// Build the manifest
var buf bytes.Buffer
err = b.Build(context.Background(), &buf)
err = b.Build(&buf)
require.NoError(t, err)
// Parse the manifest and verify signature fields are populated
@@ -257,14 +251,14 @@ func TestGPGVerify(t *testing.T) {
t.Setenv("GNUPGHOME", gpgHome)
data := []byte("test data to sign and verify")
sig, err := gpgSign(context.Background(), data, keyID)
sig, err := gpgSign(data, keyID)
require.NoError(t, err)
pubKey, err := gpgExportPublicKey(context.Background(), keyID)
pubKey, err := gpgExportPublicKey(keyID)
require.NoError(t, err)
// Verify the signature
err = gpgVerify(context.Background(), data, sig, pubKey)
err = gpgVerify(data, sig, pubKey)
require.NoError(t, err)
}
@@ -273,15 +267,15 @@ func TestGPGVerifyInvalidSignature(t *testing.T) {
t.Setenv("GNUPGHOME", gpgHome)
data := []byte("test data to sign")
sig, err := gpgSign(context.Background(), data, keyID)
sig, err := gpgSign(data, keyID)
require.NoError(t, err)
pubKey, err := gpgExportPublicKey(context.Background(), keyID)
pubKey, err := gpgExportPublicKey(keyID)
require.NoError(t, err)
// Try to verify with different data - should fail
wrongData := []byte("different data")
err = gpgVerify(context.Background(), wrongData, sig, pubKey)
err = gpgVerify(wrongData, sig, pubKey)
assert.Error(t, err)
}
@@ -290,12 +284,12 @@ func TestGPGVerifyBadPublicKey(t *testing.T) {
t.Setenv("GNUPGHOME", gpgHome)
data := []byte("test data")
sig, err := gpgSign(context.Background(), data, keyID)
sig, err := gpgSign(data, keyID)
require.NoError(t, err)
// Try to verify with invalid public key - should fail
badPubKey := []byte("not a valid public key")
err = gpgVerify(context.Background(), data, sig, badPubKey)
err = gpgVerify(data, sig, badPubKey)
assert.Error(t, err)
}
@@ -318,7 +312,7 @@ func TestManifestSignatureVerification(t *testing.T) {
// Build the manifest
var buf bytes.Buffer
err = b.Build(context.Background(), &buf)
err = b.Build(&buf)
require.NoError(t, err)
// Parse the manifest - signature should be verified during load
@@ -347,7 +341,7 @@ func TestManifestTamperedSignatureFails(t *testing.T) {
var buf bytes.Buffer
err = b.Build(context.Background(), &buf)
err = b.Build(&buf)
require.NoError(t, err)
// Tamper with the signature by replacing some bytes
@@ -381,7 +375,7 @@ func TestBuilderWithoutSigning(t *testing.T) {
// Build the manifest
var buf bytes.Buffer
err = b.Build(context.Background(), &buf)
err = b.Build(&buf)
require.NoError(t, err)
// Parse the manifest and verify signature fields are empty
@@ -396,62 +390,3 @@ func TestBuilderWithoutSigning(t *testing.T) {
assert.Empty(t, manifest.pbOuter.GetSigningPubKey(),
"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.
func TestGPGTimeoutWhenChildHoldsOutput(t *testing.T) {
t.Setenv("PATH", fakeGPGPath(t, "#!/bin/sh\nsleep 3\nexit\n"))
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)
}
+2 -1
View File
@@ -283,7 +283,8 @@ func (s *Scanner) ToManifest(
}
// Build and write manifest
return builder.Build(ctx, w)
//nolint:contextcheck // Build's GPG signing exec is not cancellable by design
return builder.Build(w)
}
// configureBuilder constructs a manifest builder configured from the
+8 -9
View File
@@ -2,7 +2,6 @@ package mfer
import (
"bytes"
"context"
"crypto/sha256"
"errors"
"fmt"
@@ -51,13 +50,13 @@ func newTimestampFromTime(t time.Time) *Timestamp {
}
}
func (m *manifest) generate(ctx context.Context) error {
func (m *manifest) generate() error {
if m.pbInner == nil {
return errInnerNotSet
}
if m.pbOuter == nil {
e := m.generateOuter(ctx)
e := m.generateOuter()
if e != nil {
return e
}
@@ -78,7 +77,7 @@ func (m *manifest) generate(ctx context.Context) error {
return nil
}
func (m *manifest) generateOuter(ctx context.Context) error {
func (m *manifest) generateOuter() error {
if m.pbInner == nil {
return errInternal
}
@@ -136,7 +135,7 @@ func (m *manifest) generateOuter(ctx context.Context) error {
// Sign the manifest if signing options are provided
if m.signingOptions != nil && m.signingOptions.KeyID != "" {
return m.signOuter(ctx)
return m.signOuter()
}
return nil
@@ -144,27 +143,27 @@ func (m *manifest) generateOuter(ctx context.Context) error {
// signOuter signs the outer message with the configured GPG key and
// embeds the signature, signer fingerprint, and public key.
func (m *manifest) signOuter(ctx context.Context) error {
func (m *manifest) signOuter() error {
sigString, err := m.signatureString()
if err != nil {
return fmt.Errorf("failed to generate signature string: %w", err)
}
sig, err := gpgSign(ctx, []byte(sigString), m.signingOptions.KeyID)
sig, err := gpgSign([]byte(sigString), m.signingOptions.KeyID)
if err != nil {
return fmt.Errorf("failed to sign manifest: %w", err)
}
m.pbOuter.Signature = sig
fingerprint, err := gpgGetKeyFingerprint(ctx, m.signingOptions.KeyID)
fingerprint, err := gpgGetKeyFingerprint(m.signingOptions.KeyID)
if err != nil {
return fmt.Errorf("failed to get key fingerprint: %w", err)
}
m.pbOuter.Signer = fingerprint
pubKey, err := gpgExportPublicKey(ctx, m.signingOptions.KeyID)
pubKey, err := gpgExportPublicKey(m.signingOptions.KeyID)
if err != nil {
return fmt.Errorf("failed to export public key: %w", err)
}