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 != ".." -}