[jonesphillip]

Adds a Microsoft 365 gatekeeper with three resources over Microsoft Graph.

SharePoint and OneDrive: Connect everything the user can open, or narrow it to a site, a library, a folder, a file, a list or a page. Agents read Word, PowerPoint and PDF files as Markdown or text and workbooks as cells, query lists, read pages, and propose uploads, edits, moves and folder changes. Each write is checked against what was connected again when it's applied.

Outlook: Connect one mailbox, yours or a shared one, optionally narrowed to a folder, a conversation or certain senders, at read, draft or send. Sending always waits for approval and happens at most once. The agent can only edit or delete drafts it wrote.

Calendar: Connect every calendar, one calendar at free/busy, titles or full detail, or availability only. Invitations and responses wait for approval.

In addition to standard tests, I ran live suite against a test tenant as three users (owner, member, outsider) and tested manually.

[github-actions]

Preview: pr902-pjones-gateke-37b725bc

https://pr902-pjones-gateke-37b725bc-router.cloudflare-os-previews.workers.dev

Dashboard · deleted when this PR closes

[ask-bonk]

Review: 6 findings.

Posted 6 actionable inline comments: 4 high-priority and 2 medium-priority. Key cases were verified with in-memory reproductions.

github run

[ask-bonk]

Review: 4 findings.

Posted 4 additional inline comments: 2 high-priority and 2 medium-priority. All were verified with in-memory reproductions.

github run

[ask-bonk]

Review: 4 findings.

Posted 4 additional inline comments: 3 high-priority and 1 medium-priority. All four were verified with in-memory reproductions.

github run

[ask-bonk]

Review: 4 findings.

Posted 4 new inline comments: 3 high-priority and 1 medium-priority. All four were verified with in-memory reproductions.

github run

[ask-bonk]

Review: 4 findings.

Posted 4 new inline comments: 1 high-priority and 3 medium-priority. All four were verified with in-memory reproductions.

github run

[ask-bonk]

Review: 3 findings.

Posted 3 new inline comments: 1 high-priority and 2 medium-priority. All three were verified with in-memory reproductions.

github run

[ask-bonk]

Review: 4 findings.

Posted 4 new actionable inline comments: 2 high-priority and 2 medium-priority. All four were verified with in-memory reproductions.

github run

[ask-bonk]

Review: 4 findings.

Posted 4 new actionable inline comments: 1 high-priority and 3 medium-priority. All four were verified with in-memory reproductions.

github run

[ask-bonk]

Review: 2 findings.

Posted 2 new medium-priority inline comments, both verified with in-memory document-generation reproductions.

Reviewed 25f8af4a, including the scope rule in packages/gatekeeper-microsoft/AGENTS.md:5: “Every ID is checked before Graph is called”.

github run

[ask-bonk]

Review: 3 findings.

Posted 3 new medium-priority inline comments. All three were verified with in-memory reproductions, including an independent XML-parser check.

Reviewed 0104c263 under the package guidance, including packages/gatekeeper-microsoft/AGENTS.md:5: “Every ID is checked before Graph is called”.

github run

[ndisidore]

Latest review of robo findings

---

Verdict: request changes. The security core holds. Bindings stay inside their scope, every side effect goes through submitAction, reads are authorized with access units, tokens stay inside UserAccount and on Graph or SharePoint hosts, and OAuth pins the tenant and user. The blockers are
elsewhere: two ways to slip a refused formula past the check on auto-approvable workbook writes, an observer hole for files awaiting approval, CPU denial of service that anyone who can email the user or share a file can trigger, and a few broken user flows.

P1 — fix before merge

┌───┬────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┬──────────────────────────────────────────────────────────┬─────────────────────────────────────────────────────────┐
│ # │ Issue                                                                                                                                                                      │ Location                                                 │ How it was checked                                      │
├───┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────┤
│ 1 │ xlsx formula check bypassed by a control character. =WEB\u0001SERVICE(...) is checked as it was written, and the \u0001 is only removed when the file is written, so the   │ content/xlsx-write.ts:300-330, :555-566                  │ Reproduced                                              │
│   │ output contains WEBSERVICE(...). Creating a file can be auto-approved.                                                                                                     │                                                          │                                                         │
├───┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────┤
│ 2 │ Functions passed by name (_xleta.) are never checked. =MAP(..., _xleta.WEBSERVICE) and _xleta.IMAGE are accepted. The same gap applies to uploaded workbooks through       │ xlsx-write.ts:260-262, :320                              │ checkFormula accepting it was reproduced; whether Excel │
│   │ package-check.ts:138.                                                                                                                                                      │                                                          │ then evaluates it was not checked                       │
├───┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────┤
│ 3 │ Observer hole for files awaiting content approval. fileRead pins a version only when it isn't a whole number, so a pending 2.0 is not pinned. A collaborator who can only  │ sharepoint-observers.ts:124-129                          │ Traced through the code                                 │
│   │ see the approved 1.0 passes the probe. The fix is to select _ModerationStatus and pin the way listItemRead does.                                                           │                                                          │                                                         │
├───┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────┤
│ 4 │ Quadratic regexes in the docx and pptx readers. /\s*\n\s*/g and /\s+$/ are slow on long runs of spaces. A 729-byte docx took 7.1 s, and about 3 KB compressed works out to │ content/markdown.ts:181, docx.ts:121, :135, pptx.ts:124, │ Timed                                                   │
│   │ roughly 700 s.                                                                                                                                                             │ pptx-describe.ts:113                                     │                                                         │
├───┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────┤
│ 5 │ Quadratic stylesheet regexes on every HTML mail read. A <style> block with no { costs 12.5 s at 200 KB. Any outside sender can make a message, its whole thread, forwards  │ outlook-content.ts:152-154                               │ Timed                                                   │
│   │ of it and calendar bodies unreadable.                                                                                                                                      │                                                          │                                                         │
├───┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────┤
│ 6 │ Quadratic trailing-whitespace trim in htmlToMarkdown. 45 KB of nested <ul> took 103 s. It is reachable through HTML mail attachments and SharePoint HTML files and pages.  │ html-markdown.ts:140                                     │ Timed                                                   │
├───┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────┤
│ 7 │ The Outlook hook cursor can stop for good. If 250 or more messages arrive within the 2-minute overlap window, every sync re-reads the same 5 pages. The alarm then loops   │ outlook-hooks.ts:247-274                                 │ Traced through the code                                 │
│   │ non-stop and nothing after the burst is ever delivered.                                                                                                                    │                                                          │                                                         │
├───┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────┤
│ 8 │ A second pending change to the same Calendar event always fails when approved. Real event IDs are fenced on the change key from proposal time, and only ~N IDs follow the  │ calendar-session.ts:934, :991, :1045;                    │ Traced through the code                                 │
│   │ chain of versions.                                                                                                                                                         │ calendar-actions.ts:275-279                              │                                                         │
├───┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────┤
│ 9 │ Choosing a page in the SharePoint configurator gives the wrong URL. new URL("SitePages/x.aspx", "https://t/sites/eng") drops /eng, so binding any page that isn't on the   │ sharepoint-configurator.ts:402                           │ Checked in node                                         │
│   │ root site fails or names another site.                                                                                                                                     │                                                          │                                                         │
└───┴────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┴──────────────────────────────────────────────────────────┴─────────────────────────────────────────────────────────┘

P2 — security and scope

• Narrowed Outlook bindings can reach subfolders of Deleted Items, Junk and Drafts (outlook-scope.ts:77-88). admits and admitsFolder check only the direct parent against privateFolders(). Two reviewers confirmed this independently against the real module. This contradicts the module's own
  comment and the test name at outlook.test.ts:63. I'd treat it as P1-adjacent.
• Files written from a template skip checkOfficePackage (office-write.ts:113-118, docx-write.ts:136-189). Reproduced: a header with INCLUDEPICTURE pointing at a remote image survives into the new file. The pptx template path has the same shape but wasn't traced.
• aFChunk (altChunk) relationships are not refused (package-check.ts:31-34). Imported RTF or HTML parts are never inspected.
• A failed prefill in the SharePoint configurator selects "All of SharePoint & OneDrive" with isReady true (sharepoint-configurator-ui.tsx:1211-1219). One click then binds everything.
• The Outlook configurator's conversation prefill silently drops senders and the folder (outlook-configurator-ui.tsx:198-201), which widens the binding.
• findPeople reads /me/people from any binding, including a read-only free/busy one (calendar-session.ts:787-810).
• listFolders gives conversation- and sender-narrowed bindings every folder name with mailbox-wide counts (outlook-session.ts:602). This contradicts Conversation.partial's own rationale.
• Any from without an @ is rewritten to the mailbox owner in every folder, not only Sent Items (outlook-session.ts:170, :356-359; confidence 0.5).
• Footnote and endnote w:id values reach the Markdown unescaped, producing javascript: links and raw HTML (content/docx.ts:620-626). Reproduced.
• mailto:%@x throws URIError in linkMismatch, which makes the message and its whole thread unreadable (outlook-content.ts:736). Reproduced.
• Listing version history doesn't pin a draft (sharepoint-observers.ts:102; confidence 0.5).
• The deploy wizard's text offers read-only, Sites.Selected and below-send permission sets that a wizard install can't configure, because the scopes come only from env vars (deploy-inputs.json:11 and the golden manifest).

P2 — resource exhaustion

• Inline Markdown parser is quadratic: 90 KB of *a took 6.6 s (content/markdown-parse.ts:371-415). It feeds every docx and pptx write.
• One pptx can make the converter parse the same slide thousands of times: 400 sldId entries pointing at one slide took 23 s (pptx.ts:27-33, :58).
• Attributes don't count toward the XML element cap: a 134 KB docx reached about 263 MB of heap in Node (content/xml.ts:48-51, :105, :227).
• htmlToMarkdown has no cap on list or quote depth: 80 KB of input used 214 MB of heap (html-markdown.ts:135, :198). Alt text going through a multiline regex is also quadratic: 13 s (:190).
• Using a large deck as a template takes roughly cubic time: 800 slides took 35 s (pptx-deck.ts:411, :507).

P2 — correctness

• SharePoint:
  • updateListItem is retried after it has already applied and then fails as "changed since proposed" (sharepoint-actions.ts:804-845). It needs claimBeforeApply.
  • Pending updates and deletes of a list item that is itself pending creation don't show in reads, and undoing one restores the wrong values (sharepoint-overlay.ts:331-345).
  • resolveUrl on a library link returns a bare drive ID that no method accepts (sharepoint-session.ts:533).
  • The 2,000-unit observer cap is reachable in normal use and then permanently blocks new reads, even on list, file and folder bindings that have nothing narrower to switch to (sharepoint-observers.ts:204).
  • "Show more" on a OneDrive search keeps returning the first page (sharepoint-configurator.ts:370, :449).
• Outlook:
  • Moving a message back to its current folder is dropped while an earlier move is pending (outlook-session.ts:1199-1201). Compare against the folder after pending moves instead.
  • Messages outside the binding still start a hook firing in the workspace (outlook-hooks.ts:256-261, :345). The Gmail hooks already filter first.
  • updateDraft reports "wasn't changed" after a retry of a change that did land, leaving the draft partly changed (outlook-actions.ts:682-710).
• Calendar:
  • updateEvent builds on Exchange's copy rather than the pending one, so an end-only change throws away a pending start change (calendar-session.ts:919-929).
  • Undoing an update to a ~N event sends {} and reports success while restoring nothing (calendar-actions.ts:600-614).
• pptx:
  • startAt can be written outside 1–32767, for example a list starting at 0. (pptx-render.ts:130).
  • duplicateSlide leaves a dangling modern-comment r:id, which the PR's own validatePptx flags (pptx-deck.ts:387).

P3

• Content readers:
  • Image targets can become absolute URLs (content/markdown.ts:156, ooxml.ts:148).
  • htmlToText takes positions from the lower-cased string and applies them to the original (text.ts:72-87).
  • Math.max(...) overflows the stack on large inputs (docx.ts:126, html-markdown.ts:253).
• Writers:
  • A template's old media and thumbnail stay in the new file (docx-write.ts:152-164).
  • Conditional-formatting and data-validation formulas aren't checked (package-check.ts:138).
  • Deleting a slide leaves dangling hlinkMouseOver references (pptx-deck.ts:432).
  • MediaStore reuses a media part that was garbage-collected (pptx-images.ts:157).
• SharePoint:
  • A replayed rename to another extension fails as "renamed into another format" (sharepoint-actions.ts:586-602).
  • Undoing a move from a ~N folder says the folder no longer exists (:963-972).
  • A twice-moved item shows its first destination (sharepoint-overlay.ts:230).
  • Undoing createListItem deletes the item even if someone edited it since (sharepoint-actions.ts:1007).
  • A list item's ~N ID resolves on any list once approved (sharepoint-actions.ts:799).
  • List-item writes skip prepareExclusively (sharepoint-session.ts:1149-1186).
  • can.write is true on page bindings (:416).
• Outlook:
  • createDraft can make a duplicate draft on retry (outlook-actions.ts:653).
  • Undoing deleteDraft can move a sent message into Drafts (:1005-1015).
  • A lapsed subscription syncs hourly instead of polling (outlook-hooks.ts:354).
  • Pending moves in listMessages ignore the focused filter (outlook-query.ts:59-68).
  • Nested pending folders never appear in listFolders (outlook-session.ts:605).
  • A pending draft's attachments read as nothing (:825, :1095).
  • mailboxBatch retries 429s on top of GraphApi.batch's own retries (outlook-configurator.ts:95-113).
• Calendar:
  • respond's notify default doesn't match its docs (calendar-session.ts:1029 vs calendar-types.d.ts:257).
  • findFreeTimes returns Graph data without an observation (:769).
  • Undo writes a plain-text body back as HTML (calendar-actions.ts:245).
  • The ACL alias match can skip an entry that should lower the role (calendar-acl.ts:44-52).
• Observations: delete and deleteDraft log reads that only prepare an action, against the write-gatekeeper skill's "What counts as an observation" (sharepoint-session.ts:1258, outlook-session.ts:1205).
• Agent API docs (outlook-types.d.ts, calendar-types.d.ts):
  • sendMail, reply and forward return id: "".
  • NewMessage.mail's type leaves out OutlookDrafter.
  • ListMessagesOptions.folderId's default is documented wrong for conversation and sender bindings.
  • EventChanges.onlineMeeting is always refused.
  • EventInfo.pending lists "delete", which can't occur.
• Tests and constants:
  • Frame tests compare against literal URL patterns instead of the server's *_RESOURCE.urlPattern, against the write-gatekeeper skill.
  • The first assertion of the new sizing test is vacuous (SandboxedResourceConfigurator.test.tsx:140).
• Docs:
  • Four mismatches in storage-schema.md: the cache TTL, the missing eventVersion: key, how scopes is stored, and stagedCredentials.
  • Small inaccuracies in the README and plans.
  • The admin-consent link uses an unregistered redirect_uri on previews (microsoft.ts:332).
  • idClaims parses claims nothing reads.

[ask-bonk]

Review: 2 findings.

Posted 2 new actionable inline comments: 1 high-priority and 1 medium-priority. Both were verified with in-memory reproductions using the real action journal and handlers.

Reviewed ba9a34c9 under the package guidance, including packages/gatekeeper-microsoft/AGENTS.md:5: “Every ID is checked before Graph is called”.

github run

[ask-bonk]

Review: 2 findings.

Posted 2 new inline comments: 1 high-priority and 1 medium-priority. Both were verified with in-memory reproductions using the validated sessions and real action journal.

Reviewed b2acee2c, following the invitation-approval boundary in packages/gatekeeper-microsoft/AGENTS.md:11.

github run

[ndisidore]

@clanky-the-clanker review this please.

[ask-bonk]

Review: 2 findings.

Posted 2 new medium-priority inline comments. Both were verified with in-memory reproductions using the current handlers and real action journal; the upload reproduction also used ActionFileStore.

Reviewed 367e34ae, following the temporary-ID resolution rule in packages/gatekeeper-microsoft/AGENTS.md:5.

github run

[ask-bonk]

Review: 2 findings.

Posted 2 new medium-priority inline comments. Both were verified with in-memory reproductions, including the real action journal and an independent XML-parser check.

Reviewed 896b64b4 under the package guides, including the temporary-ID requirements in packages/gatekeeper-microsoft/AGENTS.md:5.

github run

[ask-bonk]

Review: 2 findings.

Posted 2 new medium-priority inline comments. Both were verified with in-memory reproductions using the current code.

Reviewed ce3ef128, following packages/gatekeeper-microsoft/AGENTS.md:5: “a temporary ~N ID is resolved at every entry point.”

github run

[ndisidore]

Hey we only found 10 this time!

---

Bugs, most severe first

1. outlook-session.ts:1207: When a message already has a pending move, a new move records that move's destination as its starting folder. That destination can be a temporary ~N folder ID, and undoing the move never translates it to a real ID. So if the agent moves a message into a pending new folder ~3 and then moves it back, the undo is refused for good. If the first move is rejected, the undo sends the message to a folder it was never in.
2. outlook-hooks.ts:251: A scan that was cut short resumes from a saved @odata.nextLink. Outlook pages these links by $skip offset, so if mail leaves the folder between syncs, the offset shifts and unread messages are skipped permanently.
3. content/ooxml.ts:196: withRelationshipsNs skips adding the declaration if xmlns:r=" appears anywhere in the part, not just on the root element. It also only recognises double quotes. A slide whose only declaration is on a nested chart element gets a hyperlink outside that scope, and the file is malformed. A root that declares it with single quotes gets a duplicate attribute, which is also malformed.
4. sharepoint-actions.ts:1027: Undoing createListItem deletes the item without checking its version. A colleague's later edits are deleted along with it, unlike the file-creation and updateListItem undos, which both check.
5. sharepoint-actions.ts:281: awaitCopy only stops on completed with a resourceId. If the redirect URL has an unexpected shape, it keeps polling a finished copy until the 45 s deadline and then reports it as "still copying". The copy then can't be undone.
6. sharepoint-actions.ts:984: The updateCells undo compares Excel's stored value with the written value as strings. Excel normalises what it stores (=sum(a1:a2) becomes =SUM(A1:A2), "007" becomes 7, 1.50 becomes 1.5), so these undos are refused even though nobody touched the range.
7. calendar-session.ts:330: requireInvitesSettled only looks at pending attendee changes on the same event ID. Pending invitee changes on a recurring series don't block a personal delete of one of its occurrences, and the reverse is true too. New invitees can then get cancellations the approver never saw.

Cleanup

8. sharepoint-actions.ts:718: updateCells still fetches /versions?$top=1 and stores previousVersionId, but nothing reads it any more. That's a wasted Graph call on every approved cell edit.
9. sharepoint-actions.ts:958: The replaceFile and rename undos copy the cTag lookup inline instead of calling contentTag(). The copies throw on a 404 where the helper returns cleanly, so undoing an edit to a deleted file behaves inconsistently.
10. calendar-actions.ts:297: noteVersionLeft swallows its error with an empty catch {}. The Outlook version added in the same commit logs it with an event name, which is what packages/observability/AGENTS.md asks for.

[ask-bonk]

Review: 2 findings.

Posted 2 new medium-priority inline comments. Both were verified with in-memory reproductions, including the validated session and real action journal.

Reviewed 2189569b under the package guidance, including packages/gatekeeper-microsoft/AGENTS.md:5: “Every ID is checked before Graph is called”.

github run