Fix tenant-awareness for trusted-root command
#9638
Merged
Conversation
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
Signed-off-by: Brian DeHamer <bdehamer@github.com>
bdehamer
commented
Sep 18, 2024
Comment on lines
+119
to
+124
| // Target will be either the default trusted root, or the trust domain-qualified one | ||
| ghTR := defaultTR | ||
| if opts.TrustDomain != "" { | ||
| ghTR = fmt.Sprintf("%s.%s", opts.TrustDomain, defaultTR) | ||
| } | ||
|
|
There was a problem hiding this comment.
One subtle change from the existing logic: instead of retrieving both the default target (trusted_root.json) AND the tenant-prefixed target (some-trust-domain.trusted_root.json), the new logic will retrieve ONLY the tenant-prefixed version if the --hostname flag is supplied.
If you're identifying a specific tenant with the --hostname flag it seems reasonable to assume that you're only interested in the trusted root associated with that tenant.
There was a problem hiding this comment.
Seems fair. PGI is always included though. Given that a properly configured policy is configured, there would be no risk of confusion here during verification.
bdehamer
commented
Sep 18, 2024
Comment on lines
-140
to
-154
| targets := []string{defaultTR} | ||
| if opts.TrustDomain != "" { | ||
| targets = append(targets, fmt.Sprintf("%s.%s", | ||
| opts.TrustDomain, defaultTR)) | ||
| } |
There was a problem hiding this comment.
Note how the old logic specifies BOTH the default target AND the tenant-aware target for retrieval.
trevrosen
approved these changes
Sep 18, 2024
I take that back, there's really no reasonable way to test this without mocking out an entire TUF repository or allowing the tests to hit a remote endpoint -- I'm guessing this is why there were not tests around this functionality to begin with. |
kommendorkapten
approved these changes
Sep 23, 2024
tmeijn
pushed a commit
to tmeijn/dotfiles
that referenced
this pull request
Oct 16, 2024
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [cli/cli](https://github.com/cli/cli) | minor | `v2.57.0` -> `v2.58.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>cli/cli (cli/cli)</summary> ### [`v2.58.0`](https://github.com/cli/cli/releases/tag/v2.58.0): GitHub CLI 2.58.0 [Compare Source](cli/cli@v2.57.0...v2.58.0) #### What's Changed - Better messaging for `attestation verify` custom issuer mismatch error by [@​bdehamer](https://github.com/bdehamer) in cli/cli#9616 - Enhance gh repo create docs, fix random cmd link by [@​andyfeller](https://github.com/andyfeller) in cli/cli#9630 - Add HasActiveToken method to AuthConfig to refactor auth check for `attestation trusted-root` command by [@​BagToad](https://github.com/BagToad) in cli/cli#9635 - Improve the suggested command for creating an issue when an extension doesn't have a binary for your platform by [@​timrogers](https://github.com/timrogers) in cli/cli#9608 - Disable auth check for `attestation trusted-root` command by [@​bdehamer](https://github.com/bdehamer) in cli/cli#9610 - build(deps): bump github.com/henvic/httpretty from 0.1.3 to 0.1.4 by [@​dependabot](https://github.com/dependabot) in cli/cli#9645 - Fix tenant-awareness for `trusted-root` command by [@​bdehamer](https://github.com/bdehamer) in cli/cli#9638 - Replace "GitHub Enterprise Server" option with "other" in gh auth login prompting by [@​jtmcg](https://github.com/jtmcg) in cli/cli#9642 - build(deps): bump github.com/cpuguy83/go-md2man/v2 from 2.0.4 to 2.0.5 by [@​dependabot](https://github.com/dependabot) in cli/cli#9634 - Add `dnf5` instructions to `docs/install_linux.md` by [@​its-miroma](https://github.com/its-miroma) in cli/cli#9660 - build(deps): bump github.com/theupdateframework/go-tuf/v2 from 2.0.0 to 2.0.1 by [@​dependabot](https://github.com/dependabot) in cli/cli#9688 #### New Contributors - [@​its-miroma](https://github.com/its-miroma) made their first contribution in cli/cli#9660 **Full Changelog**: cli/cli@v2.57.0...v2.58.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
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.
Addresses the issue in the
attestation trusted-rootcommand which prevents the--tuf-urland--hostnameflags from being used together.There is already logic which calculates the correct
trusted_root.jsonprefix based on the supplied--hostnameflag, but it was only being applied in the case where the--tuf-urlwas NOT supplied.This change moves the logic to generate the tenant-aware
trusted_root.jsonprefix and applies it to all cases.Fixes #9637