Skip to content

Hardening pass: CI publish gating, entrypoint injection fix, capability cleanup - #17

Merged
libre-7 merged 6 commits into
mainfrom
fix/security-review
Aug 24, 2026
Merged

Hardening pass: CI publish gating, entrypoint injection fix, capability cleanup#17
libre-7 merged 6 commits into
mainfrom
fix/security-review

Conversation

@libre-7

@libre-7 libre-7 commented Aug 21, 2026

Copy link
Copy Markdown
Owner

Security & correctness hardening (cumulative)

Follow-up to the full-repo code/security review. One branch, five commits, grouped by area.

🔴 Critical

CI published images on pull_request events.github/workflows/docker-publish.yml

  • build-push-action ran with push: true on every PR, so any internal branch PR published sha-<pr-sha> images to GHCR (packages:write) and re-pushed them to Docker Hub via the stored secrets — without merging anything. Fork PRs were only saved by GitHub's default token restriction.
  • Now: every publish step is gated on github.event_name != 'pull_request'; PRs get build-only validation. The latest tag is additionally gated on an actual push to main.

🟡 Warnings

  • Shell-string injection in entrypointsimplex-chat args were assembled into a string with embedded quotes and run through sh -c, re-parsing env values (SIMPLEX_DISPLAY_NAME). Replaced with a bash array; values are passed verbatim now.
  • Unneeded capabilities — compose granted NET_BIND_SERVICE + NET_RAW on top of cap_drop: ALL; neither is needed (binds :5225, no raw sockets). Dropped; added no-new-privileges:true.
  • Setup marker defeated retry.setup-complete was written even when first-run setup failed, so one flaky first boot meant bot_address.txt was never created. Marker is now written only when the address was actually captured. This also fixed a latent bug in the replacement logic: the old … | sed pipe masked python's exit status (sed always exits 0), so the exit code is now captured directly and output echoed from setup.log.
  • Broken numeric comparison in installer$(grep -c … || echo 0) yields two lines ("0" twice) because grep prints 0 and exits nonzero; [ "$FIXED" -ge 2 ] threw "integer expression expected". Whitespace-stripped and the redundant fallback removed.
  • Floating Python depwebsockets was unpinned at image build despite a digest-pinned base image and SHA256-verified binaries. Pinned to 17.0.1.
  • Actions on mutable tags — all six actions pinned to full commit SHAs (Dependabot continues to manage updates).

💡 Suggestions also included

  • SIMPLEX_SOCAT_PORT validated as numeric/1–65535 before socat launch
  • First-run setup runs as the daemon user, so bot_address.txt gets correct ownership
  • Dropped unused nc wait-fallback and unused sudo package (curl kept — used by build)
  • Example compose pinned to immutable v0.16.0 per README guidance
  • Workflow concurrency group to cancel superseded runs

Verification

  • bash -n + shellcheck clean on both scripts (matches CI lint job)
  • YAML parse verified for workflow and compose
  • Marker logic exercised through a fail→retry→success→skip harness: marker absent after failure, retry preserved on next boot, written only after success
  • Installer comparison verified against grep's print-and-exit-nonzero behavior

Not addressed (deliberate)

  • daemon.log rotation, EXPOSE semantics, and the Unraid template port row are cosmetic/doc-level items — happy to fold them in if wanted.

libre-7 and others added 6 commits August 21, 2026 15:40
- build-push no longer publishes on pull_request events (internal PRs
  previously pushed sha-* tags to GHCR and Docker Hub)
- 'latest' tag now gated on actual main-branch push, not PR metadata
- all six actions pinned to full commit SHAs (Dependabot still updates)
- add concurrency group to cancel superseded runs
- websockets==17.0.1 (was floating; healthcheck + first-run setup depend on it)
- remove sudo (unused at runtime); curl retained for binary downloads
- use --no-cache-dir for pip
- build simplex-chat args as a bash array instead of an eval'd command
  string — env values (SIMPLEX_DISPLAY_NAME) are no longer re-parsed by
  a shell, closing a quoting/injection vector and fixing names with quotes
- first-run setup: write .setup-complete ONLY when the bot address was
  actually captured; a failed boot now retries on next start instead of
  being skipped forever (previously the sed pipe masked python's exit code)
- setup runs as the daemon user via gosu, so bot_address.txt gets correct
  ownership; setup.log kept under /data for debugging
- validate SIMPLEX_SOCAT_PORT is numeric/1-65535 before socat launch
- drop nc fallback (netcat not installed); quote daemon PID paths
grep -c prints 0 AND exits nonzero on no match, so '$(grep -c ... || echo 0)'
produced two lines ('0\n0') and '[ $FIXED -ge 2 ]' threw 'integer expression
expected'. Strip whitespace and drop the redundant fallback.
- NET_BIND_SERVICE and NET_RAW were unnecessary (daemon binds :5225,
  no raw sockets) — cap_drop ALL now stands alone
- security_opt: no-new-privileges:true
- pin example compose to immutable v0.16.0 per README guidance
- tmpfs /tmp for future read-only hardening
… best practice

Replaces the mutable v0.16.0 tag with an immutable digest pin, matching
the digest-pinning guidance documented in the repo README.
@libre-7

libre-7 commented Aug 24, 2026

Copy link
Copy Markdown
Owner Author

Amendment pushed: the docker-compose.yml example image pin is now an immutable digest (sha256:04cdd16a..., = v0.16.0) instead of the mutable v0.16.0 tag, matching this repo's own digest-pinning best practice.

@libre-7
libre-7 merged commit 02a9b86 into main Aug 24, 2026
2 checks passed
@libre-7
libre-7 deleted the fix/security-review branch August 24, 2026 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant