fix(machine): report unavailable protections instead of failing - #1441
Open
clement-tourriere wants to merge 6 commits into
Open
clement-tourriere wants to merge 6 commits into
clement-tourriere wants to merge 6 commits into
Conversation
The reconcile call answers 403 when the honeytoken module is disabled, the user is below Manager, or the token lacks `honeytokens:write`. Printing the raw API body left the reader guessing which one it was, and reading it as a workspace entitlement problem when a token scope was missing. Report the three prerequisites instead, matching what `honeytoken create` already prints. This is the shared path, so a direct run and the root fan-out get the message too, not only `machine setup`.
`auth login` reuses any still-valid token, so `--scopes` was a no-op for anyone already authenticated: the command printed "already authenticated" and exited without ever requesting the scope. Every message telling a user to run `auth login --scopes <scope>` was therefore dead advice unless they knew to log out first. Compare the requested scopes against the ones the token carries and fall through to a fresh login when one is missing. Only `--scopes` triggers the lookup, so a plain login keeps costing no extra round trip, and an unreadable scope list keeps the token rather than forcing a needless re-login. Comparison is on raw strings, since pygitguardian's `TokenScope` enum does not know every scope ggshield requests.
`machine setup` plants a honeytoken by default, so a token without `honeytokens:write` ended the run on a raw 403 and a non-zero exit. Doctor did the same for `honeytokens:write` and `ai-discover:send`. Both are gated on the plan, so no action on the machine can turn them green, and failing on them gates an MDM rollout on something out of the fleet's reach. Setup now checks the scope up front and skips planting with a message naming the command that grants it. Doctor gains an optional check state, rendered `!`, that still prints its fix but does not fail the run. `endpoints:send` stays required: installing the `machine_scan` plugin is an explicit opt-in, so a token that cannot upload endpoint data really is misconfigured.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1441 +/- ##
==========================================
+ Coverage 93.99% 94.00% +0.01%
==========================================
Files 200 201 +1
Lines 12660 12803 +143
==========================================
+ Hits 11900 12036 +136
- Misses 760 767 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Member
|
Thanks for taking this over! Splitting the fourth commit out removed the only feedback that a requested scope was refused,
Every attempt leaves another token behind, with no explanation and no way to succeed. On
|
This was referenced Sep 4, 2026
The backend grants the subset of requested scopes the member is eligible for and drops the rest, so what a login actually obtained is only knowable from the token afterwards. ggshield compared it against the hardcoded `DEFAULT_SCOPES`, which left two cases silent. A scope requested with `--scopes` and refused was never mentioned: the command printed nothing but success, while `machine doctor` kept advising the same flag. Compare against the effective requested set instead, the defaults plus whatever `--scopes` added, which is also the set sent in the authorize URL. A reused token was not looked at at all, since the early return skips the report entirely. One minted before a scope joined the defaults therefore stayed short until it expired, visible only in `machine doctor`. Report on that path too, with different words: nothing was refused there, the token was simply never asked for the scope, so say that and name the command that requests it. It costs one `/v1/api_tokens/self` call on an interactive, infrequent command. Warning and not re-authenticating is deliberate: forcing a login whenever a default scope is missing would mint a token on every run for any workspace whose plan cannot grant one of them. Refs: NHI-1991
…-login-reports-refused-scopes fix(auth): report the scopes login did not get
…at --scopes The `machine setup` skip and the `machine doctor` fix lines for `honeytokens:write` and `ai-discover:send` told every member to run `ggshield auth login --scopes <scope>`. The server grants what the plan and role allow and drops the rest, so for a member who is not a Manager, or whose plan does not offer honeytokens, that loop can never succeed: each attempt forces a real re-login that mints another token, and doctor keeps printing the same instruction. Lead with who can get the scope instead. The command stays, conditioned on being that person, since a Manager whose token came from a plain login still needs to know how to request it; the alternative is spelled out too, so a non-Manager reads that the plan or role cannot grant it and that nothing on the machine needs fixing. `scan` and `endpoints:send` keep their unconditional fix: missing them is a real break, not an unavailable feature.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Splits out the three uncontested commits of #1410 (@amascia-gg, authorship preserved) and rebases them onto current main. Nothing here changes
DEFAULT_SCOPES— thefeat(auth): request the honeytokens:write scope by defaultcommit stays on #1410, where the design question raised in review is still open.The problem
ggshield auth loginrequestsscan,honeytokens:check,endpoints:send,ai-discover:send— nothoneytokens:write. So a token from a plain login can never plant a honeytoken, andggshield machine setupends its run with a rawendpoint-deployments API error (403): …per target and exit code 1.ggshield machine doctorfails the same way on the missing scope.Neither is a broken machine.
honeytokens:writeis granted only on a plan that offers honeytokens, and only to a Manager;ai-discover:sendis likewise plan-gated. A member's laptop is correctly set up and still reports red — and under MDM that gates a rollout on something no machine can fix.What changes
machine setupchecks the token's scopes before planting. Withouthoneytokens:writeit reports the protection as unavailable and moves on, instead of running a doomed API call and failing the whole setup. Scopes it cannot read still go throughplant, which surfaces the real error.machine doctorgrows anoptionalcheck: printed with its fix and a yellow!, excluded from the exit code. Applied tohoneytokens:writeandai-discover:send— both plan-gated, and neither is needed by a protection that is already in place (honeytoken planting skips without its scope, andai discoveris a separate command).scanstays fatal, andendpoints:sendstays fatal when themachine_scanplugin is installed: installed-but-cannot-upload is a real break.honeytoken plantanswers a 403 with the three prerequisites (honeytoken module enabled, Manager access level,honeytokens:writeon the token) instead of the raw API body, which never said which one was missing.auth login --scopesnow falls through to a fresh login when the current token lacks a requested scope. Before, it reused the still-valid token and reported "already authenticated" without ever asking for it — which made every↳ fix:line in doctor that says "re-run auth login" a no-op. The doctor fixes now sayauth login --scopes <scope>.Testing
MockvsMagicMock).Notes for review
setup.pyimports and the area around_setup_honeytokens); whichever lands second needs a two-line resolution.honeytokens:writebelongs inDEFAULT_SCOPES, or whether honeytoken planting needs a scope/feature redesign first, given it is Manager-only and mostly run via MDM — is deliberately untouched here.