From 16426ddac099aff66a014dd20a1f205bb2c24a92 Mon Sep 17 00:00:00 2001 From: lusoris Date: Fri, 23 Oct 2026 07:10:01 +0200 Subject: [PATCH] fix(registry): restore the runner bytes the execution manifests pin The gosec change rewrote runner.go and store.go of the P1-SRC-03 acquisition runner and contract.go, plan.go, runner.go and store.go of the v3 runner. Both runners are implementation_frozen, and their embedded execution-manifest.json files pin the sha256 and size of these files, so the change broke the execution binding that verify_execution_manifest.py and TestExecutionManifestBindsExactRepositoryBytes check. The test did not show it because it stopped earlier on registry schemas that this repository does not hold. Both module directories are back to the bytes before the gosec change, together with the helper and test files that belonged to the rewrite. Every repository file the two manifests pin matches its digest again (7 of 7 in each). Their gosec findings (33 and 35) are handled by dated per-file exceptions, as for the P1-SRC-02 runner, until the runners are frozen again with fixes. Signed-off-by: lusoris --- .../p1-src03-acquisition-runner/checked_io.go | 30 -- .../checked_io_test.go | 77 ---- .../http_error_test.go | 71 --- .../p1-src03-acquisition-runner/runner.go | 93 ++-- .../p1-src03-acquisition-runner/store.go | 414 ++++++++---------- .../checked_io.go | 30 -- .../checked_io_test.go | 77 ---- .../source-acquisition-runner-v3/contract.go | 13 +- .../http_error_test.go | 71 --- .../source-acquisition-runner-v3/plan.go | 11 +- .../source-acquisition-runner-v3/runner.go | 93 ++-- .../source-acquisition-runner-v3/store.go | 414 ++++++++---------- 12 files changed, 414 insertions(+), 980 deletions(-) delete mode 100644 tools/knowledge-registry/p1-src03-acquisition-runner/checked_io.go delete mode 100644 tools/knowledge-registry/p1-src03-acquisition-runner/checked_io_test.go delete mode 100644 tools/knowledge-registry/p1-src03-acquisition-runner/http_error_test.go delete mode 100644 tools/knowledge-registry/source-acquisition-runner-v3/checked_io.go delete mode 100644 tools/knowledge-registry/source-acquisition-runner-v3/checked_io_test.go delete mode 100644 tools/knowledge-registry/source-acquisition-runner-v3/http_error_test.go diff --git a/tools/knowledge-registry/p1-src03-acquisition-runner/checked_io.go b/tools/knowledge-registry/p1-src03-acquisition-runner/checked_io.go deleted file mode 100644 index b977bb1..0000000 --- a/tools/knowledge-registry/p1-src03-acquisition-runner/checked_io.go +++ /dev/null @@ -1,30 +0,0 @@ -package main - -import ( - "errors" - "fmt" - "io" - "math" - "syscall" -) - -func checkedUID(value int) (uint32, error) { - if value < 0 || uint64(value) > math.MaxUint32 { - return 0, fmt.Errorf("effective user id %d is outside uint32 range", value) - } - return uint32(value), nil -} - -func closeWithError(primary error, closer io.Closer) error { - if err := closer.Close(); err != nil { - return errors.Join(primary, fmt.Errorf("close resource: %w", err)) - } - return primary -} - -func closeDescriptorWithError(primary error, descriptor int) error { - if err := syscall.Close(descriptor); err != nil { - return errors.Join(primary, fmt.Errorf("close descriptor: %w", err)) - } - return primary -} diff --git a/tools/knowledge-registry/p1-src03-acquisition-runner/checked_io_test.go b/tools/knowledge-registry/p1-src03-acquisition-runner/checked_io_test.go deleted file mode 100644 index c4eda17..0000000 --- a/tools/knowledge-registry/p1-src03-acquisition-runner/checked_io_test.go +++ /dev/null @@ -1,77 +0,0 @@ -package main - -import ( - "errors" - "math" - "strconv" - "testing" -) - -type failingCloser struct { - err error - calls int -} - -func (c *failingCloser) Close() error { c.calls++; return c.err } - -func TestCheckedUID(t *testing.T) { - cases := []struct { - name string - value int - want uint32 - bad bool - }{ - {"negative", -1, 0, true}, {"zero", 0, 0, false}, {"positive", 42, 42, false}, - } - if strconv.IntSize == 64 { - cases = append(cases, - struct { - name string - value int - want uint32 - bad bool - }{"maximum", math.MaxUint32, math.MaxUint32, false}, - struct { - name string - value int - want uint32 - bad bool - }{"overflow", math.MaxUint32 + 1, 0, true}) - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - got, err := checkedUID(tc.value) - if (err != nil) != tc.bad || got != tc.want { - t.Fatalf("checkedUID(%d) = %d, %v", tc.value, got, err) - } - }) - } -} - -func TestCloseWithError(t *testing.T) { - primary := errors.New("operation") - cleanup := errors.New("cleanup") - cases := []struct { - name string - primary, cleanup error - }{ - {"success", nil, nil}, {"primary", primary, nil}, {"cleanup", nil, cleanup}, {"both", primary, cleanup}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - closer := &failingCloser{err: tc.cleanup} - got := closeWithError(tc.primary, closer) - if closer.calls != 1 { - t.Fatalf("close calls = %d", closer.calls) - } - if (got == nil) != (tc.primary == nil && tc.cleanup == nil) { - t.Fatalf("error = %v", got) - } - for _, want := range []error{tc.primary, tc.cleanup} { - if want != nil && !errors.Is(got, want) { - t.Fatalf("lost error %v: %v", want, got) - } - } - }) - } -} diff --git a/tools/knowledge-registry/p1-src03-acquisition-runner/http_error_test.go b/tools/knowledge-registry/p1-src03-acquisition-runner/http_error_test.go deleted file mode 100644 index b89e426..0000000 --- a/tools/knowledge-registry/p1-src03-acquisition-runner/http_error_test.go +++ /dev/null @@ -1,71 +0,0 @@ -package main - -import ( - "context" - "errors" - "io" - "net/http" - "net/url" - "strings" - "testing" -) - -type failedResponseBody struct{ err error } - -func (b failedResponseBody) Read([]byte) (int, error) { return 0, b.err } -func (b failedResponseBody) Close() error { return nil } - -func TestGitObjectStatusError(t *testing.T) { - failedRead := errors.New("response read failed") - cases := []struct { - name string - status int - body io.ReadCloser - want error - }{ - {"success", http.StatusOK, io.NopCloser(strings.NewReader("{}")), nil}, - {"http failure", http.StatusForbidden, io.NopCloser(strings.NewReader("denied")), nil}, - {"drain failure", http.StatusForbidden, failedResponseBody{failedRead}, failedRead}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - got := gitObjectStatusError(&http.Response{StatusCode: tc.status, Body: tc.body}) - if (got == nil) != (tc.status == http.StatusOK) { - t.Fatalf("status error = %v", got) - } - if tc.want != nil && !errors.Is(got, tc.want) { - t.Fatalf("lost drain error: %v", got) - } - }) - } -} - -type responseTransport func(*http.Request) (*http.Response, error) - -func (f responseTransport) RoundTrip(req *http.Request) (*http.Response, error) { return f(req) } - -type closeErrorBody struct { - io.Reader - err error -} - -func (b closeErrorBody) Close() error { return b.err } - -func TestGetJSONPreservesResponseCloseError(t *testing.T) { - cleanup := errors.New("response close failed") - for _, closeErr := range []error{nil, cleanup} { - origin, err := url.Parse("https://api.github.com") - if err != nil { - t.Fatal(err) - } - client := &http.Client{Transport: responseTransport(func(req *http.Request) (*http.Response, error) { - return &http.Response{StatusCode: http.StatusOK, Header: http.Header{"Content-Type": []string{"application/json"}}, Request: req, Body: closeErrorBody{strings.NewReader("{}"), closeErr}}, nil - })} - r := &runner{client: client, baseURL: origin, allowedRepos: map[string]struct{}{"argoproj/argo-helm": {}}} - err = r.getJSON(context.Background(), "argoproj/argo-helm", "/git/test", &map[string]any{}) - - if !errors.Is(err, closeErr) { - t.Fatalf("response close error = %v, want %v", err, closeErr) - } - } -} diff --git a/tools/knowledge-registry/p1-src03-acquisition-runner/runner.go b/tools/knowledge-registry/p1-src03-acquisition-runner/runner.go index ce52067..1070cc9 100644 --- a/tools/knowledge-registry/p1-src03-acquisition-runner/runner.go +++ b/tools/knowledge-registry/p1-src03-acquisition-runner/runner.go @@ -3,7 +3,7 @@ package main import ( "bytes" "context" - "crypto/sha1" // #nosec G505 -- Git blob object identifiers require SHA-1; content integrity is also bound by SHA-256. + "crypto/sha1" "crypto/sha256" "encoding/base64" "encoding/hex" @@ -660,16 +660,7 @@ func (r *runner) getBlob(ctx context.Context, repository string, expected contra if blob.SHA != expected.GitBlobSHA1 || blob.Size == nil || *blob.Size != expected.Bytes || blob.Encoding != "base64" { return nil, errors.New("blob response identity, encoding, or size drifted") } - raw, err := decodeGitBlob(blob.Content, expected) - if err != nil { - return nil, err - } - r.blobCache[key] = raw - return raw, nil -} - -func decodeGitBlob(content string, expected contractFile) ([]byte, error) { - compact := strings.NewReplacer("\n", "", "\r", "").Replace(content) + compact := strings.NewReplacer("\n", "", "\r", "").Replace(blob.Content) raw, err := base64.StdEncoding.Strict().DecodeString(compact) if err != nil { return nil, errors.New("blob content is not strict base64") @@ -678,7 +669,7 @@ func decodeGitBlob(content string, expected contractFile) ([]byte, error) { return nil, errors.New("decoded blob size drifted") } identity := append([]byte("blob "+strconv.Itoa(len(raw))+"\x00"), raw...) - gitDigest := sha1.Sum(identity) // #nosec G401 -- Reconstruct the protocol-defined Git blob SHA-1 identity; SHA-256 separately binds the artifact bytes. + gitDigest := sha1.Sum(identity) if hex.EncodeToString(gitDigest[:]) != expected.GitBlobSHA1 { return nil, errors.New("decoded blob does not reproduce the exact Git SHA-1") } @@ -686,10 +677,11 @@ func decodeGitBlob(content string, expected contractFile) ([]byte, error) { if "sha256:"+hex.EncodeToString(rawDigest[:]) != expected.RawSHA256 { return nil, errors.New("decoded blob raw SHA-256 drifted") } + r.blobCache[key] = raw return raw, nil } -func (r *runner) getJSON(ctx context.Context, repository, suffix string, destination any) (err error) { +func (r *runner) getJSON(ctx context.Context, repository, suffix string, destination any) error { if _, ok := r.allowedRepos[repository]; !ok { return errors.New("repository is outside the exact three-source denominator") } @@ -697,7 +689,7 @@ func (r *runner) getJSON(ctx context.Context, repository, suffix string, destina return errors.New("HTTP request is outside the exact capability") } endpoint := r.baseURL.String() + "/repos/" + repository + suffix - request, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) // #nosec G704 -- newRunner pins the production API origin to api.github.com and forbids redirects before sending; alternate origins require explicit test mode. + request, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) if err != nil { return err } @@ -709,17 +701,32 @@ func (r *runner) getJSON(ctx context.Context, repository, suffix string, destina request.Header.Set("Authorization", "Bearer "+r.token) } r.requests++ - response, err := r.client.Do(request) // #nosec G704 -- newRunner pins the production API origin to api.github.com and forbids redirects before sending; alternate origins require explicit test mode. + response, err := r.client.Do(request) if err != nil { return err } - defer func() { err = closeWithError(err, response.Body) }() - return r.readGitObjectResponse(response, endpoint, destination) -} - -func (r *runner) readGitObjectResponse(response *http.Response, endpoint string, destination any) error { - if err := validateGitObjectResponse(response, endpoint); err != nil { - return err + defer response.Body.Close() + if response.StatusCode != http.StatusOK { + _, _ = io.Copy(io.Discard, io.LimitReader(response.Body, 4096)) + return fmt.Errorf("GitHub REST returned HTTP %d", response.StatusCode) + } + if response.Request == nil || response.Request.URL.String() != endpoint { + return errors.New("Git-object response arrived through a redirected or changed endpoint") + } + if len(response.Header.Values("Link")) != 0 { + return errors.New("Git-object endpoint returned a pagination Link header") + } + encodings := response.Header.Values("Content-Encoding") + if len(encodings) > 1 || (len(encodings) == 1 && encodings[0] != "identity") { + return errors.New("Git-object response used an unexpected content encoding") + } + contentTypes := response.Header.Values("Content-Type") + if len(contentTypes) != 1 { + return errors.New("Git-object response has an ambiguous or missing content type") + } + contentType, _, err := mime.ParseMediaType(contentTypes[0]) + if err != nil || contentType != "application/json" { + return errors.New("Git-object response is not application/json") } remaining := maxResponseBytes - r.responseBytes if remaining <= 0 { @@ -746,48 +753,6 @@ func (r *runner) readGitObjectResponse(response *http.Response, endpoint string, return nil } -func validateGitObjectResponse(response *http.Response, endpoint string) error { - if err := gitObjectStatusError(response); err != nil { - return err - } - - if response.Request == nil || response.Request.URL.String() != endpoint { - return errors.New("Git-object response arrived through a redirected or changed endpoint") - } - if len(response.Header.Values("Link")) != 0 { - return errors.New("Git-object endpoint returned a pagination Link header") - } - encodings := response.Header.Values("Content-Encoding") - if len(encodings) > 1 || (len(encodings) == 1 && encodings[0] != "identity") { - return errors.New("Git-object response used an unexpected content encoding") - } - return validateGitObjectContentType(response) -} - -func validateGitObjectContentType(response *http.Response) error { - contentTypes := response.Header.Values("Content-Type") - if len(contentTypes) != 1 { - return errors.New("Git-object response has an ambiguous or missing content type") - } - contentType, _, err := mime.ParseMediaType(contentTypes[0]) - if err != nil || contentType != "application/json" { - return errors.New("Git-object response is not application/json") - } - - return nil -} - -func gitObjectStatusError(response *http.Response) error { - if response.StatusCode != http.StatusOK { - _, drainErr := io.Copy(io.Discard, io.LimitReader(response.Body, 4096)) - if drainErr != nil { - drainErr = fmt.Errorf("drain Git-object error response: %w", drainErr) - } - return errors.Join(fmt.Errorf("GitHub REST returned HTTP %d", response.StatusCode), drainErr) - } - return nil -} - func exactTreeEntry(entries []gitTreeEntry, name string) (gitTreeEntry, error) { seen := make(map[string]struct{}, len(entries)) var match *gitTreeEntry diff --git a/tools/knowledge-registry/p1-src03-acquisition-runner/store.go b/tools/knowledge-registry/p1-src03-acquisition-runner/store.go index b559488..7223c7d 100644 --- a/tools/knowledge-registry/p1-src03-acquisition-runner/store.go +++ b/tools/knowledge-registry/p1-src03-acquisition-runner/store.go @@ -9,7 +9,6 @@ import ( "io" "os" "path/filepath" - "slices" "sort" "strings" "syscall" @@ -33,89 +32,70 @@ func newCASStore(root string) (*casStore, error) { if absolute == string(os.PathSeparator) { return nil, errors.New("output root must not be the filesystem root") } - rootFile, err := openCASRoot(absolute) + parent, err := openDirectoryPathNoFollow(filepath.Dir(absolute)) if err != nil { - return nil, err + return nil, fmt.Errorf("output parent: %w", err) + } + defer parent.Close() + leaf := filepath.Base(absolute) + rootFile, err := openDirectoryAt(parent, leaf) + if errors.Is(err, os.ErrNotExist) { + if err := syscall.Mkdirat(int(parent.Fd()), leaf, 0o700); err != nil && !errors.Is(err, syscall.EEXIST) { + return nil, fmt.Errorf("create output root: %w", err) + } + if err := syncOpenedFile(parent); err != nil { + return nil, fmt.Errorf("persist output root: %w", err) + } + rootFile, err = openDirectoryAt(parent, leaf) + } + if err != nil { + return nil, fmt.Errorf("inspect output root: %w", err) } info, err := rootFile.Stat() if err != nil { - return nil, closeWithError(fmt.Errorf("inspect output root: %w", err), rootFile) + rootFile.Close() + return nil, fmt.Errorf("inspect output root: %w", err) } if err := validateOwnedDirectory(info, "output root"); err != nil { - return nil, closeWithError(err, rootFile) + rootFile.Close() + return nil, err } store := &casStore{root: absolute, rootFile: rootFile, rootInfo: info} if err := store.verifyRootPath(); err != nil { - return nil, closeWithError(err, rootFile) + rootFile.Close() + return nil, err } - if err := store.initializeDirectories(); err != nil { - return nil, closeWithError(err, rootFile) + receipts, _, err := store.listOptionalPublishedDirectory("receipts", 2) + if err != nil { + rootFile.Close() + return nil, fmt.Errorf("preflight receipts: %w", err) } - return store, nil -} - -func (s *casStore) initializeDirectories() error { - recoveryState, err := s.hasRecoveryState() + proposals, _, err := store.listOptionalPublishedDirectory("inventory-proposals", 2) if err != nil { - return err + rootFile.Close() + return nil, fmt.Errorf("preflight inventory proposals: %w", err) + } + if len(receipts) == 0 && len(proposals) != 0 { + rootFile.Close() + return nil, errors.New("inventory proposal exists without its predecessor receipt") } + recoveryState := len(receipts) != 0 || len(proposals) != 0 for _, relative := range []string{"cas", "cas/sha256", "receipts", "inventory-proposals"} { if recoveryState { - directory, _, openErr := s.openStoreDirectory(relative, false) + directory, _, openErr := store.openStoreDirectory(relative, false) if openErr == nil { - if err := closeWithError(nil, directory); err != nil { - return err - } + directory.Close() continue } - return fmt.Errorf("recovery state is missing required directory %s: %w", relative, openErr) + rootFile.Close() + return nil, fmt.Errorf("recovery state is missing required directory %s: %w", relative, openErr) } - if _, err := s.ensureDirectory(relative); err != nil { - return err + if _, err := store.ensureDirectory(relative); err != nil { + rootFile.Close() + return nil, err } } - return nil -} - -func (s *casStore) hasRecoveryState() (bool, error) { - receipts, _, err := s.listOptionalPublishedDirectory("receipts", 2) - if err != nil { - return false, fmt.Errorf("preflight receipts: %w", err) - } - proposals, _, err := s.listOptionalPublishedDirectory("inventory-proposals", 2) - if err != nil { - return false, fmt.Errorf("preflight inventory proposals: %w", err) - } - if len(receipts) == 0 && len(proposals) != 0 { - return false, errors.New("inventory proposal exists without its predecessor receipt") - } - return len(receipts) != 0 || len(proposals) != 0, nil -} - -func openCASRoot(absolute string) (file *os.File, err error) { - parent, err := openDirectoryPathNoFollow(filepath.Dir(absolute)) - if err != nil { - return nil, fmt.Errorf("output parent: %w", err) - } - defer func() { - err = closeWithError(err, parent) - if err != nil && file != nil { - err = closeWithError(err, file) - file = nil - } - }() - leaf := filepath.Base(absolute) - file, err = openDirectoryAt(parent, leaf) - if !errors.Is(err, os.ErrNotExist) { - return file, err - } - if err := syscall.Mkdirat(int(parent.Fd()), leaf, 0o700); err != nil && !errors.Is(err, syscall.EEXIST) { - return nil, fmt.Errorf("create output root: %w", err) - } - if err := syncOpenedFile(parent); err != nil { - return nil, fmt.Errorf("persist output root: %w", err) - } - return openDirectoryAt(parent, leaf) + return store, nil } func (s *casStore) listOptionalPublishedDirectory(relative string, maximum int) ([]string, bool, error) { @@ -182,12 +162,12 @@ func (s *casStore) putProposal(proposalID string, raw []byte) (string, error) { return relative, nil } -func (s *casStore) ensureDirectory(relative string) (result string, err error) { +func (s *casStore) ensureDirectory(relative string) (string, error) { directory, _, err := s.openStoreDirectory(relative, true) if err != nil { return "", err } - defer func() { err = closeWithError(err, directory) }() + defer directory.Close() return filepath.Join(s.root, filepath.FromSlash(relative)), nil } @@ -229,7 +209,7 @@ func (s *casStore) verifyRunSet(receiptPath string, receiptRaw []byte, proposalP if err != nil { return err } - if !slices.Equal(receipts, []string{filepath.Base(receiptPath)}) || !slices.Equal(proposals, []string{filepath.Base(proposalPath)}) { + if len(receipts) != 1 || receipts[0] != filepath.Base(receiptPath) || len(proposals) != 1 || proposals[0] != filepath.Base(proposalPath) { return errors.New("published receipt/proposal set is incomplete or ambiguous") } actualReceipt, err := s.readPublished(receiptPath, maxPublishedJSONBytes) @@ -243,7 +223,7 @@ func (s *casStore) verifyRunSet(receiptPath string, receiptRaw []byte, proposalP return nil } -func (s *casStore) atomicNoReplace(relative string, raw []byte, maximum int64) (err error) { +func (s *casStore) atomicNoReplace(relative string, raw []byte, maximum int64) error { if int64(len(raw)) > maximum { return errors.New("published object exceeds its byte bound") } @@ -256,7 +236,7 @@ func (s *casStore) atomicNoReplace(relative string, raw []byte, maximum int64) ( if err != nil { return err } - defer func() { err = closeWithError(err, parent) }() + defer parent.Close() leaf := filepath.Base(relative) descriptor, err := syscall.Openat(int(parent.Fd()), leaf, syscall.O_WRONLY|syscall.O_CREAT|syscall.O_EXCL|syscall.O_CLOEXEC|syscall.O_NOFOLLOW, 0o600) if err != nil { @@ -267,50 +247,39 @@ func (s *casStore) atomicNoReplace(relative string, raw []byte, maximum int64) ( } file := os.NewFile(uintptr(descriptor), leaf) if file == nil { - return closeDescriptorWithError(errors.New("could not own exclusive published object descriptor"), descriptor) + syscall.Close(descriptor) + return errors.New("could not own exclusive published object descriptor") } - written, err := writePublishedContents(file, raw) - if err != nil { - return closeWithError(err, file) - } - return closeWithError(s.commitPublishedContents(parentRelative, parent, parentInfo, leaf, raw, maximum, written), file) -} - -func (s *casStore) commitPublishedContents(parentRelative string, parent *os.File, parentInfo os.FileInfo, leaf string, raw []byte, maximum int64, written os.FileInfo) error { - if err := s.verifyPublishedAtPath(parentRelative, parentInfo, leaf, raw, maximum, written, false, "exclusive published object changed before directory commit"); err != nil { - return err - } - if err := syncOpenedFile(parent); err != nil { - return err - } - if err := s.verifyPublishedAtPath(parentRelative, parentInfo, leaf, raw, maximum, written, false, "exclusive published object changed after directory commit"); err != nil { - return err - } - if err := s.verifyRootPath(); err != nil { - return err - } - return nil -} - -func writePublishedContents(file *os.File, raw []byte) (os.FileInfo, error) { created, err := file.Stat() if err != nil { - return nil, err + return errors.Join(err, file.Close()) } if err := validateOwnedRegular(created, "new published object"); err != nil { - return nil, err + return errors.Join(err, file.Close()) } if err := writeAll(file, raw); err != nil { - return nil, err + return errors.Join(err, file.Close()) } if err := syncOpenedFile(file); err != nil { - return nil, err + return errors.Join(err, file.Close()) } written, err := file.Stat() if err != nil || !os.SameFile(created, written) || written.Size() != int64(len(raw)) { - return nil, errors.Join(errors.New("exclusive published object write is incomplete or unstable"), err) + return errors.Join(errors.New("exclusive published object write is incomplete or unstable"), err, file.Close()) + } + if err := s.verifyPublishedAtPath(parentRelative, parentInfo, leaf, raw, maximum, written, false, "exclusive published object changed before directory commit"); err != nil { + return errors.Join(err, file.Close()) + } + if err := syncOpenedFile(parent); err != nil { + return errors.Join(err, file.Close()) + } + if err := s.verifyPublishedAtPath(parentRelative, parentInfo, leaf, raw, maximum, written, false, "exclusive published object changed after directory commit"); err != nil { + return errors.Join(err, file.Close()) + } + if err := s.verifyRootPath(); err != nil { + return errors.Join(err, file.Close()) } - return written, nil + return file.Close() } func (s *casStore) rehabilitateExisting(parentRelative string, parent *os.File, parentInfo os.FileInfo, leaf string, raw []byte, maximum int64) error { @@ -330,12 +299,24 @@ func (s *casStore) rehabilitateExisting(parentRelative string, parent *os.File, return s.verifyRootPath() } -func verifyPublishedInDirectory(parent *os.File, leaf string, raw []byte, maximum int64, required os.FileInfo, synchronize bool, conflict string) (info os.FileInfo, err error) { - file, err := openPublishedFile(parent, leaf, synchronize) +func verifyPublishedInDirectory(parent *os.File, leaf string, raw []byte, maximum int64, required os.FileInfo, synchronize bool, conflict string) (os.FileInfo, error) { + access := syscall.O_RDONLY + if synchronize { + access = syscall.O_RDWR + } + descriptor, err := syscall.Openat(int(parent.Fd()), leaf, access|syscall.O_CLOEXEC|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0) if err != nil { + if errors.Is(err, syscall.ELOOP) { + return nil, errors.New("published object path exists and is not a regular file") + } return nil, err } - defer func() { err = closeWithError(err, file) }() + file := os.NewFile(uintptr(descriptor), leaf) + if file == nil { + syscall.Close(descriptor) + return nil, errors.New("could not own published object file descriptor") + } + defer file.Close() before, err := file.Stat() if err != nil { return nil, err @@ -349,73 +330,42 @@ func verifyPublishedInDirectory(parent *os.File, leaf string, raw []byte, maximu if before.Size() > maximum { return nil, errors.New("published object exceeds its byte bound") } - existing, after, err := readStablePublished(file, before, maximum) - if err != nil { - return nil, err - } - if !bytes.Equal(existing, raw) { - return nil, errors.New(conflict) - } - if synchronize { - return synchronizePublished(file, after) - } - return after, nil -} - -func openPublishedFile(parent *os.File, leaf string, synchronize bool) (*os.File, error) { - access := syscall.O_RDONLY - if synchronize { - access = syscall.O_RDWR - } - descriptor, err := syscall.Openat(int(parent.Fd()), leaf, access|syscall.O_CLOEXEC|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0) - if err != nil { - if errors.Is(err, syscall.ELOOP) { - return nil, errors.New("published object path exists and is not a regular file") - } - return nil, err - } - file := os.NewFile(uintptr(descriptor), leaf) - if file == nil { - return nil, closeDescriptorWithError(errors.New("could not own published object file descriptor"), descriptor) - } - return file, nil -} - -func readStablePublished(file *os.File, before os.FileInfo, maximum int64) ([]byte, os.FileInfo, error) { existing, err := io.ReadAll(io.LimitReader(file, maximum+1)) if err != nil { - return nil, nil, err + return nil, err } if int64(len(existing)) > maximum { - return nil, nil, errors.New("published object exceeds its byte bound") + return nil, errors.New("published object exceeds its byte bound") } after, err := file.Stat() if err != nil { - return nil, nil, err + return nil, err } if !os.SameFile(before, after) || before.Size() != after.Size() || before.ModTime() != after.ModTime() { - return nil, nil, errors.New("published object changed while it was read") + return nil, errors.New("published object changed while it was read") } - return existing, after, nil -} - -func synchronizePublished(file *os.File, after os.FileInfo) (os.FileInfo, error) { - if err := syncOpenedFile(file); err != nil { - return nil, err + if !bytes.Equal(existing, raw) { + return nil, errors.New(conflict) } - afterSync, err := file.Stat() - if err != nil || !os.SameFile(after, afterSync) || after.Size() != afterSync.Size() || after.ModTime() != afterSync.ModTime() { - return nil, errors.Join(errors.New("published object changed while it was synchronized"), err) + if synchronize { + if err := syncOpenedFile(file); err != nil { + return nil, err + } + afterSync, err := file.Stat() + if err != nil || !os.SameFile(after, afterSync) || after.Size() != afterSync.Size() || after.ModTime() != afterSync.ModTime() { + return nil, errors.Join(errors.New("published object changed while it was synchronized"), err) + } + after = afterSync } - return afterSync, nil + return after, nil } -func (s *casStore) verifyPublishedAtPath(parentRelative string, requiredParent os.FileInfo, leaf string, raw []byte, maximum int64, requiredFile os.FileInfo, synchronize bool, conflict string) (err error) { +func (s *casStore) verifyPublishedAtPath(parentRelative string, requiredParent os.FileInfo, leaf string, raw []byte, maximum int64, requiredFile os.FileInfo, synchronize bool, conflict string) error { parent, parentInfo, err := s.openStoreDirectory(parentRelative, false) if err != nil { return err } - defer func() { err = closeWithError(err, parent) }() + defer parent.Close() if !os.SameFile(requiredParent, parentInfo) { return errors.New("published object parent changed during commit") } @@ -423,7 +373,7 @@ func (s *casStore) verifyPublishedAtPath(parentRelative string, requiredParent o return err } -func (s *casStore) readPublished(relative string, maximum int64) (result []byte, err error) { +func (s *casStore) readPublished(relative string, maximum int64) ([]byte, error) { clean := filepath.Clean(relative) if clean != relative || filepath.IsAbs(relative) || strings.HasPrefix(clean, "..") { return nil, errors.New("published object path is not canonical") @@ -433,28 +383,18 @@ func (s *casStore) readPublished(relative string, maximum int64) (result []byte, if err != nil { return nil, err } - defer func() { err = closeWithError(err, parent) }() + defer parent.Close() leaf := filepath.Base(relative) - file, err := openPublishedFile(parent, leaf, false) + descriptor, err := syscall.Openat(int(parent.Fd()), leaf, syscall.O_RDONLY|syscall.O_CLOEXEC|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0) if err != nil { return nil, err } - defer func() { err = closeWithError(err, file) }() - before, err := inspectPublished(file, maximum) - if err != nil { - return nil, err - } - raw, after, err := readStablePublished(file, before, maximum) - if err != nil { - return nil, err - } - if err := s.verifyPublishedAtPath(parentRelative, parentInfo, leaf, raw, maximum, after, false, "published object changed during path replay"); err != nil { - return nil, err + file := os.NewFile(uintptr(descriptor), leaf) + if file == nil { + syscall.Close(descriptor) + return nil, errors.New("could not own published object descriptor") } - return raw, nil -} - -func inspectPublished(file *os.File, maximum int64) (os.FileInfo, error) { + defer file.Close() before, err := file.Stat() if err != nil { return nil, err @@ -465,10 +405,21 @@ func inspectPublished(file *os.File, maximum int64) (os.FileInfo, error) { if before.Size() > maximum { return nil, errors.New("published object exceeds its byte bound") } - return before, nil + raw, err := io.ReadAll(io.LimitReader(file, maximum+1)) + if err != nil || int64(len(raw)) > maximum { + return nil, errors.Join(errors.New("published object read exceeds its bound"), err) + } + after, err := file.Stat() + if err != nil || !os.SameFile(before, after) || before.Size() != after.Size() || before.ModTime() != after.ModTime() { + return nil, errors.Join(errors.New("published object changed while it was read"), err) + } + if err := s.verifyPublishedAtPath(parentRelative, parentInfo, leaf, raw, maximum, after, false, "published object changed during path replay"); err != nil { + return nil, err + } + return raw, nil } -func (s *casStore) listPublishedDirectory(relative string, maximum int) (result []string, err error) { +func (s *casStore) listPublishedDirectory(relative string, maximum int) ([]string, error) { if maximum < 1 { return nil, errors.New("directory entry bound must be positive") } @@ -476,7 +427,7 @@ func (s *casStore) listPublishedDirectory(relative string, maximum int) (result if err != nil { return nil, err } - defer func() { err = closeWithError(err, directory) }() + defer directory.Close() entries, err := directory.ReadDir(maximum + 1) if err != nil && !errors.Is(err, io.EOF) { return nil, err @@ -484,17 +435,19 @@ func (s *casStore) listPublishedDirectory(relative string, maximum int) (result if len(entries) > maximum { return nil, errors.New("published directory exceeds its entry bound") } - names, err := publishedEntryNames(entries) - if err != nil { - return nil, err + names := make([]string, 0, len(entries)) + for _, entry := range entries { + name := entry.Name() + if name == "" || name == "." || name == ".." || strings.ContainsAny(name, "/\x00") { + return nil, errors.New("published directory contains an unsafe entry name") + } + names = append(names, name) } replayed, replayedInfo, err := s.openStoreDirectory(relative, false) if err != nil { return nil, err } - if err := closeWithError(nil, replayed); err != nil { - return nil, err - } + replayed.Close() if !os.SameFile(directoryInfo, replayedInfo) { return nil, errors.New("published directory changed while it was listed") } @@ -502,18 +455,6 @@ func (s *casStore) listPublishedDirectory(relative string, maximum int) (result return names, nil } -func publishedEntryNames(entries []os.DirEntry) ([]string, error) { - names := make([]string, 0, len(entries)) - for _, entry := range entries { - name := entry.Name() - if name == "" || name == "." || name == ".." || strings.ContainsAny(name, "/\x00") { - return nil, errors.New("published directory contains an unsafe entry name") - } - names = append(names, name) - } - return names, nil -} - func writeAll(file *os.File, raw []byte) error { for len(raw) > 0 { written, err := file.Write(raw) @@ -563,11 +504,7 @@ func validateEffectiveOwner(info os.FileInfo, label string) error { if !ok { return fmt.Errorf("%s ownership cannot be verified", label) } - uid, err := checkedUID(os.Geteuid()) - if err != nil { - return fmt.Errorf("%s ownership: %w", label, err) - } - if stat.Uid != uid { + if stat.Uid != uint32(os.Geteuid()) { return fmt.Errorf("%s is not owned by the effective user", label) } return nil @@ -584,22 +521,22 @@ func openDirectoryPathNoFollow(pathname string) (*os.File, error) { } current := os.NewFile(uintptr(descriptor), string(os.PathSeparator)) if current == nil { - return nil, closeDescriptorWithError(errors.New("could not own filesystem-root descriptor"), descriptor) + syscall.Close(descriptor) + return nil, errors.New("could not own filesystem-root descriptor") } trimmed := strings.TrimPrefix(absolute, string(os.PathSeparator)) if trimmed == "" { return current, nil } for _, component := range strings.Split(trimmed, string(os.PathSeparator)) { - if !canonicalDirectoryComponent(component) { - return nil, closeWithError(errors.New("directory path is not canonical"), current) + if component == "" || component == "." || component == ".." { + current.Close() + return nil, errors.New("directory path is not canonical") } next, err := openDirectoryAt(current, component) - if closeErr := closeWithError(err, current); closeErr != nil { - if next != nil { - closeErr = closeWithError(closeErr, next) - } - return nil, closeErr + current.Close() + if err != nil { + return nil, err } current = next } @@ -613,7 +550,8 @@ func openDirectoryAt(parent *os.File, name string) (*os.File, error) { } file := os.NewFile(uintptr(descriptor), name) if file == nil { - return nil, closeDescriptorWithError(errors.New("could not own directory descriptor"), descriptor) + syscall.Close(descriptor) + return nil, errors.New("could not own directory descriptor") } return file, nil } @@ -626,7 +564,8 @@ func duplicateFile(file *os.File, name string) (*os.File, error) { syscall.CloseOnExec(descriptor) duplicate := os.NewFile(uintptr(descriptor), name) if duplicate == nil { - return nil, closeDescriptorWithError(errors.New("could not own duplicated directory descriptor"), descriptor) + syscall.Close(descriptor) + return nil, errors.New("could not own duplicated directory descriptor") } return duplicate, nil } @@ -640,7 +579,7 @@ func validateStoreRelativeDirectory(relative string) ([]string, error) { } components := strings.Split(filepath.ToSlash(relative), "/") for _, component := range components { - if !canonicalDirectoryComponent(component) { + if component == "" || component == "." || component == ".." { return nil, errors.New("unsafe store directory component") } } @@ -657,43 +596,41 @@ func (s *casStore) openDirectoryFromRoot(relative string, create bool) (*os.File return nil, nil, err } for _, component := range components { - next, openErr := openOrCreateDirectoryAt(current, component, create) - if closeErr := closeWithError(openErr, current); closeErr != nil { - if next != nil { - closeErr = closeWithError(closeErr, next) + next, openErr := openDirectoryAt(current, component) + if errors.Is(openErr, os.ErrNotExist) && create { + if mkdirErr := syscall.Mkdirat(int(current.Fd()), component, 0o700); mkdirErr != nil && !errors.Is(mkdirErr, syscall.EEXIST) { + current.Close() + return nil, nil, mkdirErr + } + if syncErr := syncOpenedFile(current); syncErr != nil { + current.Close() + return nil, nil, syncErr } - return nil, nil, closeErr + next, openErr = openDirectoryAt(current, component) + } + current.Close() + if openErr != nil { + return nil, nil, openErr } info, statErr := next.Stat() if statErr != nil { - return nil, nil, closeWithError(statErr, next) + next.Close() + return nil, nil, statErr } if err := validateOwnedDirectory(info, relative); err != nil { - return nil, nil, closeWithError(err, next) + next.Close() + return nil, nil, err } current = next } info, err := current.Stat() if err != nil { - return nil, nil, closeWithError(err, current) + current.Close() + return nil, nil, err } return current, info, nil } -func openOrCreateDirectoryAt(current *os.File, component string, create bool) (*os.File, error) { - next, openErr := openDirectoryAt(current, component) - if errors.Is(openErr, os.ErrNotExist) && create { - if mkdirErr := syscall.Mkdirat(int(current.Fd()), component, 0o700); mkdirErr != nil && !errors.Is(mkdirErr, syscall.EEXIST) { - return nil, mkdirErr - } - if syncErr := syncOpenedFile(current); syncErr != nil { - return nil, syncErr - } - next, openErr = openDirectoryAt(current, component) - } - return next, openErr -} - func (s *casStore) openStoreDirectory(relative string, create bool) (*os.File, os.FileInfo, error) { if err := s.verifyRootPath(); err != nil { return nil, nil, err @@ -704,21 +641,22 @@ func (s *casStore) openStoreDirectory(relative string, create bool) (*os.File, o } replayed, replayedInfo, err := s.openDirectoryFromRoot(relative, false) if err != nil { - return nil, nil, closeWithError(err, held) - } - if err := closeWithError(nil, replayed); err != nil { - return nil, nil, closeWithError(err, held) + held.Close() + return nil, nil, err } + replayed.Close() if !os.SameFile(heldInfo, replayedInfo) { - return nil, nil, closeWithError(errors.New("store directory changed during path replay"), held) + held.Close() + return nil, nil, errors.New("store directory changed during path replay") } if err := s.verifyRootPath(); err != nil { - return nil, nil, closeWithError(err, held) + held.Close() + return nil, nil, err } return held, heldInfo, nil } -func (s *casStore) verifyRootPath() (err error) { +func (s *casStore) verifyRootPath() error { if s.rootFile == nil { return errors.New("store root descriptor is closed") } @@ -726,7 +664,7 @@ func (s *casStore) verifyRootPath() (err error) { if err != nil { return fmt.Errorf("reopen output root: %w", err) } - defer func() { err = closeWithError(err, current) }() + defer current.Close() info, err := current.Stat() if err != nil { return err @@ -758,7 +696,3 @@ func stripPrefixedID(value, prefix string) (string, error) { } return digest, nil } - -func canonicalDirectoryComponent(component string) bool { - return component != "" && component != "." && component != ".." -} diff --git a/tools/knowledge-registry/source-acquisition-runner-v3/checked_io.go b/tools/knowledge-registry/source-acquisition-runner-v3/checked_io.go deleted file mode 100644 index b977bb1..0000000 --- a/tools/knowledge-registry/source-acquisition-runner-v3/checked_io.go +++ /dev/null @@ -1,30 +0,0 @@ -package main - -import ( - "errors" - "fmt" - "io" - "math" - "syscall" -) - -func checkedUID(value int) (uint32, error) { - if value < 0 || uint64(value) > math.MaxUint32 { - return 0, fmt.Errorf("effective user id %d is outside uint32 range", value) - } - return uint32(value), nil -} - -func closeWithError(primary error, closer io.Closer) error { - if err := closer.Close(); err != nil { - return errors.Join(primary, fmt.Errorf("close resource: %w", err)) - } - return primary -} - -func closeDescriptorWithError(primary error, descriptor int) error { - if err := syscall.Close(descriptor); err != nil { - return errors.Join(primary, fmt.Errorf("close descriptor: %w", err)) - } - return primary -} diff --git a/tools/knowledge-registry/source-acquisition-runner-v3/checked_io_test.go b/tools/knowledge-registry/source-acquisition-runner-v3/checked_io_test.go deleted file mode 100644 index c4eda17..0000000 --- a/tools/knowledge-registry/source-acquisition-runner-v3/checked_io_test.go +++ /dev/null @@ -1,77 +0,0 @@ -package main - -import ( - "errors" - "math" - "strconv" - "testing" -) - -type failingCloser struct { - err error - calls int -} - -func (c *failingCloser) Close() error { c.calls++; return c.err } - -func TestCheckedUID(t *testing.T) { - cases := []struct { - name string - value int - want uint32 - bad bool - }{ - {"negative", -1, 0, true}, {"zero", 0, 0, false}, {"positive", 42, 42, false}, - } - if strconv.IntSize == 64 { - cases = append(cases, - struct { - name string - value int - want uint32 - bad bool - }{"maximum", math.MaxUint32, math.MaxUint32, false}, - struct { - name string - value int - want uint32 - bad bool - }{"overflow", math.MaxUint32 + 1, 0, true}) - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - got, err := checkedUID(tc.value) - if (err != nil) != tc.bad || got != tc.want { - t.Fatalf("checkedUID(%d) = %d, %v", tc.value, got, err) - } - }) - } -} - -func TestCloseWithError(t *testing.T) { - primary := errors.New("operation") - cleanup := errors.New("cleanup") - cases := []struct { - name string - primary, cleanup error - }{ - {"success", nil, nil}, {"primary", primary, nil}, {"cleanup", nil, cleanup}, {"both", primary, cleanup}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - closer := &failingCloser{err: tc.cleanup} - got := closeWithError(tc.primary, closer) - if closer.calls != 1 { - t.Fatalf("close calls = %d", closer.calls) - } - if (got == nil) != (tc.primary == nil && tc.cleanup == nil) { - t.Fatalf("error = %v", got) - } - for _, want := range []error{tc.primary, tc.cleanup} { - if want != nil && !errors.Is(got, want) { - t.Fatalf("lost error %v: %v", want, got) - } - } - }) - } -} diff --git a/tools/knowledge-registry/source-acquisition-runner-v3/contract.go b/tools/knowledge-registry/source-acquisition-runner-v3/contract.go index 9f40ae4..8f09ae3 100644 --- a/tools/knowledge-registry/source-acquisition-runner-v3/contract.go +++ b/tools/knowledge-registry/source-acquisition-runner-v3/contract.go @@ -878,16 +878,17 @@ func parseHexQuad(raw []byte) (uint16, bool) { // readRegularNoFollow opens one bounded regular file without following symlinks // and rejects any file that changes underneath the read. -func readRegularNoFollow(path string, maximum int64) (result []byte, err error) { +func readRegularNoFollow(path string, maximum int64) ([]byte, error) { descriptor, err := syscall.Open(path, syscall.O_RDONLY|syscall.O_CLOEXEC|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0) if err != nil { return nil, err } file := os.NewFile(uintptr(descriptor), path) if file == nil { - return nil, closeDescriptorWithError(errors.New("could not own file descriptor"), descriptor) + syscall.Close(descriptor) + return nil, errors.New("could not own file descriptor") } - defer func() { err = closeWithError(err, file) }() + defer file.Close() before, err := file.Stat() if err != nil { return nil, err @@ -906,7 +907,7 @@ func readRegularNoFollow(path string, maximum int64) (result []byte, err error) if err != nil { return nil, err } - if !sameStableFile(before, after) { + if !os.SameFile(before, after) || before.Size() != after.Size() || before.ModTime() != after.ModTime() { return nil, errors.New("input changed while it was read") } return raw, nil @@ -920,7 +921,3 @@ func sortedKeys(values map[string]struct{}) []string { sort.Strings(keys) return keys } - -func sameStableFile(before, after os.FileInfo) bool { - return os.SameFile(before, after) && before.Size() == after.Size() && before.ModTime() == after.ModTime() -} diff --git a/tools/knowledge-registry/source-acquisition-runner-v3/http_error_test.go b/tools/knowledge-registry/source-acquisition-runner-v3/http_error_test.go deleted file mode 100644 index b89e426..0000000 --- a/tools/knowledge-registry/source-acquisition-runner-v3/http_error_test.go +++ /dev/null @@ -1,71 +0,0 @@ -package main - -import ( - "context" - "errors" - "io" - "net/http" - "net/url" - "strings" - "testing" -) - -type failedResponseBody struct{ err error } - -func (b failedResponseBody) Read([]byte) (int, error) { return 0, b.err } -func (b failedResponseBody) Close() error { return nil } - -func TestGitObjectStatusError(t *testing.T) { - failedRead := errors.New("response read failed") - cases := []struct { - name string - status int - body io.ReadCloser - want error - }{ - {"success", http.StatusOK, io.NopCloser(strings.NewReader("{}")), nil}, - {"http failure", http.StatusForbidden, io.NopCloser(strings.NewReader("denied")), nil}, - {"drain failure", http.StatusForbidden, failedResponseBody{failedRead}, failedRead}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - got := gitObjectStatusError(&http.Response{StatusCode: tc.status, Body: tc.body}) - if (got == nil) != (tc.status == http.StatusOK) { - t.Fatalf("status error = %v", got) - } - if tc.want != nil && !errors.Is(got, tc.want) { - t.Fatalf("lost drain error: %v", got) - } - }) - } -} - -type responseTransport func(*http.Request) (*http.Response, error) - -func (f responseTransport) RoundTrip(req *http.Request) (*http.Response, error) { return f(req) } - -type closeErrorBody struct { - io.Reader - err error -} - -func (b closeErrorBody) Close() error { return b.err } - -func TestGetJSONPreservesResponseCloseError(t *testing.T) { - cleanup := errors.New("response close failed") - for _, closeErr := range []error{nil, cleanup} { - origin, err := url.Parse("https://api.github.com") - if err != nil { - t.Fatal(err) - } - client := &http.Client{Transport: responseTransport(func(req *http.Request) (*http.Response, error) { - return &http.Response{StatusCode: http.StatusOK, Header: http.Header{"Content-Type": []string{"application/json"}}, Request: req, Body: closeErrorBody{strings.NewReader("{}"), closeErr}}, nil - })} - r := &runner{client: client, baseURL: origin, allowedRepos: map[string]struct{}{"argoproj/argo-helm": {}}} - err = r.getJSON(context.Background(), "argoproj/argo-helm", "/git/test", &map[string]any{}) - - if !errors.Is(err, closeErr) { - t.Fatalf("response close error = %v, want %v", err, closeErr) - } - } -} diff --git a/tools/knowledge-registry/source-acquisition-runner-v3/plan.go b/tools/knowledge-registry/source-acquisition-runner-v3/plan.go index 4d82809..3c5a56d 100644 --- a/tools/knowledge-registry/source-acquisition-runner-v3/plan.go +++ b/tools/knowledge-registry/source-acquisition-runner-v3/plan.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "errors" - "fmt" "io" "os" "os/exec" @@ -54,7 +53,7 @@ func isArmoredSSHSIG(signature []byte) bool { // invocation used across the knowledge tools: the document is fed on stdin, the // principal and namespace are pinned, and the allowed_signers policy plus the // detached signature are written to a private temp directory. -func verifySSHSIGNamespace(document, signature, allowedSigners []byte, principal, namespace string) (err error) { +func verifySSHSIGNamespace(document, signature, allowedSigners []byte, principal, namespace string) error { info, err := os.Lstat(sshKeygenPath) if err != nil || !info.Mode().IsRegular() || info.Mode()&0o111 == 0 { return errors.New("fixed ssh-keygen verifier is unavailable") @@ -63,11 +62,7 @@ func verifySSHSIGNamespace(document, signature, allowedSigners []byte, principal if err != nil { return err } - defer func() { - if cleanupErr := os.RemoveAll(directory); cleanupErr != nil { - err = errors.Join(err, fmt.Errorf("remove signature verification directory: %w", cleanupErr)) - } - }() + defer os.RemoveAll(directory) policyPath := filepath.Join(directory, "allowed_signers") signaturePath := filepath.Join(directory, "plan.sig") if err := os.WriteFile(policyPath, allowedSigners, 0o600); err != nil { @@ -79,7 +74,7 @@ func verifySSHSIGNamespace(document, signature, allowedSigners []byte, principal ctx, cancel := context.WithTimeout(context.Background(), sshKeygenVerifyTimeout) defer cancel() command := exec.CommandContext(ctx, sshKeygenPath, "-Y", "verify", "-f", policyPath, - "-I", principal, "-n", namespace, "-s", signaturePath) // #nosec G204 -- sshKeygenPath is the fixed /usr/bin/ssh-keygen verifier; arguments are passed directly without a shell. + "-I", principal, "-n", namespace, "-s", signaturePath) command.Stdin = bytes.NewReader(document) command.Stdout = io.Discard command.Stderr = io.Discard diff --git a/tools/knowledge-registry/source-acquisition-runner-v3/runner.go b/tools/knowledge-registry/source-acquisition-runner-v3/runner.go index b3b15eb..76bb6b1 100644 --- a/tools/knowledge-registry/source-acquisition-runner-v3/runner.go +++ b/tools/knowledge-registry/source-acquisition-runner-v3/runner.go @@ -3,7 +3,7 @@ package main import ( "bytes" "context" - "crypto/sha1" // #nosec G505 -- Git blob object identifiers require SHA-1; content integrity is also bound by SHA-256. + "crypto/sha1" "crypto/sha256" "encoding/base64" "encoding/hex" @@ -704,16 +704,7 @@ func (r *runner) getBlob(ctx context.Context, repository string, expected contra if blob.SHA != expected.GitBlobSHA1 || blob.Size == nil || *blob.Size != expected.Bytes || blob.Encoding != "base64" { return nil, errors.New("blob response identity, encoding, or size drifted") } - raw, err := decodeGitBlob(blob.Content, expected) - if err != nil { - return nil, err - } - r.blobCache[key] = raw - return raw, nil -} - -func decodeGitBlob(content string, expected contractFile) ([]byte, error) { - compact := strings.NewReplacer("\n", "", "\r", "").Replace(content) + compact := strings.NewReplacer("\n", "", "\r", "").Replace(blob.Content) raw, err := base64.StdEncoding.Strict().DecodeString(compact) if err != nil { return nil, errors.New("blob content is not strict base64") @@ -722,7 +713,7 @@ func decodeGitBlob(content string, expected contractFile) ([]byte, error) { return nil, errors.New("decoded blob size drifted") } identity := append([]byte("blob "+strconv.Itoa(len(raw))+"\x00"), raw...) - gitDigest := sha1.Sum(identity) // #nosec G401 -- Reconstruct the protocol-defined Git blob SHA-1 identity; SHA-256 separately binds the artifact bytes. + gitDigest := sha1.Sum(identity) if hex.EncodeToString(gitDigest[:]) != expected.GitBlobSHA1 { return nil, errors.New("decoded blob does not reproduce the exact Git SHA-1") } @@ -730,10 +721,11 @@ func decodeGitBlob(content string, expected contractFile) ([]byte, error) { if "sha256:"+hex.EncodeToString(rawDigest[:]) != expected.RawSHA256 { return nil, errors.New("decoded blob raw SHA-256 drifted") } + r.blobCache[key] = raw return raw, nil } -func (r *runner) getJSON(ctx context.Context, repository, suffix string, destination any) (err error) { +func (r *runner) getJSON(ctx context.Context, repository, suffix string, destination any) error { if _, ok := r.allowedRepos[repository]; !ok { return errors.New("repository is outside the exact three-source denominator") } @@ -741,7 +733,7 @@ func (r *runner) getJSON(ctx context.Context, repository, suffix string, destina return errors.New("HTTP request is outside the exact capability") } endpoint := r.baseURL.String() + "/repos/" + repository + suffix - request, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) // #nosec G704 -- newRunner pins the production API origin to api.github.com and forbids redirects before sending; alternate origins require explicit test mode. + request, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) if err != nil { return err } @@ -753,17 +745,32 @@ func (r *runner) getJSON(ctx context.Context, repository, suffix string, destina request.Header.Set("Authorization", "Bearer "+r.token) } r.requests++ - response, err := r.client.Do(request) // #nosec G704 -- newRunner pins the production API origin to api.github.com and forbids redirects before sending; alternate origins require explicit test mode. + response, err := r.client.Do(request) if err != nil { return err } - defer func() { err = closeWithError(err, response.Body) }() - return r.readGitObjectResponse(response, endpoint, destination) -} - -func (r *runner) readGitObjectResponse(response *http.Response, endpoint string, destination any) error { - if err := validateGitObjectResponse(response, endpoint); err != nil { - return err + defer response.Body.Close() + if response.StatusCode != http.StatusOK { + _, _ = io.Copy(io.Discard, io.LimitReader(response.Body, 4096)) + return fmt.Errorf("GitHub REST returned HTTP %d", response.StatusCode) + } + if response.Request == nil || response.Request.URL.String() != endpoint { + return errors.New("Git-object response arrived through a redirected or changed endpoint") + } + if len(response.Header.Values("Link")) != 0 { + return errors.New("Git-object endpoint returned a pagination Link header") + } + encodings := response.Header.Values("Content-Encoding") + if len(encodings) > 1 || (len(encodings) == 1 && encodings[0] != "identity") { + return errors.New("Git-object response used an unexpected content encoding") + } + contentTypes := response.Header.Values("Content-Type") + if len(contentTypes) != 1 { + return errors.New("Git-object response has an ambiguous or missing content type") + } + contentType, _, err := mime.ParseMediaType(contentTypes[0]) + if err != nil || contentType != "application/json" { + return errors.New("Git-object response is not application/json") } remaining := maxResponseBytes - r.responseBytes if remaining <= 0 { @@ -790,48 +797,6 @@ func (r *runner) readGitObjectResponse(response *http.Response, endpoint string, return nil } -func validateGitObjectResponse(response *http.Response, endpoint string) error { - if err := gitObjectStatusError(response); err != nil { - return err - } - - if response.Request == nil || response.Request.URL.String() != endpoint { - return errors.New("Git-object response arrived through a redirected or changed endpoint") - } - if len(response.Header.Values("Link")) != 0 { - return errors.New("Git-object endpoint returned a pagination Link header") - } - encodings := response.Header.Values("Content-Encoding") - if len(encodings) > 1 || (len(encodings) == 1 && encodings[0] != "identity") { - return errors.New("Git-object response used an unexpected content encoding") - } - return validateGitObjectContentType(response) -} - -func validateGitObjectContentType(response *http.Response) error { - contentTypes := response.Header.Values("Content-Type") - if len(contentTypes) != 1 { - return errors.New("Git-object response has an ambiguous or missing content type") - } - contentType, _, err := mime.ParseMediaType(contentTypes[0]) - if err != nil || contentType != "application/json" { - return errors.New("Git-object response is not application/json") - } - - return nil -} - -func gitObjectStatusError(response *http.Response) error { - if response.StatusCode != http.StatusOK { - _, drainErr := io.Copy(io.Discard, io.LimitReader(response.Body, 4096)) - if drainErr != nil { - drainErr = fmt.Errorf("drain Git-object error response: %w", drainErr) - } - return errors.Join(fmt.Errorf("GitHub REST returned HTTP %d", response.StatusCode), drainErr) - } - return nil -} - func exactTreeEntry(entries []gitTreeEntry, name string) (gitTreeEntry, error) { seen := make(map[string]struct{}, len(entries)) var match *gitTreeEntry diff --git a/tools/knowledge-registry/source-acquisition-runner-v3/store.go b/tools/knowledge-registry/source-acquisition-runner-v3/store.go index b559488..7223c7d 100644 --- a/tools/knowledge-registry/source-acquisition-runner-v3/store.go +++ b/tools/knowledge-registry/source-acquisition-runner-v3/store.go @@ -9,7 +9,6 @@ import ( "io" "os" "path/filepath" - "slices" "sort" "strings" "syscall" @@ -33,89 +32,70 @@ func newCASStore(root string) (*casStore, error) { if absolute == string(os.PathSeparator) { return nil, errors.New("output root must not be the filesystem root") } - rootFile, err := openCASRoot(absolute) + parent, err := openDirectoryPathNoFollow(filepath.Dir(absolute)) if err != nil { - return nil, err + return nil, fmt.Errorf("output parent: %w", err) + } + defer parent.Close() + leaf := filepath.Base(absolute) + rootFile, err := openDirectoryAt(parent, leaf) + if errors.Is(err, os.ErrNotExist) { + if err := syscall.Mkdirat(int(parent.Fd()), leaf, 0o700); err != nil && !errors.Is(err, syscall.EEXIST) { + return nil, fmt.Errorf("create output root: %w", err) + } + if err := syncOpenedFile(parent); err != nil { + return nil, fmt.Errorf("persist output root: %w", err) + } + rootFile, err = openDirectoryAt(parent, leaf) + } + if err != nil { + return nil, fmt.Errorf("inspect output root: %w", err) } info, err := rootFile.Stat() if err != nil { - return nil, closeWithError(fmt.Errorf("inspect output root: %w", err), rootFile) + rootFile.Close() + return nil, fmt.Errorf("inspect output root: %w", err) } if err := validateOwnedDirectory(info, "output root"); err != nil { - return nil, closeWithError(err, rootFile) + rootFile.Close() + return nil, err } store := &casStore{root: absolute, rootFile: rootFile, rootInfo: info} if err := store.verifyRootPath(); err != nil { - return nil, closeWithError(err, rootFile) + rootFile.Close() + return nil, err } - if err := store.initializeDirectories(); err != nil { - return nil, closeWithError(err, rootFile) + receipts, _, err := store.listOptionalPublishedDirectory("receipts", 2) + if err != nil { + rootFile.Close() + return nil, fmt.Errorf("preflight receipts: %w", err) } - return store, nil -} - -func (s *casStore) initializeDirectories() error { - recoveryState, err := s.hasRecoveryState() + proposals, _, err := store.listOptionalPublishedDirectory("inventory-proposals", 2) if err != nil { - return err + rootFile.Close() + return nil, fmt.Errorf("preflight inventory proposals: %w", err) + } + if len(receipts) == 0 && len(proposals) != 0 { + rootFile.Close() + return nil, errors.New("inventory proposal exists without its predecessor receipt") } + recoveryState := len(receipts) != 0 || len(proposals) != 0 for _, relative := range []string{"cas", "cas/sha256", "receipts", "inventory-proposals"} { if recoveryState { - directory, _, openErr := s.openStoreDirectory(relative, false) + directory, _, openErr := store.openStoreDirectory(relative, false) if openErr == nil { - if err := closeWithError(nil, directory); err != nil { - return err - } + directory.Close() continue } - return fmt.Errorf("recovery state is missing required directory %s: %w", relative, openErr) + rootFile.Close() + return nil, fmt.Errorf("recovery state is missing required directory %s: %w", relative, openErr) } - if _, err := s.ensureDirectory(relative); err != nil { - return err + if _, err := store.ensureDirectory(relative); err != nil { + rootFile.Close() + return nil, err } } - return nil -} - -func (s *casStore) hasRecoveryState() (bool, error) { - receipts, _, err := s.listOptionalPublishedDirectory("receipts", 2) - if err != nil { - return false, fmt.Errorf("preflight receipts: %w", err) - } - proposals, _, err := s.listOptionalPublishedDirectory("inventory-proposals", 2) - if err != nil { - return false, fmt.Errorf("preflight inventory proposals: %w", err) - } - if len(receipts) == 0 && len(proposals) != 0 { - return false, errors.New("inventory proposal exists without its predecessor receipt") - } - return len(receipts) != 0 || len(proposals) != 0, nil -} - -func openCASRoot(absolute string) (file *os.File, err error) { - parent, err := openDirectoryPathNoFollow(filepath.Dir(absolute)) - if err != nil { - return nil, fmt.Errorf("output parent: %w", err) - } - defer func() { - err = closeWithError(err, parent) - if err != nil && file != nil { - err = closeWithError(err, file) - file = nil - } - }() - leaf := filepath.Base(absolute) - file, err = openDirectoryAt(parent, leaf) - if !errors.Is(err, os.ErrNotExist) { - return file, err - } - if err := syscall.Mkdirat(int(parent.Fd()), leaf, 0o700); err != nil && !errors.Is(err, syscall.EEXIST) { - return nil, fmt.Errorf("create output root: %w", err) - } - if err := syncOpenedFile(parent); err != nil { - return nil, fmt.Errorf("persist output root: %w", err) - } - return openDirectoryAt(parent, leaf) + return store, nil } func (s *casStore) listOptionalPublishedDirectory(relative string, maximum int) ([]string, bool, error) { @@ -182,12 +162,12 @@ func (s *casStore) putProposal(proposalID string, raw []byte) (string, error) { return relative, nil } -func (s *casStore) ensureDirectory(relative string) (result string, err error) { +func (s *casStore) ensureDirectory(relative string) (string, error) { directory, _, err := s.openStoreDirectory(relative, true) if err != nil { return "", err } - defer func() { err = closeWithError(err, directory) }() + defer directory.Close() return filepath.Join(s.root, filepath.FromSlash(relative)), nil } @@ -229,7 +209,7 @@ func (s *casStore) verifyRunSet(receiptPath string, receiptRaw []byte, proposalP if err != nil { return err } - if !slices.Equal(receipts, []string{filepath.Base(receiptPath)}) || !slices.Equal(proposals, []string{filepath.Base(proposalPath)}) { + if len(receipts) != 1 || receipts[0] != filepath.Base(receiptPath) || len(proposals) != 1 || proposals[0] != filepath.Base(proposalPath) { return errors.New("published receipt/proposal set is incomplete or ambiguous") } actualReceipt, err := s.readPublished(receiptPath, maxPublishedJSONBytes) @@ -243,7 +223,7 @@ func (s *casStore) verifyRunSet(receiptPath string, receiptRaw []byte, proposalP return nil } -func (s *casStore) atomicNoReplace(relative string, raw []byte, maximum int64) (err error) { +func (s *casStore) atomicNoReplace(relative string, raw []byte, maximum int64) error { if int64(len(raw)) > maximum { return errors.New("published object exceeds its byte bound") } @@ -256,7 +236,7 @@ func (s *casStore) atomicNoReplace(relative string, raw []byte, maximum int64) ( if err != nil { return err } - defer func() { err = closeWithError(err, parent) }() + defer parent.Close() leaf := filepath.Base(relative) descriptor, err := syscall.Openat(int(parent.Fd()), leaf, syscall.O_WRONLY|syscall.O_CREAT|syscall.O_EXCL|syscall.O_CLOEXEC|syscall.O_NOFOLLOW, 0o600) if err != nil { @@ -267,50 +247,39 @@ func (s *casStore) atomicNoReplace(relative string, raw []byte, maximum int64) ( } file := os.NewFile(uintptr(descriptor), leaf) if file == nil { - return closeDescriptorWithError(errors.New("could not own exclusive published object descriptor"), descriptor) + syscall.Close(descriptor) + return errors.New("could not own exclusive published object descriptor") } - written, err := writePublishedContents(file, raw) - if err != nil { - return closeWithError(err, file) - } - return closeWithError(s.commitPublishedContents(parentRelative, parent, parentInfo, leaf, raw, maximum, written), file) -} - -func (s *casStore) commitPublishedContents(parentRelative string, parent *os.File, parentInfo os.FileInfo, leaf string, raw []byte, maximum int64, written os.FileInfo) error { - if err := s.verifyPublishedAtPath(parentRelative, parentInfo, leaf, raw, maximum, written, false, "exclusive published object changed before directory commit"); err != nil { - return err - } - if err := syncOpenedFile(parent); err != nil { - return err - } - if err := s.verifyPublishedAtPath(parentRelative, parentInfo, leaf, raw, maximum, written, false, "exclusive published object changed after directory commit"); err != nil { - return err - } - if err := s.verifyRootPath(); err != nil { - return err - } - return nil -} - -func writePublishedContents(file *os.File, raw []byte) (os.FileInfo, error) { created, err := file.Stat() if err != nil { - return nil, err + return errors.Join(err, file.Close()) } if err := validateOwnedRegular(created, "new published object"); err != nil { - return nil, err + return errors.Join(err, file.Close()) } if err := writeAll(file, raw); err != nil { - return nil, err + return errors.Join(err, file.Close()) } if err := syncOpenedFile(file); err != nil { - return nil, err + return errors.Join(err, file.Close()) } written, err := file.Stat() if err != nil || !os.SameFile(created, written) || written.Size() != int64(len(raw)) { - return nil, errors.Join(errors.New("exclusive published object write is incomplete or unstable"), err) + return errors.Join(errors.New("exclusive published object write is incomplete or unstable"), err, file.Close()) + } + if err := s.verifyPublishedAtPath(parentRelative, parentInfo, leaf, raw, maximum, written, false, "exclusive published object changed before directory commit"); err != nil { + return errors.Join(err, file.Close()) + } + if err := syncOpenedFile(parent); err != nil { + return errors.Join(err, file.Close()) + } + if err := s.verifyPublishedAtPath(parentRelative, parentInfo, leaf, raw, maximum, written, false, "exclusive published object changed after directory commit"); err != nil { + return errors.Join(err, file.Close()) + } + if err := s.verifyRootPath(); err != nil { + return errors.Join(err, file.Close()) } - return written, nil + return file.Close() } func (s *casStore) rehabilitateExisting(parentRelative string, parent *os.File, parentInfo os.FileInfo, leaf string, raw []byte, maximum int64) error { @@ -330,12 +299,24 @@ func (s *casStore) rehabilitateExisting(parentRelative string, parent *os.File, return s.verifyRootPath() } -func verifyPublishedInDirectory(parent *os.File, leaf string, raw []byte, maximum int64, required os.FileInfo, synchronize bool, conflict string) (info os.FileInfo, err error) { - file, err := openPublishedFile(parent, leaf, synchronize) +func verifyPublishedInDirectory(parent *os.File, leaf string, raw []byte, maximum int64, required os.FileInfo, synchronize bool, conflict string) (os.FileInfo, error) { + access := syscall.O_RDONLY + if synchronize { + access = syscall.O_RDWR + } + descriptor, err := syscall.Openat(int(parent.Fd()), leaf, access|syscall.O_CLOEXEC|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0) if err != nil { + if errors.Is(err, syscall.ELOOP) { + return nil, errors.New("published object path exists and is not a regular file") + } return nil, err } - defer func() { err = closeWithError(err, file) }() + file := os.NewFile(uintptr(descriptor), leaf) + if file == nil { + syscall.Close(descriptor) + return nil, errors.New("could not own published object file descriptor") + } + defer file.Close() before, err := file.Stat() if err != nil { return nil, err @@ -349,73 +330,42 @@ func verifyPublishedInDirectory(parent *os.File, leaf string, raw []byte, maximu if before.Size() > maximum { return nil, errors.New("published object exceeds its byte bound") } - existing, after, err := readStablePublished(file, before, maximum) - if err != nil { - return nil, err - } - if !bytes.Equal(existing, raw) { - return nil, errors.New(conflict) - } - if synchronize { - return synchronizePublished(file, after) - } - return after, nil -} - -func openPublishedFile(parent *os.File, leaf string, synchronize bool) (*os.File, error) { - access := syscall.O_RDONLY - if synchronize { - access = syscall.O_RDWR - } - descriptor, err := syscall.Openat(int(parent.Fd()), leaf, access|syscall.O_CLOEXEC|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0) - if err != nil { - if errors.Is(err, syscall.ELOOP) { - return nil, errors.New("published object path exists and is not a regular file") - } - return nil, err - } - file := os.NewFile(uintptr(descriptor), leaf) - if file == nil { - return nil, closeDescriptorWithError(errors.New("could not own published object file descriptor"), descriptor) - } - return file, nil -} - -func readStablePublished(file *os.File, before os.FileInfo, maximum int64) ([]byte, os.FileInfo, error) { existing, err := io.ReadAll(io.LimitReader(file, maximum+1)) if err != nil { - return nil, nil, err + return nil, err } if int64(len(existing)) > maximum { - return nil, nil, errors.New("published object exceeds its byte bound") + return nil, errors.New("published object exceeds its byte bound") } after, err := file.Stat() if err != nil { - return nil, nil, err + return nil, err } if !os.SameFile(before, after) || before.Size() != after.Size() || before.ModTime() != after.ModTime() { - return nil, nil, errors.New("published object changed while it was read") + return nil, errors.New("published object changed while it was read") } - return existing, after, nil -} - -func synchronizePublished(file *os.File, after os.FileInfo) (os.FileInfo, error) { - if err := syncOpenedFile(file); err != nil { - return nil, err + if !bytes.Equal(existing, raw) { + return nil, errors.New(conflict) } - afterSync, err := file.Stat() - if err != nil || !os.SameFile(after, afterSync) || after.Size() != afterSync.Size() || after.ModTime() != afterSync.ModTime() { - return nil, errors.Join(errors.New("published object changed while it was synchronized"), err) + if synchronize { + if err := syncOpenedFile(file); err != nil { + return nil, err + } + afterSync, err := file.Stat() + if err != nil || !os.SameFile(after, afterSync) || after.Size() != afterSync.Size() || after.ModTime() != afterSync.ModTime() { + return nil, errors.Join(errors.New("published object changed while it was synchronized"), err) + } + after = afterSync } - return afterSync, nil + return after, nil } -func (s *casStore) verifyPublishedAtPath(parentRelative string, requiredParent os.FileInfo, leaf string, raw []byte, maximum int64, requiredFile os.FileInfo, synchronize bool, conflict string) (err error) { +func (s *casStore) verifyPublishedAtPath(parentRelative string, requiredParent os.FileInfo, leaf string, raw []byte, maximum int64, requiredFile os.FileInfo, synchronize bool, conflict string) error { parent, parentInfo, err := s.openStoreDirectory(parentRelative, false) if err != nil { return err } - defer func() { err = closeWithError(err, parent) }() + defer parent.Close() if !os.SameFile(requiredParent, parentInfo) { return errors.New("published object parent changed during commit") } @@ -423,7 +373,7 @@ func (s *casStore) verifyPublishedAtPath(parentRelative string, requiredParent o return err } -func (s *casStore) readPublished(relative string, maximum int64) (result []byte, err error) { +func (s *casStore) readPublished(relative string, maximum int64) ([]byte, error) { clean := filepath.Clean(relative) if clean != relative || filepath.IsAbs(relative) || strings.HasPrefix(clean, "..") { return nil, errors.New("published object path is not canonical") @@ -433,28 +383,18 @@ func (s *casStore) readPublished(relative string, maximum int64) (result []byte, if err != nil { return nil, err } - defer func() { err = closeWithError(err, parent) }() + defer parent.Close() leaf := filepath.Base(relative) - file, err := openPublishedFile(parent, leaf, false) + descriptor, err := syscall.Openat(int(parent.Fd()), leaf, syscall.O_RDONLY|syscall.O_CLOEXEC|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0) if err != nil { return nil, err } - defer func() { err = closeWithError(err, file) }() - before, err := inspectPublished(file, maximum) - if err != nil { - return nil, err - } - raw, after, err := readStablePublished(file, before, maximum) - if err != nil { - return nil, err - } - if err := s.verifyPublishedAtPath(parentRelative, parentInfo, leaf, raw, maximum, after, false, "published object changed during path replay"); err != nil { - return nil, err + file := os.NewFile(uintptr(descriptor), leaf) + if file == nil { + syscall.Close(descriptor) + return nil, errors.New("could not own published object descriptor") } - return raw, nil -} - -func inspectPublished(file *os.File, maximum int64) (os.FileInfo, error) { + defer file.Close() before, err := file.Stat() if err != nil { return nil, err @@ -465,10 +405,21 @@ func inspectPublished(file *os.File, maximum int64) (os.FileInfo, error) { if before.Size() > maximum { return nil, errors.New("published object exceeds its byte bound") } - return before, nil + raw, err := io.ReadAll(io.LimitReader(file, maximum+1)) + if err != nil || int64(len(raw)) > maximum { + return nil, errors.Join(errors.New("published object read exceeds its bound"), err) + } + after, err := file.Stat() + if err != nil || !os.SameFile(before, after) || before.Size() != after.Size() || before.ModTime() != after.ModTime() { + return nil, errors.Join(errors.New("published object changed while it was read"), err) + } + if err := s.verifyPublishedAtPath(parentRelative, parentInfo, leaf, raw, maximum, after, false, "published object changed during path replay"); err != nil { + return nil, err + } + return raw, nil } -func (s *casStore) listPublishedDirectory(relative string, maximum int) (result []string, err error) { +func (s *casStore) listPublishedDirectory(relative string, maximum int) ([]string, error) { if maximum < 1 { return nil, errors.New("directory entry bound must be positive") } @@ -476,7 +427,7 @@ func (s *casStore) listPublishedDirectory(relative string, maximum int) (result if err != nil { return nil, err } - defer func() { err = closeWithError(err, directory) }() + defer directory.Close() entries, err := directory.ReadDir(maximum + 1) if err != nil && !errors.Is(err, io.EOF) { return nil, err @@ -484,17 +435,19 @@ func (s *casStore) listPublishedDirectory(relative string, maximum int) (result if len(entries) > maximum { return nil, errors.New("published directory exceeds its entry bound") } - names, err := publishedEntryNames(entries) - if err != nil { - return nil, err + names := make([]string, 0, len(entries)) + for _, entry := range entries { + name := entry.Name() + if name == "" || name == "." || name == ".." || strings.ContainsAny(name, "/\x00") { + return nil, errors.New("published directory contains an unsafe entry name") + } + names = append(names, name) } replayed, replayedInfo, err := s.openStoreDirectory(relative, false) if err != nil { return nil, err } - if err := closeWithError(nil, replayed); err != nil { - return nil, err - } + replayed.Close() if !os.SameFile(directoryInfo, replayedInfo) { return nil, errors.New("published directory changed while it was listed") } @@ -502,18 +455,6 @@ func (s *casStore) listPublishedDirectory(relative string, maximum int) (result return names, nil } -func publishedEntryNames(entries []os.DirEntry) ([]string, error) { - names := make([]string, 0, len(entries)) - for _, entry := range entries { - name := entry.Name() - if name == "" || name == "." || name == ".." || strings.ContainsAny(name, "/\x00") { - return nil, errors.New("published directory contains an unsafe entry name") - } - names = append(names, name) - } - return names, nil -} - func writeAll(file *os.File, raw []byte) error { for len(raw) > 0 { written, err := file.Write(raw) @@ -563,11 +504,7 @@ func validateEffectiveOwner(info os.FileInfo, label string) error { if !ok { return fmt.Errorf("%s ownership cannot be verified", label) } - uid, err := checkedUID(os.Geteuid()) - if err != nil { - return fmt.Errorf("%s ownership: %w", label, err) - } - if stat.Uid != uid { + if stat.Uid != uint32(os.Geteuid()) { return fmt.Errorf("%s is not owned by the effective user", label) } return nil @@ -584,22 +521,22 @@ func openDirectoryPathNoFollow(pathname string) (*os.File, error) { } current := os.NewFile(uintptr(descriptor), string(os.PathSeparator)) if current == nil { - return nil, closeDescriptorWithError(errors.New("could not own filesystem-root descriptor"), descriptor) + syscall.Close(descriptor) + return nil, errors.New("could not own filesystem-root descriptor") } trimmed := strings.TrimPrefix(absolute, string(os.PathSeparator)) if trimmed == "" { return current, nil } for _, component := range strings.Split(trimmed, string(os.PathSeparator)) { - if !canonicalDirectoryComponent(component) { - return nil, closeWithError(errors.New("directory path is not canonical"), current) + if component == "" || component == "." || component == ".." { + current.Close() + return nil, errors.New("directory path is not canonical") } next, err := openDirectoryAt(current, component) - if closeErr := closeWithError(err, current); closeErr != nil { - if next != nil { - closeErr = closeWithError(closeErr, next) - } - return nil, closeErr + current.Close() + if err != nil { + return nil, err } current = next } @@ -613,7 +550,8 @@ func openDirectoryAt(parent *os.File, name string) (*os.File, error) { } file := os.NewFile(uintptr(descriptor), name) if file == nil { - return nil, closeDescriptorWithError(errors.New("could not own directory descriptor"), descriptor) + syscall.Close(descriptor) + return nil, errors.New("could not own directory descriptor") } return file, nil } @@ -626,7 +564,8 @@ func duplicateFile(file *os.File, name string) (*os.File, error) { syscall.CloseOnExec(descriptor) duplicate := os.NewFile(uintptr(descriptor), name) if duplicate == nil { - return nil, closeDescriptorWithError(errors.New("could not own duplicated directory descriptor"), descriptor) + syscall.Close(descriptor) + return nil, errors.New("could not own duplicated directory descriptor") } return duplicate, nil } @@ -640,7 +579,7 @@ func validateStoreRelativeDirectory(relative string) ([]string, error) { } components := strings.Split(filepath.ToSlash(relative), "/") for _, component := range components { - if !canonicalDirectoryComponent(component) { + if component == "" || component == "." || component == ".." { return nil, errors.New("unsafe store directory component") } } @@ -657,43 +596,41 @@ func (s *casStore) openDirectoryFromRoot(relative string, create bool) (*os.File return nil, nil, err } for _, component := range components { - next, openErr := openOrCreateDirectoryAt(current, component, create) - if closeErr := closeWithError(openErr, current); closeErr != nil { - if next != nil { - closeErr = closeWithError(closeErr, next) + next, openErr := openDirectoryAt(current, component) + if errors.Is(openErr, os.ErrNotExist) && create { + if mkdirErr := syscall.Mkdirat(int(current.Fd()), component, 0o700); mkdirErr != nil && !errors.Is(mkdirErr, syscall.EEXIST) { + current.Close() + return nil, nil, mkdirErr + } + if syncErr := syncOpenedFile(current); syncErr != nil { + current.Close() + return nil, nil, syncErr } - return nil, nil, closeErr + next, openErr = openDirectoryAt(current, component) + } + current.Close() + if openErr != nil { + return nil, nil, openErr } info, statErr := next.Stat() if statErr != nil { - return nil, nil, closeWithError(statErr, next) + next.Close() + return nil, nil, statErr } if err := validateOwnedDirectory(info, relative); err != nil { - return nil, nil, closeWithError(err, next) + next.Close() + return nil, nil, err } current = next } info, err := current.Stat() if err != nil { - return nil, nil, closeWithError(err, current) + current.Close() + return nil, nil, err } return current, info, nil } -func openOrCreateDirectoryAt(current *os.File, component string, create bool) (*os.File, error) { - next, openErr := openDirectoryAt(current, component) - if errors.Is(openErr, os.ErrNotExist) && create { - if mkdirErr := syscall.Mkdirat(int(current.Fd()), component, 0o700); mkdirErr != nil && !errors.Is(mkdirErr, syscall.EEXIST) { - return nil, mkdirErr - } - if syncErr := syncOpenedFile(current); syncErr != nil { - return nil, syncErr - } - next, openErr = openDirectoryAt(current, component) - } - return next, openErr -} - func (s *casStore) openStoreDirectory(relative string, create bool) (*os.File, os.FileInfo, error) { if err := s.verifyRootPath(); err != nil { return nil, nil, err @@ -704,21 +641,22 @@ func (s *casStore) openStoreDirectory(relative string, create bool) (*os.File, o } replayed, replayedInfo, err := s.openDirectoryFromRoot(relative, false) if err != nil { - return nil, nil, closeWithError(err, held) - } - if err := closeWithError(nil, replayed); err != nil { - return nil, nil, closeWithError(err, held) + held.Close() + return nil, nil, err } + replayed.Close() if !os.SameFile(heldInfo, replayedInfo) { - return nil, nil, closeWithError(errors.New("store directory changed during path replay"), held) + held.Close() + return nil, nil, errors.New("store directory changed during path replay") } if err := s.verifyRootPath(); err != nil { - return nil, nil, closeWithError(err, held) + held.Close() + return nil, nil, err } return held, heldInfo, nil } -func (s *casStore) verifyRootPath() (err error) { +func (s *casStore) verifyRootPath() error { if s.rootFile == nil { return errors.New("store root descriptor is closed") } @@ -726,7 +664,7 @@ func (s *casStore) verifyRootPath() (err error) { if err != nil { return fmt.Errorf("reopen output root: %w", err) } - defer func() { err = closeWithError(err, current) }() + defer current.Close() info, err := current.Stat() if err != nil { return err @@ -758,7 +696,3 @@ func stripPrefixedID(value, prefix string) (string, error) { } return digest, nil } - -func canonicalDirectoryComponent(component string) bool { - return component != "" && component != "." && component != ".." -}