Merged
Conversation
This workflow generates SLSA provenance files for the project, satisfying level 3 requirements. It includes steps for building artifacts and generating subject hashes.
Reviewer's GuideAdds two new GitHub Actions workflows: one to generate and publish SLSA level 3 provenance for built artifacts on releases, and another to build a Jekyll site using a Docker-based builder on pushes and pull requests to main. Sequence diagram for SLSA provenance generation workflowsequenceDiagram
actor Developer
participant GitHub_Repo
participant GitHub_Actions
participant Job_build
participant Job_provenance
participant SLSA_Generator_Workflow
Developer->>GitHub_Repo: Create release
GitHub_Repo-->>GitHub_Actions: release event type created
GitHub_Actions->>Job_build: Start job build
Job_build->>Job_build: actions_checkout_v4
Job_build->>Job_build: Build artifacts artifact1 and artifact2
Job_build->>Job_build: Generate sha256 hashes
Job_build->>GitHub_Actions: Output digests as base64-subjects
GitHub_Actions->>Job_provenance: Start job provenance with needs.build.outputs.digests
Job_provenance->>SLSA_Generator_Workflow: Reusable workflow call generator_generic_slsa3.yml
SLSA_Generator_Workflow->>SLSA_Generator_Workflow: Verify workflow path and sign provenance
SLSA_Generator_Workflow->>GitHub_Repo: Upload provenance as release asset
GitHub_Repo-->>Developer: Release with attached SLSA level 3 provenance
Flow diagram for Jekyll Docker-based site build workflowflowchart TD
A["Push or pull_request to main"] --> B["Trigger workflow jekyll-docker"]
B --> C["Job build on ubuntu-latest"]
C --> D["Step actions/checkout@v4"]
D --> E["Step docker run jekyll/builder:latest"]
E --> F["Mount repo to /srv/jekyll and _site to /srv/jekyll/_site"]
F --> G["chmod -R 777 /srv/jekyll"]
G --> H["jekyll build --future"]
H --> I["Static site generated into _site directory"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and found some issues that need to be addressed.
- In the SLSA workflow, the
buildjob output is nameddigestsbut theGenerate subject for provenancestep writeshashes=...toGITHUB_OUTPUT; rename the output key or the echo’d name sosteps.hash.outputs.digests(andneeds.build.outputs.digests) actually exist. - Consider pinning external dependencies to specific versions or digests (e.g.,
jekyll/builder:latestandslsa-framework/slsa-github-generator@v1.4.0) to avoid unexpected changes from moving tags and improve supply-chain stability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the SLSA workflow, the `build` job output is named `digests` but the `Generate subject for provenance` step writes `hashes=...` to `GITHUB_OUTPUT`; rename the output key or the echo’d name so `steps.hash.outputs.digests` (and `needs.build.outputs.digests`) actually exist.
- Consider pinning external dependencies to specific versions or digests (e.g., `jekyll/builder:latest` and `slsa-framework/slsa-github-generator@v1.4.0`) to avoid unexpected changes from moving tags and improve supply-chain stability.
## Individual Comments
### Comment 1
<location> `.github/workflows/generator-generic-ossf-slsa3-publish.yml:23-32` </location>
<code_context>
+ digests: ${{ steps.hash.outputs.digests }}
</code_context>
<issue_to_address>
**issue (bug_risk):** The job output name (`digests`) does not match the step output name (`hashes`), so `needs.build.outputs.digests` will be empty.
The `build` job exposes `digests: ${{ steps.hash.outputs.digests }}`, but the `hash` step writes `hashes=...` to `GITHUB_OUTPUT`, so `steps.hash.outputs.digests` is never set and `provenance` gets an empty `base64-subjects`. Please either:
- Change the step to output `digests` (e.g. `echo "digests=$(...)" >> "$GITHUB_OUTPUT"`), or
- Change the job output and `provenance` input to use `hashes` instead.
The names need to match for the provenance workflow to receive the subject list.
</issue_to_address>
### Comment 2
<location> `.github/workflows/jekyll-docker.yml:20` </location>
<code_context>
+ run: |
+ docker run \
+ -v ${{ github.workspace }}:/srv/jekyll -v ${{ github.workspace }}/_site:/srv/jekyll/_site \
+ jekyll/builder:latest /bin/bash -c "chmod -R 777 /srv/jekyll && jekyll build --future"
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Using `chmod -R 777` inside the container is overly permissive and may mask permission issues.
Within the container, this also lets any process modify the build inputs/outputs under `/srv/jekyll`, which is broader than necessary.
Consider instead:
- Matching ownership/permissions to the Jekyll user/group (e.g. `chown -R jekyll:jekyll /srv/jekyll` plus a more restrictive `chmod` like 755), or
- Configuring `docker run` (e.g. `--user` or mount options) so a blanket recursive `777` isn’t needed.
This keeps the CI environment closer to production and makes permission issues more visible rather than papered over by `777`.
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Use whatever format you need to, or give me suggestions on how to fix this
| run: | | ||
| docker run \ | ||
| -v ${{ github.workspace }}:/srv/jekyll -v ${{ github.workspace }}/_site:/srv/jekyll/_site \ | ||
| jekyll/builder:latest /bin/bash -c "chmod -R 777 /srv/jekyll && jekyll build --future" |
There was a problem hiding this comment.
🚨 suggestion (security): Using chmod -R 777 inside the container is overly permissive and may mask permission issues.
Within the container, this also lets any process modify the build inputs/outputs under /srv/jekyll, which is broader than necessary.
Consider instead:
- Matching ownership/permissions to the Jekyll user/group (e.g.
chown -R jekyll:jekyll /srv/jekyllplus a more restrictivechmodlike 755), or - Configuring
docker run(e.g.--useror mount options) so a blanket recursive777isn’t needed.
This keeps the CI environment closer to production and makes permission issues more visible rather than papered over by 777.
Owner
Author
There was a problem hiding this comment.
Can you either correct this and do this for me, or give me a step-by-step on how to
Owner
Author
There was a problem hiding this comment.
Can you either correct this and do this for me, or give me a step-by-step on how to?
There was a problem hiding this comment.
You’re right to ask for something concrete here.
I can’t directly edit the PR for you, but here’s a step‑by‑step with an updated workflow snippet you can paste in.
1. Use the Jekyll image’s default user instead of chmod 777
The jekyll/builder image already has a jekyll user configured for running the build. The main issue is that when you mount your repo from the GitHub runner into the container, the files are owned by a different UID, so the jekyll user may not have write permissions.
A cleaner pattern is:
- Run the container as the same UID/GID as the GitHub runner user, or
- Ensure the Jekyll user has ownership of the mounted dir (using
chown), but with sane permissions instead of777.
Given GitHub Actions runs as uid=1001 on ubuntu-latest, we can run the container as that user and avoid chmod -R 777 entirely.
2. Updated workflow snippet
Replace your current run step with something like this:
- name: Build the site in the jekyll/builder container
run: |
# Detect the current user and group IDs (GitHub runner user)
UID=$(id -u)
GID=$(id -g)
docker run \
--user "$UID:$GID" \
-v "${{ github.workspace }}:/srv/jekyll" \
-v "${{ github.workspace }}/_site:/srv/jekyll/_site" \
jekyll/builder:latest \
jekyll build --futureKey changes:
--user "$UID:$GID"makes the container process run as the same user as the GitHub runner, so it already has appropriate permissions on the mounted files.- Removed the
chmod -R 777entirely. - Dropped the
/bin/bash -cwrapper since we’re just running a single command.
3. If you prefer to keep using the jekyll user
If you’d rather rely on the jekyll user inside the image, you can instead adjust ownership once, but with more restrictive permissions:
- name: Build the site in the jekyll/builder container
run: |
docker run \
-v "${{ github.workspace }}:/srv/jekyll" \
-v "${{ github.workspace }}/_site:/srv/jekyll/_site" \
jekyll/builder:latest \
/bin/bash -c "\
chown -R jekyll:jekyll /srv/jekyll && \
chmod -R u+rwX,go+rX /srv/jekyll && \
jekyll build --future \
"Here:
chown -R jekyll:jekyll /srv/jekyllgives thejekylluser ownership of the source and_sitedirectories.chmod -R u+rwX,go+rXmeans:- User: read, write, execute where appropriate.
- Group/others: read and execute only (no write).
4. What you need to do
- Pick one of the two snippets above (I recommend the first one with
--user "$UID:$GID"). - Replace the existing
run:block in.github/workflows/jekyll-docker.ymlwith that snippet. - Push the change to your branch – GitHub Actions will automatically rerun the workflow on the PR.
If you paste your updated YAML here, I can sanity‑check it before you push, or we can tweak for any errors the workflow logs show.
There was a problem hiding this comment.
You’re right to ask for something concrete here.
You have two options:
- A minimal change that just avoids
777but keeps the pattern you’re using now. - A more robust change that runs the container as your GitHub Actions user so you don’t need to chmod at all.
Below I’ll show you both, with copy‑pasteable YAML.
1) Minimal fix: more restrictive permissions
The jekyll/builder image uses a jekyll user internally. A safer pattern than chmod -R 777 is to:
- set ownership to
jekyll:jekyll, and - set permissions to something like
u+rwX,go+rX(readable by others, but not world‑writable).
Update your workflow step to this:
- name: Build the site in the jekyll/builder container
run: |
docker run \
-v ${{ github.workspace }}:/srv/jekyll \
-v ${{ github.workspace }}/_site:/srv/jekyll/_site \
jekyll/builder:latest /bin/bash -c "\
chown -R jekyll:jekyll /srv/jekyll && \
chmod -R u+rwX,go+rX /srv/jekyll && \
jekyll build --future"What changed:
- Replaced
chmod -R 777with:chown -R jekyll:jekyll /srv/jekyll(make the jekyll user own the files)chmod -R u+rwX,go+rX /srv/jekyll(read/execute for everyone, write only for owner)
This keeps things permissive enough for the container to work but avoids world‑writable permissions.
2) Better approach: match the host user (no chmod at all)
Another option is to run the container as the same user ID that GitHub Actions uses (typically runner with UID 1001). This way, file permissions match by default and you don’t need chmod.
- name: Build the site in the jekyll/builder container
run: |
docker run \
--user $(id -u):$(id -g) \
-v ${{ github.workspace }}:/srv/jekyll \
-v ${{ github.workspace }}/_site:/srv/jekyll/_site \
jekyll/builder:latest jekyll build --futureWhat changed:
- Added
--user $(id -u):$(id -g)so the container uses the same UID/GID as the GitHub runner. - Removed the
chmodcompletely.
If you try this and the build fails with a permissions error, fall back to option 1.
Which should you use?
- If you want the least disruptive change that almost certainly works right away, use Option 1.
- If you want the cleanest setup that avoids permission hacks, try Option 2 first and only move to option 1 if you hit issues.
If you tell me which option you’d like, I can adapt the full .github/workflows/jekyll-docker.yml for you and show the complete file as it should look.
✅ Deploy Preview for qagultima ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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
1 participant
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.
The difference in workloads, added go functionality among other coding works
Summary by Sourcery
Add CI workflows for SLSA provenance generation and Jekyll site builds.
CI: