docs: describe a safer approach to using pinned version #1930
Conversation
WalkthroughThe documentation for the Digger action has been updated to revise versioning guidance. The warning now emphasizes using a commit SHA instead of a version tag like Changes
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR updates the versioning documentation to advise using 40-character commit SHAs for GitHub Actions, improving security for IaC management.
- Revised /docs/ce/howto/versioning.mdx to promote pinned commit SHA usage with updated examples and warnings.
- Enhanced instructions addressing risks of mutable tags.
- YAML front matter still references the old @vX.Y.Z format, potentially causing confusion.
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
docs/ce/howto/versioning.mdx (4)
2-4: Ensure header description consistency with new pinned SHA recommendationThe header (lines 2–4) still recommends using a pinned version of the form
@vX.Y.Z, which conflicts with the new advice in the Warning block. For clarity and consistency, consider updating the header description to recommend a commit SHA reference. For example:-description: "For serious usecases always use a pinned version which is of the form @vX.Y.Z since this will download a compiled binary. In addition to being faster to run, it is also more secure than using a commit from a branch" +description: "For serious usecases, always use a commit SHA reference to pin the version from one of our releases, since this will download a compiled binary. In addition to being faster, it is also more secure than using a commit from a branch or tag"
14-19: Clear distinction for using the vLatest tagThe "Use vLatest tag" section now includes a code snippet using
diggerhq/digger@vLatest. This example is useful for users who prefer performance (with pre-built binaries). However, given the context of security, ensure that it is clear that while vLatest may be acceptable in certain scenarios, for critical tasks a pinned commit SHA is strongly recommended.
32-42: Correct usage for latest commit from a branch with necessary warningsThe guidance under "Use latest commit from a branch" includes a clear warning that using a branch reference (e.g.,
@yolo-lets-do-it) is risky and should only be done in non-production scenarios. This reinforces the security message. You may consider adding a bit more context on why this practice is discouraged to enhance the user's understanding.
43-48: Example of branch commit usage is clear, but consider reinforcing the riskThe final code snippet clearly demonstrates using a branch reference for the Digger action. Since this practice is inherently riskier, you might add a brief inline comment or refer the reader back to the warning above, ensuring that the security implications are unmistakable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/ce/howto/versioning.mdx(2 hunks)
🔇 Additional comments (2)
docs/ce/howto/versioning.mdx (2)
6-8: Great update to emphasize commit SHA for improved securityThe updated Warning block now instructs users to use a commit SHA reference (in the format
@<40-character-commit-SHA>), which is an excellent approach to prevent issues with mutable tags. This change clearly aligns with security best practices.
21-30: Updated pinned version instructions with commit SHAThe section "Use a pinned version" now clearly instructs users to look up the release in our releases and use the full commit SHA. The updated code snippet using the commit SHA (
28a8ecd4ccb31f9196b1cbc9ceb14025059f97dc) accurately demonstrates the new, safer versioning practice.
|
Hi @birjj thanks for this! I totally agree with hardcoding the commit. The issue is that our current check for whether to build or download binary is based on if it starts with the letter "v" to identify release. So we need to figure out a better way of saying download binary vs build from source in the composite action before this can work with pure commit sha ref. I think We need to figure that out before updating docs, (unless I missed something) |
|
Ah, yes that would definitely be a problem. I don't have any good ideas for getting a reference to the release from the commit SHA, so that's a blocker. I'll close this for now, since it probably isn't relevant until some workaround is found. |
|
Thanks, I've created a backlog issue to look into that so we can address it and then get back to this in the near future :D |
2 participants
As exemplified by the recent
tj-actions/changed-filescompromise, pinning GitHub Action references to a tag is inherently unsafe: if the referenced action is compromised, the attacker can update any tags, including old ones, to point to whatever they want.For something as crucial as IaC management it'd probably be preferable to use actually pinned versions over pinning to mutable tags. This PR updates the "Specifying version" docs to describe that process instead. Might as well push people towards the safer workflow 🤷
Also includes a fix to the docs so the code example for
vLatestactually uses@vLatest, and removes thedescriptionMDX header, since it contains text that's immediately replicated in a more-visible warning directly below it.Feel free to make any changes you feel relevant to make the wording flow better! This PR is mostly to suggest that the pin-to-commit-SHA approach is described in the docs.
Summary by CodeRabbit
Summary by CodeRabbit