Skip to content

bazel/pgp: fix review findings (sandboxing, passphrase handling, audit, single-src signing) - #1127

Draft
phlax with Copilot wants to merge 3 commits into
bazel-gpgfrom
copilot/bazel-gpg
Draft

bazel/pgp: fix review findings (sandboxing, passphrase handling, audit, single-src signing)#1127
phlax with Copilot wants to merge 3 commits into
bazel-gpgfrom
copilot/bazel-gpg

Conversation

Copilot AI commented Sep 6, 2026

Copy link
Copy Markdown

Review of the hermetic OpenPGP signing rules in bazel/pgp/ surfaced several gaps between the stated security model and the implementation: signing actions ran unsandboxed, the passphrase was copied to a second plaintext file, the audit script's "suspicious input" check was dead code against real aquery output, and several rules/tests relied on genrules or a multi-file srcs contract that didn't match actual usage.

Sandboxing & secrets

  • Removed local (no-sandbox) from EXECUTION_REQUIREMENTS in sign.bzl, sign_test.bzl, audit_test.sh, and fixtures — signing actions now stay sandboxed as well as local/uncached.
  • signer.sh no longer copies the passphrase to a mktemp file; it now passes it to sq --password-file via process substitution (<(printf %s "$(cat "$PASSPHRASE_FILE")")), so it's never written to disk a second time. Removed the now-unneeded trap/umask scaffolding.
  • Signing/checksums actions now set an explicit, minimal env = {"PATH": "/usr/bin:/bin"} instead of relying on env = {} and libc's compiled-in default path.

Audit script

  • Check build(deps): bump actions/upload-artifact from 2 to 3 #2 (suspicious key-material inputs) previously read a nonexistent .artifacts[].execPath field, only "passing" because a fixture hand-inserted it. Rewrote it in jq to reconstruct real paths from pathFragments/parentId, scoped to inputs reachable from OpenPGPSign actions via inputDepSetIds/depSetOfFiles.
  • Added //pgp/test:live_audit (bazel run //pgp/test:live_audit), which re-runs the audit against a real bazel aquery of the example targets instead of captured JSON, wired into CI as a new matrix entry.
  • Cut the 5 committed ~1000-line JSON fixtures down to one real fixtures/audit.json; the 4 broken variants (dropped execution requirement, leaked HOME, key-material input, passphrase on argv) are now derived from it with jq at test time.

Rule/test cleanup

  • pgp_sign takes a single src = attr.label(allow_single_file = True, mandatory = True) instead of srcs + a runtime "exactly one" check.
  • Replaced the example_key/example_data/example_changes genrules with static fixture files, and the extensions_load_check genrule/.bzl with a bzl_library covering the public/private starlark surface.
  • Renamed deb_sign_changespgp_sign_changes_file with a doc/TODO clarifying it only clearsigns the .changes/.dsc itself, not a full debsign (referenced .dsc/.buildinfo re-signing is not implemented).

MODULE.bazel.lock was left untouched — this working copy doesn't have the pre-PR parent commit to diff against, and no unrelated churn was introduced here.

phlax and others added 2 commits September 6, 2026 12:56
Signed-off-by: Ryan Northey <ryan@synca.io>
Co-authored-by: phlax <454682+phlax@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix review findings in bazel/pgp signing rules bazel/pgp: fix review findings (sandboxing, passphrase handling, audit, single-src signing) Sep 6, 2026
Copilot AI requested a review from phlax September 6, 2026 12:21
@phlax
phlax force-pushed the bazel-gpg branch 21 times, most recently from 7863422 to 99a5784 Compare September 7, 2026 09:03
@phlax
phlax force-pushed the bazel-gpg branch 20 times, most recently from 1b01322 to 2f2dfe5 Compare September 8, 2026 19:34
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.

2 participants