fix(mfs): basic UnixFS sanity checks in files cp (#10701)

Signed-off-by: Abhinav Prakash <[email protected]> Co-authored-by: Marcin Rataj <[email protected]> Co-authored-by: Andrew Gillis <[email protected]>
ipfs · Mar 5, 2025 · e221e94 · e221e94
1 parent 86aee74
commit e221e94
Show file tree

Hide file tree

Showing 4 changed files with 190 additions and 2 deletions.
diff --git a/core/commands/files.go b/core/commands/files.go
@@ -402,6 +402,7 @@ func walkBlock(ctx context.Context, dagserv ipld.DAGService, nd ipld.Node) (bool
 	return local, sizeLocal, nil
 }
 
+var errFilesCpInvalidUnixFS = errors.New("cp: source must be a valid UnixFS (dag-pb or raw codec)")
 var filesCpCmd = &cmds.Command{
 	Helptext: cmds.HelpText{
 		Tagline: "Add references to IPFS files and directories in MFS (or copy within MFS).",
@@ -480,6 +481,25 @@ being GC'ed.
 			return fmt.Errorf("cp: cannot get node from path %s: %s", src, err)
 		}
 
+		// Sanity-check: ensure root CID is a valid UnixFS (dag-pb or raw block)
+		// Context: https://github.com/ipfs/kubo/issues/10331
+		srcCidType := node.Cid().Type()
+		switch srcCidType {
+		case cid.Raw:
+			if _, ok := node.(*dag.RawNode); !ok {
+				return errFilesCpInvalidUnixFS
+			}
+		case cid.DagProtobuf:
+			if _, ok := node.(*dag.ProtoNode); !ok {
+				return errFilesCpInvalidUnixFS
+			}
+			if _, err = ft.FSNodeFromBytes(node.(*dag.ProtoNode).Data()); err != nil {
+				return fmt.Errorf("%w: %v", errFilesCpInvalidUnixFS, err)
+			}
+		default:
+			return errFilesCpInvalidUnixFS
+		}
+
 		if mkParents {
 			err := ensureContainingDirectoryExists(nd.FilesRoot, dst, prefix)
 			if err != nil {

diff --git a/core/commands/files_test.go b/core/commands/files_test.go
@@ -0,0 +1,47 @@
+package commands
+
+import (
+	"context"
+	"io"
+	"testing"
+
+	dag "github.com/ipfs/boxo/ipld/merkledag"
+	cmds "github.com/ipfs/go-ipfs-cmds"
+	coremock "github.com/ipfs/kubo/core/mock"
+	"github.com/stretchr/testify/require"
+)
+
+func TestFilesCp_DagCborNodeFails(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	cmdCtx, err := coremock.MockCmdsCtx()
+	require.NoError(t, err)
+
+	node, err := cmdCtx.ConstructNode()
+	require.NoError(t, err)
+
+	invalidData := []byte{0x00}
+	protoNode := dag.NodeWithData(invalidData)
+	err = node.DAG.Add(ctx, protoNode)
+	require.NoError(t, err)
+
+	req := &cmds.Request{
+		Context: ctx,
+		Arguments: []string{
+			"/ipfs/" + protoNode.Cid().String(),
+			"/test-destination",
+		},
+		Options: map[string]interface{}{
+			"force": false,
+		},
+	}
+
+	_, pw := io.Pipe()
+	res, err := cmds.NewWriterResponseEmitter(pw, req)
+	require.NoError(t, err)
+
+	err = filesCpCmd.Run(req, res, &cmdCtx)
+	require.Error(t, err)
+	require.ErrorContains(t, err, "cp: source must be a valid UnixFS (dag-pb or raw codec)")
+}
diff --git a/docs/changelogs/v0.34.md b/docs/changelogs/v0.34.md
@@ -12,8 +12,8 @@
   - [`IPFS_LOG_LEVEL` deprecated](#ipfs_log_level-deprecated)
   - [Pebble datastore format update](#pebble-datastore-format-update)
   - [Badger datastore update](#badger-datastore-update)
-  - [Datastore Implementation updates](#datastore-implementation-updates)
-  - [One multi-error package](#one-multi-error-package) 
+  - [Datastore Implementation Updates](#datastore-implementation-updates)
+  - [One Multi-error Package](#one-multi-error-package)
   - [📦️ Important dependency updates](#-important-dependency-updates)
 - [👨‍👩‍👧‍👦 Contributors](#-contributors)
 
@@ -48,6 +48,7 @@ For more details, check out the [`AutoTLS` configuration documentation](https://
 - `ipfs config` is now validating json fields ([#10679](https://github.com/ipfs/kubo/pull/10679)).
 - Deprecated the `bitswap reprovide` command. Make sure to switch to modern `routing reprovide`. ([#10677](https://github.com/ipfs/kubo/pull/10677))
 - The `stats reprovide` command now shows additional stats for [`Routing.AcceleratedDHTClient`](https://github.com/ipfs/kubo/blob/master/docs/config.md#routingaccelerateddhtclient), indicating the last and next `reprovide` times. ([#10677](https://github.com/ipfs/kubo/pull/10677))
+- `ipfs files cp` now performs basic codec check and will error when source is not a valid UnixFS (only `dag-pb` and `raw` codecs are allowed in MFS)
 
 #### Bitswap improvements from Boxo
 

diff --git a/test/cli/files_test.go b/test/cli/files_test.go
@@ -0,0 +1,120 @@
+package cli
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/ipfs/kubo/test/cli/harness"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestFilesCp(t *testing.T) {
+	t.Parallel()
+
+	t.Run("files cp with valid UnixFS succeeds", func(t *testing.T) {
+		t.Parallel()
+
+		node := harness.NewT(t).NewNode().Init().StartDaemon()
+
+		// Create simple text file
+		data := "testing files cp command"
+		cid := node.IPFSAddStr(data)
+
+		// Copy form IPFS => MFS
+		res := node.IPFS("files", "cp", fmt.Sprintf("/ipfs/%s", cid), "/valid-file")
+		assert.NoError(t, res.Err)
+
+		// verification
+		catRes := node.IPFS("files", "read", "/valid-file")
+		assert.Equal(t, data, catRes.Stdout.Trimmed())
+	})
+
+	t.Run("files cp with unsupported DAG node type fails", func(t *testing.T) {
+		t.Parallel()
+		node := harness.NewT(t).NewNode().Init().StartDaemon()
+
+		// MFS UnixFS is limited to dag-pb or raw, so we create a dag-cbor node to test this
+		jsonData := `{"data": "not a UnixFS node"}`
+		tempFile := filepath.Join(node.Dir, "test.json")
+		err := os.WriteFile(tempFile, []byte(jsonData), 0644)
+		require.NoError(t, err)
+		cid := node.IPFS("dag", "put", "--input-codec=json", "--store-codec=dag-cbor", tempFile).Stdout.Trimmed()
+
+		// copy without --force
+		res := node.RunIPFS("files", "cp", fmt.Sprintf("/ipfs/%s", cid), "/invalid-file")
+		assert.NotEqual(t, 0, res.ExitErr.ExitCode())
+		assert.Contains(t, res.Stderr.String(), "Error: cp: source must be a valid UnixFS (dag-pb or raw codec)")
+	})
+
+	t.Run("files cp with invalid UnixFS data structure fails", func(t *testing.T) {
+		t.Parallel()
+		node := harness.NewT(t).NewNode().Init().StartDaemon()
+
+		// Create an invalid proto file
+		data := []byte{0xDE, 0xAD, 0xBE, 0xEF} // Invalid protobuf data
+		tempFile := filepath.Join(node.Dir, "invalid-proto.bin")
+		err := os.WriteFile(tempFile, data, 0644)
+		require.NoError(t, err)
+
+		res := node.IPFS("block", "put", "--format=raw", tempFile)
+		require.NoError(t, res.Err)
+
+		// we manually changed codec from raw to dag-pb to test "bad dag-pb" scenario
+		cid := "bafybeic7pdbte5heh6u54vszezob3el6exadoiw4wc4ne7ny2x7kvajzkm"
+
+		// should fail because node cant be read as a valid dag-pb
+		cpResNoForce := node.RunIPFS("files", "cp", fmt.Sprintf("/ipfs/%s", cid), "/invalid-proto")
+		assert.NotEqual(t, 0, cpResNoForce.ExitErr.ExitCode())
+		assert.Contains(t, cpResNoForce.Stderr.String(), "Error")
+	})
+
+	t.Run("files cp with raw node succeeds", func(t *testing.T) {
+		t.Parallel()
+		node := harness.NewT(t).NewNode().Init().StartDaemon()
+
+		// Create a raw node
+		data := "raw data"
+		tempFile := filepath.Join(node.Dir, "raw.bin")
+		err := os.WriteFile(tempFile, []byte(data), 0644)
+		require.NoError(t, err)
+
+		res := node.IPFS("block", "put", "--format=raw", tempFile)
+		require.NoError(t, res.Err)
+		cid := res.Stdout.Trimmed()
+
+		// Copy from IPFS to MFS (raw nodes should work without --force)
+		cpRes := node.IPFS("files", "cp", fmt.Sprintf("/ipfs/%s", cid), "/raw-file")
+		assert.NoError(t, cpRes.Err)
+
+		// Verify the file was copied correctly
+		catRes := node.IPFS("files", "read", "/raw-file")
+		assert.Equal(t, data, catRes.Stdout.Trimmed())
+	})
+
+	t.Run("files cp creates intermediate directories with -p", func(t *testing.T) {
+		t.Parallel()
+		node := harness.NewT(t).NewNode().Init().StartDaemon()
+
+		// Create a simple text file and add it to IPFS
+		data := "hello parent directories"
+		tempFile := filepath.Join(node.Dir, "parent-test.txt")
+		err := os.WriteFile(tempFile, []byte(data), 0644)
+		require.NoError(t, err)
+
+		cid := node.IPFS("add", "-Q", tempFile).Stdout.Trimmed()
+
+		// Copy from IPFS to MFS with parent flag
+		res := node.IPFS("files", "cp", "-p", fmt.Sprintf("/ipfs/%s", cid), "/parent/dir/file")
+		assert.NoError(t, res.Err)
+
+		// Verify the file and directories were created
+		lsRes := node.IPFS("files", "ls", "/parent/dir")
+		assert.Contains(t, lsRes.Stdout.String(), "file")
+
+		catRes := node.IPFS("files", "read", "/parent/dir/file")
+		assert.Equal(t, data, catRes.Stdout.Trimmed())
+	})
+}