Validate access_policies statement structure; document grammar and slugs

The build script now parses access_policies and warns on each statement that fails the grammar — bad effect, unknown resource or identifier type, an operator the identifier type does not support (a GUID takes only equals | exists), an empty identifiers array, a blank action slug. These are the shapes the add-in drops silently, so the build is the only place they get caught; an unparseable value is fatal. Docs: add a statement-grammar reference and the full feature-slug list to access-policies.md, sync the disabled_features table in manifest.md with all four slugs, add the missing access_policies field to bootstrap.md, and fix the parent-label prefix example to include the " - " separator.
2026-08-03 19:14:19 -04:00
parent 20601b0b41
commit 53e6aa67f8
4 changed files with 136 additions and 8 deletions
@@ -116,14 +116,52 @@ longer pass either. A matching `deny` still wins over a matching `allow`.
 }
 ```

-Combine as needed — the value is one JSON array of statements:
+Combine as needed — the value is one JSON array of statements. `action` also
+accepts an array to apply one statement to several actions:

 ```json
 [
  { "effect": "deny", "action": "addin.access", "resource": { ... } },
-  { "effect": "deny", "action": "file.upload", "resource": { ... } }
+  { "effect": "deny", "action": ["file.upload", "skills.authoring"] }
+]
 ```

+### Statement grammar (reference)
+
+```
+statement   := { effect, action, resource? }
+effect      := "allow" | "deny"
+action      := <slug> | [ <slug>, ... ]          # feature slugs; unknown -> skipped + reported
+resource    := { type, identifiers: [identifier, ...], description? }
+type        := "open_file" | "uploaded_file"    # the extension point
+identifier  := { type: "mip_label_guid" | "mip_label_name", <one operator> }
+```
+
+| `action` slug | Gates | Takes a `resource`? |
+|---|---|---|
+| `addin.access` | Whether the add-in runs at all on the open document — the kill switch | `open_file` |
+| `file.upload` | Whether a file may be attached to the conversation | `uploaded_file` |
+| `skills.authoring` | Creating, editing, and uploading skills; running admin-provisioned skills is unaffected | — |
+| `thumbs` | Response feedback (thumbs up / down and the follow-up prompt) | — |
+
+Unknown slugs are skipped and reported — forward-compatible. A resource-less
+statement works with any slug; today only `addin.access` and `file.upload`
+have a resource type to scope against.
+
+| Operator | Value | Semantics |
+|---|---|---|
+| `equals` | string | GUID: case-insensitive. Name: exact, case-sensitive |
+| `startsWith` | string | prefix match — `mip_label_name` only (see the parent-label rule) |
+| `endsWith` | string | suffix match — `mip_label_name` only |
+| `exists` | boolean | presence check — `false` = unlabeled, `true` = any label |
+
+Exactly one operator per identifier; `mip_label_guid` supports only `equals` and
+`exists`. A resource needs at least one identifier — an empty `identifiers`
+array never matches. Statements OR together; identifiers within a resource OR
+together; a matching `deny` beats a matching `allow`; the first
+`allow` for an `(action, resource type)` flips that scope to default-deny.
+`description` is inert prose (surfaced in UI copy / telemetry, never matched).
+
 ## 3. Rules to explain before they ship it

 Tell the admin these five things before they ship; each is a common surprise.
@@ -139,10 +177,11 @@ together.
 carries its sublabel's GUID (the row with a `ParentId`). To block a whole
 parent group ("everything under *Confidential*"), either list each sublabel's
 GUID, or match on the composed display name, which Office writes as
-`"Parent - Sublabel"`:
+`"Parent - Sublabel"` (include the ` - ` separator in the prefix so a sibling
+label like *Confidentiality Waiver* doesn't also match):

 ```json
-{ "type": "mip_label_name", "startsWith": "Confidential" }
+{ "type": "mip_label_name", "startsWith": "Confidential - " }
 ```

 **Name matching is exact and rename-hazardous.** `mip_label_name` compares the
@@ -172,10 +211,13 @@ is refused.

 Read the finished array back to the admin as a table (effect / action /
 resource / identifiers) before generating anything. Then pass it to
-[manifest](manifest.md#access_policies) as the `access_policies` key — the build
-script checks it's a JSON array. Malformed statements aren't fatal on the
-add-in side (they're skipped and reported), but validate now so the admin isn't
-surprised by a rule that silently didn't apply.
+[manifest](manifest.md#access_policies) as the `access_policies` key. The build
+script rejects a value that isn't valid JSON and warns on each statement that
+doesn't fit the grammar above (bad `effect`, unknown resource / identifier type,
+missing or duplicate operator). Fix every warning now: the add-in reports
+unknown action slugs and an unparseable value, but a statement that fails the
+grammar is dropped silently — this build-time check is the only catch, and the
+admin would otherwise ship a rule that quietly never applies.

 `access_policies` is manifest / bootstrap only — it does **not** fit in Entra
 extension attributes (256-char cap). If they use per-user
@@ -260,6 +260,20 @@ layer.
 "disabled_features": ["skills.authoring"]
 ```

+### `access_policies`
+
+Native JSON array of allow/deny statements — the per-user layer of the
+[manifest key](manifest.md#access_policies); build the array with
+[access-policies](access-policies.md). Pass it as a real array, not a string:
+
+```json
+"access_policies": [
+  { "effect": "deny", "action": "addin.access",
+    "resource": { "type": "open_file",
+      "identifiers": [{ "type": "mip_label_guid", "equals": "<guid>" }] } }
+]
+```
+
 ### `bootstrap_expires_at`

 Epoch timestamp (seconds or milliseconds — auto-detected) for when this
@@ -263,6 +263,9 @@ locked for users. Slugs use `<domain>.<action>` form. Currently enforced:
 | Slug | Effect |
 |---|---|
 | `skills.authoring` | Blocks creating, editing, and uploading skills (create/update tools, `/skillify`, `.skill` upload + drag-drop, skill editing UI). Running admin-provisioned skills is unaffected. |
+| `thumbs` | Blocks response feedback (thumbs up / down and the follow-up prompt). |
+| `addin.access` | Kill switch — the add-in refuses to run. Almost always wants a document `resource`; use [access-policies](access-policies.md). |
+| `file.upload` | Blocks attaching files to the conversation. Same — usually scoped by `resource`. |

 ```bash
 disabled_features='skills.authoring'
@@ -88,9 +88,77 @@ const KEYS = {
  access_policies: {
    pattern: /^\[.*\]$/s,
    hint: "JSON array of policy statements — see commands/access-policies.md; e.g. [{\"effect\":\"deny\",\"action\":\"addin.access\",\"resource\":{...}}]",
+    validate: (v) => {
+      let arr;
+      try {
+        arr = JSON.parse(v);
+      } catch (e) {
+        throw new Error(`access_policies is not valid JSON: ${e.message}`);
+      }
+      if (!Array.isArray(arr)) return ["expected a JSON array of statements"];
+      return arr.flatMap((st, i) => validateStatement(st, `statement[${i}]`));
+    },
  },
 };

+// Structural check for one access_policies statement. Warn-only: the add-in itself
+// skips and reports malformed statements rather than failing, so this catches typos
+// before deploy without being stricter than the runtime.
+const EFFECTS = ["allow", "deny"];
+const RESOURCE_TYPES = ["open_file", "uploaded_file"];
+const STRING_OPS = ["equals", "startsWith", "endsWith"];
+// Mirrors the add-in: a GUID supports only equals | exists (a prefix of a GUID is
+// meaningless); a name supports the string operators too. Other pairings are dropped
+// at runtime, so warn here.
+const OPERATORS_BY_TYPE = {
+  mip_label_guid: ["equals", "exists"],
+  mip_label_name: ["equals", "startsWith", "endsWith", "exists"],
+};
+
+function validateStatement(st, at) {
+  if (typeof st !== "object" || st === null || Array.isArray(st)) return [`${at}: expected an object`];
+  const problems = [];
+  if (!EFFECTS.includes(st.effect)) problems.push(`${at}.effect: expected "allow" or "deny"`);
+  const slugs = Array.isArray(st.action) ? st.action : [st.action];
+  const actionOk = slugs.length > 0 && slugs.every((a) => typeof a === "string" && a.trim());
+  if (!actionOk) problems.push(`${at}.action: expected a non-empty slug string or array of them`);
+  if (st.resource !== undefined) {
+    const r = st.resource;
+    if (typeof r !== "object" || r === null) return [...problems, `${at}.resource: expected an object`];
+    if (!RESOURCE_TYPES.includes(r.type)) problems.push(`${at}.resource.type: expected ${RESOURCE_TYPES.join(" | ")}`);
+    if (r.description !== undefined && typeof r.description !== "string") {
+      problems.push(`${at}.resource.description: expected a string`);
+    }
+    if (!Array.isArray(r.identifiers)) {
+      problems.push(`${at}.resource.identifiers: expected an array`);
+    } else if (r.identifiers.length === 0) {
+      problems.push(`${at}.resource.identifiers: empty — the statement will never match; drop \`resource\` to apply everywhere`);
+    } else {
+      r.identifiers.forEach((id, j) => problems.push(...validateIdentifier(id, `${at}.resource.identifiers[${j}]`)));
+    }
+  }
+  return problems;
+}
+
+function validateIdentifier(id, at) {
+  if (typeof id !== "object" || id === null) return [`${at}: expected an object`];
+  const problems = [];
+  const allowed = OPERATORS_BY_TYPE[id.type];
+  if (!allowed) problems.push(`${at}.type: expected ${Object.keys(OPERATORS_BY_TYPE).join(" | ")}`);
+  const ops = [...STRING_OPS.filter((op) => op in id), ..."exists" in id ? ["exists"] : []];
+  if (ops.length !== 1) {
+    problems.push(`${at}: expected exactly one operator (equals | startsWith | endsWith | exists), got ${ops.length}`);
+    return problems;
+  }
+  const [op] = ops;
+  if (allowed && !allowed.includes(op)) {
+    problems.push(`${at}: ${id.type} does not support ${op} (only ${allowed.join(" | ")})`);
+  } else if (op === "exists" ? typeof id.exists !== "boolean" : typeof id[op] !== "string" || !id[op].trim()) {
+    problems.push(`${at}.${op}: expected a ${op === "exists" ? "boolean" : "non-empty string"}`);
+  }
+  return problems;
+}
+
 const NEEDS_ENTRA = ["aws_role_arn", "graph_client_id", "entra_scope", "gateway_auth_source"];

 async function main() {
@@ -118,6 +186,7 @@ async function main() {
    if (!spec) throw new Error(`unknown key: ${k}\n  valid: ${Object.keys(KEYS).join(", ")}`);
    if (!v) throw new Error(`empty value for ${k}`);
    if (!spec.pattern.test(v)) console.warn(`warn: ${k}=${v} — expected ${spec.hint}`);
+    for (const msg of spec.validate?.(v) ?? []) console.warn(`warn: ${k} ${msg}`);
    if (spec.secret) {
      console.warn(
        `note: ${k} in the manifest applies to every user. If it varies per user, set it via update-user-attrs instead.`,