Add make rpm, fix SELinux policy, add CI RPM builds and GitHub Pages repo#5
Merged
Conversation
Add rpm and srpm-all Makefile targets for local RPM builds via mock. Fix build-srpm.sh: correct puzzlepod-selinux spec filename, inject %global macros via sed (survives SRPM re-extraction), use tar instead of git archive for working-tree tarballs, preserve source tarball for CI artifact upload. Add git-core BuildRequires and git init in %prep for regorus build.rs compatibility in puzzlectl.spec and puzzled.spec. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Adam Miller <admiller@redhat.com>
Split the single puzzlepod_t type into puzzlepod_t (daemon) and puzzlepod_agent_t (sandboxed agents). Resolves 10 allow/neverallow contradictions, fixes duplicate type declaration, replaces invalid corenet_tcp_connect_dbus_port interface, uses proper SELinux interfaces for cgroup and quota access, and adds require block for base policy types. Update all documentation and diagrams to reflect the new type. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Adam Miller <admiller@redhat.com>
Add rpm-build matrix job to CI workflow validating builds against Fedora 42, Fedora 43, and CentOS Stream+EPEL 10 via mock. Add rpm-repo.yml workflow that builds RPMs on push to main and publishes a dnf repository to GitHub Pages with per-distro repos and source tarball. Upgrade CI container to Fedora 43 with mock and createrepo_c. Add reference .repo files for Fedora and EL. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Adam Miller <admiller@redhat.com>
Switch ci and integration jobs back to using the pre-built Fedora CI container image instead of inline ubuntu apt-get installs. Use vars.CI_IMAGE with fedora42 fallback for all jobs so the image tag can be updated via repository variables without code changes. Signed-off-by: Adam Miller <admiller@redhat.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Skip test_machine_id_fallback_deterministic when /etc/machine-id is too short (< 32 chars), as commonly occurs in containers. The test already skipped when the file was missing but not when it had truncated content. Signed-off-by: Adam Miller <admiller@redhat.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Update unicode-segmentation to unyanked version. Add RUSTSEC-2026-0002 (lru unsoundness) to deny.toml ignore list — transitive dep of ratatui v0.29, fix requires ratatui >= 0.30 breaking update. No practical impact as lru is used internally for layout caching. Signed-off-by: Adam Miller <admiller@redhat.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis PR containerizes CI jobs, adds workflows to build and publish RPM repositories to GitHub Pages, refactors SRPM/RPM packaging (including source tarball behavior and spec tweaks), introduces a new SELinux domain Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Container as Container (Fedora 43)
participant Mock as mock (in Container)
participant Artifacts as Actions Artifacts
participant Pages as GitHub Pages
GH->>Container: run `build-source` (build SRPMs, source tarball)
Container->>Artifacts: upload SRPMs + source tarball
GH->>Container: run `build-rpms` matrix (each target)
Container->>Mock: rebuild SRPMs -> produce RPMs
Mock->>Container: RPMs & repo metadata (createrepo_c)
Container->>Artifacts: upload `repo-<target>` artifacts
GH->>GH: `publish` job downloads SRPMs + repo artifacts
GH->>Pages: assemble `pages/` (repo dirs + index) and deploy to GitHub Pages
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
packaging/build-srpm.sh (1)
122-130: Verify tarball preservation timing.The source tarball is correctly preserved for CI before cleanup. However, consider adding error handling if the copy fails, since the original is deleted immediately after.
🔧 Optional: Add copy verification
# Preserve source tarball for CI artifact upload TARBALL_DIR="$SCRIPT_DIR/source-tarball" mkdir -p "$TARBALL_DIR" -cp "/tmp/${TARBALL}" "$TARBALL_DIR/" +cp "/tmp/${TARBALL}" "$TARBALL_DIR/" || { + echo "ERROR: Failed to preserve source tarball" >&2 + exit 1 +} echo "" echo "Source tarball: $TARBALL_DIR/${TARBALL}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packaging/build-srpm.sh` around lines 122 - 130, The script copies the source tarball to TARBALL_DIR but immediately deletes the original; add verification and error handling around the cp of "/tmp/${TARBALL}" to ensure we don't remove the only copy on failure: after attempting the copy (the cp command that references TARBALL and TARBALL_DIR), check the command exit status and if it failed, log an error message and exit non‑zero without running the subsequent rm -rf on RPMBUILD_DIR and "/tmp/${TARBALL}"; only proceed to the cleanup step when the copy succeeded..github/workflows/ci.yml (1)
336-344: Glob expansion may fail if no SRPMs exist.If
packaging/srpms/*.src.rpmmatches nothing (e.g., ifbuild-srpm.shfails silently), the loop iterates with the literal string*.src.rpm, causing mock to fail confusingly. Consider adding a check or enablingnullglob.♻️ Proposed fix to handle empty glob
- name: Build RPMs with mock run: | + shopt -s nullglob mkdir -p packaging/rpms for srpm in packaging/srpms/*.src.rpm; do echo "--- mock: $srpm ---" mock -r ${{ matrix.mock_root }} --enable-network --isolation=simple \ --rebuild "$srpm" --resultdir=packaging/rpms/ || { echo "FAILED: $srpm" cat packaging/rpms/build.log 2>/dev/null || true exit 1 } done + if [ ! -f packaging/rpms/*.rpm ]; then + echo "ERROR: No RPMs produced" + exit 1 + fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 336 - 344, The for-loop in the CI job that iterates over packaging/srpms/*.src.rpm can run with the literal glob when no SRPMs exist; update the loop in .github/workflows/ci.yml to guard against empty globs by either enabling shell nullglob/shopt -s nullglob before the loop or by adding an explicit existence check (e.g., test -e packaging/srpms/*.src.rpm or listing files into an array) so the mock invocation only runs when there are actual .src.rpm files to process..github/workflows/rpm-repo.yml (2)
167-169: Usefindinstead oflsfor safer filename handling.Per static analysis (SC2012), using
lswith globs can misbehave with filenames containing special characters. While unlikely here,findis more robust.♻️ Proposed fix
# Determine tarball filename for the index page - TARBALL=$(ls pages/source/puzzlepod-*.tar.gz 2>/dev/null | head -1) + TARBALL=$(find pages/source/ -maxdepth 1 -name 'puzzlepod-*.tar.gz' -print -quit 2>/dev/null) TARBALL_NAME=$(basename "$TARBALL" 2>/dev/null || echo "puzzlepod-source.tar.gz")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/rpm-repo.yml around lines 167 - 169, Replace the fragile ls-based lookup for TARBALL/TARBALL_NAME with a robust find-based search: use find to locate the first matching file under pages/source (maxdepth 1) for the 'puzzlepod-*.tar.gz' pattern and assign its path to TARBALL, then derive TARBALL_NAME from that path (falling back to "puzzlepod-source.tar.gz" if no match). Update the TARBALL and TARBALL_NAME assignments to use this find approach so filenames with special characters are handled safely.
101-109: Silently ignoring copy errors may hide real failures.The
2>/dev/null || trueon thecpcommand suppresses all errors, including cases where no RPMs were built. If mock succeeds but produces no output (unusual but possible), the repo would be empty without indication.♻️ Proposed improvement to validate RPM presence
- name: Prepare repo directory run: | mkdir -p ${{ matrix.repo_path }} cp rpms/*.noarch.rpm rpms/*.x86_64.rpm \ ${{ matrix.repo_path }}/ 2>/dev/null || true + # Verify at least one RPM was copied + if ! ls ${{ matrix.repo_path }}/*.rpm >/dev/null 2>&1; then + echo "ERROR: No RPMs found for repo" + exit 1 + fi # Strip debug packages (large, not needed for users) rm -f ${{ matrix.repo_path }}/*-debuginfo-* \ ${{ matrix.repo_path }}/*-debugsource-* createrepo_c ${{ matrix.repo_path }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/rpm-repo.yml around lines 101 - 109, Remove the silent suppression on the cp step and add an explicit existence check after copying to fail the job if no RPMs were placed into the repo dir; specifically modify the block that runs mkdir -p ${{ matrix.repo_path }}, cp rpms/*.noarch.rpm rpms/*.x86_64.rpm ${{ matrix.repo_path }}, rm -f ... and createrepo_c ${{ matrix.repo_path }} so that cp errors are not redirected to /dev/null, and add a test (e.g., check for globs like ${{ matrix.repo_path }}/*.rpm) that calls exit 1 or prints an error before createrepo_c when no RPMs are found.Makefile (1)
299-328: Good error aggregation pattern; consider adding--isolation=simplefor consistency.The
rpmtarget properly aggregates failures and reports them at the end. However, the CI workflow uses--isolation=simplewith mock, while this local target does not. This could lead to different behavior between local and CI builds.♻️ Proposed fix for consistency with CI
- if mock -r "$$MOCK_ROOT" --enable-network --rebuild "$$srpm" --resultdir=packaging/rpms/; then \ + if mock -r "$$MOCK_ROOT" --enable-network --isolation=simple --rebuild "$$srpm" --resultdir=packaging/rpms/; then \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 299 - 328, The rpm Makefile target runs mock --rebuild for each SRPM but omits the --isolation=simple flag used in CI, which can cause differing local vs CI behavior; update the mock invocation in the rpm target (the line invoking mock -r "$$MOCK_ROOT" --enable-network --rebuild "$$srpm" --resultdir=packaging/rpms/) to include --isolation=simple so mock is invoked consistently with CI (leave the rest of the command, variables like MOCK_ROOT and the packaging/rpms resultdir unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/podman_puzzled_architecture.md`:
- Around line 169-170: Update the runtime SELinux label references from
puzzlepod_t to puzzlepod_agent_t: replace the string --security-opt
"label=type:puzzlepod_t" (in podman/puzzle-podman) with --security-opt
"label=type:puzzlepod_agent_t" and replace SecurityOpt=label=type:puzzlepod_t
(in podman/quadlet/puzzlepod-agent.container) with
SecurityOpt=label=type:puzzlepod_agent_t so the runtime configs match the docs.
In `@docs/technical-design.md`:
- Line 896: The sentence wrongly uses the agent sandbox domain name
puzzlepod_agent_t for the daemon; change puzzlepod_agent_t to the daemon domain
puzzlepod_t so the hardening line accurately describes the daemon (puzzled)
confinement; update the string in the paragraph containing "puzzlepod_agent_t"
to "puzzlepod_t" and ensure it now matches the daemon domain used elsewhere
(e.g., puzzlepod_t in selinux policy).
In `@selinux/puzzlepod.te`:
- Around line 14-17: Podman-managed agents are being started with
label=type:puzzlepod_t so they bypass the intended puzzlepod_agent_t domain;
update the Podman launch files (podman/puzzle-podman and
podman/quadlet/puzzlepod-agent.container) to use label=type:puzzlepod_agent_t
instead of puzzlepod_t, and ensure the SELinux policy includes the corresponding
transition via application_domain(puzzlepod_agent_t, puzzlepod_exec_t) (and
mirror any other declarations around the duplicated block at lines noted, e.g.,
the types at 96-99) so Podman-launched agents enter the puzzlepod_agent_t domain
rather than puzzlepod_t.
- Around line 166-169: The puzzlepod_agent_t SELinux domain is missing baseline
file/read and execute allows; update the selinux policy for puzzlepod_agent_t
(where files_read_usr_files(puzzlepod_agent_t), libs_use_ld_so,
libs_use_shared_libs are declared) to explicitly allow read access to
/etc/localtime, /etc/resolv.conf, /proc/self/**, /dev/null, and /dev/urandom and
to grant execute permission for the documented toolchain binaries (python3,
node, git, grep, make, cargo, curl) — i.e., add the appropriate allow rules or
use/extend macros so that puzzlepod_agent_t can read those config/device paths
and execute the binaries (ensure execute bit/context types for /usr/bin/*
entries are covered rather than only files_read_usr_files()).
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 336-344: The for-loop in the CI job that iterates over
packaging/srpms/*.src.rpm can run with the literal glob when no SRPMs exist;
update the loop in .github/workflows/ci.yml to guard against empty globs by
either enabling shell nullglob/shopt -s nullglob before the loop or by adding an
explicit existence check (e.g., test -e packaging/srpms/*.src.rpm or listing
files into an array) so the mock invocation only runs when there are actual
.src.rpm files to process.
In @.github/workflows/rpm-repo.yml:
- Around line 167-169: Replace the fragile ls-based lookup for
TARBALL/TARBALL_NAME with a robust find-based search: use find to locate the
first matching file under pages/source (maxdepth 1) for the 'puzzlepod-*.tar.gz'
pattern and assign its path to TARBALL, then derive TARBALL_NAME from that path
(falling back to "puzzlepod-source.tar.gz" if no match). Update the TARBALL and
TARBALL_NAME assignments to use this find approach so filenames with special
characters are handled safely.
- Around line 101-109: Remove the silent suppression on the cp step and add an
explicit existence check after copying to fail the job if no RPMs were placed
into the repo dir; specifically modify the block that runs mkdir -p ${{
matrix.repo_path }}, cp rpms/*.noarch.rpm rpms/*.x86_64.rpm ${{ matrix.repo_path
}}, rm -f ... and createrepo_c ${{ matrix.repo_path }} so that cp errors are not
redirected to /dev/null, and add a test (e.g., check for globs like ${{
matrix.repo_path }}/*.rpm) that calls exit 1 or prints an error before
createrepo_c when no RPMs are found.
In `@Makefile`:
- Around line 299-328: The rpm Makefile target runs mock --rebuild for each SRPM
but omits the --isolation=simple flag used in CI, which can cause differing
local vs CI behavior; update the mock invocation in the rpm target (the line
invoking mock -r "$$MOCK_ROOT" --enable-network --rebuild "$$srpm"
--resultdir=packaging/rpms/) to include --isolation=simple so mock is invoked
consistently with CI (leave the rest of the command, variables like MOCK_ROOT
and the packaging/rpms resultdir unchanged).
In `@packaging/build-srpm.sh`:
- Around line 122-130: The script copies the source tarball to TARBALL_DIR but
immediately deletes the original; add verification and error handling around the
cp of "/tmp/${TARBALL}" to ensure we don't remove the only copy on failure:
after attempting the copy (the cp command that references TARBALL and
TARBALL_DIR), check the command exit status and if it failed, log an error
message and exit non‑zero without running the subsequent rm -rf on RPMBUILD_DIR
and "/tmp/${TARBALL}"; only proceed to the cleanup step when the copy succeeded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6e7db488-6e67-4e62-877c-c415f2dd64a6
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
.github/workflows/ci.yml.github/workflows/rpm-repo.yml.gitignoreMakefileci/Containerfilecrates/puzzled/src/config.rsdeny.tomldocs/PRD.mddocs/admin-guide.mddocs/compliance/fedramp.mddocs/compliance/iso27001.mddocs/developer-guide.mddocs/diagrams/01-system-architecture.drawiodocs/diagrams/06-defense-in-depth.drawiodocs/podman_puzzled_architecture.mddocs/rogue-agent-test-report.mddocs/security-guide.mddocs/technical-design.mdpackaging/build-srpm.shpackaging/puzzlectl.specpackaging/puzzled.specpackaging/puzzlepod-el.repopackaging/puzzlepod.reposelinux/puzzlepod.ifselinux/puzzlepod.te
Comment on lines
+166
to
169
| files_read_usr_files(puzzlepod_agent_t) | ||
| libs_use_ld_so(puzzlepod_agent_t) | ||
| libs_use_shared_libs(puzzlepod_agent_t) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In SELinux reference policy, does the files_read_usr_files(domain)macro grantexecuteorexecute_no_transon/usr/bin/*, or is a separate execution interface required for a custom domain?
💡 Result:
In the SELinux reference policy, the files_read_usr_files(domain) macro does not grant execute or execute_no_trans permissions on /usr/bin/*. It provides read access (e.g., read, getattr, search on usr_t directories and files), but execution permissions require a separate interface, such as corecmd_exec_bin(domain) or files_execute_usr_files(domain) if it exists, or explicit allow rules for execute on bin_t or usr_t files. Custom domains typically need additional execution interfaces like domain_entry_file(domain, { bin_t usr_t }) or specific exec macros to run binaries from /usr/bin without denials.
Citations:
- 1: https://github.com/sjvermeu/hardened-refpolicy/blob/master/policy/modules/kernel/files.if
- 2: https://www.oreilly.com/library/view/selinux/0596007167/apc.html
- 3: https://www.linuxtopia.org/online_books/writing_SELinux_policy_guide/macros_directory_09.html
- 4: https://www.linuxtopia.org/online_books/redhat_selinux_guide/rhlcommon-section-0023.html
- 5: https://pages.cs.wisc.edu/~matyas/selinux-policy/kernel_files.html
🏁 Script executed:
# First, check if the documentation file exists and examine the referenced lines
fd -t f "technical-design.md" --exec wc -l {} \;Repository: LobsterTrap/puzzlepod
Length of output: 96
🏁 Script executed:
# Get the referenced lines from the technical design doc (617-625 and 647-655)
sed -n '617,625p;647,655p' docs/technical-design.mdRepository: LobsterTrap/puzzlepod
Length of output: 453
🏁 Script executed:
# Check the full puzzlepod.te file to see all permissions granted to puzzlepod_agent_t
cat -n selinux/puzzlepod.te | grep -A 50 "puzzlepod_agent_t"Repository: LobsterTrap/puzzlepod
Length of output: 11498
puzzlepod_agent_t is missing critical baseline runtime access documented in the design.
The domain lacks SELinux allows for the baseline reads specified in docs/technical-design.md (lines 617-625): /etc/localtime, /etc/resolv.conf, /proc/self/**, /dev/null, and /dev/urandom. Additionally, there are no execute permissions for the toolchain binaries documented in lines 647-655 (python3, node, git, grep, make, cargo, curl). The files_read_usr_files() macro grants read access to /usr/* but does not include execute permissions. Without these permissions, DNS resolution will fail (missing /etc/resolv.conf), timezone operations will break (missing /etc/localtime), and none of the documented toolchain executables will run when agents transition to this domain.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@selinux/puzzlepod.te` around lines 166 - 169, The puzzlepod_agent_t SELinux
domain is missing baseline file/read and execute allows; update the selinux
policy for puzzlepod_agent_t (where files_read_usr_files(puzzlepod_agent_t),
libs_use_ld_so, libs_use_shared_libs are declared) to explicitly allow read
access to /etc/localtime, /etc/resolv.conf, /proc/self/**, /dev/null, and
/dev/urandom and to grant execute permission for the documented toolchain
binaries (python3, node, git, grep, make, cargo, curl) — i.e., add the
appropriate allow rules or use/extend macros so that puzzlepod_agent_t can read
those config/device paths and execute the binaries (ensure execute bit/context
types for /usr/bin/* entries are covered rather than only
files_read_usr_files()).
Fix Podman runtime configs to use puzzlepod_agent_t SELinux label (puzzle-podman, quadlet container). Revert daemon hardening description to puzzlepod_t. Add corecmd_exec_bin for agent binary execution. Add error handling on tarball copy, nullglob guard for SRPM globs, find-based tarball lookup, RPM existence validation, and --isolation=simple to Makefile mock invocation for CI consistency. Signed-off-by: Adam Miller <admiller@redhat.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
.github/workflows/rpm-repo.yml (2)
82-99: Consider adding a nullglob guard for robustness.Similar to the Makefile, if
srpms/*.src.rpmdoesn't match (e.g., artifact download issue), the loop will iterate with the literal glob string. While unlikely since artifacts come from the previous job, adding a guard improves resilience.♻️ Proposed fix
- name: Build RPMs via mock run: | mkdir -p rpms FAILED="" + shopt -s nullglob for srpm in srpms/*.src.rpm; do echo "--- mock: $srpm ---" if mock -r ${{ matrix.mock_root }} --enable-network --isolation=simple \ --rebuild "$srpm" --resultdir=rpms/; then echo "--- OK: $srpm ---" else echo "--- FAILED: $srpm ---" FAILED="$FAILED $srpm" fi done + shopt -u nullglob if [ -n "$FAILED" ]; then echo "=== FAILED builds ===$FAILED" exit 1 fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/rpm-repo.yml around lines 82 - 99, The loop over srpms/*.src.rpm can iterate the literal glob when there are no matches; enable a nullglob guard before the loop or explicitly check for matches: set shell to ignore unmatched globs (e.g., run shopt -s nullglob) or test for files (e.g., compgen -G "srpms/*.src.rpm" >/dev/null) before the for-loop that assigns to srpm and uses FAILED; ensure to restore the shell option if you change it and keep the rest of the loop logic (the for srpm in srpms/*.src.rpm; do ... FAILED=...; done and the subsequent if [ -n "$FAILED" ]; then ... fi) unchanged.
172-174: Fallback tarball name may produce broken link.If
findreturns no results,TARBALL_NAMEdefaults topuzzlepod-source.tar.gz, which may not exist, creating a broken link in the index. Consider adding validation or using a more defensive pattern.♻️ Proposed improvement
# Determine tarball filename for the index page TARBALL=$(find pages/source/ -maxdepth 1 -name 'puzzlepod-*.tar.gz' -print -quit 2>/dev/null) - TARBALL_NAME=$(basename "$TARBALL" 2>/dev/null || echo "puzzlepod-source.tar.gz") + if [ -n "$TARBALL" ]; then + TARBALL_NAME=$(basename "$TARBALL") + else + echo "WARNING: No source tarball found, index link may be broken" + TARBALL_NAME="puzzlepod-source.tar.gz" + fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/rpm-repo.yml around lines 172 - 174, The fallback sets TARBALL_NAME to a hardcoded "puzzlepod-source.tar.gz" even when TARBALL is empty, which can create a broken link; update the workflow to validate TARBALL after the find call (check that the variable is non-empty and that the file exists) and then only derive TARBALL_NAME from basename("$TARBALL") when valid, otherwise handle the missing artifact defensively (for example: set TARBALL_NAME to empty and skip index link generation, or fail the job with a clear error message). Ensure you modify the logic referring to the TARBALL and TARBALL_NAME variables so downstream steps that build the index either skip the link or fail fast with an informative message.Makefile (1)
311-318: Add a guard for empty SRPM glob expansion.If
packaging/srpms/*.src.rpmmatches no files (e.g.,srpm-allproduces nothing), the loop iterates once with the literal glob string, causing a confusing mock error. Consider adding a nullglob guard or explicit check.♻️ Proposed fix
mkdir -p packaging/rpms; \ FAILED=""; \ + shopt -s nullglob; \ for srpm in packaging/srpms/*.src.rpm; do \ echo "--- mock: $$srpm ---"; \ if mock -r "$$MOCK_ROOT" --enable-network --isolation=simple --rebuild "$$srpm" --resultdir=packaging/rpms/; then \ echo "--- OK: $$srpm ---"; \ else \ echo "--- FAILED: $$srpm ---"; \ FAILED="$$FAILED $$srpm"; \ fi; \ done; \ + shopt -u nullglob; \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 311 - 318, The SRPM loop iterates even when the glob yields no files (causing mock to receive the literal pattern); replace the direct shell glob in the for-loop with a Makefile-side check using the wildcard function: compute SRPMS := $(wildcard packaging/srpms/*.src.rpm) and then iterate for srpm in $(SRPMS) or early-exit/echo when SRPMS is empty. Update the loop that uses the srpm variable and preserves the FAILED and MOCK_ROOT usage so mock only runs when real .src.rpm files exist.podman/puzzle-podman (1)
584-587: SELinux label correctly updated topuzzlepod_agent_twith graceful fallback.This addresses the prior feedback. The conditional check allows the script to work on systems without the puzzlepod SELinux policy installed.
Minor nit:
&>/dev/null 2>&1is redundant —&>/dev/nullalready redirects both stdout and stderr. Consider simplifying to just&>/dev/null.,
Optional cleanup
# Only apply puzzlepod_agent_t SELinux label if the type exists on the host - if seinfo -t puzzlepod_agent_t &>/dev/null 2>&1; then + if seinfo -t puzzlepod_agent_t &>/dev/null; then NATIVE_ARGS+=(--security-opt "label=type:puzzlepod_agent_t") fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@podman/puzzle-podman` around lines 584 - 587, The seinfo check currently uses a redundant redirection `&>/dev/null 2>&1`; simplify the command by removing the duplicate redirection so it only redirects both stdout and stderr once (e.g., use `&>/dev/null` or `>/dev/null 2>&1`) in the `seinfo -t puzzlepod_agent_t` test that controls appending `--security-opt "label=type:puzzlepod_agent_t"` to NATIVE_ARGS; update the `seinfo` conditional accordingly so the logic and behavior remain unchanged but the redirection is clean.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/rpm-repo.yml:
- Around line 155-170: Update the repository generation to document the current
disabled signature checks: add a clear comment above the two heredocs that
create pages/puzzlepod.repo and pages/puzzlepod-el.repo stating that gpgcheck is
intentionally set to 0 for now and that GPG signing will be enabled in a future
release (and/or include a TODO with the expected gpgkey location), and also add
a short note to the repo index page (index.html) documenting this limitation so
users are aware; reference the files/pages created by the heredocs
(pages/puzzlepod.repo and pages/puzzlepod-el.repo) and the gpgcheck setting to
locate where to insert the comments and index note.
In `@selinux/puzzlepod.te`:
- Around line 98-101: The entrypoint permission is incorrectly granted to the
source domain; change the SELinux rule so the entrypoint permission is granted
to the target domain by replacing the current allow that uses puzzlepod_t with
one that uses puzzlepod_agent_t so it reads as an allow for puzzlepod_agent_t
against puzzlepod_exec_t:file entrypoint; keep the type_transition and the
read/getattr/execute allow for puzzlepod_t as-is.
- Around line 14-16: The type_transition rule referencing puzzlepod_exec_t
(type_transition puzzlepod_t puzzlepod_exec_t:process puzzlepod_agent_t) is dead
because puzzlepod_exec_t has no file context in selinux/puzzlepod.fc; either add
a file context entry that assigns puzzlepod_exec_t to the agent binary (e.g.,
the agent path used at runtime) so the automatic domain transition can occur, or
remove the type_transition rule (or document that Podman must label the process
with --security-opt label=type:puzzlepod_agent_t) — update selinux/puzzlepod.fc
to include the appropriate gen_context mapping for puzzlepod_exec_t or drop the
transition rule in the policy where puzzlepod_exec_t and puzzlepod_agent_t are
declared.
---
Nitpick comments:
In @.github/workflows/rpm-repo.yml:
- Around line 82-99: The loop over srpms/*.src.rpm can iterate the literal glob
when there are no matches; enable a nullglob guard before the loop or explicitly
check for matches: set shell to ignore unmatched globs (e.g., run shopt -s
nullglob) or test for files (e.g., compgen -G "srpms/*.src.rpm" >/dev/null)
before the for-loop that assigns to srpm and uses FAILED; ensure to restore the
shell option if you change it and keep the rest of the loop logic (the for srpm
in srpms/*.src.rpm; do ... FAILED=...; done and the subsequent if [ -n "$FAILED"
]; then ... fi) unchanged.
- Around line 172-174: The fallback sets TARBALL_NAME to a hardcoded
"puzzlepod-source.tar.gz" even when TARBALL is empty, which can create a broken
link; update the workflow to validate TARBALL after the find call (check that
the variable is non-empty and that the file exists) and then only derive
TARBALL_NAME from basename("$TARBALL") when valid, otherwise handle the missing
artifact defensively (for example: set TARBALL_NAME to empty and skip index link
generation, or fail the job with a clear error message). Ensure you modify the
logic referring to the TARBALL and TARBALL_NAME variables so downstream steps
that build the index either skip the link or fail fast with an informative
message.
In `@Makefile`:
- Around line 311-318: The SRPM loop iterates even when the glob yields no files
(causing mock to receive the literal pattern); replace the direct shell glob in
the for-loop with a Makefile-side check using the wildcard function: compute
SRPMS := $(wildcard packaging/srpms/*.src.rpm) and then iterate for srpm in
$(SRPMS) or early-exit/echo when SRPMS is empty. Update the loop that uses the
srpm variable and preserves the FAILED and MOCK_ROOT usage so mock only runs
when real .src.rpm files exist.
In `@podman/puzzle-podman`:
- Around line 584-587: The seinfo check currently uses a redundant redirection
`&>/dev/null 2>&1`; simplify the command by removing the duplicate redirection
so it only redirects both stdout and stderr once (e.g., use `&>/dev/null` or
`>/dev/null 2>&1`) in the `seinfo -t puzzlepod_agent_t` test that controls
appending `--security-opt "label=type:puzzlepod_agent_t"` to NATIVE_ARGS; update
the `seinfo` conditional accordingly so the logic and behavior remain unchanged
but the redirection is clean.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3cee689d-6266-4877-a718-031a06910be8
📒 Files selected for processing (8)
.github/workflows/ci.yml.github/workflows/rpm-repo.ymlMakefiledocs/technical-design.mdpackaging/build-srpm.shpodman/puzzle-podmanpodman/quadlet/puzzlepod-agent.containerselinux/puzzlepod.te
✅ Files skipped from review due to trivial changes (1)
- docs/technical-design.md
🚧 Files skipped from review as they are similar to previous changes (2)
- packaging/build-srpm.sh
- .github/workflows/ci.yml
Comment on lines
+155
to
+170
| cat > pages/puzzlepod.repo << 'EOF' | ||
| [puzzlepod] | ||
| name=PuzzlePod - Governance for AI Agent Containers | ||
| baseurl=https://lobstertrap.github.io/puzzlepod/repo/fedora/$releasever/x86_64/ | ||
| enabled=1 | ||
| gpgcheck=0 | ||
| EOF | ||
|
|
||
| # EL .repo | ||
| cat > pages/puzzlepod-el.repo << 'EOF' | ||
| [puzzlepod] | ||
| name=PuzzlePod - Governance for AI Agent Containers | ||
| baseurl=https://lobstertrap.github.io/puzzlepod/repo/epel/$releasever/x86_64/ | ||
| enabled=1 | ||
| gpgcheck=0 | ||
| EOF |
There was a problem hiding this comment.
Document or plan for GPG signing.
Both .repo files set gpgcheck=0, which means RPM signatures won't be verified during installation. While acceptable for early-stage projects, this reduces security guarantees for users installing from this repository. Consider adding a comment noting the intent to enable GPG signing in the future, or documenting this limitation in the index page.
📝 Suggested documentation addition to index.html
<h2>Repository Files</h2>
<ul>
<li><a href="puzzlepod.repo">puzzlepod.repo</a> (Fedora)</li>
<li><a href="puzzlepod-el.repo">puzzlepod-el.repo</a> (RHEL/EL)</li>
</ul>
+
+ <p><strong>Note:</strong> Packages are not GPG-signed. GPG verification is disabled in the repository configuration.</p>
</body>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/rpm-repo.yml around lines 155 - 170, Update the repository
generation to document the current disabled signature checks: add a clear
comment above the two heredocs that create pages/puzzlepod.repo and
pages/puzzlepod-el.repo stating that gpgcheck is intentionally set to 0 for now
and that GPG signing will be enabled in a future release (and/or include a TODO
with the expected gpgkey location), and also add a short note to the repo index
page (index.html) documenting this limitation so users are aware; reference the
files/pages created by the heredocs (pages/puzzlepod.repo and
pages/puzzlepod-el.repo) and the gpgcheck setting to locate where to insert the
comments and index note.
Comment on lines
+14
to
+16
| type puzzlepod_agent_t; | ||
| type puzzlepod_exec_t; | ||
| application_domain(puzzlepod_t, puzzlepod_exec_t) | ||
| application_domain(puzzlepod_agent_t, puzzlepod_exec_t) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if puzzlepod_exec_t has any file context assignment
echo "=== Searching for puzzlepod_exec_t in .fc files ==="
rg -n "puzzlepod_exec_t" --type-add 'fc:*.fc' --type=fc
echo ""
echo "=== All file context entries in puzzlepod.fc ==="
fd -t f "puzzlepod.fc" --exec cat {}Repository: LobsterTrap/puzzlepod
Length of output: 1625
🏁 Script executed:
#!/bin/bash
# Verify 1: Check for all .fc files in repository
echo "=== All .fc files in repository ==="
fd -t f "\.fc$"
echo ""
echo "=== Total .fc files found ==="
fd -t f "\.fc$" | wc -l
echo ""
echo "=== Verify: Extract type_transition rule from puzzlepod.te around line 99 ==="
if [ -f selinux/puzzlepod.te ]; then
sed -n '95,105p' selinux/puzzlepod.te
else
echo "puzzlepod.te not found"
fi
echo ""
echo "=== Search for puzzlepod_exec_t usage in .te files ==="
rg -n "puzzlepod_exec_t" --type-add 'te:*.te' --type=te --context=2Repository: LobsterTrap/puzzlepod
Length of output: 1732
puzzlepod_exec_t has no file context assignment — type transition will never trigger.
The type_transition puzzlepod_t puzzlepod_exec_t:process puzzlepod_agent_t rule on line 99 requires that an agent binary be labeled puzzlepod_exec_t at runtime. However, examining selinux/puzzlepod.fc, there is no file context entry assigning puzzlepod_exec_t to any path — only puzzled_exec_t (for /usr/bin/puzzled) and puzzlectl_exec_t (for /usr/bin/puzzlectl) are defined.
Without a corresponding .fc entry like:
/usr/libexec/puzzle-agent -- gen_context(system_u:object_r:puzzlepod_exec_t,s0)
The type transition rule is dead code. Agents will either:
- Run in the caller's domain (if exec'd directly), or
- Only receive
puzzlepod_agent_tvia explicit Podman--security-opt label=type:puzzlepod_agent_t
If the intent is Podman-only labeling (no automatic domain transition), document this or remove the dead type_transition rule. Otherwise, add a file context entry for the agent executable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@selinux/puzzlepod.te` around lines 14 - 16, The type_transition rule
referencing puzzlepod_exec_t (type_transition puzzlepod_t
puzzlepod_exec_t:process puzzlepod_agent_t) is dead because puzzlepod_exec_t has
no file context in selinux/puzzlepod.fc; either add a file context entry that
assigns puzzlepod_exec_t to the agent binary (e.g., the agent path used at
runtime) so the automatic domain transition can occur, or remove the
type_transition rule (or document that Podman must label the process with
--security-opt label=type:puzzlepod_agent_t) — update selinux/puzzlepod.fc to
include the appropriate gen_context mapping for puzzlepod_exec_t or drop the
transition rule in the policy where puzzlepod_exec_t and puzzlepod_agent_t are
declared.
Comment on lines
+98
to
101
| # Domain transition: puzzlepod_t -> puzzlepod_agent_t when spawning agent processes | ||
| type_transition puzzlepod_t puzzlepod_exec_t:process puzzlepod_agent_t; | ||
| allow puzzlepod_t puzzlepod_exec_t:file { read getattr execute }; | ||
| allow puzzlepod_t puzzlepod_exec_t:file entrypoint; |
There was a problem hiding this comment.
entrypoint permission is on wrong domain.
Line 101 grants entrypoint to puzzlepod_t, but this permission should be on the target domain (puzzlepod_agent_t) that is being transitioned into. The entrypoint permission controls which executables a domain can use as entry points when transitioning into that domain.
Proposed fix
# Domain transition: puzzlepod_t -> puzzlepod_agent_t when spawning agent processes
type_transition puzzlepod_t puzzlepod_exec_t:process puzzlepod_agent_t;
allow puzzlepod_t puzzlepod_exec_t:file { read getattr execute };
-allow puzzlepod_t puzzlepod_exec_t:file entrypoint;
+allow puzzlepod_agent_t puzzlepod_exec_t:file entrypoint;📝 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.
Suggested change
| # Domain transition: puzzlepod_t -> puzzlepod_agent_t when spawning agent processes | |
| type_transition puzzlepod_t puzzlepod_exec_t:process puzzlepod_agent_t; | |
| allow puzzlepod_t puzzlepod_exec_t:file { read getattr execute }; | |
| allow puzzlepod_t puzzlepod_exec_t:file entrypoint; | |
| # Domain transition: puzzlepod_t -> puzzlepod_agent_t when spawning agent processes | |
| type_transition puzzlepod_t puzzlepod_exec_t:process puzzlepod_agent_t; | |
| allow puzzlepod_t puzzlepod_exec_t:file { read getattr execute }; | |
| allow puzzlepod_agent_t puzzlepod_exec_t:file entrypoint; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@selinux/puzzlepod.te` around lines 98 - 101, The entrypoint permission is
incorrectly granted to the source domain; change the SELinux rule so the
entrypoint permission is granted to the target domain by replacing the current
allow that uses puzzlepod_t with one that uses puzzlepod_agent_t so it reads as
an allow for puzzlepod_agent_t against puzzlepod_exec_t:file entrypoint; keep
the type_transition and the read/getattr/execute allow for puzzlepod_t as-is.
maxamillion
added a commit
that referenced
this pull request
Mar 30, 2026
…repo (#5) * Add make rpm target and fix SRPM/RPM build pipeline Add rpm and srpm-all Makefile targets for local RPM builds via mock. Fix build-srpm.sh: correct puzzlepod-selinux spec filename, inject %global macros via sed (survives SRPM re-extraction), use tar instead of git archive for working-tree tarballs, preserve source tarball for CI artifact upload. Add git-core BuildRequires and git init in %prep for regorus build.rs compatibility in puzzlectl.spec and puzzled.spec. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Adam Miller <admiller@redhat.com> * Introduce puzzlepod_agent_t SELinux type for agent sandbox Split the single puzzlepod_t type into puzzlepod_t (daemon) and puzzlepod_agent_t (sandboxed agents). Resolves 10 allow/neverallow contradictions, fixes duplicate type declaration, replaces invalid corenet_tcp_connect_dbus_port interface, uses proper SELinux interfaces for cgroup and quota access, and adds require block for base policy types. Update all documentation and diagrams to reflect the new type. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Adam Miller <admiller@redhat.com> * Add CI RPM build validation and GitHub Pages RPM repository Add rpm-build matrix job to CI workflow validating builds against Fedora 42, Fedora 43, and CentOS Stream+EPEL 10 via mock. Add rpm-repo.yml workflow that builds RPMs on push to main and publishes a dnf repository to GitHub Pages with per-distro repos and source tarball. Upgrade CI container to Fedora 43 with mock and createrepo_c. Add reference .repo files for Fedora and EL. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Adam Miller <admiller@redhat.com> * Restore Fedora CI container for all jobs Switch ci and integration jobs back to using the pre-built Fedora CI container image instead of inline ubuntu apt-get installs. Use vars.CI_IMAGE with fedora42 fallback for all jobs so the image tag can be updated via repository variables without code changes. Signed-off-by: Adam Miller <admiller@redhat.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix machine-id test for container environments Skip test_machine_id_fallback_deterministic when /etc/machine-id is too short (< 32 chars), as commonly occurs in containers. The test already skipped when the file was missing but not when it had truncated content. Signed-off-by: Adam Miller <admiller@redhat.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update deps and ignore lru advisory (transitive via ratatui) Update unicode-segmentation to unyanked version. Add RUSTSEC-2026-0002 (lru unsoundness) to deny.toml ignore list — transitive dep of ratatui v0.29, fix requires ratatui >= 0.30 breaking update. No practical impact as lru is used internally for layout caching. Signed-off-by: Adam Miller <admiller@redhat.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address CodeRabbit review findings Fix Podman runtime configs to use puzzlepod_agent_t SELinux label (puzzle-podman, quadlet container). Revert daemon hardening description to puzzlepod_t. Add corecmd_exec_bin for agent binary execution. Add error handling on tarball copy, nullglob guard for SRPM globs, find-based tarball lookup, RPM existence validation, and --isolation=simple to Makefile mock invocation for CI consistency. Signed-off-by: Adam Miller <admiller@redhat.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Signed-off-by: Adam Miller <admiller@redhat.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Adam Miller <admiller@redhat.com>
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.
Summary
make rpmtarget: Builds all 7 RPM packages locally via mock with auto-detected Fedora version. Also addsmake srpm-allfor local SRPM builds.puzzlepod_agent_ttype to separate daemon and agent sandbox domains, resolving 10 allow/neverallow contradictions and multiple compilation errors on Fedora 43.Key changes
Build infrastructure
rpmandsrpm-allMakefile targetsbuild-srpm.sh: correct spec filename, inject%globalmacros via sed, use working-tree tarballs, preserve source tarball for CIgit-coreBuildRequires andgit initin%prepfor regorus build.rs compatibilitySELinux policy (
selinux/puzzlepod.te,.if)puzzlepod_tintopuzzlepod_t(daemon) +puzzlepod_agent_t(sandbox)corenet_tcp_connect_dbus_portinterfacerequireblock for base policy types used in neverallow rulespuzzlepod_agent_tonlyCI/CD (
.github/workflows/)ci.yml: Addrpm-buildmatrix job (3 targets)rpm-repo.yml: Three-stage pipeline (build-source → build-rpms matrix → publish to GitHub Pages).repofiles for Fedora and ELDocumentation (13 files)
puzzlepod_agent_tas the agent sandbox domainOne-time setup required
Enable GitHub Pages in repo Settings → Pages → Source: GitHub Actions
Test plan
make rpmbuilds all 7/7 packages successfully on Fedora 43checkmodulerpm-buildmatrix passes for all 3 targetsdnf install puzzlepodworks from the published repo🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores