Repository navigation
fix: harden authorization on command mutation routes - #154
Merged
Merged
Conversation
The store re-derives authorization for command mutations instead of
relying on the API's decision, and two gaps have accumulated in that
logic:
- DELETE /v1/commands/{namespace}/{name}/{version} accepted any token
carrying `user` scope, including a `guest` token, and the handler
skips its ownership check whenever the credential has no
pipelineId. Since the store is directly reachable, a caller that
never went through the API's permission check could reach the
delete path.
- The ownership lookup requested the command record from the API at a
/v1 path, but the API serves command records under /v4. The request
therefore 404s, which is interpreted as "no owner recorded" and
allowed. The ownership guard added in #151 has consequently not
been taking effect for build credentials either.
This corrects the lookup path so the existing guard takes effect, and
excludes guest scope from the delete route. It also additionally
accepts `sdapi` scope on the route, which is inert today and prepares
it for the API to send an explicitly scoped service token instead of
forwarding the caller's token; `user` scope will be removed once that
lands.
The skip for credentials without a pipelineId is deliberately left in
place: the API currently forwards the caller's token for user-scope
deletes, so removing the skip before the API change would break
command deletion. That is a follow-up.
This was referenced Sep 15, 2026
Merged
DekusDenial
approved these changes
Sep 15, 2026
|
🎉 This PR is included in version 7.0.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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
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.
Problem
The store re-derives authorization for command mutations instead of relying on the API's
decision, and two gaps have accumulated in that logic:
DELETE /v1/commands/{namespace}/{name}/{version}accepts any token carryinguserscope, and the handler skips its ownership check whenever the credential has no
pipelineId. Since the store is directly reachable, a caller that never went through theAPI's permission check can reach the delete path.
getCommandOwnerrequests the command record from the API at a/v1path, but the API serves command records under/v4. The request therefore 404s,which the helper interprets as "no owner recorded" and allows. The ownership guard added in
fix: enforce command ownership on store POST/DELETE #151 has consequently not been taking effect.
Change
the existing guard takes effect for build credentials.
guestscope from the delete route.sdapiscope on the delete route. This is inert today and prepares theroute for the API to send an explicitly scoped service token instead of forwarding the
caller's token;
userscope will be removed once that lands.The skip for credentials without a
pipelineIdis deliberately left in place here: the APIcurrently forwards the caller's token for user-initiated deletes, so removing it before the API
change would break command deletion. That is handled as a follow-up.
Tests
guestscope is rejected.userscope still succeeds (guards the current API-mediated flow).lookup path now exercises.
Compatibility
Correcting the lookup means the ownership guard begins enforcing for build credentials where it
previously no-oped. A build that publishes or deletes a command owned by a different pipeline
will now receive a 403. That is the intended behavior of #151.