Skip to content

Add ManagedDeviceTrait / TRAIT_MANAGED_DEVICE device asset trait#995

Merged
pquerna merged 6 commits into
mainfrom
paul.querna/managed-device-trait
Jul 3, 2026
Merged

Add ManagedDeviceTrait / TRAIT_MANAGED_DEVICE device asset trait#995
pquerna merged 6 commits into
mainfrom
paul.querna/managed-device-trait

Conversation

@c1-squire-dev

@c1-squire-dev c1-squire-dev Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a new device asset trait (ManagedDeviceTrait + TRAIT_MANAGED_DEVICE) for MDM/UEM inventory. A managed-device resource represents a managed endpoint (laptop, desktop, mobile, tablet, etc.) enrolled in a device management platform. Like the other asset traits (SecretTrait, AgentTrait, NonHumanIdentityTrait) it carries no grants or entitlements and rides the resource's annotations bag.

This is a foundational SDK change that upcoming device-inventory connectors will depend on.

Changes

Proto (proto/c1/connector/v2/)

  • annotation_trait.proto: new ManagedDeviceTrait message (with DeviceType, ManagementState, Compliance enums) and a DeviceOS message (with OsType enum). Nullable booleans use google.protobuf.BoolValue; assigned_user cross-links to the synced user via ResourceId, mirroring AgentTrait.identity_resource_id.
  • resource.proto: TRAIT_MANAGED_DEVICE = 12 added to ResourceType.Trait (next free value; existing values untouched — reserve-not-reuse).

Field layout follows the OCSF device taxonomy (type_id, model, vendor_name, os). No last-seen field in v1 (deliberately omitted).

Generated (pb/c1/connector/v2/)

  • Regenerated *.pb.go / *_protoopaque.pb.go / *.pb.validate.go via buf generate (not hand-edited).

Go helpers (pkg/types/resource/)

  • New managed_device_trait.go: ManagedDeviceTraitOption, With* builders for every field, and NewManagedDeviceTrait(...) — mirrors secret_trait.go.
  • resource.go: WithManagedDeviceTrait(...) and NewManagedDeviceResource(...) — mirror WithSecretTrait / NewSecretResource.

Verification

  • buf lint — clean
  • buf breaking --against main — clean (additive only)
  • buf generate — reproducible, regenerated files committed
  • go build ./... — pass
  • go test ./... — pass
  • gofmt + golangci-lint run ./pkg/types/resource/... — 0 issues

Notes

Draft — do not merge. Human review + merge intended. Downstream connectors will pin to this branch until it lands on main.

Add a new device asset trait for MDM/UEM inventory. A managed-device
resource represents a managed endpoint (laptop, desktop, mobile, etc.)
enrolled in a device management platform. Like other asset traits it
carries no grants or entitlements and rides the resource annotations bag.

- proto: add ManagedDeviceTrait + DeviceOS messages to
  annotation_trait.proto, and TRAIT_MANAGED_DEVICE = 12 (next free value)
  to ResourceType.Trait.
- regenerate pb/ via buf generate.
- add pkg/types/resource/managed_device_trait.go with option builders and
  NewManagedDeviceTrait, mirroring secret_trait.go.
- add WithManagedDeviceTrait and NewManagedDeviceResource to resource.go.

Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
}

// NewManagedDeviceTrait creates a new `ManagedDeviceTrait` with the given options.
func NewManagedDeviceTrait(opts ...ManagedDeviceTraitOption) (*v2.ManagedDeviceTrait, error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: No test file accompanies these new helpers. Comparable recent additive traits (license_trait_test.go, vendor_trait_test.go, vendor_agreement_trait_test.go) each have coverage. Consider adding a managed_device_trait_test.go that round-trips a device through NewManagedDeviceResource and re-extracts the ManagedDeviceTrait from the annotations bag, to lock in the annotation-packing behavior downstream connectors will rely on. (confidence: high)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 87bfe5d: added pkg/types/resource/managed_device_trait_test.go. It round-trips a device through NewManagedDeviceResource and re-extracts the ManagedDeviceTrait from the annotations bag, plus covers full-population option setters, tri-state BoolValue nullability (unset stays nil; explicit false is wrapped, not dropped), WithManagedDeviceTrait merge semantics, and the defined_only enum validate rules on the trait enums and DeviceOS. go test ./pkg/types/resource/... green (131 tests).

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

General PR Review: Add ManagedDeviceTrait / TRAIT_MANAGED_DEVICE device asset trait

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 23465ff.
Review mode: incremental since 18dabed
View review run

Review Summary

The new commit removes the pre-release profile google.protobuf.Struct field (number 14) from the newly-added ManagedDeviceTrait, dropping it from the proto, the regenerated pb.go/opaque/validate output, and the WithManagedDeviceProfile option (plus its now-unused structpb import). Because ManagedDeviceTrait is introduced in this same PR and has never been released, removing field 14 carries no wire- or serialized-state-compatibility risk, and the generated artifacts are consistent with the source change. The full PR diff was scanned for security and correctness (including the dropped generated pb.go paths); no new issues were found. The prior suggestion to add a GetManagedDeviceTrait extractor for API consistency remains open but was already raised and is not re-flagged here.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

Addresses the CI review suggestion on PR #995: comparable additive
traits (license/vendor/vendor_agreement) carry test coverage while the
new ManagedDeviceTrait helpers did not.

Covers full-population option round-trip, tri-state BoolValue nullability
(unset stays nil; explicit false is wrapped, not dropped), the
NewManagedDeviceResource -> annotations-bag -> re-extract path that
downstream device-inventory connectors depend on, WithManagedDeviceTrait
merge semantics, and the defined_only enum validate rules on the trait
enums and DeviceOS.

Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

Comment thread proto/c1/connector/v2/annotation_trait.proto
string model = 5; // OCSF device.model
string vendor = 6; // OCSF device.vendor_name
DeviceOS os = 7; // OCSF device.os
connector.v2.ResourceId assigned_user = 8; // cross-link to the synced user

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be expressed as a grant? A user can have their device ownership revoked, or ownership can be granted to another device. It might also be a many-to-many relationship in some orgs.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conceptually form a Managed Device POV -- its not a grant. It's truly an Assignment to a Single user. Not a single Managed Device Provider allows multiple assignments from what i can research

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reworked per this thread: device→user is now modeled as a grant, not a scalar field. We removed assigned_user from ManagedDeviceTrait; connectors emit an assigned entitlement on the device + a grant to the user principal. This gives revocation and the many-to-many case (e.g. Intune shared devices), and — the deciding factor — it's the only way to link a device to an owner that lives in a separate directory (Entra/Okta): baton's ExternalResourceMatch (email/UPN) resolution is grant-only, so a trait ResourceId field structurally can't reference an unsynced external user. Thanks for pushing on this.

…nagedDeviceTrait

Device→user assignment is modeled connector-side as a grant, not a scalar
trait field. Remove assigned_user (field 8) from the unreleased
ManagedDeviceTrait, its WithManagedDeviceAssignedUser helper, and the
round-trip test assertions. Field number 8 is left as a gap (no reservation
needed — the trait is unreleased). Regenerated pb Go.

Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Comment thread proto/c1/connector/v2/annotation_trait.proto Outdated

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

@pquerna pquerna marked this pull request as ready for review July 2, 2026 21:01
@pquerna pquerna requested a review from a team July 2, 2026 21:01
…moved assigned_user)

Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
google.protobuf.BoolValue is_personal = 12;
ManagementState management_state = 13 [(validate.rules).enum = {defined_only: true}];
google.protobuf.Timestamp enrolled_at = 14;
google.protobuf.Struct profile = 15;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the 7th trait with a profile struct. We really should put this on the resource to prevent duplicate profiles when composing traits (since uplift only pays attention to one of them).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — and you're right about the mechanism: I checked pkg/mapper/resource/uplift.go (findAnnotationTraitProfile) and uplift returns the first trait with a non-nil profile, so a resource composing multiple profile-bearing traits does drop the others.

We'd like to keep profile on the trait here for now, for two reasons:

  1. It's really a separate, broader problem. The right fix is about uplift's one-profile behavior and/or a resource-level profile — which touches all ~7 traits and v2.Resource (no profile field today). That deserves its own change rather than being back-doored by dropping the field from this one new trait.
  2. The per-trait profile doubles as a namespace. Each trait owns its own profile keys, so composing traits can't collide on key names; a single shared resource profile would force every trait's keys into one namespace.

For managed devices the collision is also low-risk in practice — a device resource is effectively single-trait. So we'll keep it consistent with the existing traits here and track the resource-level-profile / profile-merge fix as its own item. Thanks for pushing on it.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

Comment thread proto/c1/connector/v2/annotation_trait.proto
Comment thread proto/c1/connector/v2/annotation_trait.proto
// managed endpoint (laptop, desktop, mobile, etc.) enrolled in a device
// management platform. Like other asset traits it carries no grants or
// entitlements; it rides the resource's annotations bag.
message ManagedDeviceTrait {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last thing I noticed: The comment should probably updated now that this can have grants. Also a nitpick: Should the trait name be DeviceTrait since it can show both managed & unmanaged devices (eg MANAGEMENT_STATE_UNMANAGED)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things:

  • Doc-comment: agreed — I'll update it to note that device→user assignment is now expressed as a grant (an assigned entitlement on the device), since the inline assigned_user field was removed.
  • Name: we'll keep ManagedDeviceTrait. The trait captures a device's posture as reported by a device-management source (MDM/UEM), and MANAGEMENT_STATE_UNMANAGED is one of those posture states — a device the management provider knows about but that isn't currently enrolled/managed — rather than a device with no management context at all. Keeping it scoped/namespaced to the managed-device domain also matches the product framing (device trust / MDM). So the name stays.

Appreciate the thorough review — genuinely useful pass.

…liance error vs non-compliant)

Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

…precated)

The per-trait profile Struct is being deprecated across traits; the new
ManagedDeviceTrait should not add another. profile was the last field (14),
so removal leaves fields 1-13 contiguous.

Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

@pquerna pquerna merged commit aba5768 into main Jul 3, 2026
10 checks passed
@pquerna pquerna deleted the paul.querna/managed-device-trait branch July 3, 2026 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants