feat: release config signing + moka cache - #370
Open
yuvrajjsingh0 wants to merge 4 commits into
Open
yuvrajjsingh0 wants to merge 4 commits into
yuvrajjsingh0 wants to merge 4 commits into
Conversation
yuvrajjsingh0
force-pushed
the
rc-signing
branch
from
July 16, 2026 15:11
492afe1 to
4d5dc25
Compare
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds end-to-end ES256 release-config signing, signing-key lifecycle APIs and persistence, cache invalidation, dashboard management screens, authorization checks, and documentation. ChangesSigning contracts, persistence, and runtime state
Signing-key operations and provisioning
Release response signing and invalidation
Dashboard integrity settings
Documentation and generated API descriptions
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
yuvrajjsingh0
force-pushed
the
rc-signing
branch
from
July 17, 2026 06:46
fe7c04d to
e87ab88
Compare
Contributor
Author
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
Contributor
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
smithy/models/release.smithy (1)
358-364: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRemove
@requiresauthfrom these public release operations.Both operations are documented and routed as unauthenticated SDK endpoints, but the Smithy trait still declares authentication mandatory. Generated clients and OpenAPI consumers may consequently require credentials for public boot requests.
Proposed contract fix
`@http`(method: "GET", uri: "/release/{organisation}/{application}") `@readonly` -@requiresauth operation ServeRelease { @@ `@http`(method: "GET", uri: "/release/v2/{organisation}/{application}") `@readonly` -@requiresauth operation ServeReleaseV2 {Also applies to: 375-381
🤖 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 `@smithy/models/release.smithy` around lines 358 - 364, Remove the `@requiresauth` trait from both public release operations: the operation shown in the diff and the corresponding operation near the second referenced location. Preserve their existing HTTP routes and documentation so generated clients treat these SDK boot endpoints as unauthenticated.airborne_server/src/main.rs (1)
185-195: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRun database migrations before the
signingkeysbackfill.
signingkeystriggers the backfill but does not setshould_run_db_migrations. WithMIGRATIONS_TO_RUN_ON_BOOT=signingkeys, startup skips the table migration, the backfill queries a missingsigning_keystable, logs the failure, and continues without provisioning keys.Proposed fix
let should_run_db_migrations = migrations_to_run_on_boot.iter().any(|m| m == "db"); +let should_run_signing_key_backfill = + migrations_to_run_on_boot.iter().any(|m| m == "signingkeys"); -if should_run_db_migrations || should_run_keycloak_to_casbin { +if should_run_db_migrations + || should_run_keycloak_to_casbin + || should_run_signing_key_backfill +{ @@ -if migrations_to_run_on_boot.iter().any(|m| m == "signingkeys") { +if should_run_signing_key_backfill {Also applies to: 499-513
🤖 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 `@airborne_server/src/main.rs` around lines 185 - 195, Update the startup migration selection in main so the database migration block also runs when migrations_to_run_on_boot contains "signingkeys", ensuring the signing_keys table exists before the signingkeys backfill executes. Preserve the existing behavior for "db" and "keycloaktocasbin" entries and keep the backfill flow unchanged.
🤖 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 `@airborne_dashboard/components/settings/integrity/signing-keys-card.tsx`:
- Around line 48-53: Update the useSWR state in the signing-keys card to capture
error alongside data, isLoading, and mutate, then render a dedicated error state
with a retry action using mutate when the request fails. Ensure the “No signing
keys” empty state is shown only after a successful response with no keys, not
when data is undefined because of an error.
In `@airborne_docs/docs/dashboard/integrity.mdx`:
- Around line 39-43: Update the key ID validation list in the integrity
documentation to state that IDs must be no longer than 50 characters, alongside
the existing character, dash, and boundary constraints.
In `@airborne_docs/openapi/airborne.openapi.json`:
- Line 2120: Update the signing-key list description associated with the
relevant OpenAPI endpoint to remove the guarantee that the list is never empty.
State that automatic default-key provisioning applies to newly created or
backfilled applications, while preserving the existing details about ordering,
public-key-only responses, headers, and authentication.
- Line 2865: Add operation-level security overrides with an empty array to both
ServeReleaseV2 at airborne_docs/openapi/airborne.openapi.json:2865-2865 and
ServeRelease at airborne_docs/openapi/airborne.openapi.json:2989-2989, so these
public operations do not inherit the document-level bearer authentication
requirement.
- Around line 2891-2897: Update the signing-key parameter schemas for
ServeReleaseV2 at airborne_docs/openapi/airborne.openapi.json:2891-2897 and
ServeRelease at airborne_docs/openapi/airborne.openapi.json:3015-3021 to accept
either an empty string or a valid non-empty key ID. Replace the conflicting
minLength/pattern constraints with a schema expression that preserves the
existing key-ID validation while allowing the documented empty-value default
behavior.
In `@airborne_server/src/config.rs`:
- Around line 173-175: Update the master_encryption_key handling near its
initialization and the encrypt_private_key provisioning flow so signing
private-key encryption remains mandatory regardless of USE_ENCRYPTED_SECRETS.
Require a dedicated encryption key for signing keys, or reject provisioning when
none is configured; never allow encrypt_private_key to persist plaintext PEM
data.
In `@airborne_server/src/release.rs`:
- Around line 1465-1469: Update the signing-key ID extraction in the request
handling flow to distinguish a missing header from a present header containing
invalid UTF-8. Reject the request when x-signing-key-id cannot be converted to
text, rather than passing None to signing::utils::requested_key_id and selecting
the default key; preserve existing behavior for valid and absent headers.
In `@airborne_server/src/release/utils.rs`:
- Around line 496-509: Update invalidate_release_cache to invalidate both the
existing /release/{organisation}/{application}* path and the v2 endpoint path
/release/v2/{organisation}/{application}. Submit both paths together through
invalidate_cf as a single invalidation batch, preserving the current best-effort
error logging.
In `@airborne_server/src/signing/utils.rs`:
- Around line 323-336: Update signature_cache_key and the surrounding signing
flow to resolve the authoritative active signing key before reading the cache,
then include the resolved key identity and a digest of body in the cache key
alongside the existing release context. Ensure cache hits cannot bypass key
resolution or return signatures for disabled/replaced keys, and update all
affected call sites in the signing and invalidation paths. Add regression
coverage for body mutation and concurrent mutation/invalidation so stale
in-flight signatures are not served.
- Around line 153-168: Require a configured master encryption key in
encrypt_private_key and fail with the existing ABError mechanism when it is
absent, rather than returning the plaintext key. Update the corresponding
decrypt_private_key behavior as needed to preserve encrypted-key handling, and
ensure provisioning propagates the failure instead of persisting or exposing the
raw private key.
In `@airborne_server/src/utils/db/models.rs`:
- Line 250: Remove the Debug derive from both private-key-bearing model structs
near the Queryable/Selectable declarations, including the model containing
private_key_encrypted, so debug formatting cannot expose encrypted private keys;
preserve the other derives unchanged.
In `@airborne_server/src/utils/redis.rs`:
- Around line 200-253: Make index_add and index_drop atomic by replacing their
multi-command Redis flows with Lua scripts executed through the existing Redis
connection. The index_add script must perform SADD and EXPIRE together, while
the index_drop script must read the set members, delete those members, and
delete the index within one atomic operation. Preserve the current error metrics
and ABError handling around the script executions.
In `@smithy/models/signing.smithy`:
- Around line 32-34: Update the signing model documentation around the
default-key description and the list guarantees at the referenced symbols to
state that an application may temporarily have no keys and therefore no default
key when provisioning fails; remove any claim that the key list is never empty
or that exactly one default always exists, while preserving the behavior for
applications with a configured default.
---
Outside diff comments:
In `@airborne_server/src/main.rs`:
- Around line 185-195: Update the startup migration selection in main so the
database migration block also runs when migrations_to_run_on_boot contains
"signingkeys", ensuring the signing_keys table exists before the signingkeys
backfill executes. Preserve the existing behavior for "db" and
"keycloaktocasbin" entries and keep the backfill flow unchanged.
In `@smithy/models/release.smithy`:
- Around line 358-364: Remove the `@requiresauth` trait from both public release
operations: the operation shown in the diff and the corresponding operation near
the second referenced location. Preserve their existing HTTP routes and
documentation so generated clients treat these SDK boot endpoints as
unauthenticated.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f601f139-b710-49ec-8e89-00e3e44b228a
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockairborne_docs/static/docs_static/img/screenshots/dark/integrity-keys.pngis excluded by!**/*.pngairborne_docs/static/docs_static/img/screenshots/light/integrity-keys.pngis excluded by!**/*.png
📒 Files selected for processing (42)
airborne_dashboard/app/dashboard/[orgId]/[appId]/settings/integrity/page.tsxairborne_dashboard/app/dashboard/[orgId]/[appId]/settings/layout.tsxairborne_dashboard/app/dashboard/[orgId]/[appId]/settings/page.tsxairborne_dashboard/components/settings/integrity/create-key-dialog.tsxairborne_dashboard/components/settings/integrity/key-actions.tsxairborne_dashboard/components/settings/integrity/public-key-dialog.tsxairborne_dashboard/components/settings/integrity/signing-keys-card.tsxairborne_dashboard/components/settings/settings-tabs.tsxairborne_dashboard/components/shared-layout.tsxairborne_dashboard/lib/name-validation.tsairborne_dashboard/next.config.mjsairborne_dashboard/types/integrity.tsairborne_docs/docs/dashboard/integrity.mdxairborne_docs/docs/dashboard/overview.mdxairborne_docs/docs/guides/verify-the-release-config-signature.mdxairborne_docs/docs/server/configuration.mdairborne_docs/openapi/airborne.openapi.jsonairborne_docs/sidebars.tsairborne_server/.env.exampleairborne_server/Cargo.tomlairborne_server/migrations/20260714120000_add_signing_keys/down.sqlairborne_server/migrations/20260714120000_add_signing_keys/up.sqlairborne_server/src/config.rsairborne_server/src/main.rsairborne_server/src/organisation/application.rsairborne_server/src/organisation/application/properties.rsairborne_server/src/release.rsairborne_server/src/release/utils.rsairborne_server/src/signing.rsairborne_server/src/signing/types.rsairborne_server/src/signing/utils.rsairborne_server/src/types.rsairborne_server/src/utils.rsairborne_server/src/utils/advisory_lock.rsairborne_server/src/utils/db/models.rsairborne_server/src/utils/db/schema.rsairborne_server/src/utils/moka.rsairborne_server/src/utils/redis.rssmithy/models/errors.smithysmithy/models/main.smithysmithy/models/release.smithysmithy/models/signing.smithy
yuvrajjsingh0
force-pushed
the
rc-signing
branch
from
August 17, 2026 08:23
366ebf8 to
22b4abe
Compare
yuvrajjsingh0
force-pushed
the
rc-signing
branch
from
August 17, 2026 09:48
39ea2fb to
31a5fb0
Compare
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 by CodeRabbit