Merge pull request #2155 from mtrmac/append

Don't use append() on slices with unclear origin

copy/multiple.go

@@ -340,7 +340,7 @@ func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte,
  if err != nil {
  return nil, err
  }
- sigs = append(sigs, newSigs...)
+ sigs = append(slices.Clone(sigs), newSigs...)
 
  c.Printf("Storing list signatures\n")
  if err := c.dest.PutSignaturesWithFormat(ctx, sigs, nil); err != nil {

copy/single.go

@@ -277,7 +277,7 @@ func (c *copier) copySingleImage(ctx context.Context, unparsedImage *image.Unpar
  if err != nil {
  return copySingleImageResult{}, err
  }
- sigs = append(sigs, newSigs...)
+ sigs = append(slices.Clone(sigs), newSigs...)
 
  if len(sigs) > 0 {
  c.Printf("Storing signatures\n")

docker/distribution_error.go

@@ -24,6 +24,7 @@ import (
 
  "github.com/docker/distribution/registry/api/errcode"
  dockerChallenge "github.com/docker/distribution/registry/client/auth/challenge"
+ "golang.org/x/exp/slices"
 )
 
 // errNoErrorsInBody is returned when an HTTP response body parses to an empty
@@ -105,7 +106,7 @@ func makeErrorList(err error) []error {
 }
 
 func mergeErrors(err1, err2 error) error {
- return errcode.Errors(append(makeErrorList(err1), makeErrorList(err2)...))
+ return errcode.Errors(append(slices.Clone(makeErrorList(err1)), makeErrorList(err2)...))
 }
 
 // handleErrorResponse returns error parsed from HTTP response for an

docker/docker_image_dest.go

@@ -688,6 +688,10 @@ func (d *dockerImageDestination) putSignaturesToSigstoreAttachments(ctx context.
  }
  }
 
+ // To make sure we can safely append to the slices of ociManifest, without adding a remote dependency on the code that creates it.
+ ociManifest.Layers = slices.Clone(ociManifest.Layers)
+ // We don’t need to ^^^ for ociConfig.RootFS.DiffIDs because we have created it empty ourselves, and json.Unmarshal is documented to append() to
+ // the slice in the original object (or in a newly allocated object).
  for _, sig := range signatures {
  mimeType := sig.UntrustedMIMEType()
  payloadBlob := sig.UntrustedPayload()

internal/image/docker_schema1_test.go

@@ -14,6 +14,7 @@ import (
  imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1"
  "github.com/stretchr/testify/assert"
  "github.com/stretchr/testify/require"
+ "golang.org/x/exp/slices"
 )
 
 var schema1FixtureLayerInfos = []types.BlobInfo{
@@ -372,7 +373,7 @@ func TestManifestSchema1UpdatedImage(t *testing.T) {
  original := manifestSchema1FromFixture(t, "schema1.json")
 
  // LayerInfos:
- layerInfos := append(original.LayerInfos()[1:], original.LayerInfos()[0])
+ layerInfos := append(slices.Clone(original.LayerInfos()[1:]), original.LayerInfos()[0])
  res, err := original.UpdatedImage(context.Background(), types.ManifestUpdateOptions{
  LayerInfos: layerInfos,
  })

internal/image/docker_schema2_test.go

@@ -448,7 +448,7 @@ func TestManifestSchema2UpdatedImage(t *testing.T) {
  original := manifestSchema2FromFixture(t, originalSrc, "schema2.json", false)
 
  // LayerInfos:
- layerInfos := append(original.LayerInfos()[1:], original.LayerInfos()[0])
+ layerInfos := append(slices.Clone(original.LayerInfos()[1:]), original.LayerInfos()[0])
  res, err := original.UpdatedImage(context.Background(), types.ManifestUpdateOptions{
  LayerInfos: layerInfos,
  })

internal/image/oci_test.go

@@ -419,7 +419,7 @@ func TestManifestOCI1UpdatedImage(t *testing.T) {
  original := manifestOCI1FromFixture(t, originalSrc, "oci1.json")
 
  // LayerInfos:
- layerInfos := append(original.LayerInfos()[1:], original.LayerInfos()[0])
+ layerInfos := append(slices.Clone(original.LayerInfos()[1:]), original.LayerInfos()[0])
  res, err := original.UpdatedImage(context.Background(), types.ManifestUpdateOptions{
  LayerInfos: layerInfos,
  })

internal/manifest/docker_schema2_list.go

@@ -133,7 +133,9 @@ func (index *Schema2ListPublic) editInstances(editInstances []ListEdit) error {
  }
  }
  if len(addedEntries) != 0 {
- index.Manifests = append(index.Manifests, addedEntries...)
+ // slices.Clone() here to ensure a private backing array;
+ // an external caller could have manually created Schema2ListPublic with a slice with extra capacity.
+ index.Manifests = append(slices.Clone(index.Manifests), addedEntries...)
  }
  return nil
 }

internal/manifest/docker_schema2_list_test.go

@@ -10,6 +10,7 @@ import (
  imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1"
  "github.com/stretchr/testify/assert"
  "github.com/stretchr/testify/require"
+ "golang.org/x/exp/slices"
 )
 
 func TestSchema2ListPublicFromManifest(t *testing.T) {
@@ -81,11 +82,11 @@ func TestSchema2ListEditInstances(t *testing.T) {
  err = list.EditInstances(editInstances)
  require.NoError(t, err)
 
- // Add new elements to the end of old list to maintain order
- originalListOrder = append(originalListOrder, digest.Digest("sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"))
- originalListOrder = append(originalListOrder, digest.Digest("sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"))
- // Verify order
- assert.Equal(t, list.Instances(), originalListOrder)
+ // Verify new elements are added to the end of old list
+ assert.Equal(t, append(slices.Clone(originalListOrder),
+  digest.Digest("sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
+  digest.Digest("sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"),
+ ), list.Instances())
 }
 
 func TestSchema2ListFromManifest(t *testing.T) {

internal/manifest/oci_index.go

@@ -167,7 +167,9 @@ func (index *OCI1IndexPublic) editInstances(editInstances []ListEdit) error {
  }
  }
  if len(addedEntries) != 0 {
- index.Manifests = append(index.Manifests, addedEntries...)
+ // slices.Clone() here to ensure the slice uses a private backing array;
+ // an external caller could have manually created OCI1IndexPublic with a slice with extra capacity.
+ index.Manifests = append(slices.Clone(index.Manifests), addedEntries...)
  }
  if len(addedEntries) != 0 || updatedAnnotations {
  slices.SortStableFunc(index.Manifests, func(a, b imgspecv1.Descriptor) int {

internal/testing/gpgagent/gpg_agent.go

@@ -3,12 +3,14 @@ package gpgagent
 import (
  "os"
  "os/exec"
+
+ "golang.org/x/exp/slices"
 )
 
 // Kill the running gpg-agent to drop unlocked keys.
 // This is useful to ensure tests don’t leave processes around (in TestMain), or for testing handling of invalid passphrases.
 func KillGPGAgent(gpgHomeDir string) error {
  cmd := exec.Command("gpgconf", "--kill", "gpg-agent")
- cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHomeDir)
+ cmd.Env = append(slices.Clone(os.Environ()), "GNUPGHOME="+gpgHomeDir)
  return cmd.Run()
 }

oci/layout/oci_dest.go

@@ -19,6 +19,7 @@ import (
  digest "github.com/opencontainers/go-digest"
  imgspec "github.com/opencontainers/image-spec/specs-go"
  imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1"
+ "golang.org/x/exp/slices"
 )
 
 type ociImageDestination struct {
@@ -271,8 +272,8 @@ func (d *ociImageDestination) addManifest(desc *imgspecv1.Descriptor) {
  return
  }
  }
- // It's a new entry to be added to the index.
- d.index.Manifests = append(d.index.Manifests, *desc)
+ // It's a new entry to be added to the index. Use slices.Clone() to avoid a remote dependency on how d.index was created.
+ d.index.Manifests = append(slices.Clone(d.index.Manifests), *desc)
 }
 
 // Commit marks the process of storing the image as successful and asks for the image to be persisted.

pkg/shortnames/shortnames.go

@@ -11,6 +11,7 @@ import (
  "github.com/containers/image/v5/types"
  "github.com/manifoldco/promptui"
  "github.com/opencontainers/go-digest"
+ "golang.org/x/exp/slices"
  "golang.org/x/term"
 )
 
@@ -169,7 +170,7 @@ func (r *Resolved) Description() string {
 // pull errors must equal the amount of pull candidates.
 func (r *Resolved) FormatPullErrors(pullErrors []error) error {
  if len(pullErrors) > 0 && len(pullErrors) != len(r.PullCandidates) {
- pullErrors = append(pullErrors,
+ pullErrors = append(slices.Clone(pullErrors),
  fmt.Errorf("internal error: expected %d instead of %d errors for %d pull candidates",
  len(r.PullCandidates), len(pullErrors), len(r.PullCandidates)))
  }

pkg/tlsclientconfig/tlsclientconfig.go

@@ -66,7 +66,7 @@ func SetupCertificates(dir string, tlsc *tls.Config) error {
  if err != nil {
  return err
  }
- tlsc.Certificates = append(tlsc.Certificates, cert)
+ tlsc.Certificates = append(slices.Clone(tlsc.Certificates), cert)
  }
  if strings.HasSuffix(f.Name(), ".key") {
  keyName := f.Name()

signature/fulcio_cert_test.go

@@ -18,6 +18,7 @@ import (
  "github.com/sigstore/sigstore/pkg/cryptoutils"
  "github.com/stretchr/testify/assert"
  "github.com/stretchr/testify/require"
+ "golang.org/x/exp/slices"
 )
 
 // assert that crypto.PublicKey matches the on in certPEM.
@@ -132,7 +133,7 @@ func TestFulcioIssuerInCertificate(t *testing.T) {
  extensions: []pkix.Extension{
  {
  Id: certificate.OIDIssuerV2,
- Value: append(asn1MarshalTest(t, "https://", "utf8"), asn1MarshalTest(t, "example.com", "utf8")...),
+ Value: append(slices.Clone(asn1MarshalTest(t, "https://", "utf8")), asn1MarshalTest(t, "example.com", "utf8")...),
  },
  },
  errorFragment: "invalid ASN.1 in OIDC issuer v2 extension, trailing data",

signature/internal/sigstore_payload_test.go

@@ -14,6 +14,7 @@ import (
  "github.com/sigstore/sigstore/pkg/cryptoutils"
  "github.com/stretchr/testify/assert"
  "github.com/stretchr/testify/require"
+ "golang.org/x/exp/slices"
 )
 
 // A short-hand way to get a JSON object field value or panic. No error handling done, we know
@@ -283,7 +284,7 @@ func TestVerifySigstorePayload(t *testing.T) {
  for _, invalidSig := range [][]byte{
  {}, // Empty signature
  []byte("invalid signature"),
- append(validSignatureBytes, validSignatureBytes...),
+ append(slices.Clone(validSignatureBytes), validSignatureBytes...),
  } {
  recorded = acceptanceData{}
  res, err = VerifySigstorePayload(publicKey, sigstoreSig.UntrustedPayload(), base64.StdEncoding.EncodeToString(invalidSig), recordingRules)