All checks were successful
check / check (push) Successful in 53s
- Add canonical .golangci.yml (v2 schema, default: all, project thresholds for lll/funlen/cyclop/dupl) - Bump golangci-lint pins from v2.0.2 to v2.12.2 in Makefile (go install, new /v2 module path) and Dockerfile (tagged+digest Debian image pin) - Fix all lint findings surfaced by the new linter set across cmd/mfer, internal/bork, internal/cli, internal/log, and mfer: static sentinel errors (err113), context-aware HTTP and exec (noctx), guarded integer conversions and stricter permissions (gosec), named constants (mnd, goconst), function decomposition (funlen, cyclop, gocognit, nestif), declaration ordering (funcorder), t.Parallel/t.TempDir/t.Setenv adoption in tests (paralleltest, usetesting), protobuf getters (protogetter), plus formatting and style cleanups (wsl_v5, nlreturn, lll, revive, testifylint, and others) - Serialize CLI runs in tests behind a mutex so parallel tests do not cross-wire the process-global logger's captured output The decompositions are behavior-preserving. In particular: - REPO_POLICIES.md is untouched and stays byte-identical to the authoritative copy in the prompts repo - the mfer.manifest type stays unexported; whether to export it is an open owner design question (README question 13) - directories created by fetch keep mode 0755, because fetched trees are content meant to be readable by other uids - an absent MFFilePath.Mtime is handled explicitly and identically in freshen, list, and export rather than being read as the Unix epoch, which would classify every entry as changed and rewrite the manifest on every freshen - every user-visible error message renders byte-identically to what it did before, with the err113 sentinels wrapped mid-sentence where needed; the rendered strings are now pinned by tests Also fixes an argument-injection defect the lint pass surfaced: key IDs reach gpg as bare positional arguments, so a key ID beginning with "-" was parsed by gpg as an option. All positional arguments now follow an explicit "--" end-of-options marker. The symlink-escape gap in fetch's path handling, which sanitizePath does not and cannot address, is filed separately as #86.
169 lines
4.7 KiB
Go
169 lines
4.7 KiB
Go
//nolint:testpackage // white-box tests exercise unexported internals
|
|
package cli
|
|
|
|
import (
|
|
"fmt"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
)
|
|
|
|
// errMsgCase is one pinned user-visible error message.
|
|
type errMsgCase struct {
|
|
name string
|
|
err error
|
|
want string
|
|
}
|
|
|
|
const (
|
|
msgFpA = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
|
|
msgFpB = "BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"
|
|
)
|
|
|
|
func checkErrMsgCases(t *testing.T, cases []errMsgCase) {
|
|
t.Helper()
|
|
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
assert.Equal(t, tc.want, tc.err.Error())
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestErrorMessagesVerbatim pins the exact rendered text of the CLI's
|
|
// user-visible error messages.
|
|
//
|
|
// These strings are an interface: they are grepped for in CI pipelines
|
|
// and quoted in bug reports. The messages are assembled by wrapping
|
|
// static sentinels, and it is easy to change what a user sees while
|
|
// only meaning to make an error matchable with errors.Is - which is
|
|
// precisely what happened once already. Any change to a string below is
|
|
// therefore a deliberate, separately stated change, never a side effect
|
|
// of a refactor.
|
|
func TestErrorMessagesVerbatim(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
checkErrMsgCases(t, []errMsgCase{
|
|
{
|
|
name: "check: no manifest found",
|
|
err: fmt.Errorf("%w in %s (looked for index.mf and .index.mf)",
|
|
errNoManifestFound, "/tmp/x"),
|
|
want: "no manifest found in /tmp/x " +
|
|
"(looked for index.mf and .index.mf)",
|
|
},
|
|
{
|
|
name: "check: invalid fingerprint length",
|
|
err: fmt.Errorf("%w, got %d", errInvalidFingerprint, 8),
|
|
want: "invalid fingerprint: must be exactly 40 hex characters, got 8",
|
|
},
|
|
{
|
|
name: "check: manifest not signed",
|
|
err: fmt.Errorf("%w, but signature from %s is required",
|
|
errManifestNotSigned, msgFpA),
|
|
want: "manifest is not signed, but signature from " + msgFpA +
|
|
" is required",
|
|
},
|
|
{
|
|
name: "check: signer mismatch",
|
|
err: fmt.Errorf("embedded signing key fingerprint %s %w %s",
|
|
msgFpA, errSignerMismatch, msgFpB),
|
|
want: "embedded signing key fingerprint " + msgFpA +
|
|
" does not match required " + msgFpB,
|
|
},
|
|
{
|
|
name: "gen: path does not exist",
|
|
err: fmt.Errorf("%w: %s", errPathNotExist, "nope"),
|
|
want: "path does not exist: nope",
|
|
},
|
|
{
|
|
name: "gen: output file exists",
|
|
err: fmt.Errorf("output file %s %w", "index.mf", errOutputExists),
|
|
want: "output file index.mf already exists " +
|
|
"(use --force to overwrite)",
|
|
},
|
|
{
|
|
name: "mfer: unknown command",
|
|
err: fmt.Errorf("%w %q", errUnknownCommand, "bogus"),
|
|
want: `unknown command "bogus"`,
|
|
},
|
|
})
|
|
}
|
|
|
|
// TestFetchErrorMessagesVerbatim pins the fetch and manifest-loader
|
|
// messages; see TestErrorMessagesVerbatim for why.
|
|
func TestFetchErrorMessagesVerbatim(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
checkErrMsgCases(t, []errMsgCase{
|
|
{
|
|
name: "manifest_loader: http status",
|
|
err: fmt.Errorf("failed to fetch %s: %w %d",
|
|
"https://example.com/index.mf", errHTTPStatus, 404),
|
|
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
|
|
// messages above remain matchable with errors.Is, which is the reason
|
|
// the sentinels exist at all.
|
|
func TestSentinelsAreMatchable(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
wrapped := fmt.Errorf("embedded signing key fingerprint %s %w %s",
|
|
"a", errSignerMismatch, "b")
|
|
require.ErrorIs(t, wrapped, errSignerMismatch)
|
|
|
|
wrapped = fmt.Errorf("output file %s %w", "index.mf", errOutputExists)
|
|
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)
|
|
}
|