Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Basic UnixFS sanity checks #10701

Merged
merged 25 commits into from
Mar 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
f8a2fe5
fix::> Added checks for UnixFS + added force flag
PsychoPunkSage Feb 6, 2025
9db46b1
docs: update min requirements (#10687)
lidel Feb 5, 2025
3031cf3
docs(release): update RELEASE_CHECKLIST.md after v0.33.1 (#10697)
lidel Feb 5, 2025
b28bce9
Merge branch 'master' into issue-10331
PsychoPunkSage Feb 6, 2025
e922ef0
fix::> Removed confusing shorthand
PsychoPunkSage Feb 15, 2025
3297308
fix::> impl changes
PsychoPunkSage Feb 15, 2025
4a8bb62
fix::> Basic Setup of Tests
PsychoPunkSage Feb 15, 2025
a687b2b
fix::> test 1
PsychoPunkSage Feb 15, 2025
de94ec6
fix::> test passing
PsychoPunkSage Feb 15, 2025
58e4c90
fix::> formatting
PsychoPunkSage Feb 16, 2025
8e6796f
fix::> Proper test env ready
PsychoPunkSage Feb 16, 2025
fa7c769
fix::> Proper test ready
PsychoPunkSage Feb 16, 2025
1296054
fix::> Test Passing
PsychoPunkSage Feb 19, 2025
888e1e8
Merge branch 'master' into issue-10331
gammazero Feb 20, 2025
8ab8a83
Rename files cp "force" option to "no-validate"
gammazero Feb 20, 2025
d945049
Use require package for more compact test code
gammazero Feb 20, 2025
34f05a2
Rename file_test.go to files_test.go to match the files command
gammazero Feb 20, 2025
b77142d
Update core/commands/files.go
PsychoPunkSage Feb 27, 2025
9f7f8b6
Update core/commands/files.go
PsychoPunkSage Feb 27, 2025
9a59b8d
Update core/commands/files.go
PsychoPunkSage Feb 27, 2025
7ad6847
fix::> Added dedicated tests....
PsychoPunkSage Feb 27, 2025
548f630
fix::> more random tests
PsychoPunkSage Feb 27, 2025
5c93132
refactor: remove --force, simplify checks
lidel Mar 5, 2025
bf04ec1
Merge branch 'master' into issue-10331
lidel Mar 5, 2025
23c8b6e
docs: changelog
lidel Mar 5, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions core/commands/files.go
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,7 @@
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).",
Expand Down Expand Up @@ -480,6 +481,25 @@
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
}

Check warning on line 491 in core/commands/files.go

View check run for this annotation

Codecov / codecov/patch

core/commands/files.go#L490-L491

Added lines #L490 - L491 were not covered by tests
case cid.DagProtobuf:
if _, ok := node.(*dag.ProtoNode); !ok {
return errFilesCpInvalidUnixFS
}

Check warning on line 495 in core/commands/files.go

View check run for this annotation

Codecov / codecov/patch

core/commands/files.go#L494-L495

Added lines #L494 - L495 were not covered by tests
if _, err = ft.FSNodeFromBytes(node.(*dag.ProtoNode).Data()); err != nil {
return fmt.Errorf("%w: %v", errFilesCpInvalidUnixFS, err)
}
default:
return errFilesCpInvalidUnixFS

Check warning on line 500 in core/commands/files.go

View check run for this annotation

Codecov / codecov/patch

core/commands/files.go#L499-L500

Added lines #L499 - L500 were not covered by tests
}

if mkParents {
err := ensureContainingDirectoryExists(nd.FilesRoot, dst, prefix)
if err != nil {
Expand Down
47 changes: 47 additions & 0 deletions core/commands/files_test.go
Original file line number Diff line number Diff line change
@@ -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)")
}
5 changes: 3 additions & 2 deletions docs/changelogs/v0.34.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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

Expand Down
120 changes: 120 additions & 0 deletions test/cli/files_test.go
Original file line number Diff line number Diff line change
@@ -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

Check failure on line 68 in test/cli/files_test.go

View workflow job for this annotation

GitHub Actions / spellcheck

cant ==> can't
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())
})
}
Loading