security: verify self-update checksum and stop leaking GitHub token via argv - #37
Merged
Merged
Conversation
…ia argv - self_update now verifies the downloaded script against the sha256 digest GitHub's Releases API reports for the asset, before replacing the running (root-executed) script. A missing or mismatched digest aborts the update. - fetch_api_key passes GITHUB_TOKEN to curl via a stdin config file (curl -K -) instead of as a command-line argument, so it no longer appears in /proc/<pid>/cmdline or ps output for other local users. - Bump SCRIPT_VERSION to 0.1.10. - Update README security section and copilot-instructions.md line references to match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
locus313
added a commit
that referenced
this pull request
Aug 28, 2026
Two gaps found while reviewing CI coverage: - Self-update's sha256 digest verification (added in #37) had zero test coverage — the existing check only grepped for function names, not behavior. test_self_update_round_trip runs a full --self-update against the real latest GitHub release on a scratch copy of the script, so a broken digest extraction or verification would fail CI. - Integration tests checked authorized_keys/.ssh permission bits (700/600) but never verified chown ownership, even though get_user_gid/chown is its own code path that could silently leave files root-owned. test_file_ownership_correct checks both are owned by the target user. Verified locally end-to-end: 11/11 functional tests and 5/5 integration tests pass, including the new round-trip and ownership checks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
locus313
added a commit
that referenced
this pull request
Aug 28, 2026
* ci: extract test.yml assertions into standalone scripts test.yml previously embedded every test case as inline bash in the workflow YAML, so adding a test meant editing the workflow. Split the assertions into .github/scripts/functional-tests.sh and integration-tests.sh (using the same run_test-harness pattern as test.sh); the workflow now only provisions CI fixtures (packages, test users) and calls the scripts. New tests are added as a test_* function + run_test line in the appropriate script, no workflow changes needed. Behavior is unchanged — same test users, same fixtures, same pass/fail messages, verified by running both scripts locally end-to-end (including the symlink-attack integration tests) against a live GitHub API/keys endpoint. Also fixes a `set -o pipefail` bug introduced during extraction: piping the (intentionally failing) sync script into `grep -q` made pipefail report the script's exit code instead of grep's match result, flipping an assertion that checks for an expected error message. Updates AGENTS.md, .github/copilot-instructions.md, and TESTING.md references to point at the new script locations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: add self-update round-trip and file ownership coverage Two gaps found while reviewing CI coverage: - Self-update's sha256 digest verification (added in #37) had zero test coverage — the existing check only grepped for function names, not behavior. test_self_update_round_trip runs a full --self-update against the real latest GitHub release on a scratch copy of the script, so a broken digest extraction or verification would fail CI. - Integration tests checked authorized_keys/.ssh permission bits (700/600) but never verified chown ownership, even though get_user_gid/chown is its own code path that could silently leave files root-owned. test_file_ownership_correct checks both are owned by the target user. Verified locally end-to-end: 11/11 functional tests and 5/5 integration tests pass, including the new round-trip and ownership checks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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
Fixes two findings from a security review:
self_updatereplaced the running (root) script after only checking it was non-empty and started with#!/bin/bash— no integrity verification. Now verifies the downloaded script's sha256 against thedigestGitHub's Releases API reports for that asset before replacing the current script. Missing/mismatched digest aborts the update.fetch_api_keypassedGITHUB_TOKENtocurlas a literal argv arg, visible to other local users via/proc/<pid>/cmdline/ps. Now passed via acurl -K -stdin config instead.Also bumped
SCRIPT_VERSIONto0.1.10(required by CI) and updated README security notes + stale line-number references in.github/copilot-instructions.md.Testing
bash -n sync-ssh-keys.shbash test.sh— all 10 tests + shellcheck passget_latest_release_url/download_latest_scriptagainst the real GitHub API: correct digest downloads and passes, tampered/mismatched digest is rejectedfetch_api_keystill authenticates correctly via the new curl config approachCo-authored-by: Copilot 223556219+Copilot@users.noreply.github.com