feat: deployment attestation — GET /version, release manifest, verify script#54
Open
alokit-bot wants to merge 1 commit into
Open
feat: deployment attestation — GET /version, release manifest, verify script#54alokit-bot wants to merge 1 commit into
alokit-bot wants to merge 1 commit into
Conversation
… script
- GET /version endpoint: { version, commit, buildTime, repoUrl, attestationUrl }
COMMIT_SHA and BUILD_TIME injected at deploy time; null when not set (dev).
- GET /health now includes commit field alongside version.
- deploy.yml: after git pull, writes COMMIT_SHA (full SHA) and BUILD_TIME (ISO-8601
UTC) into .env; post-deploy smoke test confirms /version reflects the commit.
- .github/workflows/release-attestation.yml: triggers on v* tags; computes SHA-256
for src/index.js, src/db.js, src/providers.js, package.json, package-lock.json;
writes attestation.json (schema byok-relay-attestation/v1) and uploads it as a
GitHub Release asset via softprops/action-gh-release (SHA-pinned).
- scripts/verify-attestation.js: CLI tool — clone repo at commit, point at the
downloaded attestation.json, get PASS/FAIL per file + overall exit code.
- .env.example: documents COMMIT_SHA/BUILD_TIME as auto-injected (do not set manually).
- README Security section: 'Verifying the managed relay' subsection — 3-step curl
workflow any user can run independently of trusting the relay operator.
📝 WalkthroughWalkthroughThe PR adds attestation metadata to runtime responses, injects matching values during deploy, publishes a release attestation manifest on tagged releases, and adds a CLI plus README steps for verifying file hashes against that manifest. ChangesAttestation and verification flow
Sequence Diagram(s)sequenceDiagram
participant Deploy as GitHub Actions deploy workflow
participant SSH as remote SSH script
participant Relay as byok-relay
participant Version as GET /version
Deploy->>SSH: compute COMMIT and BUILD_TS
SSH->>Relay: write COMMIT_SHA and BUILD_TIME into .env
SSH->>Relay: restart byok-relay and run the health check
SSH->>Version: request the version payload
Version-->>SSH: return the commit field
SSH->>SSH: compare commit to COMMIT
sequenceDiagram
participant Release as release-attestation.yml
participant GitHub as GitHub Release
participant Verify as scripts/verify-attestation.js
participant Repo as checked-out repository files
Release->>GitHub: upload attestation.json
Verify->>GitHub: read attestation.json
Verify->>Repo: hash attested files
Repo-->>Verify: file contents
Verify-->>Verify: compare hashes and print PASS or FAIL
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 1
🧹 Nitpick comments (6)
.github/workflows/deploy.yml (2)
42-44: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCommit mismatch only logs a warning; the deploy still reports success.
Line 44 echoes
WARNING: /version commit mismatchbut returns success, so a stale or wrong build silently passes deployment. If the attestation contract matters, a mismatch should fail the step.♻️ Fail on mismatch
- [ "$VERSION_COMMIT" = "$COMMIT" ] && echo "Attestation OK: commit ${COMMIT}" || echo "WARNING: /version commit mismatch" + if [ "$VERSION_COMMIT" = "$COMMIT" ]; then + echo "Attestation OK: commit ${COMMIT}" + else + echo "ERROR: /version commit ${VERSION_COMMIT} != deployed ${COMMIT}" && exit 1 + fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/deploy.yml around lines 42 - 44, The deploy attestation check in the workflow currently only prints a warning when /version returns a different commit, so the step still succeeds. Update the VERSION_COMMIT validation in the deploy job to fail the step on mismatch instead of continuing, using the same commit comparison block that echoes the attestation message.
43-43: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winHarden the
/versionprobe against hangs and empty responses.
curl -shas no--failor--max-time, so a hung relay can stall the deploy job, and on a non-JSON/empty body thenodeparse throws — underset -ethat aborts the assignment without a clear message. Add a timeout and a small parse guard.♻️ Add timeout and guard
- VERSION_COMMIT=$(curl -s http://localhost:3000/version | node -e "const d=require('fs').readFileSync('/dev/stdin','utf8'); console.log(JSON.parse(d).commit)") + VERSION_COMMIT=$(curl -sf --max-time 5 http://localhost:3000/version | node -e "try{const d=require('fs').readFileSync('/dev/stdin','utf8');console.log((JSON.parse(d).commit)||'')}catch(e){console.log('')}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/deploy.yml at line 43, The VERSION_COMMIT probe in the deploy workflow can hang or fail अस्पष्टly because the `curl` call to `/version` has no timeout/fail handling and the `node` JSON parse assumes a valid response. Update that probe to use a bounded request and explicit failure behavior, and add a small guard in the parsing step so empty or non-JSON responses produce a clear error instead of aborting silently under `set -e`..github/workflows/release-attestation.yml (2)
26-27: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueSet
persist-credentials: falseon checkout.This job only reads source to hash it; leaving the default credential persistence stores the
GITHUB_TOKENin.git/configon the runner, widening the exfiltration surface. zizmor (artipacked) flags this.🔒 Proposed change
- name: Checkout tagged commit uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release-attestation.yml around lines 26 - 27, Update the Checkout tagged commit step in the release attestation workflow to disable credential persistence by setting persist-credentials to false on the actions/checkout usage. This workflow only needs read-only source access for hashing, so adjust the checkout configuration directly in the checkout step to avoid storing the GITHUB_TOKEN in the runner’s git config.Source: Linters/SAST tools
36-58: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd
set -euo pipefailso a missing/unhashable file fails the build.The step has no error-exit guard. If any attested file is missing or
sha256sumfails,HASHbecomes empty and the loop silently emits an entry like"src/db.js":"", producing a malformed attestation that ships to the release.♻️ Proposed guard
run: | + set -euo pipefail COMMIT=$(git rev-parse HEAD) TAG=${GITHUB_REF_NAME} BUILD_TIME=$(date -u +%Y-%m-%dT%H:%M:%SZ)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release-attestation.yml around lines 36 - 58, The attestation build step in the release workflow should fail immediately on missing or unhashable files. Update the shell block that computes COMMIT, TAG, BUILD_TIME, and the ATTESTED_FILES hash loop to enable strict shell handling with set -euo pipefail so any sha256sum or file access error stops the job instead of producing empty hash entries in HASH_JSON.scripts/verify-attestation.js (2)
59-86: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winVerification never confirms the checked-out commit matches the manifest.
The script prints
manifest.commitbut never compares it against the repo's actualHEAD. A user who skips or mistypes thegit checkout <commit>step (README Line 266) can still get an all-PASS result against a different tree, since the hashes are computed against whatever is currently checked out. Assertinggit rev-parse HEAD === manifest.commitcloses the gap between "/version commit" and "hashed files".🔒 Suggested check before hashing
const repoRoot = path.resolve(__dirname, '..'); let allPassed = true; + +try { + const head = require('child_process') + .execSync('git rev-parse HEAD', { cwd: repoRoot }) + .toString().trim(); + if (manifest.commit && head !== manifest.commit) { + console.error(`Error: checked-out commit ${head} does not match manifest commit ${manifest.commit}.`); + console.error('Run: git checkout ' + manifest.commit); + process.exit(1); + } +} catch (_) { + console.warn('Warning: could not determine git HEAD; skipping commit match check.'); +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/verify-attestation.js` around lines 59 - 86, The attestation verifier in verify-attestation.js prints manifest.commit but never checks it against the checked-out repository state. Add a HEAD validation near the existing manifest logging and before the attestedFiles loop so the script compares the current git HEAD to manifest.commit and fails early if they differ. Use the existing repoRoot setup and keep the hash checks in the for...of over manifest.attestedFiles unchanged once the commit match is confirmed.
54-57: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueGuard against a missing
attestedFilesmap.A manifest that passes the schema check but lacks
attestedFilesmakesObject.entries(manifest.attestedFiles)(Line 70) throw a rawTypeErrorinstead of a clear diagnostic.🛡️ Optional guard
if (manifest.schema !== 'byok-relay-attestation/v1') { console.error(`Error: unrecognised schema "${manifest.schema}" — expected "byok-relay-attestation/v1"`); process.exit(2); } +if (!manifest.attestedFiles || typeof manifest.attestedFiles !== 'object') { + console.error('Error: manifest is missing a valid "attestedFiles" map.'); + process.exit(2); +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/verify-attestation.js` around lines 54 - 57, The manifest validation in verify-attestation.js only checks schema, so a manifest with no attestedFiles can still reach Object.entries(manifest.attestedFiles) and throw a TypeError. Add an explicit guard in the manifest validation flow, near the existing schema check and before the attestedFiles iteration, to verify manifest.attestedFiles exists and is a map/object; if it is missing or invalid, print a clear error via console.error and exit with a non-zero status instead of allowing the raw exception.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/index.js`:
- Around line 144-153: The /version response in app.get currently builds
attestationUrl from PKG_VERSION, which can point to a release tag that does not
match the deployed COMMIT_SHA. Update the version payload logic so
attestationUrl is derived from the actual deployed commit (for example using
COMMIT_SHA) or is only set when the deployed commit matches the tagged release
commit. Keep the change localized to the /version handler and preserve the
existing fields version, commit, buildTime, and repoUrl.
---
Nitpick comments:
In @.github/workflows/deploy.yml:
- Around line 42-44: The deploy attestation check in the workflow currently only
prints a warning when /version returns a different commit, so the step still
succeeds. Update the VERSION_COMMIT validation in the deploy job to fail the
step on mismatch instead of continuing, using the same commit comparison block
that echoes the attestation message.
- Line 43: The VERSION_COMMIT probe in the deploy workflow can hang or fail
अस्पष्टly because the `curl` call to `/version` has no timeout/fail handling and
the `node` JSON parse assumes a valid response. Update that probe to use a
bounded request and explicit failure behavior, and add a small guard in the
parsing step so empty or non-JSON responses produce a clear error instead of
aborting silently under `set -e`.
In @.github/workflows/release-attestation.yml:
- Around line 26-27: Update the Checkout tagged commit step in the release
attestation workflow to disable credential persistence by setting
persist-credentials to false on the actions/checkout usage. This workflow only
needs read-only source access for hashing, so adjust the checkout configuration
directly in the checkout step to avoid storing the GITHUB_TOKEN in the runner’s
git config.
- Around line 36-58: The attestation build step in the release workflow should
fail immediately on missing or unhashable files. Update the shell block that
computes COMMIT, TAG, BUILD_TIME, and the ATTESTED_FILES hash loop to enable
strict shell handling with set -euo pipefail so any sha256sum or file access
error stops the job instead of producing empty hash entries in HASH_JSON.
In `@scripts/verify-attestation.js`:
- Around line 59-86: The attestation verifier in verify-attestation.js prints
manifest.commit but never checks it against the checked-out repository state.
Add a HEAD validation near the existing manifest logging and before the
attestedFiles loop so the script compares the current git HEAD to
manifest.commit and fails early if they differ. Use the existing repoRoot setup
and keep the hash checks in the for...of over manifest.attestedFiles unchanged
once the commit match is confirmed.
- Around line 54-57: The manifest validation in verify-attestation.js only
checks schema, so a manifest with no attestedFiles can still reach
Object.entries(manifest.attestedFiles) and throw a TypeError. Add an explicit
guard in the manifest validation flow, near the existing schema check and before
the attestedFiles iteration, to verify manifest.attestedFiles exists and is a
map/object; if it is missing or invalid, print a clear error via console.error
and exit with a non-zero status instead of allowing the raw exception.
🪄 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: c01b20b8-2cc2-4e57-899b-969dcc6d85b9
📒 Files selected for processing (6)
.env.example.github/workflows/deploy.yml.github/workflows/release-attestation.ymlREADME.mdscripts/verify-attestation.jssrc/index.js
Comment on lines
+144
to
+153
| app.get('/version', (req, res) => { | ||
| res.json({ | ||
| version: PKG_VERSION, | ||
| commit: COMMIT_SHA, | ||
| buildTime: BUILD_TIME, | ||
| repoUrl: REPO_URL, | ||
| attestationUrl: COMMIT_SHA | ||
| ? `${REPO_URL}/releases/tag/v${PKG_VERSION}` | ||
| : null, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
attestationUrl points at a version tag that may not correspond to the deployed commit.
The deploy workflow injects COMMIT_SHA from git rev-parse HEAD on main, but attestationUrl is derived from v${PKG_VERSION}. The release-attestation manifest (attestation.json) is generated per v* tag and pins commit to the tagged commit. If main HEAD is ahead of (or otherwise differs from) the last tagged release, /version returns a commit that won't match the manifest at releases/tag/v${PKG_VERSION}, and the attested file hashes may differ — breaking the documented verification flow.
Consider linking by the actual deployed commit (e.g. ${REPO_URL}/tree/${COMMIT_SHA}) or only emitting attestationUrl when the deployed commit equals a tagged release commit.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/index.js` around lines 144 - 153, The /version response in app.get
currently builds attestationUrl from PKG_VERSION, which can point to a release
tag that does not match the deployed COMMIT_SHA. Update the version payload
logic so attestationUrl is derived from the actual deployed commit (for example
using COMMIT_SHA) or is only set when the deployed commit matches the tagged
release commit. Keep the change localized to the /version handler and preserve
the existing fields version, commit, buildTime, and repoUrl.
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.
What
Users of
relay.byokrelay.comcurrently cannot verify that the managed relay runs the public repo code. This PR adds a transparent, independently verifiable attestation chain.Changes
GET /versionendpoint (new)Returns
{ version, commit, buildTime, repoUrl, attestationUrl }.COMMIT_SHAandBUILD_TIMEare injected at deploy time (null in dev).GET /healthnow also includescommit.deploy.yml(updated)After
git pull, writesCOMMIT_SHA(full SHA fromgit rev-parse HEAD) andBUILD_TIME(ISO-8601 UTC) into.env. Post-deploy smoke test now verifies that/versionreflects the deployed commit..github/workflows/release-attestation.yml(new)Triggers on
v*tags. Computes SHA-256 hashes for the five files that cover all relay behaviour:src/index.js,src/db.js,src/providers.jspackage.json,package-lock.jsonWrites
attestation.json(schemabyok-relay-attestation/v1) and uploads it as a GitHub Release asset viasoftprops/action-gh-release(SHA-pinned). No external trust required — the manifest is generated by GitHub Actions from the tagged commit.scripts/verify-attestation.js(new)CLI verifier. Clone the repo at the reported commit, point at the downloaded
attestation.json, get PASS/FAIL per file + exit code 0/1.README.md— Security sectionNew "Verifying the managed relay" subsection with the 3-step
curlworkflow:GET /version→ note commit + versionattestation.jsonfrom the matching GitHub Releasenode scripts/verify-attestation.js.env.exampleDocuments
COMMIT_SHA/BUILD_TIMEas auto-injected (do not set manually).Verification
Related BACKLOG item: Deployment attestation (Compliance/Trust Documentation section).
Summary by CodeRabbit
New Features
Documentation