fix(scope)!: resolve --computer/--mobile-device to an ID and send <id> alone

CHANGELOG.md

@@ -11,6 +11,32 @@ commit types the repo already uses (`feat!`/`build!` for a breaking change).
 
 ## Unreleased
 
+### Breaking — `scope add`/`remove` resolve `--computer` and `--mobile-device` to an ID first
+
+`scope add --computer <UDID>` answered `409 Unable to match computer` on every
+computer-scoped resource: a Mac's UUID-shaped UDID was sent as `<name>`, and a
+matched UDID went out beside an empty `<name>`, which the Classic computer
+matcher reads first and refuses. The `scope add` and `scope remove` commands
+of the eight scopeable Classic resources now look a `--computer` or
+`--mobile-device` value up in inventory — as an ID, name, UDID or serial
+number — and send the device's `<id>` alone. That is the one form both Classic
+matchers resolve unconditionally.
+
+Two things a script can see change:
+
+- **A name shared by more than one device is refused**, naming the matching
+  ids. The Classic API's own name match picked one of them silently.
+- **The API client needs Read Computers or Read Mobile Devices**, because the
+  lookup reads inventory. Without it, a numeric ID that worked before now
+  fails with a 403 (exit 5), and the hint names the privilege.
+
+`scope remove` by serial number now works; before, it could never match,
+because the scope GET carries no serial. A value that no longer names any
+device still removes a member listed under that ID or name.
+
+**Migration.** Pass the numeric ID for a device whose name is not unique, and
+grant the API client Read Computers / Read Mobile Devices.
+
 ### Breaking — device secrets are read from a file, not a flag value
 
 Three flags took a device secret as their value, where `ps`, shell history and

internal/resolve/identifier.go

@@ -0,0 +1,179 @@
+// Copyright 2026, Jamf Software LLC
+
+package resolve
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"net/url"
+	"strings"
+
+	"github.com/Jamf-Concepts/jamf-cli/internal/exitcode"
+	"github.com/Jamf-Concepts/jamf-cli/internal/registry"
+)
+
+// ErrNoDeviceMatch is what a NoDeviceMatchError answers errors.Is with, so a
+// caller can tell "nothing is called that" from a failed lookup.
+var ErrNoDeviceMatch = errors.New("no matching device")
+
+// NoDeviceMatchError reports a value that names no device.
+type NoDeviceMatchError struct {
+	Label string // "computer" / "mobile device"
+	Value string
+}
+
+func (e *NoDeviceMatchError) Error() string {
+	return fmt.Sprintf("no %s found with ID, name, UDID or serial number %q", e.Label, e.Value)
+}
+
+// Is makes errors.Is(err, ErrNoDeviceMatch) hold.
+func (e *NoDeviceMatchError) Is(target error) bool { return target == ErrNoDeviceMatch }
+
+// identifierSpec describes one device family's inventory endpoint for
+// ResolveComputerIdentifier / ResolveMobileDeviceIdentifier.
+type identifierSpec struct {
+	label   string // "computer" / "mobile device"
+	path    string // inventory list path, with the sections parse needs
+	idField string // RSQL field holding the numeric ID
+	// fields are the RSQL fields holding the name, UDID and serial number.
+	nameField, udidField, serialField string
+	parse                             func(map[string]any) (*DeviceIdentifiers, error)
+	// readPrivilege is the Jamf Pro privilege the lookup needs, named in the
+	// hint on a 401 or 403.
+	readPrivilege string
+}
+
+// Both endpoints are published on the platform gateway; the per-ID inventory
+// paths the other resolvers use are not all, so the ID goes through the filter
+// as well.
+var computerIdentifierSpec = identifierSpec{
+	label:         "computer",
+	path:          "/v4/computers-inventory?section=GENERAL&section=HARDWARE",
+	idField:       "id",
+	nameField:     "general.name",
+	udidField:     "udid",
+	serialField:   "hardware.serialNumber",
+	parse:         parseComputerInventory,
+	readPrivilege: "Read Computers",
+}
+
+var mobileIdentifierSpec = identifierSpec{
+	label:         "mobile device",
+	path:          "/v2/mobile-devices/detail?section=GENERAL&section=HARDWARE",
+	idField:       "mobileDeviceId",
+	nameField:     "displayName",
+	udidField:     "udid",
+	serialField:   "serialNumber",
+	parse:         parseMobileDevice,
+	readPrivilege: "Read Mobile Devices",
+}
+
+// identifierPageSize bounds the lookup. One exact identifier names a handful of
+// records at most (names are not unique); anything past this is reported as
+// ambiguous all the same.
+const identifierPageSize = 20
+
+// ResolveComputerIdentifier resolves a value that may be a computer's Jamf Pro
+// ID, name, UDID or serial number to one computer, in a single request.
+//
+// See resolveIdentifier for how a value matching more than one record is
+// handled.
+func ResolveComputerIdentifier(ctx context.Context, client registry.HTTPClient, value string) (*DeviceIdentifiers, error) {
+	return resolveIdentifier(ctx, client, computerIdentifierSpec, value)
+}
+
+// ResolveMobileDeviceIdentifier is ResolveComputerIdentifier for mobile devices.
+func ResolveMobileDeviceIdentifier(ctx context.Context, client registry.HTTPClient, value string) (*DeviceIdentifiers, error) {
+	return resolveIdentifier(ctx, client, mobileIdentifierSpec, value)
+}
+
+// resolveIdentifier ORs the value across every identifier field and then
+// re-checks each result exactly, since RSQL `==` treats `*` as a wildcard.
+//
+// A numeric value that is some record's ID resolves to that record even when
+// another record carries the same digits as its name, because the numeric-ID
+// reading is the CLI's documented contract and the one the Classic API itself
+// applies first. Any other value must name exactly one record: Classic names
+// are not unique, and the Classic API's own name matching would silently pick
+// one of the duplicates.
+func resolveIdentifier(ctx context.Context, client registry.HTTPClient, spec identifierSpec, value string) (*DeviceIdentifiers, error) {
+	value = strings.TrimSpace(value)
+	if value == "" {
+		return nil, fmt.Errorf("empty %s identifier", spec.label)
+	}
+
+	quoted := `"` + EscapeRSQL(value) + `"`
+	clauses := []string{
+		spec.nameField + "==" + quoted,
+		spec.udidField + "==" + quoted,
+		spec.serialField + "==" + quoted,
+	}
+	if isNumericID(value) {
+		clauses = append([]string{spec.idField + "==" + value}, clauses...)
+	}
+	path := fmt.Sprintf("%s&page-size=%d&filter=%s", spec.path, identifierPageSize,
+		url.QueryEscape(strings.Join(clauses, ",")))
+
+	records, _, err := fetchInventoryPage(ctx, client, path)
+	if err != nil {
+		return nil, lookupError(spec, value, err)
+	}
+
+	var matches []*DeviceIdentifiers
+	for _, record := range records {
+		d, err := spec.parse(record)
+		if err != nil {
+			continue
+		}
+		if isNumericID(value) && d.ID == value {
+			return d, nil
+		}
+		if matchedBy(d, value) != "" {
+			matches = append(matches, d)
+		}
+	}
+
+	switch len(matches) {
+	case 0:
+		return nil, &NoDeviceMatchError{Label: spec.label, Value: value}
+	case 1:
+		return matches[0], nil
+	}
+	described := make([]string, len(matches))
+	for i, d := range matches {
+		described[i] = fmt.Sprintf("id %s (%s)", d.ID, matchedBy(d, value))
+	}
+	return nil, fmt.Errorf("%q matches %d %ss: %s; pass the numeric ID of the one you mean",
+		value, len(matches), spec.label, strings.Join(described, ", "))
+}
+
+// lookupError wraps a failed inventory lookup. A 401 or 403 gets a hint naming
+// the read privilege. Resolving a device is a new inventory read, so an API
+// client that could scope a computer by numeric ID before now fails here, and
+// the bare status does not say which privilege is missing.
+func lookupError(spec identifierSpec, value string, err error) error {
+	msg := fmt.Sprintf("looking up %s %q", spec.label, value)
+	var ee *exitcode.Error
+	if !errors.As(err, &ee) || (ee.Code != exitcode.PermissionDenied && ee.Code != exitcode.Authentication) {
+		return fmt.Errorf("%s: %w", msg, err)
+	}
+	hint := fmt.Sprintf("resolving a %s to its ID reads inventory, so the API client needs %s", spec.label, spec.readPrivilege)
+	if ee.Hint != "" {
+		hint += "; " + ee.Hint
+	}
+	return &exitcode.Error{Code: ee.Code, Message: msg, Err: err, Hint: hint, Details: ee.Details}
+}
+
+// matchedBy names the identifier a record matched value on, or "" for none.
+func matchedBy(d *DeviceIdentifiers, value string) string {
+	switch {
+	case strings.EqualFold(d.Name, value):
+		return "name"
+	case strings.EqualFold(d.UDID, value):
+		return "UDID"
+	case strings.EqualFold(d.SerialNumber, value):
+		return "serial number"
+	}
+	return ""
+}

internal/resolve/identifier_test.go

@@ -0,0 +1,182 @@
+// Copyright 2026, Jamf Software LLC
+
+package resolve
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"io"
+	"net/http"
+	"strings"
+	"testing"
+
+	"github.com/Jamf-Concepts/jamf-cli/internal/exitcode"
+	"github.com/Jamf-Concepts/jamf-cli/internal/registry"
+)
+
+func inventoryPage(records ...string) string {
+	return fmt.Sprintf(`{"totalCount":%d,"results":[%s]}`, len(records), strings.Join(records, ","))
+}
+
+// mobileDetailRecord is the /v2/mobile-devices/detail shape as the wire sends
+// it with section=GENERAL&section=HARDWARE: udid and displayName under
+// general, serialNumber under hardware, nothing at the top level but the id.
+func mobileDetailRecord(id, name, udid, serial string) string {
+	return fmt.Sprintf(`{"mobileDeviceId":%q,"general":{"displayName":%q,"udid":%q,"managementId":"m-%s"},"hardware":{"serialNumber":%q}}`,
+		id, name, udid, id, serial)
+}
+
+func TestResolveComputerIdentifier_EachIdentifierKind(t *testing.T) {
+	rec := `{"id":"107","udid":"96050be1-e53b-454d-9752-2306c709f192","general":{"name":"ARMADA-058JG5"},"hardware":{"serialNumber":"FWWT058JG5"}}`
+	for _, value := range []string{"107", "armada-058jg5", "96050BE1-E53B-454D-9752-2306C709F192", "fwwt058jg5"} {
+		client := &mockClient{responses: map[string]mockResponse{"v4/computers-inventory?": {200, inventoryPage(rec)}}}
+		d, err := ResolveComputerIdentifier(context.Background(), client, value)
+		if err != nil {
+			t.Fatalf("%s: %v", value, err)
+		}
+		if d.ID != "107" {
+			t.Errorf("%s: ID = %q, want 107", value, d.ID)
+		}
+		if len(client.calls) != 1 {
+			t.Errorf("%s: want one request, got %v", value, client.calls)
+		}
+	}
+}
+
+func TestResolveComputerIdentifier_FilterORsEveryField(t *testing.T) {
+	client := &mockClient{responses: map[string]mockResponse{"v4/computers-inventory?": {200, inventoryPage(computerRecord("7"))}}}
+	if _, err := ResolveComputerIdentifier(context.Background(), client, "7"); err != nil {
+		t.Fatal(err)
+	}
+	want := `filter=id==7,general.name=="7",udid=="7",hardware.serialNumber=="7"`
+	if !strings.Contains(client.calls[0], want) {
+		t.Errorf("request %q does not carry %s", client.calls[0], want)
+	}
+
+	client = &mockClient{responses: map[string]mockResponse{"v4/computers-inventory?": {200, inventoryPage(computerRecord("7"))}}}
+	_, _ = ResolveComputerIdentifier(context.Background(), client, "SER7")
+	if strings.Contains(client.calls[0], "filter=id==") {
+		t.Errorf("a non-numeric value must not be compared against the numeric id field: %s", client.calls[0])
+	}
+}
+
+// A numeric value that is a record's ID wins over another record that carries
+// the same digits as its name: the numeric reading is the documented contract.
+func TestResolveComputerIdentifier_IDWinsOverNumericName(t *testing.T) {
+	named := `{"id":"9","udid":"u9","general":{"name":"42"},"hardware":{"serialNumber":"S9"}}`
+	client := &mockClient{responses: map[string]mockResponse{"v4/computers-inventory?": {200, inventoryPage(named, computerRecord("42"))}}}
+	d, err := ResolveComputerIdentifier(context.Background(), client, "42")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if d.ID != "42" {
+		t.Errorf("ID = %q, want 42", d.ID)
+	}
+}
+
+func TestResolveComputerIdentifier_DuplicateNameIsRefused(t *testing.T) {
+	a := `{"id":"4","udid":"u4","general":{"name":"FVFZCAK0LYWH"},"hardware":{"serialNumber":"FVFZCAK0LYWH"}}`
+	b := `{"id":"31","udid":"u31","general":{"name":"FVFZCAK0LYWH"},"hardware":{"serialNumber":"OTHER"}}`
+	client := &mockClient{responses: map[string]mockResponse{"v4/computers-inventory?": {200, inventoryPage(a, b)}}}
+	_, err := ResolveComputerIdentifier(context.Background(), client, "FVFZCAK0LYWH")
+	if err == nil {
+		t.Fatal("expected an ambiguity error")
+	}
+	for _, want := range []string{"id 4", "id 31", "numeric ID"} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("error %q does not mention %q", err, want)
+		}
+	}
+	if errors.Is(err, ErrNoDeviceMatch) {
+		t.Error("an ambiguous value is not a missing one")
+	}
+}
+
+// RSQL == treats * as a wildcard, so a server-side match is re-checked exactly.
+func TestResolveComputerIdentifier_WildcardMatchIsNotExact(t *testing.T) {
+	client := &mockClient{responses: map[string]mockResponse{"v4/computers-inventory?": {200, inventoryPage(computerRecord("1"), computerRecord("2"))}}}
+	_, err := ResolveComputerIdentifier(context.Background(), client, "mac-*")
+	if !errors.Is(err, ErrNoDeviceMatch) {
+		t.Fatalf("want ErrNoDeviceMatch, got %v", err)
+	}
+}
+
+func TestResolveComputerIdentifier_NotFoundAndLookupFailure(t *testing.T) {
+	client := &mockClient{responses: map[string]mockResponse{"v4/computers-inventory?": {200, inventoryPage()}}}
+	if _, err := ResolveComputerIdentifier(context.Background(), client, "nope"); !errors.Is(err, ErrNoDeviceMatch) {
+		t.Errorf("want ErrNoDeviceMatch, got %v", err)
+	}
+	client = &mockClient{responses: map[string]mockResponse{"v4/computers-inventory?": {403, `{}`}}}
+	_, err := ResolveComputerIdentifier(context.Background(), client, "nope")
+	if err == nil || errors.Is(err, ErrNoDeviceMatch) {
+		t.Errorf("a failed lookup must not read as no match: %v", err)
+	}
+}
+
+func TestResolveMobileDeviceIdentifier_DetailShape(t *testing.T) {
+	rec := mobileDetailRecord("64", "ARMADA-66B185", "5f3644dc5303edf83f28a8d6070c1e955ba91af1", "GMJR66B185")
+	for _, value := range []string{"64", "armada-66b185", "5F3644DC5303EDF83F28A8D6070C1E955BA91AF1", "gmjr66b185"} {
+		client := &mockClient{responses: map[string]mockResponse{"v2/mobile-devices/detail?": {200, inventoryPage(rec)}}}
+		d, err := ResolveMobileDeviceIdentifier(context.Background(), client, value)
+		if err != nil {
+			t.Fatalf("%s: %v", value, err)
+		}
+		if d.ID != "64" || d.UDID == "" || d.SerialNumber == "" {
+			t.Errorf("%s: got %+v", value, d)
+		}
+		if !strings.Contains(client.calls[0], "section=HARDWARE") {
+			t.Errorf("%s: the serial is only populated with section=HARDWARE: %s", value, client.calls[0])
+		}
+	}
+}
+
+// statusErrClient answers every request the way client.Do answers a non-2xx:
+// with an *exitcode.Error and no response.
+type statusErrClient struct{ err error }
+
+func (c statusErrClient) Do(context.Context, string, string, io.Reader) (*http.Response, error) {
+	return nil, c.err
+}
+
+// A 401 or 403 on the lookup names the read privilege the resolution needs,
+// keeps the exit code, and keeps the hint the client already attached.
+func TestResolveDeviceIdentifier_PermissionFailureNamesThePrivilege(t *testing.T) {
+	cases := []struct {
+		resolve   func(context.Context, registry.HTTPClient, string) (*DeviceIdentifiers, error)
+		code      int
+		privilege string
+	}{
+		{ResolveComputerIdentifier, exitcode.PermissionDenied, "Read Computers"},
+		{ResolveMobileDeviceIdentifier, exitcode.PermissionDenied, "Read Mobile Devices"},
+		{ResolveComputerIdentifier, exitcode.Authentication, "Read Computers"},
+	}
+	for _, tc := range cases {
+		upstream := exitcode.New(tc.code, "permission denied (HTTP 403)").WithHint("upstream hint")
+		_, err := tc.resolve(context.Background(), statusErrClient{upstream}, "5")
+		var ee *exitcode.Error
+		if !errors.As(err, &ee) {
+			t.Fatalf("want an *exitcode.Error, got %T %v", err, err)
+		}
+		if ee.Code != tc.code {
+			t.Errorf("exit code = %d, want %d", ee.Code, tc.code)
+		}
+		if !strings.Contains(ee.Hint, tc.privilege) || !strings.Contains(ee.Hint, "upstream hint") {
+			t.Errorf("hint %q should name %s and keep the upstream hint", ee.Hint, tc.privilege)
+		}
+		if !strings.Contains(err.Error(), `looking up`) || !strings.Contains(err.Error(), "HTTP 403") {
+			t.Errorf("message should say what was looked up and keep the status: %v", err)
+		}
+		if errors.Is(err, ErrNoDeviceMatch) {
+			t.Error("a permission failure must not read as no match")
+		}
+	}
+
+	// Any other failure is wrapped as before, with no privilege hint.
+	other := exitcode.New(exitcode.General, "server error (HTTP 500)")
+	_, err := ResolveComputerIdentifier(context.Background(), statusErrClient{other}, "5")
+	var ee *exitcode.Error
+	if errors.As(err, &ee) && strings.Contains(ee.Hint, "Read Computers") {
+		t.Errorf("a 5xx must not be blamed on a missing privilege: %q", ee.Hint)
+	}
+}