Pass JWT public key parameter to components#2789
Merged
Merged
Conversation
WalkthroughAdds optional PEM-encoded UAA JWT signing public-key properties to Director, health monitor, and NATS configurations. Propagates the values through generated templates into authentication providers. Token decoders conditionally verify signatures when a key is configured, with tests covering default, configured-key, and invalid-signature behavior. Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
mkocher
previously approved these changes
Jul 17, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR propagates a configurable UAA JWT signing public key through BOSH job templates into the relevant Ruby components, enabling optional signature verification when decoding UAA access tokens (while preserving the existing non-verifying behavior by default).
Changes:
- Add
uaa_public_key/public_keyconfig fields and pass them into UAA token helper classes. - Decode UAA tokens with
{ verify: false }by default, and with{ pkey: <public key>, verify: true }when a public key is configured. - Extend unit/template specs to cover default passthrough and configured-key behavior.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/bosh-nats-sync/lib/nats_sync/auth_provider.rb | Pass uaa_public_key into UAAToken and enable optional signature verification during token decoding. |
| src/bosh-nats-sync/spec/nats_sync/auth_provider_spec.rb | Add coverage asserting decoding options differ based on presence of uaa_public_key. |
| src/bosh-monitor/lib/bosh/monitor/auth_provider.rb | Pass uaa_public_key into UAAToken and enable optional signature verification during token decoding. |
| src/bosh-monitor/spec/unit/bosh/monitor/auth_provider_spec.rb | Add coverage asserting decoding options differ based on presence of uaa_public_key. |
| src/bosh-director/lib/bosh/director/config_server/uaa_auth_provider.rb | Thread public_key from config into UAAToken and conditionally verify JWT signatures when decoding. |
| src/bosh-director/spec/unit/bosh/director/config_server/uaa_auth_provider_spec.rb | Add coverage asserting decoding options differ based on presence of public_key. |
| jobs/nats/templates/bosh_nats_sync_config.yml.erb | Render nats.director_account.uaa_public_key into the nats-sync director config. |
| jobs/nats/spec | Define new job property nats.director_account.uaa_public_key with default ''. |
| jobs/health_monitor/templates/health_monitor.yml.erb | Render hm.director_account.uaa_public_key into the HM director config. |
| jobs/health_monitor/spec | Define new job property hm.director_account.uaa_public_key with default ''. |
| jobs/director/templates/director.yml.erb | Render director.config_server.uaa.public_key into the config_server UAA config. |
| jobs/director/spec | Define new job property director.config_server.uaa.public_key with default ''. |
| spec/nats_templates_spec.rb | Assert rendered nats-sync config includes uaa_public_key (default empty string). |
| spec/health_monitor_templates_spec.rb | Assert rendered HM config includes uaa_public_key and passes through a configured PEM. |
| spec/director.yml.erb_spec.rb | Assert rendered director config_server UAA config includes public_key and passes through a configured PEM. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bosh-monitor/lib/bosh/monitor/auth_provider.rb (1)
84-94: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAvoid partial token state on decode failure.
Insrc/bosh-monitor/lib/bosh/monitor/auth_provider.rbandsrc/bosh-nats-sync/lib/nats_sync/auth_provider.rb, compute the new token and decoded payload in locals first, then assign both only after decode succeeds. The current order can leave@uaa_tokenset while@token_dataisnil, so the nextauth_headercall can raise inexpires_soon?.🤖 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/bosh-monitor/lib/bosh/monitor/auth_provider.rb` around lines 84 - 94, Update fetch in src/bosh-monitor/lib/bosh/monitor/auth_provider.rb (lines 84-94) and src/bosh-nats-sync/lib/nats_sync/auth_provider.rb (lines 84-94) to obtain the token and decode its payload into local variables first, then assign `@uaa_token` and `@token_data` only after decoding succeeds; preserve the existing rescue behavior and ensure failed decoding cannot leave partial token state in either file.
🤖 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/bosh-director/lib/bosh/director/config_server/uaa_auth_provider.rb`:
- Around line 21-23: In the surrounding class, remove the ineffective private
modifier before UAAToken, then add private_constant :UAAToken immediately after
the UAAToken class definition and before the enclosing class ends, preserving
the class implementation unchanged.
In `@src/bosh-monitor/lib/bosh/monitor/auth_provider.rb`:
- Around line 96-101: The decode_options method in
src/bosh-monitor/lib/bosh/monitor/auth_provider.rb (lines 96-101) must use a
whitespace-aware emptiness check for `@uaa_public_key`. Apply the same change in
src/bosh-nats-sync/lib/nats_sync/auth_provider.rb (lines 96-101), using
`@uaa_public_key.strip.empty`? so whitespace-only keys select verify: false while
valid keys retain signature verification.
In `@src/bosh-monitor/spec/unit/bosh/monitor/auth_provider_spec.rb`:
- Around line 176-177: Add a unit test for the token verification failure path
in the auth provider specs, exercising the relevant verification method and
asserting that the provider handles the error gracefully without partially
updating its state.
In `@src/bosh-nats-sync/spec/nats_sync/auth_provider_spec.rb`:
- Around line 182-183: Add a test in the auth provider specs covering token
verification failure, asserting the provider handles the error gracefully and
leaves its existing state unchanged. Anchor the test to the token verification
behavior and use the existing provider setup and error-handling expectations.
---
Outside diff comments:
In `@src/bosh-monitor/lib/bosh/monitor/auth_provider.rb`:
- Around line 84-94: Update fetch in
src/bosh-monitor/lib/bosh/monitor/auth_provider.rb (lines 84-94) and
src/bosh-nats-sync/lib/nats_sync/auth_provider.rb (lines 84-94) to obtain the
token and decode its payload into local variables first, then assign `@uaa_token`
and `@token_data` only after decoding succeeds; preserve the existing rescue
behavior and ensure failed decoding cannot leave partial token state in either
file.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: d1975aad-036d-4674-ac3e-60ddcf47511f
📒 Files selected for processing (15)
jobs/director/specjobs/director/templates/director.yml.erbjobs/health_monitor/specjobs/health_monitor/templates/health_monitor.yml.erbjobs/nats/specjobs/nats/templates/bosh_nats_sync_config.yml.erbspec/director.yml.erb_spec.rbspec/health_monitor_templates_spec.rbspec/nats_templates_spec.rbsrc/bosh-director/lib/bosh/director/config_server/uaa_auth_provider.rbsrc/bosh-director/spec/unit/bosh/director/config_server/uaa_auth_provider_spec.rbsrc/bosh-monitor/lib/bosh/monitor/auth_provider.rbsrc/bosh-monitor/spec/unit/bosh/monitor/auth_provider_spec.rbsrc/bosh-nats-sync/lib/nats_sync/auth_provider.rbsrc/bosh-nats-sync/spec/nats_sync/auth_provider_spec.rb
github-project-automation
Bot
moved this from Inbox
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
aramprice
force-pushed
the
pass-jwt-params
branch
from
July 17, 2026 00:33
9d67a93 to
24666be
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/bosh-monitor/lib/bosh/monitor/auth_provider.rb (1)
96-101: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse
.strip.empty?to handle whitespace-only public keys safely.If the configured
@uaa_public_keycontains only whitespace (e.g., due to trailing padding or multi-line misconfiguration),.empty?returnsfalse. This causes the token coder to attempt signature verification with an invalid blank key.
src/bosh-monitor/lib/bosh/monitor/auth_provider.rb#L96-L101: Update the condition to use.strip.empty?.src/bosh-nats-sync/lib/nats_sync/auth_provider.rb#L96-L101: Update the condition to use.strip.empty?.🛡️ Proposed fix to handle whitespace
def decode_options - if `@uaa_public_key.empty`? + if `@uaa_public_key.strip.empty`? { verify: false } else { pkey: `@uaa_public_key`, verify: true }🤖 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/bosh-monitor/lib/bosh/monitor/auth_provider.rb` around lines 96 - 101, Update the decode_options method in src/bosh-monitor/lib/bosh/monitor/auth_provider.rb at lines 96-101 to use a stripped emptiness check for `@uaa_public_key`, preserving the existing verification options. Apply the same change in src/bosh-nats-sync/lib/nats_sync/auth_provider.rb at lines 96-101 so whitespace-only keys disable verification in both auth providers.src/bosh-monitor/spec/unit/bosh/monitor/auth_provider_spec.rb (1)
170-176: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest error handling during token verification.
Consider adding a test case to verify that the provider gracefully handles a token verification failure without partially updating its state.
src/bosh-monitor/spec/unit/bosh/monitor/auth_provider_spec.rb#L170-L176: Add a test context for signature verification failure asserting thatauth_provider.auth_headerreturnsnil.src/bosh-nats-sync/spec/nats_sync/auth_provider_spec.rb#L177-L183: Add the same test context for signature verification failure.🧪 Proposed test case addition
it 'decodes UAA tokens with signature verification using the configured public key' do expect(CF::UAA::TokenCoder).to receive(:decode) .with(anything, { pkey: uaa_public_key, verify: true }) .and_return('exp' => Time.now.to_i + 3600) auth_provider.auth_header end + + context 'when signature verification fails' do + it 'logs the error and avoids partial state updates' do + allow(CF::UAA::TokenCoder).to receive(:decode).and_raise(StandardError.new('invalid signature')) + expect(logger).to receive(:error).with(/Failed to obtain token from UAA/) + + expect(auth_provider.auth_header).to be_nil + end + end end🤖 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/bosh-monitor/spec/unit/bosh/monitor/auth_provider_spec.rb` around lines 170 - 176, Add signature-verification failure examples alongside the existing TokenCoder decode tests in src/bosh-monitor/spec/unit/bosh/monitor/auth_provider_spec.rb (lines 170-176) and src/bosh-nats-sync/spec/nats_sync/auth_provider_spec.rb (lines 177-183). Stub CF::UAA::TokenCoder.decode to raise during verification, call auth_provider.auth_header, and assert it returns nil without partially updating provider state.
🤖 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/bosh-monitor/lib/bosh/monitor/auth_provider.rb`:
- Around line 93-95: Update the error handler in fetch to reset both `@uaa_token`
and `@token_data` when token retrieval or verification fails, ensuring failed
authentication cannot expose the unverified token or leave nil token data. Apply
this change in src/bosh-monitor/lib/bosh/monitor/auth_provider.rb at lines 93-95
and src/bosh-nats-sync/lib/nats_sync/auth_provider.rb at lines 93-95; both sites
require the same direct fix.
---
Duplicate comments:
In `@src/bosh-monitor/lib/bosh/monitor/auth_provider.rb`:
- Around line 96-101: Update the decode_options method in
src/bosh-monitor/lib/bosh/monitor/auth_provider.rb at lines 96-101 to use a
stripped emptiness check for `@uaa_public_key`, preserving the existing
verification options. Apply the same change in
src/bosh-nats-sync/lib/nats_sync/auth_provider.rb at lines 96-101 so
whitespace-only keys disable verification in both auth providers.
In `@src/bosh-monitor/spec/unit/bosh/monitor/auth_provider_spec.rb`:
- Around line 170-176: Add signature-verification failure examples alongside the
existing TokenCoder decode tests in
src/bosh-monitor/spec/unit/bosh/monitor/auth_provider_spec.rb (lines 170-176)
and src/bosh-nats-sync/spec/nats_sync/auth_provider_spec.rb (lines 177-183).
Stub CF::UAA::TokenCoder.decode to raise during verification, call
auth_provider.auth_header, and assert it returns nil without partially updating
provider state.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 1e21adeb-f509-41f6-bb5d-0c9f5d23ccd0
📒 Files selected for processing (15)
jobs/director/specjobs/director/templates/director.yml.erbjobs/health_monitor/specjobs/health_monitor/templates/health_monitor.yml.erbjobs/nats/specjobs/nats/templates/bosh_nats_sync_config.yml.erbspec/director.yml.erb_spec.rbspec/health_monitor_templates_spec.rbspec/nats_templates_spec.rbsrc/bosh-director/lib/bosh/director/config_server/uaa_auth_provider.rbsrc/bosh-director/spec/unit/bosh/director/config_server/uaa_auth_provider_spec.rbsrc/bosh-monitor/lib/bosh/monitor/auth_provider.rbsrc/bosh-monitor/spec/unit/bosh/monitor/auth_provider_spec.rbsrc/bosh-nats-sync/lib/nats_sync/auth_provider.rbsrc/bosh-nats-sync/spec/nats_sync/auth_provider_spec.rb
aramprice
force-pushed
the
pass-jwt-params
branch
from
July 17, 2026 00:44
24666be to
654de81
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/bosh-director/lib/bosh/director/config_server/uaa_auth_provider.rb`:
- Around line 86-92: Update decode_options to use `@public_key.strip.empty`? when
determining whether verification should be disabled, so whitespace-only keys
follow the empty-key branch while valid keys retain the existing pkey and
verification behavior.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: f535df39-4e69-4611-b06b-f893eadf94ee
📒 Files selected for processing (15)
jobs/director/specjobs/director/templates/director.yml.erbjobs/health_monitor/specjobs/health_monitor/templates/health_monitor.yml.erbjobs/nats/specjobs/nats/templates/bosh_nats_sync_config.yml.erbspec/director.yml.erb_spec.rbspec/health_monitor_templates_spec.rbspec/nats_templates_spec.rbsrc/bosh-director/lib/bosh/director/config_server/uaa_auth_provider.rbsrc/bosh-director/spec/unit/bosh/director/config_server/uaa_auth_provider_spec.rbsrc/bosh-monitor/lib/bosh/monitor/auth_provider.rbsrc/bosh-monitor/spec/unit/bosh/monitor/auth_provider_spec.rbsrc/bosh-nats-sync/lib/nats_sync/auth_provider.rbsrc/bosh-nats-sync/spec/nats_sync/auth_provider_spec.rb
aramprice
force-pushed
the
pass-jwt-params
branch
from
July 17, 2026 01:10
654de81 to
446f16d
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/bosh-director/lib/bosh/director/config_server/uaa_auth_provider.rb (2)
66-84: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPrevent partial state update on token verification failure.
To achieve parity with the identical fixes correctly applied in
bosh-monitorandbosh-nats-sync, avoid assigning the fetched token to@uaa_tokenuntil it has been successfully verified.If
decodethrows an exception (e.g., due to an invalid signature), the current implementation leaves@uaa_tokenpopulated with the unverified token. If a consumer catches the resultingUAAAuthorizationErrorand re-invokesauth_headeron the same instance, it will return the unverified token and bypass signature verification.🛡️ Proposed fix
def fetch - `@uaa_token` = retryable.retryer do + token = retryable.retryer do `@uaa_token_issuer.client_credentials_grant` end - `@token_data` = decode + `@token_data` = decode_token(token) + `@uaa_token` = token rescue CF::UAA::SSLException => e error_message = "Failed to obtain valid token from UAA: Invalid SSL Cert for '#{`@uaa_url`}'" `@logger.error`("#{error_message}. Error thrown: #{e.inspect}") raise Bosh::Director::UAAAuthorizationError, error_message rescue Exception => e error_message = "Failed to obtain valid token from UAA: #{e.inspect}" `@logger.error`(error_message) raise Bosh::Director::UAAAuthorizationError, error_message end - def decode - access_token = `@uaa_token.info`['access_token'] || `@uaa_token.info`[:access_token] + def decode_token(token) + access_token = token.info['access_token'] || token.info[:access_token] CF::UAA::TokenCoder.decode(access_token, decode_options) end🤖 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/bosh-director/lib/bosh/director/config_server/uaa_auth_provider.rb` around lines 66 - 84, Update fetch so the token returned by client_credentials_grant is held in a local variable, verified via decode before assigning it to the instance variable `@uaa_token`. Ensure decode operates on the fetched token during verification, and leave `@uaa_token` unchanged when verification raises an exception, preventing auth_header from returning an unverified token.
66-84: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPrevent partial state update on token verification failure.
To achieve parity with the identical fixes correctly applied in
bosh-monitorandbosh-nats-sync, avoid assigning the fetched token to@uaa_tokenuntil it has been successfully verified.If
decodethrows an exception (e.g., due to an invalid signature), the current implementation leaves@uaa_tokenpopulated with the unverified token. If a consumer catches the resultingUAAAuthorizationErrorand re-invokesauth_headeron the same instance, it will return the unverified token and bypass signature verification.🛡️ Proposed fix
def fetch - `@uaa_token` = retryable.retryer do + token = retryable.retryer do `@uaa_token_issuer.client_credentials_grant` end - `@token_data` = decode + `@token_data` = decode_token(token) + `@uaa_token` = token rescue CF::UAA::SSLException => e error_message = "Failed to obtain valid token from UAA: Invalid SSL Cert for '#{`@uaa_url`}'" `@logger.error`("#{error_message}. Error thrown: #{e.inspect}") raise Bosh::Director::UAAAuthorizationError, error_message rescue Exception => e error_message = "Failed to obtain valid token from UAA: #{e.inspect}" `@logger.error`(error_message) raise Bosh::Director::UAAAuthorizationError, error_message end - def decode - access_token = `@uaa_token.info`['access_token'] || `@uaa_token.info`[:access_token] + def decode_token(token) + access_token = token.info['access_token'] || token.info[:access_token] CF::UAA::TokenCoder.decode(access_token, decode_options) end🤖 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/bosh-director/lib/bosh/director/config_server/uaa_auth_provider.rb` around lines 66 - 84, Update fetch so the client-credentials result is held in a temporary local variable, verified by decode using that candidate token, and assigned to `@uaa_token` only after verification succeeds. Preserve the existing rescue behavior and ensure subsequent auth_header calls cannot use an unverified token after decode raises.
🤖 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.
Outside diff comments:
In `@src/bosh-director/lib/bosh/director/config_server/uaa_auth_provider.rb`:
- Around line 66-84: Update fetch so the token returned by
client_credentials_grant is held in a local variable, verified via decode before
assigning it to the instance variable `@uaa_token`. Ensure decode operates on the
fetched token during verification, and leave `@uaa_token` unchanged when
verification raises an exception, preventing auth_header from returning an
unverified token.
- Around line 66-84: Update fetch so the client-credentials result is held in a
temporary local variable, verified by decode using that candidate token, and
assigned to `@uaa_token` only after verification succeeds. Preserve the existing
rescue behavior and ensure subsequent auth_header calls cannot use an unverified
token after decode raises.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b4a5b09b-0c0b-4273-b4a4-653317bd1615
📒 Files selected for processing (21)
jobs/director/specjobs/director/templates/director.yml.erbjobs/health_monitor/specjobs/health_monitor/templates/health_monitor.yml.erbjobs/nats/specjobs/nats/templates/bosh_nats_sync_config.yml.erbspec/director.yml.erb_spec.rbspec/health_monitor_templates_spec.rbspec/nats_templates_spec.rbsrc/bosh-director/lib/bosh/director/config_server/uaa_auth_provider.rbsrc/bosh-director/spec/support/uaa_helpers.rbsrc/bosh-director/spec/unit/bosh/director/config_server/uaa_auth_provider_spec.rbsrc/bosh-monitor/lib/bosh/monitor/auth_provider.rbsrc/bosh-monitor/spec/support/uaa_helpers.rbsrc/bosh-monitor/spec/unit/bosh/monitor/auth_provider_spec.rbsrc/bosh-monitor/spec/unit/bosh/monitor/director_spec.rbsrc/bosh-monitor/spec/unit/bosh/monitor/plugins/event_logger_spec.rbsrc/bosh-monitor/spec/unit/bosh/monitor/plugins/resurrector_spec.rbsrc/bosh-nats-sync/lib/nats_sync/auth_provider.rbsrc/bosh-nats-sync/spec/nats_sync/auth_provider_spec.rbsrc/bosh-nats-sync/spec/support/uaa_helpers.rb
aramprice
force-pushed
the
pass-jwt-params
branch
2 times, most recently
from
July 17, 2026 01:30
570986f to
e25f7aa
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
spec/nats_templates_spec.rb (1)
18-44: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd coverage for a configured multiline PEM key.
This only verifies the empty default. Add a non-empty PEM case to ensure the NATS template preserves YAML quoting and newlines before the value reaches signature verification.
🤖 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 `@spec/nats_templates_spec.rb` around lines 18 - 44, Extend the existing nats template spec’s input configuration and expected_content coverage with a non-empty multiline PEM value for uaa_public_key. Assert that the rendered YAML preserves the required quoting and newline structure, while retaining the existing empty-key case.
🤖 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.
Outside diff comments:
In `@spec/nats_templates_spec.rb`:
- Around line 18-44: Extend the existing nats template spec’s input
configuration and expected_content coverage with a non-empty multiline PEM value
for uaa_public_key. Assert that the rendered YAML preserves the required quoting
and newline structure, while retaining the existing empty-key case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3e92d2e2-4fea-47f1-97f2-220109aaac11
📒 Files selected for processing (21)
jobs/director/specjobs/director/templates/director.yml.erbjobs/health_monitor/specjobs/health_monitor/templates/health_monitor.yml.erbjobs/nats/specjobs/nats/templates/bosh_nats_sync_config.yml.erbspec/director.yml.erb_spec.rbspec/health_monitor_templates_spec.rbspec/nats_templates_spec.rbsrc/bosh-director/lib/bosh/director/config_server/uaa_auth_provider.rbsrc/bosh-director/spec/support/uaa_helpers.rbsrc/bosh-director/spec/unit/bosh/director/config_server/uaa_auth_provider_spec.rbsrc/bosh-monitor/lib/bosh/monitor/auth_provider.rbsrc/bosh-monitor/spec/support/uaa_helpers.rbsrc/bosh-monitor/spec/unit/bosh/monitor/auth_provider_spec.rbsrc/bosh-monitor/spec/unit/bosh/monitor/director_spec.rbsrc/bosh-monitor/spec/unit/bosh/monitor/plugins/event_logger_spec.rbsrc/bosh-monitor/spec/unit/bosh/monitor/plugins/resurrector_spec.rbsrc/bosh-nats-sync/lib/nats_sync/auth_provider.rbsrc/bosh-nats-sync/spec/nats_sync/auth_provider_spec.rbsrc/bosh-nats-sync/spec/support/uaa_helpers.rb
aramprice
force-pushed
the
pass-jwt-params
branch
from
July 17, 2026 02:25
e25f7aa to
7dc9c3e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/bosh-director/spec/support/uaa_helpers.rb`:
- Around line 5-7: Update the uaa_token_key helper in both
src/bosh-director/spec/support/uaa_helpers.rb (lines 5-7) and
src/bosh-monitor/spec/support/uaa_helpers.rb (lines 5-7) to memoize the
generated RSA key using module-level state rather than the per-example
`@uaa_token_key` instance variable, preserving the existing 2048-bit key
generation and return behavior.
In `@src/bosh-monitor/lib/bosh/monitor/auth_provider.rb`:
- Around line 85-90: Update the token-refresh rescue in AuthProvider to clear
both `@uaa_token` and `@token_data` before logging the failure, preventing reuse of
the previous token. Apply this change in
src/bosh-monitor/lib/bosh/monitor/auth_provider.rb (lines 85-90) and
src/bosh-nats-sync/lib/nats_sync/auth_provider.rb (lines 85-90), and add
regression coverage confirming a refresh failure after successful initialization
leaves both token fields cleared.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: e93a28db-d6b2-48da-9e05-06cf94aac1c6
📒 Files selected for processing (21)
jobs/director/specjobs/director/templates/director.yml.erbjobs/health_monitor/specjobs/health_monitor/templates/health_monitor.yml.erbjobs/nats/specjobs/nats/templates/bosh_nats_sync_config.yml.erbspec/director.yml.erb_spec.rbspec/health_monitor_templates_spec.rbspec/nats_templates_spec.rbsrc/bosh-director/lib/bosh/director/config_server/uaa_auth_provider.rbsrc/bosh-director/spec/support/uaa_helpers.rbsrc/bosh-director/spec/unit/bosh/director/config_server/uaa_auth_provider_spec.rbsrc/bosh-monitor/lib/bosh/monitor/auth_provider.rbsrc/bosh-monitor/spec/support/uaa_helpers.rbsrc/bosh-monitor/spec/unit/bosh/monitor/auth_provider_spec.rbsrc/bosh-monitor/spec/unit/bosh/monitor/director_spec.rbsrc/bosh-monitor/spec/unit/bosh/monitor/plugins/event_logger_spec.rbsrc/bosh-monitor/spec/unit/bosh/monitor/plugins/resurrector_spec.rbsrc/bosh-nats-sync/lib/nats_sync/auth_provider.rbsrc/bosh-nats-sync/spec/nats_sync/auth_provider_spec.rbsrc/bosh-nats-sync/spec/support/uaa_helpers.rb
aramprice
force-pushed
the
pass-jwt-params
branch
2 times, most recently
from
July 17, 2026 02:43
98bc0ee to
d5e8a27
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/bosh-director/spec/support/uaa_helpers.rb (1)
5-12: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMemoize the RSA key at the module level to improve test performance.
The addition of
module_function :uaa_token_keydoes not prevent the instance variable@uaa_token_keyfrom being re-initialized for every test. WhenSupport::UaaHelpersis included into RSpec examples, the method still executes in the context of the test instance, meaning a new 2048-bit RSA key is generated per test block, keeping the test suite slow.To genuinely cache the key across tests, store it on the module itself and delegate to it.
src/bosh-director/spec/support/uaa_helpers.rb#L5-L12: Updateuaa_token_keyanduaa_token_public_keyto delegate to a module-level method.src/bosh-monitor/spec/support/uaa_helpers.rb#L5-L12: Updateuaa_token_keyanduaa_token_public_keyto delegate to a module-level method.src/bosh-nats-sync/spec/support/uaa_helpers.rb#L5-L12: Updateuaa_token_keyanduaa_token_public_keyto delegate to a module-level method.♻️ Proposed refactor (apply to all 3 files)
- def uaa_token_key - `@uaa_token_key` ||= OpenSSL::PKey::RSA.generate(2048) - end - module_function :uaa_token_key - - def uaa_token_public_key - `@uaa_token_public_key` ||= uaa_token_key.public_key.to_pem + def self.uaa_token_key + `@uaa_token_key` ||= OpenSSL::PKey::RSA.generate(2048) end + + def uaa_token_key + Support::UaaHelpers.uaa_token_key + end + + def self.uaa_token_public_key + `@uaa_token_public_key` ||= uaa_token_key.public_key.to_pem + end + + def uaa_token_public_key + Support::UaaHelpers.uaa_token_public_key + end🤖 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/bosh-director/spec/support/uaa_helpers.rb` around lines 5 - 12, Memoize the RSA key on the helper module rather than each included RSpec example instance. In src/bosh-director/spec/support/uaa_helpers.rb lines 5-12, src/bosh-monitor/spec/support/uaa_helpers.rb lines 5-12, and src/bosh-nats-sync/spec/support/uaa_helpers.rb lines 5-12, update uaa_token_key and uaa_token_public_key to delegate to module-level methods while preserving their existing public helper behavior.
🤖 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.
Duplicate comments:
In `@src/bosh-director/spec/support/uaa_helpers.rb`:
- Around line 5-12: Memoize the RSA key on the helper module rather than each
included RSpec example instance. In
src/bosh-director/spec/support/uaa_helpers.rb lines 5-12,
src/bosh-monitor/spec/support/uaa_helpers.rb lines 5-12, and
src/bosh-nats-sync/spec/support/uaa_helpers.rb lines 5-12, update uaa_token_key
and uaa_token_public_key to delegate to module-level methods while preserving
their existing public helper behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f32979f5-59a9-4e0d-87cc-e4acb8e769e8
📒 Files selected for processing (9)
src/bosh-director/spec/support/uaa_helpers.rbsrc/bosh-director/spec/unit/bosh/director/config_server/uaa_auth_provider_spec.rbsrc/bosh-monitor/spec/support/uaa_helpers.rbsrc/bosh-monitor/spec/unit/bosh/monitor/auth_provider_spec.rbsrc/bosh-monitor/spec/unit/bosh/monitor/director_spec.rbsrc/bosh-monitor/spec/unit/bosh/monitor/plugins/event_logger_spec.rbsrc/bosh-monitor/spec/unit/bosh/monitor/plugins/resurrector_spec.rbsrc/bosh-nats-sync/spec/nats_sync/auth_provider_spec.rbsrc/bosh-nats-sync/spec/support/uaa_helpers.rb
github-project-automation
Bot
moved this from Waiting for Changes | Open for Contribution
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
Previously specs used HMAC token signing / validation which is not used in production. This commit updates the uaa helper code to use RSA based "pkey" token signing / validation.
aramprice
force-pushed
the
pass-jwt-params
branch
from
July 17, 2026 03:20
d5e8a27 to
2e9a4fa
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/bosh-director/spec/support/uaa_helpers.rb (1)
1-13: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCorrect the module-level memoization to ensure tests reuse the RSA key.
Using
module_functioncopies the instance method to the module's singleton class but leaves the instance method intact. Because test examplesinclude Support::UaaHelpers, they call the instance method, assigning the@uaa_token_keyinstance variable to the test'sselfrather than the module. This causes the slow 2048-bit RSA key generation to still run once per test example.
src/bosh-director/spec/support/uaa_helpers.rb#L1-L13: Replacemodule_functionwith module-level methods for proper memoization.src/bosh-monitor/spec/support/uaa_helpers.rb#L1-L13: Apply the same memoization fix.♻️ Proposed refactor
module UaaHelpers + def self.uaa_token_key + `@uaa_token_key` ||= OpenSSL::PKey::RSA.generate(2048) + end + def uaa_token_key - `@uaa_token_key` ||= OpenSSL::PKey::RSA.generate(2048) + Support::UaaHelpers.uaa_token_key end - module_function :uaa_token_key + def self.uaa_token_public_key + `@uaa_token_public_key` ||= uaa_token_key.public_key.to_pem + end + def uaa_token_public_key - `@uaa_token_public_key` ||= uaa_token_key.public_key.to_pem + Support::UaaHelpers.uaa_token_public_key end🤖 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/bosh-director/spec/support/uaa_helpers.rb` around lines 1 - 13, Replace module_function-based definitions in Support::UaaHelpers with module-level singleton methods so `@uaa_token_key` and `@uaa_token_public_key` are memoized on the module and reused across included test examples. Apply the same change to src/bosh-director/spec/support/uaa_helpers.rb lines 1-13 and src/bosh-monitor/spec/support/uaa_helpers.rb lines 1-13; preserve the existing uaa_token_key and uaa_token_public_key behavior.
🤖 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.
Duplicate comments:
In `@src/bosh-director/spec/support/uaa_helpers.rb`:
- Around line 1-13: Replace module_function-based definitions in
Support::UaaHelpers with module-level singleton methods so `@uaa_token_key` and
`@uaa_token_public_key` are memoized on the module and reused across included test
examples. Apply the same change to src/bosh-director/spec/support/uaa_helpers.rb
lines 1-13 and src/bosh-monitor/spec/support/uaa_helpers.rb lines 1-13; preserve
the existing uaa_token_key and uaa_token_public_key behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 32e08c4f-660b-4563-a5b9-7b429ee75763
📒 Files selected for processing (21)
jobs/director/specjobs/director/templates/director.yml.erbjobs/health_monitor/specjobs/health_monitor/templates/health_monitor.yml.erbjobs/nats/specjobs/nats/templates/bosh_nats_sync_config.yml.erbspec/director.yml.erb_spec.rbspec/health_monitor_templates_spec.rbspec/nats_templates_spec.rbsrc/bosh-director/lib/bosh/director/config_server/uaa_auth_provider.rbsrc/bosh-director/spec/support/uaa_helpers.rbsrc/bosh-director/spec/unit/bosh/director/config_server/uaa_auth_provider_spec.rbsrc/bosh-monitor/lib/bosh/monitor/auth_provider.rbsrc/bosh-monitor/spec/support/uaa_helpers.rbsrc/bosh-monitor/spec/unit/bosh/monitor/auth_provider_spec.rbsrc/bosh-monitor/spec/unit/bosh/monitor/director_spec.rbsrc/bosh-monitor/spec/unit/bosh/monitor/plugins/event_logger_spec.rbsrc/bosh-monitor/spec/unit/bosh/monitor/plugins/resurrector_spec.rbsrc/bosh-nats-sync/lib/nats_sync/auth_provider.rbsrc/bosh-nats-sync/spec/nats_sync/auth_provider_spec.rbsrc/bosh-nats-sync/spec/support/uaa_helpers.rb
mkocher
approved these changes
Jul 17, 2026
github-project-automation
Bot
moved this from Pending Merge | Prioritized
to Done
in Foundational Infrastructure Working Group
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
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.
Depends on
masterbranch