From 56eab758ed8b2b21dc902babf11c635d01fadd3b Mon Sep 17 00:00:00 2001
From: Fabio Bozzo <fabio.bozzo@consensys.net>
Date: Mon, 2 Dec 2024 12:24:06 +0100
Subject: [PATCH] move args int validation to their creation

---
 pkg/args/args.go                |  5 +++
 pkg/args/args_test.go           | 66 +++++++++++++++++++++++++++++++++
 pkg/policy/selector/selector.go |  5 ---
 3 files changed, 71 insertions(+), 5 deletions(-)

diff --git a/pkg/args/args.go b/pkg/args/args.go
index 3cee3ce..8fdb48c 100644
--- a/pkg/args/args.go
+++ b/pkg/args/args.go
@@ -16,6 +16,7 @@ import (
 	"github.com/ipld/go-ipld-prime/node/basicnode"
 	"github.com/ipld/go-ipld-prime/printer"
 
+	"github.com/ucan-wg/go-ucan/pkg/policy/limits"
 	"github.com/ucan-wg/go-ucan/pkg/policy/literal"
 )
 
@@ -62,6 +63,10 @@ func (a *Args) Add(key string, val any) error {
 		return err
 	}
 
+	if err := limits.ValidateIntegerBoundsIPLD(node); err != nil {
+		return fmt.Errorf("value for key %q: %w", key, err)
+	}
+
 	a.Values[key] = node
 	a.Keys = append(a.Keys, key)
 
diff --git a/pkg/args/args_test.go b/pkg/args/args_test.go
index 2a44d0f..938151f 100644
--- a/pkg/args/args_test.go
+++ b/pkg/args/args_test.go
@@ -15,6 +15,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/ucan-wg/go-ucan/pkg/args"
+	"github.com/ucan-wg/go-ucan/pkg/policy/limits"
 	"github.com/ucan-wg/go-ucan/pkg/policy/literal"
 )
 
@@ -185,6 +186,71 @@ func TestInclude(t *testing.T) {
 	}, maps.Collect(a1.Iter()))
 }
 
+func TestArgsIntegerBounds(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name    string
+		key     string
+		val     int64
+		wantErr string
+	}{
+		{
+			name: "valid int",
+			key:  "valid",
+			val:  42,
+		},
+		{
+			name: "max safe integer",
+			key:  "max",
+			val:  limits.MaxInt53,
+		},
+		{
+			name: "min safe integer",
+			key:  "min",
+			val:  limits.MinInt53,
+		},
+		{
+			name:    "exceeds max safe integer",
+			key:     "tooBig",
+			val:     limits.MaxInt53 + 1,
+			wantErr: "exceeds safe integer bounds",
+		},
+		{
+			name:    "below min safe integer",
+			key:     "tooSmall",
+			val:     limits.MinInt53 - 1,
+			wantErr: "exceeds safe integer bounds",
+		},
+		{
+			name:    "duplicate key",
+			key:     "duplicate",
+			val:     42,
+			wantErr: "duplicate key",
+		},
+	}
+
+	a := args.New()
+	require.NoError(t, a.Add("duplicate", 1)) // tests duplicate key
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := a.Add(tt.key, tt.val)
+			if tt.wantErr != "" {
+				require.Error(t, err)
+				require.Contains(t, err.Error(), tt.wantErr)
+			} else {
+				require.NoError(t, err)
+				val, err := a.GetNode(tt.key)
+				require.NoError(t, err)
+				i, err := val.AsInt()
+				require.NoError(t, err)
+				require.Equal(t, tt.val, i)
+			}
+		})
+	}
+}
+
 const (
 	argsSchema = "type Args { String : Any }"
 	argsName   = "Args"
diff --git a/pkg/policy/selector/selector.go b/pkg/policy/selector/selector.go
index 9c3c2ef..249cd44 100644
--- a/pkg/policy/selector/selector.go
+++ b/pkg/policy/selector/selector.go
@@ -10,7 +10,6 @@ import (
 	"github.com/ipld/go-ipld-prime/datamodel"
 	"github.com/ipld/go-ipld-prime/fluent/qp"
 	"github.com/ipld/go-ipld-prime/node/basicnode"
-	"github.com/ucan-wg/go-ucan/pkg/policy/limits"
 )
 
 // Selector describes a UCAN policy selector, as specified here:
@@ -23,10 +22,6 @@ type Selector []segment
 //   - a resolutionerr error if not being able to resolve to a node
 //   - nil and no errors, if the selector couldn't match on an optional segment (with ?).
 func (s Selector) Select(subject ipld.Node) (ipld.Node, error) {
-	if err := limits.ValidateIntegerBoundsIPLD(subject); err != nil {
-		return nil, fmt.Errorf("node contains integer values outside safe bounds: %w", err)
-	}
-
 	return resolve(s, subject, nil)
 }