Skip to content

feat(ci): real sealed-box action secrets + fix double-encryption#378

Merged
zoetaka38 merged 1 commit into
mainfrom
feat/ci-real-secrets
Jul 7, 2026
Merged

feat(ci): real sealed-box action secrets + fix double-encryption#378
zoetaka38 merged 1 commit into
mainfrom
feat/ci-real-secrets

Conversation

@zoetaka38

Copy link
Copy Markdown
Contributor

Final PR completing the Actions executor: make GitHub-compatible action secrets actually work.

The problem

The secrets API was non-functional end-to-end:

  • It advertised a placeholder all-zeros public key — libsodium refuses to seal against it, so no real client could store a secret.
  • DecryptSealedBox "decrypted" the client's sealed box with the symmetric key, not the sealed-box private key.

Enabling it also surfaced a latent double-encryption bug that corrupted every stored secret.

Fixes

  • Derive a real curve25519 keypair deterministically from the server secret (shared across replicas, stable across restarts) and advertise the true public key. DecryptSealedBox now opens the client's anonymous ("sealed") box — the libsodium crypto_box_seal format GitHub uses.
  • Double-encryption: the upsert usecase encrypted the value and the repository encrypted it again, while reads decrypt only once — so the CI worker's single decrypt yielded ciphertext, and steps failed with a corrupt env var. The repository is now the single at-rest encryption boundary; the usecase passes plaintext through.

Verified live end-to-end

GET public-key           -> 3foa/oZGMiVr...      (real key, not all-zeros)
seal with pynacl + PUT   -> 201
workflow: echo "env=$API_TOKEN expr=${{ secrets.API_TOKEN }} len=${#API_TOKEN}"
  -> env=*** expr=*** len=11      (len(topsecret42)=11: correct plaintext, and masked)
run -> success

Unit tests: sealed-box round trip, public key is real + deterministic, usecase hands plaintext to the repo.

This closes the last known gap in the Actions executor.

@orange-codens

orange-codens Bot commented Jul 7, 2026

Copy link
Copy Markdown

🍊 Orange Codens レビュー

密封ボックス化 (sealed box) の実装と double-encryption 修正の方向性は正しいです。テストも round-trip / 決定性 / 不正入力拒否を網羅しており、PR 本文の claims を裏付ける内容になっています。

修正推奨: sealedBoxKeypair 失敗時の silent propagation を避けるため、エラーを伝播させるか、少なくとも PublicKeyBase64 でゼロ鍵を検出して sentinel error を返すこと。 X25519 の失敗自体は稀ですが、dev 環境の e.key が nil / 短いのケースや、将来のアルゴリズム入れ替え時に silent failure だと原因特定に時間がかかります。

参考: OpenSealedBox のエラーメッセージを失敗原因別に分けると、本番デバッグが楽になります (low priority)。

全体としてマージに問題ない水準ですが、silent error 部分だけを修正すると本番でのトラブル時に気づきにくい事故を防げます。

🔒 セキュリティ: 本PRは sealed-box の動作不全 (placeholder キー / 暗号化レイヤーの二重化) を修正する実装改善であり、diff に暗号誤用・認可漏れ・入力量の脆弱性パターンは見られない。nacl/box による sealed box 形式・決定論的鍵導出 (blake2b + curve25519) は正しく実装されており、テストも round-trip / ガベージ入力の拒否を含んでいる。修正として安全と判断し、findings は出さない。

🟡 medium: 1 / 🔵 low: 2

その他の指摘 (3 件)
  • 🟡 [medium] backend/internal/infrastructure/crypto/action_secret_sealedbox.go:21 — 鍵導出失敗時にゼロ鍵が静かに伝播し、エラーメッセージが誤解を招く (confidence: 0.75, crypto.key_derivation.silent_error)
  • 🔵 [low] backend/internal/infrastructure/crypto/action_secret_sealedbox.go:43OpenSealedBox のエラーメッセージが失敗原因を区別していない (confidence: 0.85, crypto.sealedbox.error_message.imprecise)
  • 🔵 [low] backend/internal/infrastructure/crypto/action_secret_sealedbox_test.go:55TestOpenSealedBoxRejectsGarbage が極端に短い入力のみの不正入力テスト (confidence: 0.60, crypto.test.input_coverage.short_input)

head: be00a7a | 🍊 Reviewed by Orange Codens — AI code review & security audit

The GitHub-compatible secrets API was non-functional: it advertised a
placeholder all-zeros public key (which libsodium refuses to seal against) and
"decrypted" the client's sealed box with the symmetric key, so no real client
could ever store a secret. Enabling it also surfaced a latent double-encryption
bug that corrupted every stored secret.

- Derive a real curve25519 keypair deterministically from the server secret
  (shared across replicas, stable across restarts) and advertise the true
  public key. DecryptSealedBox now opens the client's anonymous ("sealed") box
  with the private key — the libsodium crypto_box_seal format GitHub uses.
- Fix double-encryption: the upsert usecase encrypted the value AND the
  repository encrypted it again, while reads decrypt only once — so the CI
  worker's single decrypt yielded ciphertext, not the secret (steps then failed
  with a corrupt env var). The repository is now the single at-rest encryption
  boundary; the usecase passes the plaintext through.

Verified live end-to-end: fetch the real public key, seal a value with
libsodium (pynacl), PUT it (201), then a workflow reads it via `$API_TOKEN`
and `${{ secrets.API_TOKEN }}` — both resolve to the correct plaintext
(shell sees the right length) and are masked to *** in the logs.

Unit tests cover the sealed-box round trip, that the public key is real and
deterministic, and that the usecase hands plaintext to the repository.
@zoetaka38
zoetaka38 force-pushed the feat/ci-real-secrets branch from 4e216e7 to be00a7a Compare July 7, 2026 03:51
@zoetaka38
zoetaka38 merged commit 36020eb into main Jul 7, 2026
20 checks passed
@zoetaka38
zoetaka38 deleted the feat/ci-real-secrets branch July 7, 2026 03:56
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