mirror of
https://github.com/mattermost/mattermost.git
synced 2026-11-03 12:48:37 -06:00
9ef793301a
* Split the ABAC attribute matview by object type ABAC reads custom attributes through a materialized view, AttributeView, whose rows are keyed by target id and whose attributes are a jsonb object keyed by property-field name. That view is filtered to user-typed fields, so it can only ever describe a user. Replace it with two views of identical shape, UserAttributeView and ChannelAttributeView, differing only in the pf.ObjectType they filter on, so a channel can carry attributes of its own — and can carry a field with the same name as a user field without the two colliding in a single keyed-by-name row. Nothing reads the channel view yet; this lays the schema and the store seam. - Migration 000212: drop AttributeView, create the two per-object-type views. The down migration restores the combined user-scoped view exactly as migration 000200 defined it, rank branch included. - AttributesStore.GetSubject takes an objectType and selects the view through attributeViewFor. An unrecognized type is an error rather than a fallback to the user view: a silent coercion would surface a miswired caller as a wrong-but-successful user lookup instead of a failure. Because the view is now chosen per call, the shared select builder carries only its Select clause. - RefreshAttributes refreshes both views on one cadence, so a caller sees a consistent snapshot without knowing which object types it is about to read. Refreshing only the changed view is a scale follow-up. - SearchUsers, GetChannelMembersToRemove and GetTeamMembersToRemove pin their joins to UserAttributeView; they only ever matched users. BuildAccessControlSubject passes PropertyFieldObjectTypeUser. - store.Store's interface change ripples into the generated retrylayer, timerlayer and mockery mock, regenerated with `go generate ./channels/store` and mockery v2.53.4. - Retarget the comments that named the dropped AttributeView, in the simulator redaction helpers and in refreshAttributeViewIfStale, at the views that now exist. Also correct a stale doc comment on applyFieldReadAccessControl, which described a user-editable passthrough and a source-only-versus-unknown split that the code does not implement. Tests: migration_000212_test asserts the view topology, that each view filters to its own ObjectType, and that down/up round-trips; migration_000185_test follows the renamed views. storetest gains a channel-subject case — a channel row is readable through the channel view and invisible through the user view — and pins the empty-multiselect contract: the view builds a multiselect with jsonb_agg over joined option rows, and jsonb_agg over zero rows yields SQL NULL, never '[]'. That contract was an untested side effect of the view definition, so a refactor wrapping it in COALESCE(..., '[]') would have flipped an empty tag set from failing an "in" rule to satisfying it, with the suite still green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Carry a resource channel id on SubjectSearchOptions An ABAC rule can reference `resource.attributes.<field>` — the accessed channel's own custom attribute values. Resolving such a reference needs to know which channel, and for the stored-policy search paths that is implicit: the policy names its resource. The ad-hoc expression test (`cel/test`) has no policy, so it has nothing to resolve against. Add ResourceID to SubjectSearchOptions to carry that channel explicitly. Doing it as a search option rather than widening the access-control service and policy administration interfaces keeps a single optional value out of those signatures and their generated mocks. Nothing sets the field yet — the endpoint that will supply it is not wired here, and the search paths that consume it live in the access control engine. This just adds the seam. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Mask channel attribute values and redact them in the simulator Two masking systems keep attribute values a caller is not entitled to see out of the ABAC policy API: attribute-value masking, which blanks rule literals and simulator expected/actual values on the policy read, save, validate and merge paths; and the simulator's visibility redaction, which strips ActualValues from the evaluation tree. Both understood only `user.attributes.*`. A policy rule can now also reference the accessed channel's own attributes as `resource.attributes.<field>`, so a channel admin could read a hidden channel value straight out of a policy GET or a simulation trace. Attribute-value masking is now resolved per CPA object type. model.MaskingFieldResolver.Resolve takes the object type alongside the field name, because a user field and a channel field may share a name and differ in visibility, so each must be resolved against its own schema. Resolving a channel attribute needs two different fields, which is the subtle part. A user never holds channel-side values, so the caller's *holdings* — the option names they can see — have to be read from the user-side field linked to the same template field; linked fields copy the template's options verbatim at creation, so the option IDs match. The *access mode*, however, is read from the channel field itself: a linked field does not inherit its access mode from the template, so taking it from the sibling could silently under-protect a channel attribute whose user sibling is public. New holdingsFieldFor centralises that pairing and is shared by the CEL-walker resolver and the visual-AST path. It fails closed at every gap: an unlinked channel field or one with no user sibling falls back to its own caller-filtered field (which for a protected field yields no visible values); a protected channel field whose sibling is not itself protected has the sibling's unfiltered option list dropped rather than trusted; and two user fields linking the same template is an error rather than a guess, since nothing enforces that uniqueness in the database and picking the wrong sibling could disclose a value. An unrecognised object type errors instead of falling back to a user lookup. The simulator's redactor now tracks protected field names per attribute root (protectedCPAAttributes) and matches each evaluation-tree leaf against the set for its own root. The top-level Attributes maps carry only the simulated user's snapshot, so they are still pruned against the user set alone. Also corrects the two self-match guard comments on the expression-test paths. They claimed the check stops an admin from discovering who matches a rule they do not match themselves; it is a speed bump, not a boundary — an admin can OR in a term they always satisfy and read the match count. What actually protects the values is the masking above, which the comments now say. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Return channel attribute fields from the ABAC autocomplete endpoint An access control rule compares the requesting user's attributes (user.attributes.*) against the accessed channel's (resource.attributes.*). Those are custom attribute fields of two different object types, and GET /access_control_policies/cel/autocomplete/fields only ever returned the user-object-type ones — so an editor had no way to discover a channel field to author a resource.attributes.* comparison against. Select the searched object types per request instead of hardcoding "user": - A channelId in scope adds the channel object type. The handler already parsed and permission-checked channelId; it now forwards it. - include_resource_fields=true adds the channel object type with no channelId. A policy that many channels import has no single channel to scope by, so scoping is not available to its editor; with no channelId the handler's existing check requires ManageSystem or team-admin access-rule permission on team_id. Each returned field already carries its ObjectType, so a caller can tell the two namespaces apart. Native user attributes stay user-only — there are no native channel attributes. Also forward cel/test's channelId into SubjectSearchOptions.ResourceID, so an ad-hoc expression test resolves resource.attributes.* against that channel's values instead of finding them unset. On the client, Client4.getAccessControlFields, the redux action and the useChannelAccessControlActions hook take an optional includeResourceFields, and CHANNEL_ATTRIBUTES_OBJECT_TYPE names the object_type value a caller compares against. Nothing in the System Console asks for the flag yet: this is the API and client surface the policy editors need before they can offer channel attributes as comparison targets. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Author resource.attributes.* rules in the ABAC policy editors An access rule compares the requesting user's attributes against literal values: user.attributes.clearance == "secret". The engine now also resolves resource.attributes.*, the accessed channel's own attributes, so one rule can express "the user's clearance must be at least the channel's classification" without naming a value. Nothing in the System Console could write such a rule; this adds it to both editors. Table editor: the left side of a row stays the requesting user's attribute, and the right-hand side becomes one consolidated dropdown with two sections, Values and Channel attributes. Picking a literal keeps the row in literal mode; picking a channel field switches it to a resource.attributes.<name> target and clears the literals, since the two are mutually exclusive. Only type-compatible channel fields are offered - the same field type, and for option-based types (select/multiselect/rank) the same option scale, enforced structurally by both sides linking to one template field - so an author cannot build a comparison the server rejects at save. Multiselect rows offer "has any of" / "has all of", which the engine holds verbatim as a member call on the user's list. Advanced CEL editor: resource.attributes.* is declared as an autocomplete root, but only when channel fields are in scope, so an editor that cannot reference a channel (a team policy) is not offered an empty root. The help modal documents the two namespaces and the deny-on-miss rule - a channel missing an attribute its policy references denies access until the attribute is set. Both system-console editors request channel fields explicitly (getAccessControlFields with include_resource_fields), since neither has a single channel to scope the autocomplete by, then split the result by object_type: user fields drive the left picker and the user.attributes.* autocomplete, channel fields back resource.attributes.*. Channel fields are comparison targets rather than standalone rules, so they no longer count toward "this policy has no usable attributes". Also adds the missing server string for the error the access-control simulator returns when a resource-referencing policy is submitted without a channel. Testing such a rule needs a concrete channel to resolve against, and the test button still only uses the channel an editor is already scoped to. Picking one is a follow-up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Add a channel-picker step to the ABAC test modal The test modal takes an expression and lists the users it matches. A rule that compares against resource.attributes.* - the accessed channel's own attributes - cannot be evaluated that way: the server needs a concrete channel to read those values from, and without one it fails to resolve the resource side and the modal reports no matching users. A channel-settings editor already has a channel to use, but the system-console policy editors do not, which is exactly where such rules are authored. The modal now opens on a channel-picker step when it is told a channel is required, and threads the chosen id through the members search. The picker searches public and private channels (access policies apply to both, and both carry channel attributes), debounces its input, keeps a request generation counter so a slow earlier search cannot overwrite a newer one's results, and surfaces a search failure as an error rather than as "No channels found" - the latter reads as "nothing matches" and invites the admin to give up instead of retrying. Choosing a channel advances to the members list with a back arrow in the header; a modal that needs no channel opens straight to the members list and is unchanged. Deciding whether a channel is needed is now shared rather than left to each editor: both the table editor and the advanced CEL editor render one TestResults wrapper that checks the expression for a resource.attributes.* reference and asks for the picker only when the editor has no channel of its own. The check ignores quoted string literals, so a value that merely spells out "resource.attributes.something" does not force a pointless picker step. The picked channel takes precedence over an editor's own scope all the way down to the plugin-facing searchUsers override, which grows a trailing channel-id argument. An override that resolves its own channel can ignore it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Add Classification Enforcement and a linked Clearance user attribute The Classification Markings page defines a template property field whose ranked options are the install's classification levels, and links a channel field to that template so channels can be marked. There was no user-side counterpart, so an admin had nothing to compare a channel's marking against. Add a Classification Enforcement section to the page, shown when attribute-based access control is enabled. Its checkbox creates — or, when unchecked, deletes — a "Clearance" user property field linked to the same template, so it inherits the level scale wholesale rather than restating it. The field is admin-managed and admin-only for values and options: clearance is granted, not self-declared. Creation is deferred to save because a linked field needs the template to exist first, and the save path re-fetches before creating so a slow initial load cannot produce a duplicate. Disabling classification markings deletes the clearance field between the channel field and the template, which the server's deletion protection requires. The section gates nothing by itself. An admin still writes a membership policy comparing user.attributes.Clearance against the channel's classification level; the section's help text links to that page. Also lock the type and the option values on any template-linked user attribute in System Attributes. The server rejects both changes on a linked field — it propagates the template's options to dependents instead — so the screen was offering edits that could only fail: the type menu, the values cell, the rank-value chips, and the dot menu's Edit ranking, AD/LDAP and SAML items (the latter two coerce a field to text). Each locked menu item says why. Duplicating a linked attribute now drops the link, since the create request carries only name, type and attrs and a new field is therefore never linked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Add resource.attributes.* end-to-end coverage An ABAC membership policy can compare the requesting user's attribute against the accessed channel's attribute (user.attributes.<a> == resource.attributes.<a>), but the coverage of that path so far is all unit-level: the api4 tests mock the access-control engine, and the engine's own tests run below the HTTP boundary. Nothing exercises save-time validation, the membership sync job and the runtime decision point together against one real server. Add a Playwright suite under specs/functional/system_console/abac/resource_attributes/: - authoring.spec.ts — a parent policy mixing user and channel attributes saves; a has(resource.attributes.*) guard is rejected at check time; a parent that references channel attributes cannot be assigned to a team. - membership_sync.spec.ts — the SQL sync lane and the runtime decision point agree on the same fixtures for a scalar equality rule, and a channel that never sets the referenced attribute denies and is emptied. - multiselect_sync.spec.ts — the same lane agreement for hasAnyOf / hasAllOf over a user field and a channel field sharing one option scale. - masking.spec.ts — a policy whose condition still carries the masked-value sentinel is rejected on the write path. - test_picker.spec.ts — the test modal's channel-picker step resolves a rule that has no channel scope of its own against the picked channel. - helpers.ts — provisions the resource side (channel-object-type attributes in the access_control group, per-channel values, and a shared multiselect template) and authors policies over the REST API, so a spec exercising the resource side does not also have to drive the policy editor. Scope is explicit per-channel assignment only: every spec puts its channel under the parent policy through the /assign endpoint. Also adds fillSingleConditionValue() to the shared abac support helpers. The table editor's value cell renders an inline input only for an attribute with no comparable channel-attribute targets; once channel attributes exist it becomes a dropdown whose value field is revealed by opening the menu. Channel attributes are system-wide, so any spec filling a single-value condition can now meet either shape — createBasicPolicy and the display_name_in_selector spec both route through the helper rather than waiting on the inline input. These specs need a licensed Enterprise server with the access-control engine and its sync jobs, which this environment does not provide, so they have not been executed. eslint, prettier, tsc and the test-doc linter pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Fix RefreshAttributes' comment about snapshot consistency RefreshAttributes issues one REFRESH MATERIALIZED VIEW per object-type attribute view (migration 000212 split the single view in two). Its comment claimed that because both refresh on one cadence, every caller sees a consistent snapshot. They refresh in separate statements that commit independently, so a read straddling a refresh can see UserAttributeView and ChannelAttributeView a generation apart. No behavior change, and none needed: every consumer already tolerates stale attribute values, and the membership sync re-runs on cadence and converges. The guarantee was never required, only mis-stated. Describe what the code actually does, since this path feeds access-control decisions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Make the ABAC e2e value helper work in the dropdown variant fillSingleConditionValue fills the value cell of a rule row in the access-policy table editor. That cell renders two ways: single_value_selector_menu.tsx shows a plain inline input when the attribute has no options and no comparable channel attribute exists, and a dropdown otherwise. The helper already branched on both, but its dropdown path could never succeed: - It waited for input[placeholder*="Add value"]. The menu autofocuses that input, and widgets/inputs/input/input.tsx only sets a placeholder attribute while the input is unfocused -- focused, the text moves into the floating legend. The element therefore never matched. Match the accessible name instead, which input.tsx sets from the same string either way. - The click that opens the menu is swallowed when a sibling menu is still closing, which happens when the caller selects an operator immediately before. Callers that paused first never hit this. Retry the click once, gated on a waitFor rather than isVisible -- the latter returns immediately and would toggle the freshly opened menu shut. Only the inline variant was exercised until channel attributes became something a test server can carry, since they are system-wide: any spec that creates one switches every later value cell to the dropdown. That made four specs fail on a shared server -- MM-T5794, MM-T5795, MM-T5796 and the display_name attribute selector spec -- all of which pass with this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Hide the informational-only notice while clearance enforcement is on The Classification Markings page opens with a standing notice: "Classification markings are informational only / Markings are not tied to access control decisions at this time and are for display purposes only." That is no longer unconditionally true. The Classification Enforcement section on the same page creates a ranked Clearance user attribute linked to these levels, and a membership policy comparing it against a channel's classification does decide who may be a member. The notice and the section immediately beneath it therefore contradicted each other whenever the clearance attribute was enabled. Render the notice only while clearanceEnabled is false. The wording still holds in that state -- nothing enforces the levels -- so no copy change is needed, and an admin who turns enforcement off sees the notice return. Gated on the live checkbox state rather than the persisted one, matching how the rest of the page reveals and hides sections before a save. Covered by a test asserting the notice is absent when a linked Clearance field already exists, and back once the checkbox is cleared. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Gate channel attributes in access rules behind a feature flag Access rules that compare a user's attributes against the accessed channel's ship dark, behind ResourceAttributesInPolicies (default false). The gate is a single choke point: GetAccessControlFieldsAutocomplete stops offering channel-object-type fields. Every authoring surface is driven by what that endpoint returns, so with the flag off the value column has no channel targets to list, the advanced editor has nothing to autocomplete under resource.attributes, and the test dialog never needs to ask for a channel -- none of which needs its own check. The engine refuses such a rule at save time as well, which is the boundary that matters for an API client. Nothing is gated on the Classification Markings page. The clearance attribute is useful on its own: a rule comparing it against a literal level is ordinary user-attribute access control, and the channel-side classification field belongs to the per-channel banner feature, which predates this. Authoring only, deliberately. A rule stored while the flag was on keeps being evaluated, because such a rule denies for any channel that has not set a value -- dropping enforcement on a flag flip would remove every member of every channel the policy governs. Turning the feature off is not a way to stop enforcing; editing the policy is. The flag is checked without re-checking the licence or the ABAC config setting. Those are enforced by the surfaces that need them, ABAC is unusable without both, and re-checking them in the autocomplete would make it stricter for channel fields than it is for user fields today. Two notes for anyone extending the tests. A feature flag set through UpdateConfig is discarded unless the config store's read-only guard is cleared first, which neither Setup() nor SetupEnterprise() does unless handed a config updater -- invisible until now because the flags these suites set already defaulted to true. And a flag cannot be set through the config API at all, so the e2e server takes it from the environment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Move property field options into their own table A select-style property field (select, multiselect, rank) kept its list of selectable options inside its own Attrs JSON blob, under an "options" key. That makes an individual option unaddressable: it cannot be paged, counted, or pointed at by a row-level relationship, and every edit rewrites the whole array. It also forced a linked field to hold a byte-identical copy of the options belonging to the template it links to, kept in step by a propagating UPDATE on every write. Migration 000213 creates PropertyOptions, backfills a row per option, strips the blob key, and redefines both attribute materialized views to resolve option names from the rows. The store reads and writes the table, and a field's option list is now derived rather than copied: its effective options are the ones it owns plus the ones owned by the field named in its LinkedFieldID, unioned on every read. The copy-propagating UPDATE is gone. A template option edit is therefore visible through every linked field immediately, with no write to any of them. Nothing outside the store can tell. Reads re-coalesce the option list into Attrs["options"] in the shape it was written in, so REST and plugin responses are unchanged and every in-memory consumer of the blob keeps working. The columns exist to make that reconstruction exact: id, name, color, and rank become columns when the JSON value has the matching type, everything else on the option object is kept verbatim in the option's own Attrs, and a NULL column means the key was absent -- so an option that never carried a color does not grow one. SortOrder holds the option's position in the array, because clients render options in array order and that order is data. Above a hard-coded 1,000 options a field serializes with the list left out and replaced by options_count and options_omitted, so a field with a large option set still lists instead of erroring. Consumers that need to tell an omitted list from an empty one have to check options_omitted; auditing the existing readers for that is follow-up work, not done here. Two things the option list allowed that a table cannot, both found in existing data and both handled without failing the backfill: - An option ID only had to be unique within one field, and two unrelated fields could use the same one. The primary key is therefore (FieldID, ID) rather than ID alone. This is also what unlinking a field from its template needs: the field takes over the options it was deriving, under the IDs its property values already point at, while the template keeps its own. - Nothing rejected an array holding the same ID twice, or an option with no ID or no name at all. A second occurrence of an ID and a missing ID are minted one, and Name is nullable. The migration reports every option it could not carry over under the ID it had, and every option on a linked field that its template did not have, as warnings in the PostgreSQL server log. Both UserAttributeView and ChannelAttributeView had to be redefined in the same migration -- they resolved option names out of the blob, which is now empty -- and their lookups are field-scoped to the effective option set, because a linked field no longer holds a copy of the options it serves. Each type's projection is otherwise untouched: select yields the option name, multiselect an array of names in value order, rank an object of name and rank. This is larger than one commit would normally carry, and does not split. The migration that strips the blob, the store code that reads the rows, and the view redefinition that depends on them cannot land separately without an intermediate tree where reads and writes disagree about where options live. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Handle withheld option lists in property field consumers A select-style property field's options are rows in PropertyOptions, inlined into the field's Attrs["options"] when it is read. Above model.PropertyFieldMaxHydratedOptions the list is left out and replaced by options_count and options_omitted, so a field with a very large option set still lists instead of erroring. The options key is then absent -- exactly as it is for a field with no options at all -- and every consumer that reads an absent list as "this field has none" quietly does the wrong thing. On a read-masking path that is a leak or an over-hide; on a write path it is deleted options. model gains the single definition of the distinction, PropertyFieldOptionsOmitted, and PropertyField.HideOptions: the fail-secure operation the masking paths need, which empties the list *and* drops the count, because on a shared_only field the number of options is itself controlled information. Read masking. filterSharedOnlyFieldOptions and filterSharedOnlyRankFieldOptions returned an oversized field untouched, handing an unentitled caller its option count. Both now hide everything instead. The source-only default and the masking resolver's fail-closed copy (fieldWithEmptyOptions) drop the count too. buildOptionRankMap still returns an empty map, which every caller already reads as "the caller holds no clearance" and answers by hiding -- the withheld case is now its own branch so that stays deliberate rather than incidental. Writes. PropertyField.Patch drops the two keys when a patch supplies a non-empty option list: the caller supplied the list, so it is authoritative, and leaving the keys on makes the store skip option reconciliation and silently ignore it. A supplied *empty* list keeps them -- that is a caller echoing back the list it was never given, and honouring it would delete every option the field has. optionsChanged, the guard that refuses option edits on a linked field, now refuses anything it cannot verify. sanitizeAndValidateFieldAttrs drops the two keys along with the options when a field is converted to a type that has none, so a former select field does not read back claiming its non-existent options were withheld. MigrateBackfillCPADisplayName no longer round-trips a stored field through model.CPAField. CPAField holds a fixed set of typed attrs, so ToPropertyField rebuilds the whole blob from them and drops the withheld marker with it; the backfill's write then reads as "this field now has no options" and soft-deletes every one. It sets the display_name attr on the PropertyField in place instead, and ToPropertyField is documented as unsafe for that round trip. Consumers where the cap is unreachable, or where the existing behaviour is already the fail-secure one, are marked as such rather than changed: IsValidSessionAttributeValue (the seeded session attribute schema is fixed, six options at most, and an API caller cannot edit an options list), mergeBoardsStatusColors (three seeded options, and its early return preserves the marker so the store leaves them alone), propertyFieldOptionsEqual (nil compares equal to nil and unequal to any supplied list, which is exactly what its guard wants), and extractVisibleOptionNames (no options key yields no visible names, so every condition value masks). One consumer fails secure but is a dead end. extractOptionIDs cannot check a value against a list it does not have, so every value assignment to a field above the cap is refused; it now says so rather than reporting the option the caller sent as non-existent. Making such a field usable needs an existence check against the option rows rather than against the inlined list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Refuse an option list supplied for a field whose options were not loaded A property field with more than model.PropertyFieldMaxHydratedOptions options reads back with its option list left out, replaced by options_count and options_omitted. An editor that reads such a field therefore holds no option list, and anything it sends back as "the options" is built on nothing. The admin console's add-an-option handler spreads the attrs it read and appends to `options ?? []`, so on a field with 1,001 options it produces a list of one -- a request that reads as "delete the other thousand" but that nobody asked for. The store already declines to reconcile options for a field carrying options_omitted, so that request was silently doing nothing: a 200 for a change that did not happen. Neither obeying nor ignoring it is right, so UpdatePropertyFields now refuses it, with a message that says why. Editing the options of a field this large needs an interface that addresses one option at a time. A supplied *empty* list is not refused. That is what a read-modify-write of any other attr looks like once a client normalises the absent list it was given, and refusing it would leave a field this large unrenamable; the store leaves its options alone as before. Also checked before the PSAv1 skip, since a legacy field's options are as destroyable as a current one's. model.PropertyFieldSuppliesOptions draws that line -- a non-empty list of any slice shape is an assertion about the field's options, an absent or empty one is not. It replaces the two narrower guards the same problem had grown: optionsChanged no longer needs a withheld-options branch, because a supplied list never reaches it now, and PropertyField.Patch no longer edits the withheld markers, because whether a supplied list may be applied is a question only the service can answer -- it is the one that can see the stored field. The webapp types record the contract: options_count and options_omitted beside options on SelectPropertyField, and a note that an editor holding no list must not send one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Run the resource-attribute e2e specs against a server that has the flag on All five specs under abac/resource_attributes failed in CI, every one of them at createParentPolicyViaAPI with "Unable to save access control policy" -- and the sixth, masking.spec.ts, passed for the wrong reason, since it asserts only that a save is refused. The cause is which harness the Playwright suite runs on. The flag was added to e2e-tests/.ci/server.generate.sh, which builds the docker-compose stack the Cypress suite uses; the Playwright suite starts its server through testcontainers, whose MM_* environment comes from SERVER_ENV_BASELINE. The CI server logged "ResourceAttributesInPolicies = false" and refused every rule naming a channel attribute. Add the flag to the baseline so both harnesses agree. Each spec now also guards with skipIfFeatureFlagNotSet. A feature flag cannot be turned on through the config API -- the config store restores flags on write -- so a server started without the environment variable can only skip, and skipping is the honest outcome for a precondition the spec cannot establish itself. That is what the helpers docstring already told the reader would happen; now it does. Separately, the three runtime-enforcement checks swallowed any error from addToChannel and then asserted the user was absent from the channel. Both halves were weak: a bare catch also passes on a 500 or a transport fault, and the absence check passes on its own because the sync job removed the user seconds earlier, so neither half actually established that the PDP refused the add. They now go through expectAddToChannelDenied, which asserts the 403 and the api.channel.add_user.to.channel.rejected id the ABAC branch of AddUserToChannel returns. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Keep the table editor's row mutators pure addRow, removeRow, updateRowAttribute, updateRowOperator, updateRowValues and updateRowTarget each moved their work inside the setRows updater callback, calling updateExpression -- which invokes onChange and onValidate and flips the isInternalChange ref -- and setAutoOpenAttributeMenuForRow from within it. A state updater must be pure. React may invoke one more than once for a single update (StrictMode double-invocation, and render rebasing under concurrent rendering), so the parent could receive duplicate onChange/onValidate calls and isInternalChange could desync, which re-triggers a getVisualAST reparse of an expression the editor itself just produced. Compute the next rows from `rows` and run the side effects after setRows, the shape these callbacks had before this branch. Each mutator regains `rows` in its dependency list, which is only a stale-closure risk if two of them fire within one render batch; every one is invoked from a distinct handler on a distinct control, and none calls another, so there is no path that reads a stale array. Two details preserved from the functional form: updateRowAttribute still returns early without regenerating the expression when the attribute changed and values were cleared, and it reads the pre-update operator through `current`, which is captured before the row's slot is replaced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Let the rule-table value trigger grow, and stop tinting it blue Two fixes to .field-selector-menu-button, the shared trigger behind the attribute, operator and value cells of a rule row. Height. The trigger was pinned to 40px while its multi-value container still carries flex-wrap: wrap under overflow: hidden, so a row with more selected values than fit on one line clipped them. 40px is the floor, not the height: restore min-height with height: auto so the row grows to fit its chips, which is what the container was built to do. Colour. Focus and the open state both painted the trigger with the accent tint, rgba(--button-bg-rgb, 0.08). Because clicking a menu trigger leaves it focused, that fill stayed on the cell long after the menu closed and read as "this cell is selected" -- and it out-ranked the neutral hover wash, so hovering a cell that had been clicked went blue too. The rule predates this branch, but the branch is what makes it worth fixing: the only blue in a row should be the channel-attribute token, and a blue cell background competes with it. The open state keeps a background, one step deeper than the hover wash and in the same neutral family, so it still reads as open. Focus moves to the standard button-focus mixin, a ring rather than a fill, which the mixin suppresses for pointer focus and keeps for keyboard -- the accessibility affordance stays, the stuck highlight goes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Delete every clearance field when enforcement or classifications go off The Classification Enforcement checkbox creates one linked Clearance user field, and both teardown paths on the page assumed exactly that. The save path read fetchUserLinkedFields(...)[0] and deleted that one, then recorded enforcement as disabled. Any further linked field stayed live and reappeared on the next load, with the page's own state saying enforcement was off. fetchUserLinkedFields returns the whole set precisely so callers do not have to assume; delete all of them, and dispatch the deletion into Redux the way the sibling teardown already does. The disable-classifications path deleted clearance fields only when ABAC was on. A clearance field outlives the setting that allowed it: create one, turn ABAC off, then disable classification markings, and the template delete fails on a dependent that nothing cleaned up. Cleanup is not conditional on the setting -- only creation is. Three existing tests fed getPropertyFields a fixed mockResolvedValueOnce chain and now need one more empty page for the clearance lookup the disable path always makes. Two new assertions cover the fixes: the save path deletes both fields when two exist, and disabling classifications with ABAC off still deletes the clearance field ahead of the template. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Correct two stale claims in the CEL help modal The combine-conditions example wrote user.attrs.Program and user.attrs.Team. No such root exists -- the supported one is user.attributes.* , which every other example on the same screen uses. Predates this branch; the typo fix one commit over is what surfaced it. The notes below then said ordering operators are "forbidden due to incorrect string comparison", which this branch made self-contradictory by adding user.attributes.Clearance >= resource.attributes.MinClearance as a documented example a few lines above. The engine allows <, >, <= and >= : the lint's allowedFunctions carries all four. What the original note was reaching for is that they compare alphabetically on a text attribute, which is rarely what an admin means, and meaningfully only on a ranked one. Say that instead. Both strings are inline Markdown passed to <Markdown>, not FormattedMessage ids, so nothing needs re-extracting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Drop the template link when duplicating, not when creating newPendingField stripped linked_field_id from every field it built. It is the generic create path behind the whole user-attributes table, and the server only forbids adding a link on a later update -- a create request may carry one. So the strip quietly made "create a linked field through the pending collection" impossible, to solve a problem that belongs to exactly one caller. That caller is handleDuplicate, which spreads the source field into the new one. A copy must not inherit the link: it would become a second dependent on the template and would claim a type and option set the copy cannot actually hold. handleDuplicate already drops attrs.owners for the same reason -- a copy does not inherit the original's provenance -- so the link is dropped next to it, and newPendingField goes back to passing its patch through untouched. The util test now pins the restored contract, that an explicitly requested link survives newPendingField. The behaviour it used to cover moves to the dot menu suite, next to the other duplication cases, where it fails without the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Assert why the last two e2e rejections happen, not just that they do Two specs still proved a rejection by observing that a call threw. A bare catch passes on a transport fault, a 500, or a gating rejection that never reached the rule under test -- which is not hypothetical here: every spec in this directory was passing or failing for the wrong reason until the feature flag reached the Playwright server, and masking.spec.ts in particular was green the whole time because the flag-off refusal looks identical to the one it meant to assert. Generalise the check behind expectRejection and give each site a named expectation: authoring.spec.ts assigning a resource-referencing parent to a team masking.spec.ts saving an expression that still carries the mask sentinel The team boundary reports through SavePolicy, which returns one error id for every rejection, so that assertion pins the status and rules out a permission, feature-gate or server error rather than naming the reason; the enterprise unit test owns the message. The masked-token rejection has its own id, app.pap.save_policy.masked_token_in_expression, so that one does name the reason and would now catch a repeat of the flag-off false pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Assert the has() rejection's reason, and push the new Clearance field to Redux Two review follow-ups, both of them a gap this branch's own earlier commits widened. The has(resource.attributes.*) check-time rejection was asserted with errors.length > 0, which passes on any validation error -- including the one the feature flag returns for every resource reference, so the test would have stayed green through the whole period the flag was off on the Playwright server. Assert the message, which cel_utils pins and the enterprise unit test already asserts on its side. The Clearance creation branch was the one field operation in persistLevels that did not push its result into Redux: the channel field dispatches RECEIVED_PROPERTY_FIELDS on create, and clearance dispatches PROPERTY_FIELD_DELETED on delete -- a dispatch this branch added, which is what left creation looking like the exception it was. Without it a consumer reading the properties slice does not see the new field until a full reload. saveCreateUserLinkedField already returns the created field, so it is only a matter of dispatching it. The existing creation test gains the store assertion, alongside the one the already-exists channel-field case uses; it fails without the dispatch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Ask SupportsOptions() in the masking resolver, so rank fields agree The two paths that decide which literals a caller may see for a shared_only field disagreed about rank. maskConditionValues asks h.field.Type.SupportsOptions(), which covers select, multiselect and rank. fieldToMaskingInfo spelled the list out as select-or-multiselect, so a rank field fell through to getCallerTextValues. For a rank field that is the wrong source: the caller's stored value is an option ID, while the condition literals it is compared against are option names. Every option therefore read as hidden, so the resolver reported a shared_only rank field as fully masked while the visual-AST path showed the held option — and the resolver is what the write path validates against, so a value an admin could see in the editor could be refused on save. Reachable on this branch specifically: Classification Enforcement mints Clearance as a rank field linked to a template the channel classification field also links to, which is exactly the shared-template shape this resolver bridges. Both spellings predate this branch, but this commit's two lines are the ones it already rewrote when it moved them onto maskingHoldings, and the branch's own new sibling-resolution code asks SupportsOptions() -- leaving one hand-written list as the odd one out was how the types drifted apart in the first place. TestAppMaskingResolver_ChannelFieldUsesUserHoldings now runs over select and rank. The rank case fails on the old predicate with "value the caller holds user-side must be visible on the channel field"; select passes either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Name the clearance field `clearance`, labelled Clearance The field Classification Enforcement mints was created as `Clearance`, but the name is the CEL identifier an author writes -- user.attributes.Clearance -- and the channel half of the same pairing is already lowercase (`classification`). Name it `clearance` and carry the capitalised form as attrs.display_name, which is what the System Console table and profile popovers render. Nothing looks the field up by name (fetchUserLinkedFields matches on linked_field_id), so a field already created as `Clearance` is still found, deleted and counted as enforcement being on. Only newly created ones get the new name; an existing install keeps whatever its policies already reference. Also gives the channel-picker rows in the test modal a real keyboard focus indicator. `&:hover, &:focus` shared one 0.08 background and set outline: none, so a focused row was indistinguishable from a hovered one and keyboard users had nothing to locate. Hover is unchanged; :focus-visible adds a 2px ring on top of it. The ring is inset because the row is full-bleed inside &__list, whose overflow-y makes overflow-x compute to auto and would clip an outset ring or a UA outline at the sides. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Address review feedback on the resource-attribute branch - masking.spec.ts: guard on AttributeValueMasking. The sentinel rejection the spec asserts lives inside the server's AttributeValueMasking branch (app/access_control.go), so with the flag off the save fails for an unrelated reason. Dropped the enableMaskingFlag call it replaces: the config store restores feature flags on write, so that call never did anything. - cel_help_modal.tsx: the supported-operator list omitted the ordering operators the examples right above it use, and hasAnyOf/hasAllOf, which authors can type (isSimpleCondition accepts them). - test_modal.tsx: guard fetchUsers with a request sequence, mirroring test_channel_picker.tsx. Backing out of the members list and re-picking could otherwise let the first channel's in-flight response replace the second's results. - user_properties_values.tsx: don't offer the LDAP/SAML link editor on a linked field. Saving it rewrites the field as `text`, which the server refuses when linked_field_id is set (properties/property_field.go); the dot menu already disables the same action. - attributes_store.go: derive both the per-object-type view lookup and the refresh sweep from one map, so adding an object type can't leave a new view perpetually stale. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Save the e2e parent policy through the typed client createParentPolicyViaAPI hand-rolled the PUT through an `as any` cast to doFetch, so a signature change would surface as a runtime failure in CI rather than a compile error. Client4.updateOrCreateAccessControlPolicy issues the same request against the same route. Also records why the version field here is advisory: the app layer overwrites it, and no policy version gates resource.attributes.* — the gate is the ResourceAttributesInPolicies flag plus the rule that a team policy has no attributes to resolve against. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Keep showing the channel target when the target list is empty A row in target mode fell through to the bare text input whenever no comparable channel fields were available — while the autocomplete fields are still loading, with the feature turned off, or after the targeted attribute is deleted. That hid the "Channel: X" label and the first keystroke cleared targetAttribute, silently rewriting the rule as a literal comparison. Exclude target mode from the bare-input path, matching the guard the masked-values branch already applies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Correct what the pending linked-field test actually pins Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Wait for the search round-trip before asserting the user is absent toHaveCount(0) resolved against the intermediate empty render: the previous result row is cleared the moment the search term changes, so the assertion could pass before the query for the non-matching user had come back. Wait for the cel/test response first, matching how the rest of the ABAC suite synchronises on API calls. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Ellipsize the channel-attribute chip, and correct a stale contract note A long channel-attribute display name was clipped mid-glyph: the chip has overflow/nowrap but no ellipsis, and it could not shrink inside the button's flex row once min-width:0 came off the inner wrapper. Give the label text its own box so text-overflow has something to apply to — on the chip itself it would only see flex items, not the text. Also correct the targetAttribute comment: rowToCEL and updateRowOperator have kept targets for "has any of" / "has all of" since multiselect targeting landed, so comparison operators are not the only case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Use property option rows where the inlined option list may be absent A select-style property field carries its options over the wire inline under Attrs["options"], hydrated from the PropertyOptions rows behind them. Above model.PropertyFieldMaxHydratedOptions the read leaves that list out and reports options_count/options_omitted instead, so the key is present exactly when there are options to show — and absent both when the field has none and when it has too many to inline. Two paths derived option state from that list and so read the second case as the first. Both now ask the rows. The store rebuilt a field's owned options from the list when the field stopped linking to a template. That takeover is necessary: the field's property values point at the option identifiers it was deriving, so it has to keep serving them once it no longer derives them. But a caller unlinking a field above the cap has no list to send, so the takeover found nothing to do and the field unlinked owning no options at all, with every one of its values left dangling. takeOverLinkSourceOptions copies the rows from the link source instead, keeping their identifiers and their CreateAt, which makes the takeover independent of what the caller sends. The previous LinkedFieldID is read inside the update transaction before the UPDATE overwrites it, since the row is the only record of it. Value validation for a managed property group checked a value's option identifiers against the same list, so once a field grew past the cap every value assigned to it was refused — including identifiers the field really had. It now resolves them through PropertyFieldStore.GetExistingOptionIDs, one indexed query against the field's effective option set, which has no such ceiling. This is the first exported access to option rows; the generated store layers and mocks were regenerated for it. Both cases are pinned by tests that fail without the change: unlinking a field of 1,001 options keeps all of them, and a value naming one of them is accepted while a value naming an option the field does not have is still refused. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Add the graph property field type behind a feature flag A PropertyField carries a Type from a Postgres enum, and the types that bear options (select, multiselect, rank) keep those options as rows in PropertyOptions. This adds a fourth option-bearing type, 'graph': a multi-value select whose options are meant to form a hierarchy, so access rules can eventually reason about one option standing above another rather than about exact equality. What lands here is the type and its gate, nothing else. A graph field's options are stored, validated, and served exactly like a multiselect field's — there is no storage for the links between options yet, and nothing traverses them. Migration 000214 adds 'graph' to the property_field_type enum. Its down migration is deliberately empty: Postgres cannot remove a value from an enum in place, and rebuilding the type without it would mean dropping and recreating both attribute materialized views and coercing live field rows to some other type, which is a data change dressed up as a schema reversal. The value is therefore permanent from here on, which is also why the name is the unqualified 'graph' — with no single-value variant there is nothing to disambiguate it from, and the name outlives whatever the API layer calls the type. Two rules arrive with it: - The PropertyFieldGraph feature flag, off by default, blocks creating a graph field and converting a field to graph — and nothing else. A graph field that already exists stays editable while the flag is off, so switching the flag off cannot strand a field somebody is halfway through populating. graphPropertyFieldGate carries the reasoning. - No field converts to or from graph, in either direction. A conversion would quietly change what the field's values mean: out of graph, options authored as a hierarchy become a flat list; into graph, a flat list becomes a hierarchy nobody wrote. Creating a field of the type actually wanted and moving the values across is always available. Options on a graph field may not carry a rank. The option store promotes that key into a column of its own for every option-bearing type, so left alone it would be persisted, served back, and read as an ordering a graph field does not have. make i18n-check needs mmgotool from a separate tools repo that is not checked out here, so it could not be run; the two new en.json strings were verified by hand as valid JSON in the correct alphabetical position. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Gate graph fields created by linking to a graph template The PropertyFieldGraph flag is meant to stop new graph fields appearing while it is off, and graphPropertyFieldGate enforces that on the type carried by the create request. That is not the only way a graph field comes into existence: a field created with a linked_field_id inherits its type from the template it links to, and the property service copies that type over whatever the request said, after the gate has already run. So with the flag off, linking a new user or channel field to an existing graph template produced a graph field — and that is the shape the feature is built around, one template holding the hierarchy with object-scoped fields linked to it, so it was the likeliest path rather than an obscure one. CreatePropertyField now resolves the link source and gates on its type too. The lookup is skipped when the flag is on, so the usual path costs nothing, and a source that does not exist is left to the service, which has the specific errors for an unusable link target — a test pins that down, since the gate could otherwise mask linked_source_not_found with a less useful message. Kept as its own commit rather than folded into the one that added the gate: the gap is worth seeing, because any check placed before the service's schema copy has it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Add a table for property option parent edges The options of a select-style property field are rows in PropertyOptions, and for a `graph`-typed field they are more than a list: an option may sit below several others, forming a hierarchy that access rules ask about ("does this user hold this program, or one above it?"). This adds the table that records those parent links, and the store seam for reading and writing them. Migration 000215 creates PropertyOptionEdges: one row per link, keyed (FieldID, ChildOptionID, ParentOptionID), with a second index on (FieldID, ParentOptionID, ChildOptionID) for the downward walk. Both keys lead with FieldID because an option is identified by (FieldID, ID) and not by its ID alone -- unlinking a field from its template deliberately gives two fields options under the same identifiers -- so a query keyed on an option ID alone would read another field's hierarchy. There is no delete marker: removing a parent deletes the row, and deleting an option deletes every edge it appears in, in the same transaction. PropertyFieldStore gains four methods -- create, delete, load a field's whole edge set, and load the edges below a set of options, which is what tells a caller whether those options are leaves. Creating an edge enforces the two invariants an edge is meaningless without: its field is a live `graph` field, and both endpoints are live options that field owns itself. An option inherited from a link source belongs to the template, and so do its parents. Unlinking a field now copies the inherited edges alongside the option rows it already copied. Without that a graph field that stops linking would keep every option and lose every relationship between them: a flat list of options that cover nothing but themselves, which an access rule reads as a denial. It fails in the safe direction, which is exactly why it would go unnoticed. Nothing creates or walks edges yet -- this is the schema and the store seam. The structure of the resulting graph is unchecked: an edge set can still hold a cycle or a hierarchy deeper than anything can usefully walk, since those are properties of the whole set rather than of one row. The store interface change was regenerated into the retry layer, the timer layer, and the mocks with `make store-layers` / `make store-mocks`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Add hierarchy resolution for graph property field options The options of a graph property field carry parent links, stored in PropertyOptionEdges. Until now those rows could be written and read back one at a time, but nothing could answer a question about the hierarchy they form. This adds that, as store queries plus a narrow interface on the property service. Two recursive queries do the walking, one per direction, seeded by a set of options and returning what each one reached. They are the same query with the edge columns swapped, and three things about them carry weight that is not visible from reading the SQL: the walk is scoped to the field that owns the hierarchy, which for a linked field is its template, since option identifiers are unique within a field and not across them; the seeds are read from PropertyOptions so an identifier naming nothing live reaches nothing; and the recursive term is UNION rather than UNION ALL, which is the difference between enumerating the options reachable from a seed and enumerating the routes to them. On a grid of 121 options and 220 edges -- the shape representing a second dimension as extra parents produces -- one seed reaches 121 options along 705,431 routes, so the dedup is what makes the query usable at all. It is also what stops the walk if the edges ever do form a cycle. A regression test pins the row count on exactly that grid. Identifier lists are batched at ten thousand per statement. Postgres accepts 65,535 bind parameters and refuses the statement outright past that, and a field is allowed far more options than that, so a query naming one option per parameter could not run at all on a large field -- the same class of ceiling as any other per-element emission, reached here by asking about a whole level of a hierarchy. Every entry point takes a set of options rather than one, because the callers ask about sets: masking one object's values or deciding one policy resolves several options at once, and a set-seeded recursive query costs no more than a single- option one. That is also the one shape a cache added later could not fix, so the interface has it from the start. On the property service, on top of those walks: - AncestorsOrSelf and DescendantsOrSelf, which log any option they could not resolve. - CoversAll, CoversAny, WithinAll and WithinAny -- whether a holder of options is at-or-above, or at-or-below, another set of them. All four are one coverage walk read two ways, so they agree by construction. An empty side is false rather than vacuously true, and two options on separate branches are an ordinary false rather than an error. - CommonGround, the most specific options that every one of several holders covers. It descends from one holder's options and stops each branch at the first option the others cover, rather than collecting and intersecting whole subtrees, because a holder of a root has the entire hierarchy below them. - WouldCreateCycle and DepthAfterAdding, for a caller about to write edges. Both read the part of the hierarchy the proposed edges can reach -- above their parents and below their children -- and answer over it in memory, so a payload whose edges only form a cycle together is caught as readily as one edge that closes an existing chain. Anything that cannot be computed is answered with the negative and the error together, so a caller that drops the error denies rather than grants. Nothing consumes any of this yet: no endpoint, policy, or read path calls it, and in particular nothing yet refuses to write a cyclic or over-deep hierarchy -- DepthAfterAdding reports a stored cycle as an error rather than picking a number, which is the honest answer until a write path runs the checks. CommonGround has no caller in sight either; it is the data behind showing what is safe to discuss among a set of people, and it ships tested so that display has something to call. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Project graph property values as option identifiers in the attribute views UserAttributeView and ChannelAttributeView flatten every property value into one JSON object per user or channel, keyed by field name, and that object is what an access-control rule reads. Each type that carries options resolves its value there: select yields the option's name, multiselect an array of names, rank a name and a rank. A graph value had no branch of its own and fell through the type CASE to the catch-all, which projects the stored value as it is -- an array of the option identifiers the object holds. Migration 000216 redefines both views to state that branch. The projection stays an array of identifiers rather than names, because a rule over a hierarchy is compiled against the identifiers of the options at or above the ones it names, and identifiers survive an option being renamed. It carries only the options the object holds, with no ancestors mixed in, because that at-or-above set is computed when the rule is compiled and the view never has to walk PropertyOptionEdges. Writing the branch out buys three things the catch-all could not give: * The next type added to property_field_type cannot inherit graph's meaning by accident. * Identifiers of soft-deleted options are dropped, as select, multiselect, and rank all do. This is belt and braces rather than a hole being closed -- the at-or-above sets a rule is compiled with are built from live options only, so an identifier left dangling by a deletion matches nothing either way. * The projection is always an array. Filtering deleted options means aggregating over rows that may be none, and an aggregate over no rows is NULL, so the branch coalesces to an empty array; an object holding nothing but deleted options holds nothing. A value that is not an array projects the same way rather than passing through as itself, as it did before. Leaving a JSON null there would be worse than untidy, because a rule that expands the attribute with jsonb_array_elements_text raises "cannot extract elements from a scalar" on it -- where an object with no value at all is SQL NULL and expands to no rows harmlessly. Every other type's projection is byte-identical, and the down migration restores 000213's two view bodies verbatim. migration_000216_test.go pins the projection through a down/up cycle: held identifiers on both a graph field that owns its options and one that links to the template owning them, a deleted option's identifier dropped, a non-array value projecting nothing, and select, multiselect, and rank unchanged. The pin matters because the array of identifiers is also what the catch-all produced, which made the projection an accident of where the CASE ended that a later redefinition could have undone unnoticed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Validate graph option structure on write The options of a graph property field form a hierarchy of parent links, held in PropertyOptionEdges. Until now the store wrote any link it was given as long as both endpoints were live options of the same graph field: nothing checked that the result had no cycles, that it stayed shallow enough to walk, or that it stayed within any size at all. The cycle is the case that matters, because an access rule reads one as every option in it covering every other, which grants access nobody granted. Add those checks as PropertyService.ValidateOptionEdges, over the hierarchy a whole change produces rather than over one link at a time -- half a change can break a limit that no single link in it breaks. The limits are hard-coded in public/model beside the hierarchy they bound: 100,000 options and 1,000,000 parent links per field, 100 options on one chain, 100 parents per option. ValidateOptionEdges enforces three of them. The option count is a property of the payload rather than of the stored hierarchy, so PropertyField.IsValid checks it, which is where a field written with its option list inline passes. CreateOptionEdges and DeleteOptionEdges are replaced by a single MutateOptionEdges(groupID, fieldID, expectedUpdateAt, add, remove), because a change that re-parents an option removes one link and makes another and the two halves have to land together. That also closes two gaps the separate methods left open: - The field's UpdateAt is bumped in the same transaction. A hierarchy change alters what a field serves without altering a single column of it, so clients syncing on UpdateAt would otherwise never hear about it -- and the cached list of the group's fields is invalidated for the same reason. - The bump is a compare-and-swap against the UpdateAt the caller read before deciding what to write, in the pattern Update already uses for a field's own columns, and zero rows affected is a conflict. Validation reads the hierarchy as stored, outside the transaction, so the swap is what keeps what it checked true when the change lands: two changes that are each acyclic and that jointly put an option below itself cannot both be applied. UpdateAt moves to at least one past what the row held rather than to the current millisecond, or two changes inside one millisecond would both swap successfully against the same value. WouldCreateCycle and DepthAfterAdding now take the links a change removes as well as the ones it adds, and read the stored hierarchy without them. Otherwise they answer for a hierarchy the change does not produce: re-parenting an option counts the chain through the link being dropped, and inverting a relationship reads as a cycle against the link it replaces. ApplyOptionEdges validates a change and writes it, and is the entry point for anything changing a hierarchy. Nothing calls it yet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Drop a false claim from the graph attribute view migration test The test that pins how a graph property value projects into UserAttributeView and ChannelAttributeView exercises the down migration, and afterwards restored both views through a t.Cleanup, on the stated grounds that the views are shared with every other test in the package. They are not. Each of these tests builds its settings through storetest.MakeSqlSettings, which generates a random database name, CREATE DATABASEs it, and lets New() apply every migration into it -- so the views one test drops and redefines are its own, and no other test can observe them. The restore was dead code and the comment justifying it was false, which is worse than the dead code: it tells the next reader these tests share state. Both are gone. The test still re-applies the up migration explicitly, because the assertions after it are what prove the migration is re-appliable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Deduplicate graph option endpoints through a map The write path for a graph field's option hierarchy collected the option identifiers a change names by scanning what it had already collected -- slices.Contains over a growing slice, in three places: the endpoints an added link must prove are live options, the options whose parent links the change moves, and the region its cycle and depth checks load. A whole hierarchy can arrive in one change. Creating a field with its options and their parents inline is the intended way to author one, and a field may hold 100,000 options with as many as 100 options directly above each of them, so the scan is quadratic in something the caller chooses the size of. Measured here, deduplicating the endpoints of a change of N links: 1,000 links 7ms scanning, 0.3ms through a map 10,000 links 682ms scanning, 2.9ms through a map 50,000 links 16.87s scanning, 13ms through a map utils.RemoveDuplicatesFromStringArray uses a map and keeps the first occurrence of each identifier, which these callers depend on: the failure a caller is told about is the earliest offending link in the payload, and that has to be the same on every run. Also cover the removal half of a hierarchy change against the field it belongs to. A removal matches on the field and on pairs of options, which is two conditions rather than one, and no test asked for more than one pair at a time. Nothing would have caught a change reaching past its own field to a link between options carrying the same two identifiers -- which is a state the unlink takeover deliberately produces, since a field that stops linking takes its template's options over under their existing identifiers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Add REST endpoints for property field options A select-style property field's options are rows in PropertyOptions, but the only way to change one has been to write the whole field with its complete option list inline under attrs.options. That does not scale to a graph field, whose options are meant to form a hierarchy of thousands: past model.PropertyFieldMaxHydratedOptions a field reads back with no option list at all, so a caller has nothing to modify and no way to name the option it means. Add a collection under the existing field route, /api/v4/properties/groups/{group}/{object_type}/fields/{field_id}/options, with four handlers: GET one page of the field's effective option set -- the options it owns plus the ones it inherits from the template it links to -- ordered by (CreateAt, ID), with read_only set on an inherited option and, for a graph field, the options directly above each one named inline. POST create N options, each optionally naming the options it sits below, including others in the same payload. PATCH change N options. A part the payload leaves out is left as it was. DELETE remove N options, judged as one set so a whole branch of a hierarchy goes in a single call. Parents are referenced by name throughout, which is the same reference the field's inline option list can offer: identifiers are assigned in the store, after a request is parsed, and an option name is unique across a field's effective set. PropertyFieldOption is the wire type, new in public/model/property_option.go; it deliberately carries neither an option's rank nor its position in the field's option list, both of which belong to the list a field is written with and are left untouched by a write here. Writes only ever touch the options a field owns. An inherited option is reported read-only and refused, and a field linking to a graph template may own no options at all -- a local option there could never be given a parent from the inherited hierarchy, so it could only form a second hierarchy permanently disconnected from the one the field exists to serve. Every mutation is gated on the field's own options permission level, the same one an options-only field patch uses. Option rows and parent links have to move together, so SqlPropertyFieldStore gains MutateOptions -- one transaction covering option upserts, link removals, link additions, and the compare-and-swap on the field's UpdateAt that serializes concurrent changes. MutateOptionEdges becomes its hierarchy-only case, so there is one implementation of the transaction and the swap rather than two. DeleteOptions is the soft-delete half, with the same swap and the edge cascade. A rejected payload answers with the position of the first item at fault and the reason, as an already-shaped bad request rather than as an error detail: the HTTP layer strips a detail from the response unless the server is in developer mode, and there is no fixing a payload from "the options are not valid". The shared graph validation still reports its reasons as details, so the option code restates them; its doc comment now says so rather than claiming otherwise. *store.ErrInvalidInput also gains a case in mapPropertyServiceError, so a payload the store refuses -- a link naming an option another writer has since deleted -- answers 400 rather than 500. Every mutation writes an audit record carrying the field, the options, and their parents before and after. A parent link has no delete marker, so for a change that detaches or deletes one the audit log is the only record it existed. Two things clients do not get yet. There is no websocket event for an option change: the field's UpdateAt moves, which existing delta-sync clients follow, and republishing property_field_updated for the owning field and its linked dependents is a follow-up. And the access-control caches are dropped for the field whose options changed but not for fields linking to it, which serve the same options -- so a policy compiled against a linked field can hold a stale option set until that field itself changes. Both belong with the republication work. This is larger than a commit here usually is. It is one seam -- four verbs over one collection -- and every layer of it is needed for any of it to be reachable: the wire type, the store transaction, the validation and name resolution above it, and the handlers. Splitting it would land a store method with no caller. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Decide an option change against the field as the store has it Three corrections to the property field options endpoints, all found reviewing what the field a caller hands in is actually trusted for. A rank field's options are no longer writable one at a time. Every option on one has to carry a rank that is positive and unique within the field — validateRankOptions is the single source of truth for that, and the comment above it says invalid rank data must never be persisted because it fails open, denying access at read time rather than erroring. PropertyFieldOption does not model a rank and this seam has no way to choose one, so an option created here carried none: the field was left unwritable through its own option list and read as covering nothing where a policy clamps against it. Reading a rank field's options is still allowed; writing them is what the field's own option list is for, where the ranks are validated together. The compare-and-swap every option change is written under is now anchored on a read of the field from the master, taken by the property service immediately before it validates. It was anchored on the field the caller passed in, which comes from PropertyFieldStore.Get on a background context — a replica read where one is configured. Replication lag alone therefore made a caller lose a race against a write that had already finished, answering 409 to a request that was in conflict with nothing. The anchor now precedes every read the validation does, all of which already went to the master, so the window the swap covers is the window the checks were decided in. Nothing is weakened: a competing change landing on either side of the re-read still fails the swap. A change aimed at a deleted field is refused. There is no undeleting a property field, so an option written to one is written where nothing will look for it. It was refused already for a graph field whose change touched the hierarchy, by the liveness check behind the edge write, and accepted otherwise — one shape of a request answering differently from another for no reason a caller could see. Listing options now requires a positive page size instead of answering with an empty page. An empty page is indistinguishable from a field that has no options, which is the reading the option rows exist to stop being possible; the HTTP layer always supplies a size, so this is for the callers that come later. Also: a rejected change now carries its reason as the error's detail as well as in its message, so the reason reaches the server log and not only the response. And two comments claiming a write is decided against the caller's copy of the field are no longer true of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Accept option parents when creating and updating property fields A select-style property field is written with its options inline under attrs.options, and a graph field's options additionally sit in a hierarchy held as parent links in PropertyOptionEdges. Those links could only be created one call at a time through the options endpoints, so a field and its hierarchy could never arrive together: a graph field was created flat and then rearranged. An inline option may now carry the options directly above it, by name, under a "parents" key. Names resolve within the option list itself -- that list is the whole of the field's option set once the write lands -- so one write can build a hierarchy, naming an option that appears further down the list. PropertyField.OptionParentLinks reads the links off the list; the property service checks the hierarchy they produce through the same ValidateOptionEdges the options endpoints use (acyclicity, depth, fan-in, edge total); and SqlPropertyFieldStore writes them inside the transaction that writes the option rows, so an option never exists without the links the same write gave it. What was checked stays true when the write lands because the field write already swaps on the UpdateAt of a master read taken before the checks, and every change to a field's options or hierarchy moves that UpdateAt. Reads are deliberately asymmetric: an option still reads back without its parents, which are reported by the endpoints that address options one at a time. That is what makes a read-modify-write of a field leave the hierarchy alone instead of flattening it -- an option quietly left with no parents becomes a root, covered by nothing, and every access rule that reached it through an option above stops matching. For the same reason a list that says nothing about an option's parents leaves them as they are, and only an explicitly empty list detaches one. Three refusals come with it: - "parents" on a field whose options form no hierarchy, which the options endpoints already refuse. Two write paths answering differently about the same payload key is the kind of difference nobody finds until a client hits it. It does mean a field of another type whose stored options happen to carry a "parents" attribute can no longer be written back unchanged: the key is structural now, not an attribute. - An option list on a field created by linking to a graph template. Such a field serves the template's hierarchy and owns no part of it, and an option of its own could never be linked into that hierarchy, because an edge never crosses fields -- it could only form a second hierarchy covered by nothing. The check reads the type from the field after the link source's schema has been copied over, since a request creating a linked field need not mention the graph type at all. - A name that an option local to a linked field already has. An option name has to identify one option across everything a field serves, and a field linking to a template serves that template's options beside its own -- so a name free on the template can still be taken on a dependent, which no read of the template can see. GetLinkedFieldOptionNames is the store read that answers it, and the options endpoints now make the same check for the names they add or rename. CustomProfileAttributesSelectOption gains a Parents field. An option list written to a managed property group is normalized by decoding it into that type and encoding it again, which drops every key the type does not name -- so without it a graph field's parents would be discarded silently, in exactly the group whose fields access-control policies are written against. A hierarchy change made this way marks the field as changed, so the fields linking to it are republished and their compiled policies dropped, as already happens for a change to the options themselves. The store interface gained a method, so retrylayer, timerlayer, and the mocks were regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Pin the option-list write path against the hierarchy limits A review pass after the previous commit found the checks it relies on were not driven through the path it added. An option list stating a hierarchy is held to the same limits as a change made one option at a time, and for a field being created it is held to them with nothing stored: every read behind the check answers empty and what it measures is the list alone. Nothing pinned that, so an early return for a field with no option rows yet -- an obvious-looking optimization, since all those reads are known to be empty -- would have made a single create the one way past every limit on a hierarchy, with no test failing. Now driven: a chain past the depth limit and an option past the fan-in limit, both refused on create; and a cycle that a list closes only in combination with a link already stored, which is what the check has to read the stored hierarchy to see at all. Two more that were untested anywhere. A local option taking a name the field inherits from its template was checked but never exercised -- only the reverse direction was, where a template takes a name a dependent has locally. And a hierarchy change reports the fields linking to the one that changed, so their clients are told and their compiled policies are dropped, while a list restating the hierarchy it already had reports nothing; both of those are consequences of where the change is detected rather than of anything visible at the call site. Tests only; no behaviour changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Validate graph field values and notify linked fields of option changes Two holes in how a graph field's options are treated after they are written. The first is that nothing checked a value written to one. The property value validation for managed groups switches on the field's type and ended in a bare `return nil`, so a value written to a `graph` field was accepted with no validation whatsoever -- the wrong shape, or option identifiers naming options the field does not have. That is a fail-open default in an access-control path: policies decide from these values. A graph value now has to be an array of option identifiers the field's effective option set actually contains, with no identifier repeated, and every other type the model allows has an explicit case so that a type with none is refused outright rather than accepted unchecked. The check goes through `PropertyFieldStore.GetExistingOptionIDs`, which queries the option rows, and not through the field's inlined `attrs.options`. That matters more than it looks: a field with more than `model.PropertyFieldMaxHydratedOptions` options reads back without its option list, and a graph field is meant to hold thousands, so a check against the inlined list would refuse every value written to a realistic one. There is a test at that boundary. The second is that a field linking to a template serves the template's options without holding a copy of them, so an option change on the template writes only the template's row -- and nothing told the fields linking to it. Their compiled access policies, cached under their own IDs, went on deciding from options that had been renamed or deleted, and their clients went on offering them. Every option mutation now announces a change for the field it names and for every live field linking to it, dropping each one's caches and publishing `property_field_updated` for each: one event per field, since each carries the option list from that field's own perspective and is broadcast to that field's own target. `PropertyFieldStore.GetLinkedFields` is the read behind it, promoted out of `SqlPropertyFieldStore.Update`, which was already doing exactly this select-and-hydrate for the field-blob write path. Those three endpoints also pass the requesting connection through, so the caller is left out of the event for the field it just wrote -- but not out of the events for the dependents, which are fields it did not write. One gap stays open, deliberately. An option or edge change moves what a masked read of a *value* returns without changing any `PropertyValue` row: option mutations bump the owning field's `UpdateAt`, so a client delta-syncing fields refetches, but nothing makes a client delta-syncing values refetch, and the values it holds may now mask differently. Nothing reads graph values yet, so it is recorded rather than fixed. `GetLinkedFields` is a new store method, so the retry and timer layers and the store mocks are regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Cascade option and edge cleanup through property field deletion Deleting a property field soft-deleted the field row and nothing else, leaving the field's options in PropertyOptions and the parent links between them in PropertyOptionEdges. Every query about an option filters on the option's own DeleteAt and none of them joins to the field's, so those rows went on answering as live options of a field nobody could read: a value validated against one still passed, and a hierarchy walk seeded from one still found it. SqlPropertyFieldStore.Delete now runs in a transaction and, once the field row is marked, soft-deletes the options that field owns and deletes the parent links between them outright. The links go rather than being marked because an edge has no delete marker of its own: it is a link between two options, and an option that is gone is not in the hierarchy for it to link. Both statements are scoped to the field's own ID, so deleting a field that links to a template leaves the template's options untouched -- the template owns them, and they outlive any number of fields deriving them. Two rules that were already enforced are now covered in the graph shape as well: a template with a live field linking to it cannot be deleted at all, and an option with something still below it cannot be deleted on its own. The comment on the second one now also records its consequence, which is what a later reader needs -- deletion requires leaf status and both endpoints of a new parent link have to be live options, so a deleted option is never in the middle of a hierarchy, and no walk needs a rule for whether one conducts reachability from what is above it to what is below. Dangling property values are deliberately left alone. A value naming an option that no longer exists is ignored wherever it is read, which is how the property system has always treated one, and cleaning those up is a wider change than this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Correct why a deleted option needs no reachability rule DeleteFieldOptions refuses to delete an option that still has options below it, and the comment on that check claimed the refusal is what keeps a deleted option out of the middle of a hierarchy -- and therefore what lets the hierarchy walks skip a rule for whether a deleted option conducts reachability from what is above it to what is below. The premise is wrong. That check guards one of the two ways an option can be deleted. The other is a field write whose inline option list omits the option: syncPropertyFieldOptions soft-deletes every option the list no longer carries, with no leaf check, so an interior option can be deleted that way and its children are left as roots. Measured by writing a three-option chain and then updating the field with the middle option omitted -- the write succeeds, both of the middle option's parent links are gone, and the bottom option reads back with only itself above it. The conclusion still holds, for a reason that does not depend on the leaf check: every one of the three places an option's DeleteAt is set deletes the edges touching that option in the same transaction, and both endpoints of a new link have to be live options. A deleted option therefore has no links on either side of it on any path, so no walk needs the rule. The comment now says that, and says plainly that the leaf check does not cover the field-write path, which is a real gap rather than a documentation detail: through the options endpoint a caller that would orphan a subtree is told, and through a field write the same caller is not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Refuse a field write that orphans a graph option's children An option of a graph field that still has options below it cannot be deleted: whatever is under it would be left hanging off nothing, which turns it into a root covered by nothing but itself, so every access rule that reached it through an option above stops matching. DeleteFieldOptions enforced that. The other way to delete an option did not. A field write states the field's option list in full, so an option missing from the list is an option being deleted -- syncPropertyFieldOptions soft-deletes every option of the field's own that the list no longer carries, and deletes that option's parent links with it. No leaf check ran on that path, so writing a field with an interior option left out succeeded and silently orphaned its subtree. Reproduced on a three-option chain: an update omitting the middle option returned no error, both of that option's parent links were gone, and the bottom option read back with only itself above it. The outcome was fail-secure -- an option that loses its ancestors is covered by less, so policies deny rather than grant -- but silent, and the two write paths answered the same question differently. The check is now one function, blockingChildBelow, asked by both paths: it reports the first option of a set that has something below it which is not itself in the set, so removing a whole branch at once stays legal. The options endpoint reports it against the position in the caller's payload; a field write reports which option the list left out and which option is still below it, and is refused with a 400 before anything is written. Scoped to the field's own hierarchy, so an option a field merely inherits from a template never blocks a write to the field that inherits it -- it is not that field's to delete, and a write to it does not delete it. A field read above the option-inlining cap comes back without its list; writing that copy back is not a request to drop every option, and is skipped here exactly as the store skips it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Run the property hooks when a field's options change A property field's options can be written two ways: as the field's own inline option list, through UpdatePropertyField, or one at a time through the option rows they are stored in. The first goes through the property service's hooks -- which is where the access_control group's protected flag, its owners list, and its Enterprise license requirement are enforced. The second went through nothing, so the same change was refused on one path and accepted on the other. The gap opened when options moved out of the field's Attrs blob and into rows of their own: an option change stopped writing the field's row, and every hook is keyed to a field or value operation. What it allows today: any caller with permission to manage a field's options -- including an administrator, and including another plugin -- can add, rename, re-parent, and delete the options of a field that a plugin protects, and can list the options of a field whose access mode says only its source plugin may read them. PropertyHook gains two methods for that path: - PreChangePropertyFieldOptions runs before options are created, changed, or deleted. One method covers all three verbs because what they have in common, the caller's authority over the field's option set, is the whole of what there is to decide before the change itself is validated. - PostGetPropertyFieldOptions filters a page of options after it is read. A hook may return fewer than it received; unlike the field post-hooks there is no cardinality rule, since a shorter page is the answer when a caller may not see all of them. Both receive the field as the store has it, and the write hook runs inside writableField -- after the master re-read the compare-and-swap is anchored on, before the checks on the field's shape, so a caller with no authority over a field is told that rather than being told what state it is in. AccessControlHook implements both with the rules it already applies to a field update: enforceFieldUpdateAccess for a change, hasUnrestrictedFieldReadAccess for a listing. The field stands for both sides of the update, which is accurate -- an option change alters no attribute of the field, so there is no incoming copy to judge and no owners list a caller could be adding itself to. LicenseCheckHook implements both as it does every other operation on a group it manages. Two behaviour changes follow, both on fields in the access_control group; every other group registers no hook and is unaffected: - A protected or owner-managed field's options are now refused to a caller that is not its source plugin or a listed owner, whichever governs. That includes administrators, matching what the field write path already answers. - A field whose access mode is not public now lists no options to a caller other than its source plugin. For source_only that is what the field read already does. For shared_only it is stricter: the field read filters the inline list down to the options the caller shares with the target, and there is no equivalent for the rows yet, so serving them unfiltered would answer a question the field read refuses. Serving none of them is the secure side of that until the filter exists. The four option methods on the property service take a request context so the hooks can see the caller. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Expose property field options through the plugin API A property field's options can be read and changed one at a time over HTTP, which is what a field whose options form a hierarchy needs: past the point where the field stops carrying its option list inline, a change has to name the options it touches. A plugin could only write the whole list, as part of the field, so a plugin could not maintain a hierarchy at all beyond the one it created. Four methods mirror the HTTP collection one for one, taking the same arrays and naming parents the same way: GetPropertyFieldOptions(groupID, fieldID, cursorCreateAt, cursorID, perPage) CreatePropertyFieldOptions(groupID, fieldID, options) UpdatePropertyFieldOptions(groupID, fieldID, options) DeletePropertyFieldOptions(groupID, fieldID, optionIDs) A field is addressed by group and identifier, without the object type the URL carries -- that exists to bucket fields by object type without leaking cross-bucket existence, and a plugin reads fields by identifier already. Nothing is exposed beyond the collection: no hierarchy traversal, no ancestor or common-ground helpers. The app layer needs those internally and they can be exposed later; a plugin contract cannot be taken back, so it starts as the predictable half. Two smaller differences from the HTTP surface, both because there is nothing on the other side to carry them: no connection is excluded from the websocket event a change publishes, since a plugin is not one of the clients being told to read the field again; and an update does not return the options as they stood, which exist for the audit record the HTTP layer writes. Three checks the HTTP layer was making on the way in move to the property service, where every caller passes: - At most 200 options per call, and pages of at most 200. The HTTP layer clamps a page size and refuses an oversized payload; a caller choosing its own numbers had neither bound. The number is now model.PropertyFieldOptionsMaxPerRequest, which the HTTP layer's own limit is defined as. - A cursor is both halves or neither. Half of one silently starts from the beginning, so a caller paging until a short page would never stop. - A legacy field is refused. It has no object type, no permission levels, and none of the access attributes the option hooks read, so there is nothing to gate its options by; its option list is part of the field, which is how it was written. Consequently the methods work on fields in a version 2 property group, which a group a plugin registers for itself is not -- the access_control group is the one a plugin reaches today. Nothing here changes that; it is why the tests are written against that group. The RPC glue, the timer layer, and the plugin API mocks are regenerated rather than hand-edited (make pluginapi, make plugin-mocks). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Answer a fully filtered page of options with an empty page A page of a property field's options runs through the read hooks before it is returned, and a hook that may not show a caller anything returns nothing. That nothing was a nil slice, which serializes as null rather than [] -- so the options endpoint answered a caller who is not entitled to a field's options with a body no caller looping over the page can read, where the same endpoint answers a field that simply has no options with an empty array. Two ways of having nothing to show, two wire shapes. Normalized where the hooks are dispatched rather than in the hook, because the next filter written will have the same shape: a hook that keeps what a caller may see by appending to a slice ends up with a nil one exactly when it keeps nothing, which is the case most likely to be the interesting one. The hook that has the behaviour today returns an empty slice as well, so the intent is visible at both ends. Also covers the four option methods on the plugin API's property client, which nothing exercised: each passes several arguments straight through, and what is worth pinning is that every one arrives where it was meant to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Say what an unhooked option hierarchy write would have to do first ApplyOptionEdges changes a graph property field's option hierarchy and checks the shape of that change, but not the authority behind it: the caller's right to change this field's options is decided by the property hooks, which it does not run. Nothing outside tests calls it -- a hierarchy change reaches the store as part of an option change, which writes the rows and their links together and does run them -- so this is a note on the method rather than a fix to it, saying what a future caller owes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Add end-to-end coverage for graph property field authoring A graph property field's options form a hierarchy that is authored on one field and served by others: a template field owns the options and their parent links, and a user or channel field linking to that template serves the same option set without owning any of it. Options are written either inline on the field, through attrs.options with a per-option parents list of names, or one at a time through .../fields/{field_id}/options. Each of those pieces has coverage of its own; nothing put them together over HTTP. channels/api4/property_graph_test.go does. TestGraphPropertyFieldAuthoring builds a three-option hierarchy in a single field POST with a forward name reference, links a user field and a channel field to it, tags a user and a channel with the options they hold, and reads all of it back. It then asserts what only shows up across several requests: - the same options, identifiers and parent links reach both linked fields, and are read-only through them, the refusal naming the field to edit instead - an option added to the template is served by both linked fields immediately and neither of their rows is written for it: their UpdateAt does not move, where the owning field's does - a field's inlined option list stays flat, so a client that knows nothing about hierarchies sees an ordinary option list - a value naming an option the field has none of, or naming one twice, is refused, and the value the object already held survives - a rejected option payload names the index of the item at fault and the reason, in the response rather than only in the server log, and writes nothing TestGraphPropertyFieldAboveHydrationCutoff covers the state a graph field of the size this type exists for is normally in. Past model.PropertyFieldMaxHydratedOptions a field reads back with its option list withheld and replaced by a count, so a caller holds no list to write back: the field-blob write path refuses a supplied one with a 400, while everything else about the field still writes and the options endpoints keep working on it. They address one option at a time and resolve a parent's name against the option rows, so they never need the whole list. A value naming an option no read ever listed is accepted, since the check behind a value write reads those rows too. Both run against the access_control property group rather than a group registered for the test, because it is the only group whose property values are validated at all, and a graph field is only useful where a value naming an option that does not exist is refused. Every assertion above was confirmed to fail with the behaviour it describes removed. Tests only; no production code changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Mask graph property values to the viewer's own hierarchy A property field with access_mode shared_only shows one object's values to another caller only so far as the caller shares them: select and multiselect intersect the two option sets, rank clamps the target's rung down to the caller's own, and every other type is all-or-nothing on an exact match. A graph field fell into that last case, so a caller saw a target's options only when their own value was byte-identical -- which ignores the hierarchy the type exists for and hides what a caller is entitled to see. Graph values are now masked against that hierarchy. Each option the target is marked with that the caller covers -- one of the caller's own options is at-or-above it -- shows as it stands. Each option they do not cover is replaced by the options below it that they do cover, keeping only those that none of the others sits above. An option with nothing covered below it drops out, and a value left with nothing at all is hidden. So a caller marked with one program in a family learns that a target marked with the whole family shares their part of it, and nothing about the rest of the family. The two halves are computed differently. Coverage is one walk up from every option the target holds at once, in PropertyService.coveredBy, which the four hierarchy predicates now read their answers from as well. The replacement descends from the option instead, stopping each branch at the first option the caller covers, which is what leaves the result maximal without materializing anything. Stopping prunes only the branch it happens on, though, so the descent costs the size of the subtree below the option whenever the caller's clearance is narrow: on a 1,110-option subtree it costs the same whether the caller covers nothing below it or holds an option one level down, and about four times what intersecting the two down-sets costs. Neither half is capped -- a cap would mask a value to less than the caller may see with nothing to say why, and the subtree bounds the descent already. Intersecting is the upgrade path if masking shows up in a read profile; both formulations sit on the same store primitives and agree option for option. Stopping at the first covered option does not by itself leave the result maximal -- where branches rejoin, an option can be reached by a route that crossed none of the ones already taken -- so the candidates are compared against each other at the end, in maximalElements. CommonGround, whose walk has the same shape, now shares that comparison and the descent step with it. Any failure to establish what a caller may see hides the value and logs, since hiding is also the answer for a caller who genuinely shares nothing and there is otherwise nothing to tell the two apart. The hierarchy is read from the option rows rather than from the option list inlined into a field read, so a field with more options than a read inlines masks like any other instead of hiding everything for want of a list to check against. The option list on a field read is left as it was: a shared_only graph field still filters it by exact match, which tells a caller less than they may see, one graph option standing for everything below it. Filtering that list against the hierarchy is a follow-up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Mask graph option lists to the viewer's own hierarchy A property field with an access mode of shared_only shows a caller only what they have in common with whatever they are reading. For a field whose options form a hierarchy, what they have in common is settled by that hierarchy: a caller covers an option when one of the options they hold themselves sits at or above it. That is already how such a field's values are masked. Its option lists were still answered by exact membership, or not answered at all. Three surfaces change. The options endpoint, and its plugin equivalent, served a shared_only field's options to nobody but the source plugin, for want of a filter over the option rows. It now serves the options the caller covers, judged one page at a time by walking up from that page rather than by building the set of everything the caller reaches, which can be the whole hierarchy. Each option's parents are reported by name and a parent sits above the option it belongs to, so a masked page carries no parents -- an absent parents key already means "not reported" to the write path, where an empty one would mean "this is a root". The option list a field read carries inline was filtered to the options the caller holds themselves, which under-reports what they may see, since an option stands for everything below it. It is filtered by coverage now. A field with more than PropertyFieldMaxHydratedOptions options has no inline list to filter and still shows none; the endpoint above is how such a field's hierarchy is read. The option names shown inside an access control policy, in channels/app/access_control_masking.go, reached the field's option list only for select and multiselect fields. Every other type went to the caller's stored text value instead, which for a hierarchy value fails to parse and for a rank value yields an option identifier where a name is wanted -- so every value in a condition against either type masked, and a policy written on a hierarchy rendered blank to the administrator reading it. Both types read the option list now, as the neighbouring maskConditionValues already did. Filling a page is new, and is what makes a masked listing traversable at all. A caller pages until a page comes back shorter than the size it asked for, so a page that lost most of its options to masking would end the listing early and one that lost all of them would end it immediately -- on a large hierarchy where a caller covers a handful of options, the listing would answer nothing. GetFieldOptions therefore reads on until the page is full or the field's options run out. That costs a scan when little of a large field is visible, which includes a source_only field, where the answer is nothing and used to cost a single page. The way out of the scan is to ask the rows for the options the caller may see rather than to filter them afterwards. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Cover the option listing on a linked hierarchy A field whose options form a hierarchy is meant to be used by defining the hierarchy once, on a template, and linking the fields that objects are marked through to it. Nothing covered the option listing in that shape: the options such a field lists are the template's rows, while the values saying what a caller holds are the linked field's own -- so a filter resolving the hierarchy against the field holding the values would find every option unrelated to every other and show nothing. Asserts that a caller listing the linked field is shown the options they cover, that those options are reported read-only because the template owns them, and that a caller addressing the template itself is shown nothing, since nobody holds a template's values. Test only; the behaviour is already correct. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Cover filling an option page from several pages of rows Options a caller may not see are dropped from a page and the page is then filled from the rows behind it, which can carry the answer past the size that was asked for; the surplus is dropped, and the caller picks it up again by continuing from the last option they were shown. Nothing exercised that. Every existing case either fits in one page of rows or asks for one option at a time, so the branch that trims a filled page had no test at all, and neither did the property that matters around it: paging on from what was shown skips no option and repeats none. Asks for two options from a field where the caller may not see the first, so the answer has to span two pages of rows and comes back one option longer than it may be. Test only; the behaviour is already correct. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Say which callers pay for an option listing's scan Filling a page of options from the rows behind it scans the whole field when little of it is visible, and the comment saying so named only one caller that pays it: a source_only field's, other than the source plugin. The likelier one was left out. A field that shows a caller only what they have in common with it shows nothing at all to a caller who holds nothing for it -- an administrator included, on a field whose options they may nonetheless change -- and that caller now reads every row of the field to be told so. Comment only. It also states why the obvious short circuit is not available: distinguishing these callers needs a hook that can say "nothing here, ever", where "nothing on this page" is what the loop exists to read past. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Author graph hierarchy rules in the access policy editor A graph property field's options form a hierarchy, and an access rule reasons about it through four member calls on the holder's attribute: coversAll, coversAny, withinAll and withinAny. The visual rule editor knows none of them, so such a rule can only be written and read as raw CEL — isSimpleExpression refuses it, which also disables the toggle back to the table view. Teach the table editor the four predicates. Each becomes an operator whose label is the CEL function name, so a condition the server hands back needs no translation in either direction: it reports the function as the operator. A graph attribute's operator menu offers those four plus "has any of"/"has all of", which lower to a chain of `in` tests — the exact, hierarchy-blind membership check, and the only other form the policy engine accepts on a graph attribute. A predicate's one argument is a list of option names or the accessed channel's graph attribute. Two graph attributes may be compared when they draw on the same option pool: the template they both link to, or one being the other's link source. That is looser than the shared-template rule the other option-bearing types use, and it is what the server checks at save. The membership operators take no channel target on a graph attribute — hasAllOf against a channel attribute requires both sides to be multiselect — so the target is withheld from the picker, dropped when the operator changes, and never serialized even if a stale one survives. Also render graph fields read-only under User Attributes in the System Console. Their options carry parent edges the values cell knows nothing about, so saving from there would keep the option names and drop the hierarchy between them. The type now names itself ("Graph", not the fallback "Text" label), and both the type menu and the values cell are locked on the field type — the equivalent of what isLinkedField already does for a field that inherits its options. Two gaps left open. The option picker holds the whole option list in memory, which needs an asynchronous searchable option search at thousands of options. And nothing renders the withheld-options state: a field whose options the server declined to inline — routinely a graph field, and possible for any large option-bearing field — shows an empty picker instead of saying the names are not visible to this caller. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Add end-to-end coverage for graph hierarchy access policies A graph property field carries a hierarchy of options, and an access policy can ask whether the programs a user holds cover the ones a channel is tagged with. Two browser specs now exercise that against a licensed server, split into an editor half and an evaluation half the way the surrounding ABAC suite already splits its coverage. policies/graph_operators.spec.ts drives the Membership Policy editor. Selecting a graph attribute offers the four hierarchy predicates plus the two exact-membership operators and none of the equality, string or ordinal ones, and a coversAll rule authored over option names, and again over a channel attribute, survives save and reopen. A graph predicate is not stored as written — it is desugared into a marker over group and field ids — so each round trip also asserts the expression the server serves back afterwards. That is what makes it evidence of the server rehydrating the rule rather than of the browser remembering what it just submitted. resource_attributes/graph_sync.spec.ts drives both evaluation lanes over the REST API against a channel holding F-18 Program. The membership sync keeps a member holding an ancestor of it, removes one holding only a sibling, auto-adds a covering team member, and removes a member holding nothing; the runtime decision point then agrees on the same fixtures. A second test moves a member's value sideways in the hierarchy and re-runs the sync to assert the removal. The fixture builder sits beside the multiselect one it mirrors in resource_attributes/helpers.ts. Both specs skip when the graph field type is not enabled on the server: it is gated on a feature flag that is off by default, and a feature flag is read-only through the config API, so a spec cannot turn it on for itself the way it can with a config setting. Both specs delete the fields and policies they create. The access_control group caps how many fields may exist in it at once and the policy editor reads only the first page of them, so a spec that leaks fixtures degrades later runs rather than its own. Deleting a parent policy needs its channels unassigned first, or the delete is refused. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Delete the fixtures the ABAC policy specs create Seven specs under system_console/abac created property fields and access policies and left every one of them on the server. That is not a problem for the spec that leaks — it is a problem for whichever spec runs next, and it had made the suite unusable on a development server: - The access_control property group allows at most 20 fields on the user object type. Once reached, every later spec that creates a user attribute fails in its setup, which reads as unrelated breakage far from the cause. The server this was found on had all 20 slots held by leaked fixtures. - The System Console pages through the policy list. A spec that saves a policy and then finds it by scanning that list starts failing as soon as accumulated policies push a new one off the first page — and the failure looks like the save was rejected. Each spec now tears down what it created: a list of cleanup callbacks drained in afterEach, or an afterAll where the fixture is shared by a serial describe. Two ordering constraints are load-bearing and commented at the helper rather than left for the next reader to rediscover: - A policy has to go before the fields its rules reference. While attribute-value masking is on, deleting a policy first asks which of its values the caller may see; that check fails outright when a referenced field is already gone, and the policy can then never be deleted through the API. - A parent policy has to have its channels unassigned first, or the delete is refused for having child policies. Teardown is best-effort throughout, so a failure to clean up cannot mask the assertion that already ran, and deleteCustomProfileAttributes only removes fields the call actually created — a shared attribute like Department that a spec reused rather than created survives. deleteGraphHierarchy becomes deleteLinkedFieldTrio, since the template/user/ channel fixture it deletes is the same shape whether the fields are graph or multiselect and both now need tearing down. Verified by measurement rather than by inspection: with the group emptied first, the resource_attributes suite and advanced_policies.spec.ts each leave the field and policy counts exactly where they started, where before they added 6 and 14 rows respectively. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Keep a masked graph condition's shape when the table editor saves it A condition whose values are all hidden from the reader renders with no values, so the table editor cannot serialize it from what it holds. It emits `user.attributes.<field> in []` instead -- a placeholder carrying the attribute and nothing else -- and the server puts the hidden values back before the policy is stored. That placeholder does not work for the four graph hierarchy predicates. The server pairs a stored condition with the submitted one by call shape, so an `in` list does not stand in for a stored `coversAll(...)`: the save is refused with "masked condition node was deleted", and every other condition in the policy goes down with it. Exclude the predicates from the placeholder and let them serialize through their own branch, which for a row with no values yields `coversAll([])` -- an empty target list, which is the same placeholder idea in the shape the pairing expects. Exact membership on a graph attribute is left on the generic placeholder. It is stored as an `in` test, so `in []` is already the shape the server pairs on. Also corrects the comment on the placeholder's own test, which said the server takes the operator from the submitted expression. It does not -- it pairs on the submitted shape and restores the stored operator -- which is the whole reason a shape the placeholder cannot stand in for has to be excluded rather than tolerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Skip the graph policy specs that need a channel attribute when it is gated off Comparing a user's attribute against the accessed channel's is gated by the ResourceAttributesInPolicies feature flag, which is off by default. With it off, saving a policy that references resource.attributes.* is rejected outright. Three graph specs author exactly that shape -- a coversAll predicate whose target is the channel's own graph attribute -- and carried no guard for it, so on a server in its default configuration they failed rather than skipping. Every other spec in resource_attributes/ already skips on that flag; these were written against the attribute type rather than against the resource-attribute surface, and so missed the convention. Both tests in graph_sync.spec.ts need it. In graph_operators.spec.ts only the channel-attribute round-trip does: the other two tests name option names literally, and skipping those on a flag that has nothing to do with them would lose coverage of the hierarchy predicates themselves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Say who is asking when the properties routes read property fields and values A shared_only field's options and values are masked against what the caller themselves holds, so every read of one has to state the caller. The two read paths under /properties did not: searchPropertyFieldsCore and getPropertyValuesCore passed the untagged request context straight to the service, the access control hook read no caller ID, and every caller -- the owner of the values included, and a sysadmin -- was answered as someone who holds nothing and shown an empty list. Nothing reported it. Hiding is the correct answer for a caller who cannot be identified, and it is the same answer a legitimate caller who shares nothing with the target gets, so the handlers looked like masking doing its job. Only the write handlers on the same routes tagged their context, which is why the equivalent CPA routes masked correctly throughout. The four affected read handlers -- getPropertyFields, searchPropertyFields, getPropertyValues and getSystemPropertyValues -- all funnel through those two functions, so this is two lines. The options endpoints under .../fields/{field_id}/options were already tagged, in propertyFieldForOptions, and are unaffected; so is source_only, which answers an unidentified caller exactly as it answers a non-source one. There is no structural guard against a future handler forgetting, and deliberately no log either: the service cannot tell a request that forgot to tag itself from one that is genuinely anonymous, so warning on the second to catch the first would mean a line per anonymous read forever. The reasoning is left on sessionCallerID, which every handler already calls. The tests read one target's values back over the properties route as four callers -- the owner, a caller sharing part, a caller sharing nothing, and a sysadmin -- and assert what each of them gets rather than that anything came back. Both masking branches are covered, since multiselect intersects holdings and graph resolves a hierarchy, and an untagged caller fell into the same hole in both. The sysadmin case asserts they see nothing: shared_only masks admins like anyone else, matching the CPA route, stated so that granting them a bypass has to be deliberate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Fix Access Rule Test modal alignment and add the new save-error strings Review feedback from @lindalumitchell on #37755. Search box alignment. Both steps of the Test Access Rule modal inset their search box 16px, while the modal title and the members list rows sit at the 32px .modal-header gutter — so the search visibly overhung everything above and below it. Moves both searches (and the channel picker's list rows, which followed the search) to 32px. Missing magnify icon. The members step has no icon element of its own, so it draws one with a ::before on the row wrapper, and pads the input 38px to make room. The input's opaque background painted over the glyph, so the icon never appeared and the 38px read as unexplained blank space before the "Search users" placeholder. A z-index on the pseudo-element lifts it above the input. Back arrow. It was pulled out of flow into the header's left gutter to keep the title at a fixed x-position across both steps, but the gutter is only as wide as the arrow plus its 8px gap, which left the arrow flush against the modal edge. It now leads the title in normal flow, starting at the same 32px gutter as everything else; the title shifts right by the arrow's width on this step, which is the ordinary modal-header layout. Also adds the en.json translations for the two access-control policy save errors introduced in mattermost/enterprise (a team cannot be given rules that reference channel attributes, because a team has no attributes). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Move the clearance checkbox out of its own section Review feedback from @abhijit-singh on #37755: "This can just be a checkbox after selecting the preset/source." The Classification Enforcement section held a single checkbox, so it now renders as a Setting in the main form directly below the classification preset dropdown. Same ABAC gate, same state, same save path — only the placement changes. The section's title and description strings are dropped; the three clearance strings move to the page, which also puts them in searchableStrings for the first time. Deletes classification_enforcement.tsx and its test. The main page test already covers the checkbox creating and deleting the linked user field, so only the membership-policy link test moves over with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Match the channel attribute chip to the Figma design Measured against the export Abhijit attached (318x40 @2x, i.e. a 159x20 chip): - brackets were 8px tall, design has 9px. The glyph fills 16 of the icon's 24-unit viewBox, so size 14 renders it at 9px; 12 rendered 8px. - Menlo made the chip 200px wide for the same string against the design's 159. The design uses the body font, so drop the monospace family. - no border, and the fill is 12% of --button-bg rather than 8%. The border was also what made the chip 22px tall instead of 20px. - 6px of horizontal padding, not 8px, and an explicit 16px line-height so the height doesn't ride on the font's default leading. The result is 161x20 against the design's 159x20 — the 2px is Chrome measuring the string wider than Figma does. Bracket glyph, fill, text colour and corner radius are pixel-identical. * Open the policy editor from its own fetch, not the list's copy A rank comparison is stored desugared into a marker call, so the policies list — filled by /access_control_policies/search, which returns rules in their stored form — leaves `_rank_ge(user.attributes.x, "Secret", "<fieldID>")` in the store. Clicking through to the editor seeded its expression from that copy and TableEditor immediately POSTed it to /cel/visual_ast, which rejects marker calls: "unsupported function/operator: _rank_ge", HTTP 400. onParseError then flips the editor to Advanced mode, and nothing flips it back. Whether it flips is a race. fetchPolicy runs on the same mount and replaces the expression with the rehydrated operator form, and the effect's `cancelled` guard drops the 400 if that lands first — which is why this reproduces on some machines and not others, and only for rank rules: every other type is stored as authored, so search returns something the endpoint accepts. The seed was redundant to begin with — fetchPolicy sets the name, rules and active flag as well as the expression — so this drops it and the doomed parse with it, one round trip lighter per policy opened. Pre-existing on master, not introduced by the resource-attributes work. * Open the permission policy editor from its own fetch too Same defect as the membership editor, same store: the permission policies list is filled by /access_control_policies/search, whose rules are in stored form, so a rank comparison arrives as `_rank_ge(...)` and seeding the editor with it fires a /cel/visual_ast parse that 400s. This editor already recomputes its mode from the fetched expression, so the flip to Advanced only sticks when the 400 lands after the fetch — a narrower race than the membership editor's, but the same one. fetchPolicy sets the name, role and permissions as well as the expression, so dropping the seed loses nothing. * Warn when a policy's rule reads a channel attribute A channel with no value for a referenced channel attribute cannot be evaluated, so the server fails closed: resource_resolution.go swaps the whole rule for the constant false, and every member of that channel loses access. Nothing said so while authoring — the policy saved cleanly and the members disappeared. The notice fires on the pair (the rule references any channel attribute, the policy governs at least one channel) rather than on the precise condition "this channel has no value". Reading values would cost one request per assigned channel — GET /properties/groups/{group}/{object_type}/values/{target_id} takes a single target id and there is no bulk-by-ids form — and the broad version is what the reviewer asked for. Both halves reuse existing helpers: referencesResourceAttributes strips quoted literals first, so a value that spells out an attribute path is not mistaken for a reference, and hasChannels already counts unsaved additions, so the warning arrives while it can still change the outcome rather than after save. * Assert the team-boundary rejection's own error id The e2e helper pinned app.pap.save_policy.app_error, which the enterprise commit "Give the team/channel-attribute save rejections their own error messages" replaced with app.pap.save_policy.team_resource_attributes for this path. The spec was the only thing left asserting the old generic id, so playwright-full-enterprise failed on it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Warn on a channel-attribute rule before any channel is assigned Keying the notice on the policy already governing a channel made it arrive too late to act on: an admin authors the rule first and assigns channels after, so the warning appeared only once the damage was configurable. The rule reference alone is enough to warn on. Also pads the channel-attribute chip to match the Figma spacing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Remove a comment restating the next line in the property field update path Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix stale migration numbers and drop restating comments The migration renumbering left two comments pointing at pre-renumbering migration numbers: the down migration referenced the split-attribute-view migration by its old number, and a migration test referenced its own old number. Also removes four test comments that only restated the line below them. * Carry the withheld-options markers through the CPA field shape CPAAttrs had no field for options_count/options_omitted, so converting a field whose option list was withheld dropped both markers and served it as an ordinary empty list. * Recognize withheld option lists in webapp property field consumers A field with more than 1,000 options is served without attrs.options and attrs.options_omitted set instead, but the admin validator still flagged it as missing options, and the Profile Settings editable select crashed on the absent list. * Regenerate migration manifest for the property options migrations * Batch property value option-existence checks by field validateValueAgainstField now returns the option IDs a value names instead of checking them itself, so validateValues can look each field's options up once per field rather than once per value. A batch spanning N option-bearing values against one field used to cost N sequential store round-trips; it now costs one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Pin that a store failure fails the whole option-existence batch A GetExistingOptionIDs error must propagate as a store failure, not be mistaken for an empty option set that rejects every pending value as naming an option that does not exist. * Fix stale comments and redundant coverage in options-withheld tests One comment claimed the property service could not validate a value against an oversized field's option list, which no longer holds now that validation checks the option rows instead of the inlined list. Another doc comment said the opposite of what its subtest asserts. A test named after the linked-field guard actually pins the withheld guard, which fires first, so rename it and drop the CPA group setup it never used. Delete two app-layer subtests whose behaviour is already covered one layer down. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Inline the single-call optionsWithheld helper model.PropertyFieldOptionsOmitted is called directly everywhere else in this code; having a same-named wrapper in this file only meant the same check read under two names depending on which file you were in. * Trim duplicate rationale from the withheld-options round-trip comment * Group and dedupe option IDs in one pass during batch validation A second walk over the pending values built the per-field option-ID list without checking for duplicates. A batch naming the same option on the same field more than once sent that ID to the store repeated, and a large enough batch could push the lookup query past PostgreSQL's bind parameter limit. * Move withheld-options round-trip subtests into their own test They lived inside TestCPAField_ToPropertyField_DisplayName, whose other subtests are about DisplayName and Owners, so the options_count / options_omitted coverage was easy to miss. * Cover the withheld-options case in the admin field validator Nothing tested the options_omitted escape hatch that lets an admin edit a select-style field whose option list the server withheld for being too large. Add cases for both sides: a withheld list raises no warning, a genuinely empty one still does. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Check option existence against a set instead of scanning a slice A batch naming a few thousand distinct options on one field turned this into O(values x optionIDs x distinct options) string comparisons; a map lookup makes it linear. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix stale migration number in TestMigration000218 comment and drop a comment restating the helper's name * Drop the redundant graph-type gate on the property field update path PropertyService.updatePropertyFields already rejects any conversion to or from the graph type unconditionally, so the app-layer gate on update duplicated that check while only ever answering a flag-off caller with a less specific error. graphPropertyFieldGate now runs on create only; comments describing it are updated to match. * Check the graph field option-count limit before scanning for ranks The rank scan marshals the whole inline option list to JSON and back just to look for one key, while the option-count check only reads the slice length through reflection. Running the count check first means an over-limit write no longer pays for that round-trip on a list that's about to be rejected anyway. Also corrects the SupportsOptions doc comment: the webapp does render graph field options (read-only), it just excludes graph from supportsOptions because that helper also gates the "at least one option" save requirement, which options the server withheld can't meet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Clarify why the create-path linked-field gate check skips its read Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Drop the unused caller-name parameter from graphPropertyFieldGate Both call sites always passed the same literal, so the parameter carried no information; inline it into the AppError construction instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix migration numbers left stale by a later renumbering A prior merge renumbered these migrations, and three comments still named their old numbers, which now belong to unrelated migrations elsewhere in the sequence. * Log unresolved graph property options at debug rather than error A value naming a deleted option is invisible-but-tolerated state, not a failure: the walk succeeded and simply found nothing for that identifier. The live read path re-enters this on every descent level, so one such value logged an error line on every profile popover or channel attribute read for as long as the value stayed. A failed hierarchy read is still an error, and is still logged where the caller handles it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Replace the stale rationale sentence in the unresolved-options doc comment The doc comment still said a decision "leaves no trace of why it went the way it did" after the debug-logging sentence was added alongside it, so the comment argued two different justifications instead of one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Delete the predicate and common-ground methods nothing calls The enterprise policy engine reads the hierarchy through the store directly and evaluates the four coverage predicates itself over the expanded sets, so CoversAll/CoversAny/WithinAll/WithinAny and the coverage helper behind them were a second, unused implementation of the same relation. DescendantsOrSelf and CommonGround have no callers at all. The remaining coverage: the at-or-above relation over stored rows is covered by testPropertyFieldOptionHierarchy in the store tests, the predicates' semantics are covered by the enterprise decision tests, and the read-masking direction stays covered by TestGraphClampToCoverage. * Delete the unused ApplyOptionEdges hierarchy writer A hierarchy change always reaches the store as part of an option change, through CreateFieldOptions and UpdateFieldOptions, which write option rows and edges together. Nothing else called this function, and keeping it around invited a caller to write a hierarchy change without first checking write authority on the field. Replace its one caller in the test suite with the store call it wraps, and drop the arithmetic subtest that restated the same limit the adjoining table already covers. * Delete MutateOptionEdges from the property field store It was a one-line delegation to MutateOptions with the options-to-upsert argument fixed to nil, kept only for tests. Every caller now uses MutateOptions directly with nil in that position, so the store carries one entry point into the hierarchy cache invalidation instead of two. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix a stale comment about which mutations invalidate the property field cache The comment described the invalidation test as exercising a hierarchy change, but the call it sits above passes nil for every argument, so it exercises no hierarchy edges at all. * Correct two comments that describe code that is not there MutateOptions's doc comment still named ApplyOptionEdges, which no longer exists; point at CreateFieldOptions and UpdateFieldOptions instead, matching the wording already used in graph_validation.go. The cache invalidation subtest's comment claimed MutateOptions always moves the field's UpdateAt, which is false for the real store when all three change lists are empty -- it returns immediately without writing anything. What the subtest actually pins is that the cache layer's wrapper invalidates the group on any accepted call, regardless of what changed. * Batch option edge writes and existence lookups past the parameter ceiling MutateOptions built one INSERT/DELETE statement per call for all added or removed edges, and getExistingOptionIDs built one WHERE ID IN (...) for the whole option ID list. At 200 options with 100 parents each, a single statement could carry tens of thousands of bind parameters, past PostgreSQL's 65,535-per-statement limit, so validated requests could still fail in the driver. Chunk both at maxOptionIDsPerQuery, the same treatment applyOptionParentLinks already uses. requireOwnedOptions now checks membership through a set built once instead of scanning the existing-ID slice per option. * Cover a denied patch and a denied delete on property field options The permission-gating subtest only tried a denied create, then returned. A caller with the field visible but not its options permission could still hit patch or delete, and this test would not catch it. * Record an audit entry when listing a field's options is refused Move the audit record creation ahead of the scope and permission check in getPropertyFieldOptions, matching the convention the field listing and the option write handlers already follow. A caller denied access to a field's options now leaves a failure record instead of no record at all. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Document that an option's name is required on every write A patch that omits Name is refused by validation before any stored option is read, so there is nothing to fall back to. Say so instead of implying it behaves like Color and Attrs, which really are filled in from the stored option when omitted. * Serve a page of field options from a replica GetFieldOptions backs a plain GET that any caller who may see the field can call, and the masking loop above it may page through every option a shared_only field has before finding one the caller may see. None of that needs the primary. attachOptionParentNames now takes the connection to query so GetFieldOptions can pass the replica while the write-path callers (GetOptionsByID, and the transaction inside MutateOptions) keep the master they need to see their own uncommitted work. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Read a page of graph options and their parent edges from the same connection GetFieldOptions called GetReplica() twice, once for the page query and once to attach parent names. Two calls can round-robin to different replicas at different points in replication, so an option with parents could come back with an empty parent list if the second replica lagged behind the first. Take the replica once and reuse it for both queries so both read from the same point in replication. * Move the inline-hierarchy comment back next to the code it describes A later insertion split it from validateOptionBlobLinks, leaving it floating above an unrelated function and pointing "below" at the wrong checks. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Tighten the rank-field comment in requireWritableOptions Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Refuse a graph field's option list that names one option twice A name has to identify one option: a rule naming one is stored as the identifier the name resolved to, and a lookup over two rows answering to one name keeps whichever the query returned last, so every holder of the other is denied with nothing reporting a problem. The options endpoints already refuse a repeat within their own payload. The list a field is written with did not, except for fields in the managed profile-attributes group, whose hook decodes the list through a type that checks. Held to graph fields only -- the types with no hierarchy have never been held to it, and a field already holding two options of one name has to stay writable for the name to be fixable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Prune the comments on the repeated-option-name check Cut a clause restating the function's own callers, an aside naming the one caller that would go stale the moment a second appeared, a sentence the one after it already said concretely, and a duplicate of the source comment in the test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Keep an access-controlled field's option list out of the websocket broadcast A broadcast has no recipient to filter options against, so a non-public field must go out with none at all -- a client that needs the list reads the field back, and that read is already filtered per caller. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Route access-control cache drops through the shared helper Both UpdatePropertyFields and DeletePropertyField duplicated the same nil-guard around AccessControl.OnPropertyFieldOptionsChanged; route them through invalidatePolicyCachesForOptionChange instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Document that rank fields share the select option-existence check Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Refuse a field write that drops the withheld-options marker An oversized field's options are withheld on read, and the store skips reconciling a field's options only when its attrs still carry the options_omitted marker. A write whose attrs supply no option list and have also lost that marker looked like a harmless read-modify-write, so it slipped past the existing guard and the store deleted every option the field owned. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Keep the withheld-options marker through masked field reads A field whose option list is too large to inline reads back carrying options_omitted instead of the list, and a write that has lost that marker is now refused because it looks identical to a caller asserting the field has no options. But every read-path masking branch called HideOptions, which deletes options_omitted along with the option count, so a masked read-modify-write could never succeed even when the patch never touched options. Add maskedFieldCopy, which masks a field and then restores the marker (never the count, which stays controlled), and route the five masking sites through it. * Collect broadcast field events through one test helper Two subtests duplicated the same Eventually loop that collects websocket property-field-updated events, and each unmarshaled the event payload twice -- once inside the loop, again in the assertion below. Worse, the unmarshal's require.NoError ran on Eventually's own goroutine, where a failure would strand the test for five seconds instead of failing it. collectFieldEvents centralizes the loop and returns the parsed field alongside the event, asserting any unmarshal error back on the test goroutine. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Drop boilerplate step comments from the property field option API tests Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Decide an option listing's read gate from the stored field, not the caller's copy GetFieldOptions used the *model.PropertyField the caller passed in to decide addressability, which rows to page, and what the post-hook filters against. A caller could hand in a bare field carrying only an ID and a group, which the access-control hook read as public and served every option through. Re-read the field once before any of those decisions and use that copy for all three, matching what the write path already does. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Clamp a property field options page to the service's own bound The HTTP layer's own per_page maximum happened to match the property service's page limit, so a request above it never actually reached the service's refusal. Clamp explicitly instead of relying on that coincidence, so raising the HTTP maximum for unrelated reasons can't turn an oversized page request into a 400. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Inline the plugin API's option field lookup Each of the four plugin option methods opened with a one-line helper call that just wrapped GetPropertyField plus its error check. The helper cost more (a name, a doc comment, an extra hop) than it saved, since every call site is the same length either way. * Export the graph hierarchy coverage lookup CoveredBy is about to gain a caller outside the properties package that has no other way to ask which options a set of holders reaches. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Answer a policy's option names from the option rows when a graph field's list is withheld A shared_only graph field past the hydration cap reads back with its option list withheld rather than inlined, so masking had nothing to read a caller's visible option names from and hid every literal against it, even ones the caller holds. Resolve those names on demand instead, through the store and the graph coverage lookup, memoized per literal within one MaskingFieldInfo. Scope is graph only; a withheld rank, select, or multiselect list keeps failing closed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Let a property hook say a caller may see none of a field's options at all Adds MayShowAnyPropertyFieldOptions to the PropertyHook interface, answering once per listing whether a caller can ever see anything from a field's option page, rather than only whether the current page has anything. The access-control implementation reuses the same unrestricted-access and access-mode checks PostGetPropertyFieldOptions already applies, so the two cannot disagree about the same caller. Nothing calls the new hook yet. * Stop the option listing before it scans a field nothing in it is visible in A caller who may see none of a field's options used to read every page of it to find that out. Ask the hooks once, before the loop, whether the caller may see any of the field's options at all; if none of them may, return an empty page without reading a row. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Batch the caller's holdings and coverage walk when masking a field's graph values Reading a page of a channel's members' attributes calls into this masking once per member, and each call re-read the caller's own held options and re-walked the hierarchy from scratch even though the caller and field are the same throughout. Now the holdings are read once per field and the coverage question is asked once over the union of every value's options, with each value still masked on its own. * Assert the unmasked option listing actually contains the option before checking its parents The loop only ran its assertions when it found F-18 Program; if the listing came back empty or without it, the subtest passed anyway on the masked half alone, which is exactly the regression this half exists to catch. * Share one copy of the graph clamping rule between single and batch value reads filterSharedOnlyGraphValueBatch had its own copy of clampToCoverage's replacement rule so a batch could share one CoveredBy walk and memoize each option's descent; that left the rule duplicated and its only unit tests exercising a copy the batch path no longer calls. clampToCoverage now takes an optional graphClampBatch carrying the shared CoveredBy answer and descent cache, so the batch path calls the one implementation and the existing tests exercise the code the read path actually runs. Also drops filterSharedOnlyGraphValueBatch's error return: every one of its failure paths already masks the value and logs why, so the error was always nil, and its caller's dead error-handling branch is removed with it. * Cover graph field masking on the policy editor's visual AST path TestGetMaskedVisualAST_GraphWithheldOptionList drives GetMaskedVisualAST over a shared_only graph field whose option list is withheld for size, covering both single- and multi-literal conditions. The existing resolver-path test only exercised MaskingFieldInfo.IsValueHidden; the two paths resolve visibility with separate code and could silently disagree. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Require a license on the option listing's early exit The license check hook answered the field option listing's per-page check but had no implementation of the new hook that lets a caller skip the scan entirely when nothing in a field would be visible to them. That left an unlicensed managed group answering an empty page instead of a license error for callers the early exit fires for. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Drop an unreachable nil check from the option listing's license hook The only caller reaches it after the listing has already refused a nil field, and every sibling method on this hook reads field.GroupID without such a guard. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Make the runtime PDP assertion in the coversAll graph sync test exercise a real join The covering user was already a channel member from the sync job, so adding them again was a no-op that never reached the policy engine. Remove them first so the add is a real join under test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Show option names, not ids, for graph attributes on the user detail page A graph field's value is a list of option ids just like select and multiselect, but supportsOptions excluded it because that helper also gates whether the option list is editable here, which graph correctly opts out of. Add valueRefersToOptions to answer the display question separately, and use it when resolving option ids to names in the save changes summary. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Render graph attribute values as option names on the profile page An admin-managed graph attribute showed raw option ids because the profile page only resolved ids to names for select/multiselect/rank. An editable graph attribute fell to a plain text input holding an array of ids instead of the option picker. Both paths now check valueRefersToOptions, and the picker accepts multiple choices for graph the same way it does for multiselect. * Show option counts for graph attributes on the Manage Attributes page The Options column only checked whether a field carries an option list at all, so a graph field read as free text there even though it has options loaded elsewhere on the page. * Prune and sharpen comments left by the graph attribute display fixes Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Read the option-backed question through one predicate on the attribute pages valueRefersToOptions already means "supportsOptions or graph", so the user attributes value cell can ask it directly instead of restating the graph half inline, and the user detail page's option-name resolver no longer names only the three types it handled before graph joined them. * Render a graph custom attribute as an option picker on the user detail page A graph attribute's value is a list of option ids, exactly like a multiselect value, so it shares the multiselect editor instead of falling through to a plain text input that showed raw ids. * Trim redundant and provenance-laden comments in shared-only read tests Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Restate shared-only read comments as standing behaviour, not bug history Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Add translation strings dropped by a master merge A master merge silently lost strings this branch had already added: the graph option for the user-properties type picker, the four graph-hierarchy operator labels in the access-control table editor, and the server-side error string for an unreadable graph property value, which had never had an entry at all. * Create property option indexes concurrently in dedicated migrations check-style failed because CREATE INDEX on PropertyOptions and PropertyOptionEdges did not use CONCURRENTLY. Split those indexes into their own morph:nontransactional migrations so they can be built without locking DML. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Fix golangci-lint findings in property option code Address govet shadow, minmax, and mapsloop issues that check-style reached after the concurrent-index vet passed. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Address CodeRabbit findings on graph option parents and empty values Reject duplicate parent names in OptionParentLinks, normalize a cleared graph attribute to an empty array, and correct the session-attribute option-count comment. Co-authored-by: mattermost-code <matty-code@mattermost.com> * chore: retrigger CI after GitHub Actions startup_failure Server CI on2635e3d7concluded startup_failure with Compute Go Version and Server CI Complete stuck queued (no job logs). Last fully executed Server CI and Web App CI succeeded on071f3d84. Co-authored-by: mattermost-code <matty-code@mattermost.com> * docs: note why graph clears persist [] and parent names must be unique Empty graph values must be an option-id array, not a string; duplicate parent names on one option would write the same edge twice. Co-authored-by: mattermost-code <matty-code@mattermost.com> * fix: keep omitted graph options read-only and reject negative cursors Web App check-lint failed on a missing blank line before the graph-clear comment. A graph field whose option list was withheld could drop stored IDs on save; treat options_omitted as read-only and keep unresolved IDs for display. Reject negative cursor_create_at on the options listing API. Co-authored-by: mattermost-code <matty-code@mattermost.com> * fix: show omitted-option select values in user settings When options are withheld, getDisplayValue only preserved array values. A scalar select stored ID was rendered as empty while the field is read-only. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Render omitted-option graph fields read-only on admin user detail When options_omitted is set, CPAMultiSelect had no options and dropped stored IDs from the picker. Saving that empty selection would wipe the user's graph/select values. Show the stored IDs as a disabled field with an explanation instead. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Batch option-edge deletes to stay under Postgres parameter limits deletePropertyOptionEdgesForOptions binds each ID twice plus FieldID. Chunk by (maxOptionIDsPerQuery-1)/2 so a large option list cannot produce a statement Postgres refuses. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Initialize graphClampBatch maps on first clampToCoverage use A non-nil batch with a nil below map would panic on write. Seed covered and below on the batch when they are missing, and cover the reuse path in TestGraphClampToCoverage. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Tighten CodeRabbit test nits for omitted options and policy cleanup Pin options_omitted tests to the withheld-options branch, assert aria-disabled=true on graph values, derive the rank fixture from the hydration cap, and register policy cleanup as soon as the policy exists. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Assert omitted-option values on the disabled input, not label text Input values are not part of the container's textContent, so the new admin user detail test now checks the input value directly. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Close two gaps the master merge opened Neither was a textual conflict; both are this branch's new code landing next to master's new code. Bounce a graph field out of the attribute details editor. Master made the Edit row action live, pointing at an editor whose type list is ['text','select','multiselect','rank']. A graph field loaded there was coerced to 'text' -- and originalFieldTypeRef with it, so typeChanged stayed false and no guard fired -- while every save sends `type`. Renaming a standalone graph attribute's display name would have retyped the field, and TypeChangeValueCleanupHook would have deleted every value it held. Added the type to the guard that already redirects plugin-owned and classification fields to the list, which also removes the 'text' fallback that did the coercing. Authoring UI for graph fields is a separate work stream. Invalidate the user attribute caches on an option write. Master's rule is that anything changing what the AttributeView materializes calls invalidateAllUserAttributeCaches(). The matview resolves stored option IDs to names through PropertyOptions, so the option endpoints this branch adds change what every subject resolves to -- but propertyFieldOptionsChanged only dropped the policy caches. The view stayed stale until its next 30s refresh, and the per-user epoch never moved at all, since it is computed from PropertyValues, which an option write does not touch: clients went on serving renamed and deleted option names from cache indefinitely. Not covered by tests: no fixture pins the bounce-out, and TestPropertyFieldCacheInvalidation covers the field path rather than the options endpoints. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Invalidate the attribute caches unconditionally when dependents can't be read propertyFieldOptionsChanged's degraded path -- taken when the field serving the changed options and its dependents cannot be re-read -- tested the caller's copy of the field for object type "user" before invalidating the user attribute caches. That copy is a template whenever the write went to a template field, and a template's user-scoped dependents are exactly what could not be read, so the test skipped the invalidation in the case most likely to need it: the AttributeView would go on resolving option IDs to names that have been renamed or deleted until its next scheduled refresh. Invalidate unconditionally there instead. The success path below can still test, because it has the dependents in hand. The cost of invalidating when nothing was affected is one matview refresh. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Deduplicate the property field link-source lookup into one method Two packages each carried a byte-identical optionSourceID helper that dereferenced LinkedFieldID. Replaced both with model.PropertyField.LinkSourceID() so the two call sites can't drift. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Thread request context into createFieldWithOptionLinks Both call sites already hold rctx; passing it through keeps the private helper consistent with the rest of the package ahead of standardizing store calls on request-scoped context. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Merge the two options loops in UpdateFieldOptions Fold the rename-collection loop into the validation loop that already checks each option exists and isn't read-only, using the already-looked-up option instead of a second map lookup for the name comparison. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Take a group and field ID in FieldWithDependents and propertyFieldOptionsChanged Both re-read the authoritative field from the store before doing anything with it, so a caller that already holds a field only needed its two ID fields, not the field itself. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Take a group and field ID in the property option service methods The four option methods and writableField re-read the field from the store anyway to decide access and anchor the compare-and-swap, so the *model.PropertyField argument was only ever used for its two IDs. Take groupID, fieldID directly instead, and drop the nil-field guards that no longer apply with two required strings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Take groupID and fieldID in the App property-field-options methods The four plugin-API option methods pre-fetched the field only to hand its two ID fields to the App layer, which discarded the rest and read it again. Passing groupID/fieldID straight through drops the redundant fetch; the property service still enforces existence and group ownership as before, and the license and access-control gates on the option methods are unaffected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Remove duplicated caller-ID comments from property search handlers The explanation now lives once, on sessionCallerID's doc comment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Compress the option-limit constant's comment Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Refuse a null options-array element at decode time A null element in the JSON options array previously reached the property service before being rejected. Catching it in the HTTP decode step gives a clearer 400 naming the element's position. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Note the null check in decodePropertyFieldOptions's doc comment Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Add a method to purge every compiled access-control policy cache A caller that cannot enumerate which compiled policies a change made stale has no sound alternative to dropping all of them, both the resource-policy and permission-policy caches, locally and cluster-wide. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Purge every policy cache when a property field's dependents can't be read Which fields' policies went stale is unknown when that read fails, so per-field invalidation for the changed field alone can leave a stale compiled policy that still grants access through a since-removed parent edge. Drop every compiled policy instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Keep plugin-owned attributes on the details page instead of bouncing to the list View for a plugin-sourced field was redirecting because the load path treated plugin ownership like an uneditable type. Plugin-owned fields stay on this page read-only; only classification markings and types this editor cannot author (graph) still send the user back. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Give graph field API tests an Enterprise Advanced license Channel attributes in the access_control group now require Advanced. TestGraphPropertyFieldAuthoring creates a channel-linked field, so an Enterprise-only license made the fixture fail before any graph assertions ran. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Open plugin-owned attributes read-only instead of redirecting The attribute details editor redirected every plugin-owned field to the listing, so a plugin-sourced attribute could not be viewed. Only a type this editor can't render (graph) should redirect; plugin-owned text/select/etc. fields open read-only via effectiveDisabled. A graph field is caught by its type, not by plugin ownership. Add a test locking the graph-type redirect. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Keep the options blob after migration 000221's backfill A node still running pre-upgrade code reads select/multiselect/rank option lists straight out of PropertyFields.Attrs->'options'. Stripping that key once PropertyOptions rows exist made every such field look optionless to that node for the length of a rolling upgrade. Upgraded code never reads the blob back anyway, so leave it in place instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Clear the stale options blob in the 000221 down migration The rehydrate only rewrites fields with a live option row, so a field whose options were all soft-deleted after the upgrade kept its old blob untouched on rollback -- resurrecting an option that attribute views, and the access-control policies matching against them, had already stopped seeing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Document the last-writer-wins ceiling on field option changes The compare-and-swap on a field's UpdateAt closes the server's own validation window but not the gap between two callers who each read options and then edit -- state that ceiling once at the package level, why it's acceptable (a masked caller can never write the field), and point to it with one line on each option-change entry point. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Add a withheld-value marker for masked broadcast values Non-public field values must not ride in a websocket broadcast, but dropping or nulling the value is indistinguishable from a client clearing or deleting it. BroadcastValue substitutes a non-null JSON object marker instead, which every existing real value's JSON (a string or array of strings) cannot collide with. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Withhold non-public field values from the property values upsert broadcast A websocket broadcast for a source-only or shared-only field carried its value in the clear, letting any client on the channel read it regardless of access mode. Broadcast a withheld marker for those rows instead, while the HTTP response to the caller who wrote them still carries the real value. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Withhold non-public field values from the legacy CPA broadcast The custom profile attributes websocket event carried every patched value verbatim, so a non-public field's value leaked to every connected client even though the newer property_values_updated event already masks it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Add a client-side predicate for the withheld property value marker Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Refetch a target instead of storing its withheld property values A property_values_updated websocket event carries a withheld marker in place of any value the recipient isn't allowed to see. Storing that marker verbatim would show as the field's value; treating an all-withheld event as empty would wipe the target's other values. Split withheld rows out before routing the event, refetch each affected group's values through the masked read path, and only ever store the visible rows. * Refetch instead of storing a withheld custom profile attribute value A masked field value in the CPA websocket event would otherwise overwrite the store with the withheld marker itself. Drop withheld entries from the dispatched map and trigger a refetch of the user's values instead, so the field goes stale for a moment rather than reading as a real value. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Assert the refetch fires on a mixed withheld/real event, not just all-withheld Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Name the fixed bounds on hierarchy-walk work Add PropertyGraphMaxWalkRows, PropertyGraphMaxMaskedOptions, and PropertyFieldOptionCandidatesMaxPerRequest alongside the existing graph-size limits, each documented with what a hierarchy would have to look like to reach it. These bound the work one request may do over an already-legal hierarchy, distinct from the limits above them that bound what a hierarchy may hold. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Bound the recursive option hierarchy walk by row count An option hierarchy large enough, walked from enough seeds at once, produces more (seed, option) rows than a masking or validation caller can afford to process. Cap the total at PropertyGraphMaxWalkRows: each batch's query stops pulling early via LIMIT, and a running total across batches catches a walk that stays under any one batch's limit but adds up past the bound. Going over refuses the walk outright rather than returning a partial hierarchy, since every caller already treats an error as "hide" or "refuse". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Bound the downward walk that masks a graph property value A masked value naming an option with a very large subtree below it could send the coverage descent through the whole subtree on every read. Cap the options one descent may visit, held as a budget shared across every descent one masked value's clamp does, and error out (masking to nothing, as an unresolvable option already does) when it runs over. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix contradictory hierarchy-walk comments and drop a restating one coveredBelow's doc claimed a cache hit "reaches here at all" before spending nothing, when clampToCoverage actually skips the call entirely on a cache hit. walkOptionHierarchy's doc claimed each batch's LIMIT tracked what the row budget had left, when it is the same fixed maxRows+1 on every batch regardless of prior spend. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Add option page and page filter shapes to the property model Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Filter and bound a graph field's option page in the store query A shared_only graph field's option listing used to mask each page in Go after the database had already applied LIMIT, so a page could come back mostly empty while options the caller covers were still further down the table. The coverage filter now runs inside the page query itself, via a recursive CTE that walks down from the caller's own options and intersects it with a bounded window of candidate rows, so a page comes back full while options remain. The candidate window is capped independently of the page size (PropertyFieldOptionCandidatesMaxPerRequest, 2000): once it runs out without filling the page, a second cheap query reports the window's tail so the cursor can resume past the rows that were dropped instead of re-scanning them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Decide a graph field's option-page filter once per listing MayShowAnyPropertyFieldOptions answered a bare bool before the page scan, then PostGetPropertyFieldOptions re-read the caller's held options on every page to apply the same masking. Replace the bool hook with PreGetPropertyFieldOptions, which returns a filter carrying the caller's held option IDs (or ShowNothing), and hand that filter into the post-hook so it only falls back to reading the holdings itself when the filter carries nothing. Behaviour is unchanged: the listing still passes nil to the store and still loops page by page. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Read one bounded page of a graph field's options instead of rescanning GetFieldOptions used to loop over the store, re-reading pages until it filled one or ran out of rows, because the post-hook could drop most of what a page held. Now that the coverage filter narrows the store query itself, one call returns a page that is already full while options remain, so the service just runs the post-hook over it and hands back the store's own HasMore and cursor unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Report a graph field's option listing continuation on the wire The options endpoint returned a bare array, so a caller had no way to tell a short page apart from the end of the listing once the page query started filtering rows. Return an object carrying has_more and the resume cursor instead, and update Client4 and its tests to follow it. * Return the property field option page from the plugin API The plugin GetPropertyFieldOptions method returned a bare option slice, so a plugin could not tell a short page from the end of a listing once a filtered page could be short with options still to come. It now returns the same PropertyFieldOptionPage the HTTP endpoint does, carrying HasMore and the resume cursor, and the generated RPC and mock glue are regenerated to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Sharpen comments added for the bounded option page Drop references to "this PR" and "the old code", and fix a comment pointing at a test file it doesn't actually sit next to. None of these describe standing behaviour a future reader needs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Replace t.Fatal in graph shared-only helper with require.FailNow mattermost-govet's tFatal checker rejects t.Fatal in tests. Use require.FailNow so check-style matches the rest of the property tests. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Advance a property field's UpdateAt when it is deleted * Refuse option changes on a soft-deleted property field * Clarify comments on refusing option writes to a deleted field * Stamp a field delete's UpdateAt from the same clock reading as DeleteAt * Chunk PropertyOptions writes to stay under the driver parameter limit * Correct comments on chunked PropertyOptions writes * Refuse option endpoints on channel attributes without Enterprise Advanced * Refuse option requests on a template serving channel attributes without Enterprise Advanced * Refuse patching or deleting a template serving channel attributes without Enterprise Advanced Co-authored-by: Cursor <cursoragent@cursor.com> * Drop inert permission pins from the license-gate test fixture The access-control validation hook overwrites PermissionField and PermissionOptions and default-fills PermissionValues for fields in this group, so the fixture builds identically without them. Co-authored-by: Cursor <cursoragent@cursor.com> * Reject a repeated option id in a field's option list Two options carrying one id made the store's upsert conflict on (FieldID, ID) twice in one statement, surfacing a Postgres 21000 as a 500. PropertyOptions.IsValid now refuses the repeat before the store is reached, naming the id, so the caller gets a 400. Blank ids are skipped: option-create payloads require them empty. Co-authored-by: Cursor <cursoragent@cursor.com> * Mark a masked field broadcast's option list as withheld * Repair a withheld option list in the property field websocket handler A broadcast property field marked options_omitted would replace the cached field wholesale, emptying the option list every client held. Carry the cached options onto the dispatched field and refetch the field's scope so the authoritative per-caller list replaces it. * Clarify the options_omitted comment in the field websocket test Co-authored-by: Cursor <cursoragent@cursor.com> * Document why option name lookups use the unmasked set * Trim self-referential clause from the option name-lookup invariant comment * Collapse the property-options migrations into two 000222 adds the graph enum value and nothing else; 000223 does the rest in one transaction: PropertyOptions and its backfill, the postcondition checks, PropertyOptionEdges, the three indexes as plain CREATE INDEX, and both attribute views defined once with the graph branch included. The backfill and the down migration's blob clear now cover graph fields, so a down/up cycle carries graph options through the blob instead of losing them, and the rehydrate treats a JSON-null Attrs as empty rather than failing jsonb_set on a scalar. Co-authored-by: Cursor <cursoragent@cursor.com> * Point attribute-view comments at migration 000223 * Guard the property-options backfill with a Systems sentinel so both migration directions re-run Co-authored-by: Cursor <cursoragent@cursor.com> * Record the option-backfill check results in the sentinel row Co-authored-by: Cursor <cursoragent@cursor.com> * Prune restating comments in the property-options migration tests * Move property-option secondary indexes into CONCURRENTLY migrations check-style's concurrentIndex checker (minMigration 166) rejects CREATE INDEX inside the transactional 000223 backfill. CONCURRENTLY cannot run in that transaction, so the three keys move to 000224-000226, matching the post-166 pattern. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Use integer range in the graph option-list window test golangci-lint's intrange checker failed check-style on the classic for-i loop that fills filler options. range total-1 is the same iteration and matches the rest of the properties tests. Co-authored-by: mattermost-code <matty-code@mattermost.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: mattermost-code <matty-code@mattermost.com>
94 lines
6.2 KiB
Go
94 lines
6.2 KiB
Go
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
|
|
// See LICENSE.txt for license information.
|
|
|
|
package einterfaces
|
|
|
|
import (
|
|
"github.com/mattermost/mattermost/server/public/model"
|
|
"github.com/mattermost/mattermost/server/public/shared/request"
|
|
)
|
|
|
|
// PolicyAdministrationPointInterface is the service that manages access control policies.
|
|
// It is responsible for creating, updating, and deleting policies.
|
|
// Also, it provides methods to check the validity of expressions and to retrieve policies.
|
|
type PolicyAdministrationPointInterface interface {
|
|
// Init initializes the policy administration point and intiates the CEL engine.
|
|
// It is an idempotent operation, meaning that it can be called multiple times.
|
|
Init(rctx request.CTX) *model.AppError
|
|
// GetPolicyRuleAttributes retrieves the attributes of the given policy.
|
|
// It returns a map of attribute names to their values for given action.
|
|
GetPolicyRuleAttributes(rctx request.CTX, policyID string, action string) (map[string][]string, *model.AppError)
|
|
// CheckExpression checks the validity of the given expression using the CEL engine.
|
|
// It returns a list of CELExpressionError if the expression is invalid.
|
|
// If the expression is valid, it returns an empty list.
|
|
CheckExpression(rctx request.CTX, expression string) ([]model.CELExpressionError, *model.AppError)
|
|
// ExpressionToVisualAST converts the given expression to a visual AST.
|
|
ExpressionToVisualAST(rctx request.CTX, expression string) (*model.VisualExpression, *model.AppError)
|
|
// NormalizePolicy normalizes the given policy by restoring ids back to names.
|
|
NormalizePolicy(rctx request.CTX, policy *model.AccessControlPolicy) (*model.AccessControlPolicy, *model.AppError)
|
|
// QueryUsersForExpression evaluates the given expression using the CEL engine.
|
|
// It returns a list of users that match the expression.
|
|
QueryUsersForExpression(rctx request.CTX, expression string, opts model.SubjectSearchOptions) ([]*model.User, int64, *model.AppError)
|
|
// QueryUsersForResource evaluates finds the users match to the resource.
|
|
QueryUsersForResource(rctx request.CTX, resourceID, action string, opts model.SubjectSearchOptions) ([]*model.User, int64, *model.AppError)
|
|
// GetChannelMembersToRemove retrieves the channel members that need to be removed from the given channel.
|
|
GetChannelMembersToRemove(rctx request.CTX, channelID string) ([]*model.ChannelMember, *model.AppError)
|
|
// GetTeamMembersToRemove retrieves the team members that need to be removed from the given team.
|
|
GetTeamMembersToRemove(rctx request.CTX, teamID string) ([]*model.TeamMember, *model.AppError)
|
|
// SavePolicy saves the given access control policy.
|
|
SavePolicy(rctx request.CTX, policy *model.AccessControlPolicy) (*model.AccessControlPolicy, *model.AppError)
|
|
// GetPolicy retrieves the access control policy with the given ID.
|
|
GetPolicy(rctx request.CTX, id string) (*model.AccessControlPolicy, *model.AppError)
|
|
// DeletePolicy deletes the access control policy with the given ID.
|
|
DeletePolicy(rctx request.CTX, id string) *model.AppError
|
|
// GetPoliciesForFieldIDs returns the policies that reference any of the given
|
|
// property field IDs in their CEL rule expressions.
|
|
GetPoliciesForFieldIDs(rctx request.CTX, fieldIDs []string) ([]*model.AccessControlPolicy, *model.AppError)
|
|
// SimulatePolicyForUsers evaluates a DRAFT policy against an explicit
|
|
// user list (with optional per-user session attribute overrides) and
|
|
// returns per-user, per-action ALLOW/DENY decisions plus blame
|
|
// attribution. The draft is compiled in-memory only; nothing is
|
|
// persisted. Backs the picker-based "Simulate access" UX in the
|
|
// System Console and Channel Settings.
|
|
SimulatePolicyForUsers(rctx request.CTX, params model.PolicySimulationByUsersParams) (*model.PolicySimulationResponse, *model.AppError)
|
|
|
|
// OnPropertyFieldOptionsChanged signals the access control service that a
|
|
// property field's options changed — renamed or re-ranked, and for a field
|
|
// whose options form a hierarchy, added, deleted, or moved under a
|
|
// different parent. The service invalidates any cached per-field metadata
|
|
// and clears compiled-policy cache entries for policies that reference the
|
|
// field so subsequent evaluations re-read the authoritative values. Called
|
|
// once per field the change touched, which includes the fields serving
|
|
// another field's options as their own. Safe to call for any field type;
|
|
// the service no-ops for fields it does not track.
|
|
OnPropertyFieldOptionsChanged(rctx request.CTX, fieldID string)
|
|
|
|
// InvalidateAllPolicyCaches purges every compiled policy, local and
|
|
// cluster-wide, both the resource-policy cache and the permission-policy
|
|
// cache. For a caller that cannot enumerate which compiled policies a
|
|
// change made stale, dropping all of them is the only sound alternative.
|
|
InvalidateAllPolicyCaches(rctx request.CTX)
|
|
|
|
// HasMaskedValuesForCaller reports whether expression contains any literal
|
|
// value hidden from the caller according to resolver. Returns an error when
|
|
// field resolution or CEL parsing fails — callers must treat any error as
|
|
// fail-closed (assume values are hidden).
|
|
HasMaskedValuesForCaller(rctx request.CTX, expression string, resolver model.MaskingFieldResolver) (bool, *model.AppError)
|
|
|
|
// MaskExpressionForCaller rewrites hidden literal values in expression to
|
|
// the masked token and returns the modified CEL string. The logical tree
|
|
// shape is preserved (|| / grouping / function calls are not flattened).
|
|
MaskExpressionForCaller(rctx request.CTX, expression string, resolver model.MaskingFieldResolver) (string, bool, *model.AppError)
|
|
|
|
// ValidateExpressionValuesForCaller walks expression and returns an error
|
|
// if any non-token literal is forbidden for the caller under resolver's
|
|
// visibility rules.
|
|
ValidateExpressionValuesForCaller(rctx request.CTX, expression string, resolver model.MaskingFieldResolver) *model.AppError
|
|
|
|
// MergeExpressionWithMaskedValuesCanonical re-injects hidden stored
|
|
// literals from storedExpr into submittedExpr using a canonical CEL AST
|
|
// walk. Returns the merged CEL string. Fails closed when the submitted
|
|
// tree's shape diverges from stored while hidden values are present.
|
|
MergeExpressionWithMaskedValuesCanonical(rctx request.CTX, submittedExpr, storedExpr string, resolver model.MaskingFieldResolver) (string, *model.AppError)
|
|
}
|