fix: expose M365 read-only pilot CLI#42
Conversation
|
Warning Review limit reached
More reviews will be available in 42 minutes and 42 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR adds a user-facing Microsoft 365 read-only pilot command surface and integrates M365 authentication scopes into the CLI's auth metadata display. It registers M365 as a top-level command with Outlook (search, message get) and Calendar (events, free-busy) subcommands, enforces explicit ChangesMicrosoft 365 Read-Only Pilot Surface
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces read-only pilot commands for Microsoft 365 (M365) integration, covering Outlook message search/retrieval and Calendar events/free-busy checks. It also integrates M365 service details into the auth services command. Feedback suggests refactoring the tests to use the standard library's slices.Contains instead of a custom helper function, and ensuring that an empty users list in the calendar free-busy command marshals to an empty JSON array rather than null.
| import ( | ||
| "encoding/json" | ||
| "strings" | ||
| "testing" | ||
| ) |
There was a problem hiding this comment.
To use the standard library's slices.Contains function instead of a custom helper, we should add "slices" to the imports.
| import ( | |
| "encoding/json" | |
| "strings" | |
| "testing" | |
| ) | |
| import (\n\t\"encoding/json\"\n\t\"slices\"\n\t\"strings\"\n\t\"testing\"\n) |
| for _, scope := range []string{"User.Read", "Mail.Read", "Calendars.Read"} { | ||
| if !stringSliceContains(scopes, scope) { | ||
| t.Fatalf("m365 auth services missing %s: %#v", scope, scopes) | ||
| } | ||
| } | ||
| for _, forbidden := range []string{"Mail.Send", "Calendars.ReadWrite"} { | ||
| if stringSliceContains(scopes, forbidden) { | ||
| t.Fatalf("m365 auth services exposed write scope %s: %#v", forbidden, scopes) | ||
| } | ||
| } |
There was a problem hiding this comment.
We can use the standard library's slices.Contains function here for better readability and to align with modern Go practices.
\tfor _, scope := range []string{\"User.Read\", \"Mail.Read\", \"Calendars.Read\"} {\n\t\tif !slices.Contains(scopes, scope) {\n\t\t\tt.Fatalf(\"m365 auth services missing %s: %#v\", scope, scopes)\n\t\t}\n\t}\n\tfor _, forbidden := range []string{\"Mail.Send\", \"Calendars.ReadWrite\"} {\n\t\tif slices.Contains(scopes, forbidden) {\n\t\t\tt.Fatalf(\"m365 auth services exposed write scope %s: %#v\", forbidden, scopes)\n\t\t}\n\t}| func stringSliceContains(values []string, want string) bool { | ||
| for _, value := range values { | ||
| if value == want { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } |
There was a problem hiding this comment.
Since we are now using slices.Contains from the standard library, this custom helper function is no longer needed and can be safely removed.
| func (c *M365CalendarFreeBusyCmd) Run(ctx context.Context, flags *RootFlags) error { | ||
| return writeM365PilotResult(ctx, flags, "m365.calendar.freebusy", map[string]any{ | ||
| "users": splitCommaList(c.Users), | ||
| "from": strings.TrimSpace(c.From), | ||
| "to": strings.TrimSpace(c.To), | ||
| }) | ||
| } |
There was a problem hiding this comment.
If c.Users is empty, splitCommaList returns nil, which marshals to null in the JSON output. It is more idiomatic for JSON arrays to be represented as an empty array [] rather than null when empty. We can ensure a non-nil slice is returned.
func (c *M365CalendarFreeBusyCmd) Run(ctx context.Context, flags *RootFlags) error {\n\tusers := splitCommaList(c.Users)\n\tif users == nil {\n\t\tusers = []string{}\n\t}\n\treturn writeM365PilotResult(ctx, flags, \"m365.calendar.freebusy\", map[string]any{\n\t\t\"users\": users,\n\t\t\"from\": strings.TrimSpace(c.From),\n\t\t\"to\": strings.TrimSpace(c.To),\n\t})\n}![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5bd774b134
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if flags == nil || !flags.ReadOnly { | ||
| return usage("m365 pilot commands require explicit --read-only") | ||
| } |
There was a problem hiding this comment.
When --force is supplied with an allowlisted M365 read command, this only checks --read-only, so wk --read-only --force m365 calendar events ... is still emitted as a safe read_only_pilot operation. The M365 approval contract in docs/plans/2026-05-30-workit-m365-write-approval-contract.md classifies any command using --force as approval-required, so the CLI should fail closed here (or otherwise refuse flags.Force) instead of presenting the operation as approval-free read-only.
Useful? React with 👍 / 👎.
namastex888
force-pushed
the
fix/m365-readonly-cli
branch
from
5bd774b to
f0d39db
Compare
|
Addressed Gemini feedback in f0d39db: replaced custom slice helper with stdlib slices.Contains and added/fixed freebusy empty users so JSON emits [] instead of null. Re-ran focused tests, make lint, make deadcode, go test ./..., and dogfooded the binary. |
|
KHAW alignment follow-up is open too: https://git.namastex.io/khal-apps/khaw/pulls/28. It fixes the classifier edge where the newly exposed \ includes an id positional after the allowlisted prefix. Local KHAW verification: 17 policy/audit tests passing and classifier dogfood green for all four Workit commands. |
|
KHAW alignment follow-up is open too: https://git.namastex.io/khal-apps/khaw/pulls/28. It fixes the classifier edge where the newly exposed Local KHAW verification: 17 policy/audit tests passing and classifier dogfood green for all four Workit commands. |
2 participants
Summary
Fixes #41 by exposing the M365 read-only pilot CLI surface that the KHAW gate was already designed to protect.
Root cause
The previous integration added the M365 scope guard in
internal/msauthand KHAW approval policy, but Workit never registered a user-facingm365top-level command or surfaced Microsoft Graph pilot scopes inwk auth services --json.Changes
wk m365top-level command with aliasesmicrosoftandgraph.wk --read-only m365 outlook searchwk --read-only m365 outlook message get <id>wk --read-only m365 calendar eventswk --read-only m365 calendar freebusy--read-onlyfor every M365 pilot command; missing flag fails closed.wk auth services --json/ Markdown / table output with only:User.ReadMail.ReadCalendars.ReadTests / dogfood
unexpected argument m365auth services missing m365 servicego test ./internal/cmd -run 'TestM365|TestAuthServicesJSONIncludesM365' -count=1 -vgo test ./internal/msauth ./internal/cmd -count=1go test ./...make lintmake deadcode./bin/wk --helpshowsm365 (microsoft,graph)./bin/wk --json auth servicesreturns M365 with only read-only Graph scopes./bin/wk --json --read-only m365 outlook search --query 'from:felipe' --top 2succeeds./bin/wk --json m365 outlook search --query 'from:felipe'fails closed and mentions--read-onlyread_only_pilotJSONNote
This PR intentionally exposes the CLI/pilot metadata only. It does not add write commands or write scopes. The KHAW policy remains the approval/write gate for any future write surface.
Summary by CodeRabbit
New Features
m365CLI command (aliases:microsoft,graph)