-
Notifications
You must be signed in to change notification settings - Fork 560
enable fips140 builds #1952
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
enable fips140 builds #1952
Conversation
WalkthroughThis update introduces two new GitHub Actions workflows for building and releasing FIPS-compliant Docker images and CLI binaries for the enterprise edition. The Dockerfile and CLI code are updated to support FIPS mode via build arguments and runtime logging. Minor formatting changes and blank lines are also added to existing files. The action.yml is enhanced to conditionally support FIPS builds and downloads. New documentation on FIPS 140 usage is added. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Release Event
participant GitHub Actions Workflow (FIPS)
participant Docker Build
participant Container Registry
GitHub Release Event->>GitHub Actions Workflow (FIPS): Trigger on release (tag starts with 'v')
GitHub Actions Workflow (FIPS)->>Docker Build: Build Docker image with FIPS args
Docker Build->>Container Registry: Push image with FIPS tags
sequenceDiagram
participant GitHub Release Event (go branch)
participant GitHub Actions Workflow (CLI FIPS)
participant Go Build
participant GitHub Releases
GitHub Release Event (go branch)->>GitHub Actions Workflow (CLI FIPS): Trigger on publish
GitHub Actions Workflow (CLI FIPS)->>Go Build: Build CLI with FIPS env vars
Go Build->>GitHub Releases: Publish FIPS-compliant CLI binary
Poem
✨ Finishing Touches🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
Added FIPS 140 compliance support across Digger's backend and CLI components, enabling secure cryptographic operations in regulated environments.
- Added FIPS build configuration in
Dockerfile_backend_ee
withGODEBUG_VALUE
andGOFIPS140_VALUE
build args - Added new workflow
.github/workflows/ee_backend_docker_release_fips.yml
for FIPS-compliant Docker image builds - Added FIPS status logging in
ee/cli/cmd/digger/default.go
usingcrypto/fips140.Enabled()
- Added
.github/workflows/ee_cli_release_fips.yml
for FIPS-compliant CLI builds (note: Go 1.24.0 version needs correction) - Potential issue:
CGO_ENABLED=0
in CLI workflow may conflict with FIPS requirements
6 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
project_path: ./ee/cli/cmd/digger | ||
binary_name: digger | ||
pre_command: export CGO_ENABLED=0 | ||
ldflags: ${{ matrix.ldflags }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: matrix.ldflags
is referenced but no matrix is defined in the workflow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.github/workflows/ee_backend_docker_release.yml (1)
48-49
: Remove trailing spaces in YAML file.The static analysis tools flagged trailing spaces on line 49, which violates YAML best practices.
- +🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 49-49: trailing spaces
(trailing-spaces)
.github/workflows/ee_backend_docker_release_fips.yml (2)
48-51
: Remove trailing whitespace
YAMLLint flagged trailing spaces on the last two lines ofbuild-args
. Trim these to clean up the YAML.Suggested diff:
- build-args: | - GODEBUG_VALUE=fips140=only - GOFIPS140_VALUE=v1.0.0 + build-args: | + GODEBUG_VALUE=fips140=only + GOFIPS140_VALUE=v1.0.0🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 50-50: trailing spaces
(trailing-spaces)
[error] 51-51: trailing spaces
(trailing-spaces)
40-48
: Enhance Docker build performance with caching
Consider enabling layer caching to speed up future builds. You can leverage theactions/cache
step and thecache-from
/cache-to
fields in the Docker action.Example snippet:
- uses: actions/cache@v3 with: path: /tmp/.build-cache key: ${{ runner.os }}-docker-cache-${{ github.sha }} - name: Build and push Docker image uses: docker/build-push-action@v5.3.0 with: context: . file: Dockerfile_backend_ee push: true cache-from: type=local,src=/tmp/.build-cache cache-to: type=local,dest=/tmp/.build-cache tags: ... labels: ... build-args: | GODEBUG_VALUE=fips140=only GOFIPS140_VALUE=v1.0.0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/ee_backend_docker_release.yml
(1 hunks).github/workflows/ee_backend_docker_release_fips.yml
(1 hunks).github/workflows/ee_cli_release_fips.yml
(1 hunks)Dockerfile_backend_ee
(1 hunks)cli/cmd/digger/default.go
(1 hunks)ee/cli/cmd/digger/default.go
(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/ee_backend_docker_release.yml
[error] 49-49: trailing spaces
(trailing-spaces)
.github/workflows/ee_backend_docker_release_fips.yml
[error] 50-50: trailing spaces
(trailing-spaces)
[error] 51-51: trailing spaces
(trailing-spaces)
🪛 actionlint (1.7.7)
.github/workflows/ee_cli_release_fips.yml
6-6: "branches" filter is not available for release event. it is only for merge_group, push, pull_request, pull_request_target, workflow_run events
(events)
36-36: property "ldflags" is not defined in object type {}
(expression)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
🔇 Additional comments (5)
cli/cmd/digger/default.go (1)
21-22
: LGTM! Minor formatting improvement.The added blank line improves code readability by separating the function signature from its implementation.
Dockerfile_backend_ee (1)
24-31
: Well-structured FIPS 140 support implementation.The added build arguments and environment variables provide a clean way to enable FIPS 140 compliance at build time. Using default values of "off" maintains backward compatibility while allowing explicit activation when needed for FIPS-compliant builds.
ee/cli/cmd/digger/default.go (2)
4-4
: LGTM! Appropriate import for FIPS 140 functionality.The addition of the crypto/fips140 package import is necessary for checking FIPS mode status.
29-29
: Good addition of FIPS 140 status logging.This log statement provides valuable runtime visibility into whether FIPS 140 mode is enabled, which is essential for validating that FIPS-compliant builds are working correctly.
.github/workflows/ee_cli_release_fips.yml (1)
36-36
: Static analysis false positive: custom action input
actionlint flaggedldflags
as undefined, but this is a valid input forwangyoucao577/go-release-action
. You can safely ignore this warning.🧰 Tools
🪛 actionlint (1.7.7)
36-36: property "ldflags" is not defined in object type {}
(expression)
"on": | ||
release: | ||
branches: | ||
- 'go' | ||
types: | ||
- 'released' | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix invalid branches
filter on release
event
GitHub Actions’ release
event does not support a branches
filter. This filter will be ignored, potentially causing unintended triggers. Remove the branches
key and, if you need to scope to the go
branch, use a job-level if
condition on github.event.release.target_commitish
.
Suggested diff:
-on:
- release:
- branches:
- - 'go'
- types:
- - 'released'
+on:
+ release:
+ types:
+ - 'released'
And at the job level:
jobs:
binary:
if: github.event.release.target_commitish == 'go'
runs-on: ubuntu-latest
...
🧰 Tools
🪛 actionlint (1.7.7)
6-6: "branches" filter is not available for release event. it is only for merge_group, push, pull_request, pull_request_target, workflow_run events
(events)
🤖 Prompt for AI Agents
In .github/workflows/ee_cli_release_fips.yml around lines 4 to 10, the release
event incorrectly uses a branches filter, which is unsupported and ignored by
GitHub Actions. Remove the branches key under the release event entirely. To
restrict the workflow to the 'go' branch, add an if condition at the job level
checking if github.event.release.target_commitish equals 'go'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
action.yml (2)
11-14
: Add input type forfips
for stronger validation
To avoid misconfiguration and automatically coerce values, consider declaring thefips
input as a boolean.fips: description: build with fips140 standard? required: false default: 'false' + type: boolean
478-482
: Reduce duplication in the FIPS vs standard download block
You can consolidate the twocurl
calls by prepending the-fips
suffix via a variable. This makes the logic easier to read and maintain:- if [[ ${{ inputs.fips }} == "true" ]]; then - curl -sL https://github.com/diggerhq/digger/releases/download/${actionref}/digger-ee-cli-${{ runner.os }}-${{ runner.arch }}-fips -o digger - else - curl -sL https://github.com/diggerhq/digger/releases/download/${actionref}/digger-ee-cli-${{ runner.os }}-${{ runner.arch }} -o digger - fi + suffix=${{ inputs.fips == 'true' && '-fips' || '' }} + curl -sL https://github.com/diggerhq/digger/releases/download/${actionref}/digger-ee-cli-${{ runner.os }}-${{ runner.arch }}${suffix} -o digger
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
action.yml
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
action.yml
Outdated
if [[ ${{ inputs.fips }} == "true" ]]; then | ||
export GODEBUG=fips140=only | ||
export GOFIPS140=v1.0.0 | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: FIPS env vars not persisted across steps
Using export
inside this step only affects the local shell. Subsequent steps (e.g., the Go build) won’t see GODEBUG
or GOFIPS140
. You need to write to $GITHUB_ENV
so the variables persist for later steps.
- if [[ ${{ inputs.fips }} == "true" ]]; then
- export GODEBUG=fips140=only
- export GOFIPS140=v1.0.0
- fi
+ if [[ ${{ inputs.fips }} == "true" ]]; then
+ echo "GODEBUG=fips140=only" >> "$GITHUB_ENV"
+ echo "GOFIPS140=v1.0.0" >> "$GITHUB_ENV"
+ fi
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if [[ ${{ inputs.fips }} == "true" ]]; then | |
export GODEBUG=fips140=only | |
export GOFIPS140=v1.0.0 | |
fi | |
if [[ ${{ inputs.fips }} == "true" ]]; then | |
echo "GODEBUG=fips140=only" >> "$GITHUB_ENV" | |
echo "GOFIPS140=v1.0.0" >> "$GITHUB_ENV" | |
fi |
🤖 Prompt for AI Agents
In action.yml around lines 365 to 368, the environment variables GODEBUG and
GOFIPS140 are set using export, which only affects the current shell and does
not persist across steps. To fix this, write these variables to the $GITHUB_ENV
file instead of using export, so they are available in subsequent steps like the
Go build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
action.yml (3)
11-14
: Clarify valid values forfips
input
The newfips
input is a string flag that drives FIPS‐mode builds, but the description doesn’t specify its accepted values. Consider updating it to something like:description: 'Build with FIPS140 standard? Accepts "true" or "false"'Or add an input‐validation step to catch invalid inputs.
434-437
: Export within run block is correct, but trim trailing whitespace
Since theexport GODEBUG
andexport GOFIPS140
lines live inside the same multi‐linerun
block asgo build
, the variables will be in scope for that build step. No persistence across steps is required here. However, line 437 (fi
) has trailing spaces—please remove them to satisfy YAML linting.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 437-437: trailing spaces
(trailing-spaces)
478-482
: DRY up the conditional download logic
The twocurl
invocations for enterprise CLI only differ by the-fips
suffix. You can collapse them into a single command by computing aSUFFIX
variable:SUFFIX=${{ inputs.fips == 'true' && '-fips' || '' }} curl -sL \ https://github.com/diggerhq/digger/releases/download/${actionref}/digger-ee-cli-${{ runner.os }}-${{ runner.arch }}${SUFFIX} \ -o diggerThis reduces duplication and simplifies maintenance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
action.yml
(3 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
action.yml
[error] 437-437: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
docs/ee/fips-140.mdx (5)
5-6
: Refine description and casing.
Use consistent casing for product name and standard, and merge sentences for brevity:- You can use digger binary with FIPS140 standard. FIPS 140 (Federal Information Processing Standard Publication 140) is a U.S. government standard that specifies security requirements for cryptographic modules protecting sensitive information. + The Digger binary supports FIPS 140 compliance, a U.S. government standard that specifies security requirements for cryptographic modules protecting sensitive information.
7-7
: Fix grammar and style.
Capitalize and correct spelling, add missing comma, and clarify context:- as of version v0.6.101 digger backend and cli are both compiled seperately with FIPS140 enabled. In order to enable it for github follow these steps: + Since version v0.6.101, the Digger backend and CLI are compiled separately with FIPS 140 enabled. To enable FIPS 140 on GitHub Actions, follow these steps:🧰 Tools
🪛 LanguageTool
[style] ~7-~7: Consider a more concise word here.
Context: ...mpiled seperately with FIPS140 enabled. In order to enable it for github follow these steps...(IN_ORDER_TO_PREMIUM)
[uncategorized] ~7-~7: A comma might be missing here.
Context: ...S140 enabled. In order to enable it for github follow these steps: - For the backend ...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
9-10
: Clarify bullet points and naming.
Use imperative voice, correct casing, and inline code formatting:- - For the backend you need to ensure you use the right docker image: `_backend_ee_fips` during the pull - - For the cli you need to add the following argument in addition to `ee: true` : + - Use the `_backend_ee_fips` Docker image for the enterprise backend. + - For the Digger CLI, add `fips: 'true'` alongside `ee: 'true`:
12-17
: Enhance code snippet clarity.
Specify YAML keys for GitHub Actions syntax and correct indentation:- ``` - - diggerhq/digger@vLatest - with: - ee: 'true' - fips: 'true' - ``` + ```yaml + - uses: diggerhq/digger@vLatest + with: + ee: 'true' + fips: 'true' + ```
19-19
: Improve VCS instructions.
Clarify and correct casing, add punctuation:- If you are using gitlab or other VCS then just ensure that you are downloading the fips enabled binary which is suffixed with '_fips' + For GitLab or other VCS, download the FIPS-enabled binary, which is suffixed with `_fips`.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/ee/fips-140.mdx
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/ee/fips-140.mdx
[style] ~7-~7: Consider a more concise word here.
Context: ...mpiled seperately with FIPS140 enabled. In order to enable it for github follow these steps...
(IN_ORDER_TO_PREMIUM)
[uncategorized] ~7-~7: A comma might be missing here.
Context: ...S140 enabled. In order to enable it for github follow these steps: - For the backend ...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
Summary by CodeRabbit
New Features
Chores