feat: add open bao for credential secret storage#1679
Open
sujitaw wants to merge 19 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds OpenBao-backed secret loading, wires it into app startup, and refactors shared email and storage clients to fetch credentials at runtime. It also adds the ChangesOpenBao Secret Provider Integration
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com>
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com>
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com>
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com>
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com>
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com>
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com>
sujitaw
marked this pull request as ready for review
June 29, 2026 13:06
![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
🧹 Nitpick comments (3)
apps/oid4vc-verification/src/main.ts (1)
11-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the config package root export instead of the internal file path.
This PR already re-exports
loadConfigSecretsfromlibs/config/src/index.ts, so importing@credebl/config/secret-storage/secrets-loaderkeeps this service coupled to an internal module path. The same cleanup applies to the sibling bootstraps.♻️ Proposed change
-import { loadConfigSecrets } from '`@credebl/config/secret-storage/secrets-loader`'; +import { loadConfigSecrets } from '`@credebl/config`';🤖 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 `@apps/oid4vc-verification/src/main.ts` at line 11, The import in the main bootstrap is using an internal config module path instead of the package root export. Update the `loadConfigSecrets` import in `main.ts` to come from the `@credebl/config` root entrypoint, matching the re-export in `libs/config/src/index.ts`, and apply the same import cleanup in the sibling bootstraps that use this secret loader.apps/geo-location/src/main.ts (1)
12-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
loadConfigSecretsfrom the config package root.
libs/config/src/index.tsnow exportsloadConfigSecrets, so keeping the deep import here couples the app to@credebl/config's internal layout and makes the new public export easy to bypass.♻️ Proposed change
-import { loadConfigSecrets } from '`@credebl/config/secret-storage/secrets-loader`'; +import { loadConfigSecrets } from '`@credebl/config`';🤖 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 `@apps/geo-location/src/main.ts` at line 12, The import in main.ts is still using a deep path for loadConfigSecrets, which bypasses the new public export and couples the app to internal config layout. Update the app entrypoint to import loadConfigSecrets from the `@credebl/config` package root instead of the secrets-loader path, keeping the existing usage in main() unchanged.apps/agent-provisioning/src/main.ts (1)
12-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the config package’s public export here.
This reaches into
@credebl/configinternals even though the PR adds a root export forloadConfigSecrets. Importing from the package boundary avoids coupling every service entrypoint to an internal file path.Proposed fix
-import { loadConfigSecrets } from '`@credebl/config/secret-storage/secrets-loader`'; +import { loadConfigSecrets } from '`@credebl/config`';🤖 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 `@apps/agent-provisioning/src/main.ts` at line 12, The import in the agent provisioning entrypoint is reaching into an internal path instead of using the package boundary. Update the `main.ts` import to use `loadConfigSecrets` from `@credebl/config`’s public root export rather than `@credebl/config/secret-storage/secrets-loader`, so the entrypoint depends only on the supported API.
🤖 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 `@apps/agent-provisioning/src/main.ts`:
- Around line 1-19: The NATS environment values are being read too early because
AgentProvisioningModule evaluates process.env.AGENT_PROVISIONING_NKEY_SEED and
process.env.NATS_CREDS_FILE at import time, before main bootstrap calls
loadConfigSecrets(). Move that lookup out of module scope and into a lazy path
such as bootstrap or a provider factory used by getNatsOptions so the secrets
are loaded before the values are consumed.
In `@apps/geo-location/src/main.ts`:
- Around line 1-19: `geo-location.module.ts` is reading NATS env values during
import, before `main.ts` finishes `dotenv.config()` and `loadConfigSecrets()`.
Move the `ClientsModule.register(...)` NATS options construction out of module
initialization and into a post-secrets-loaded path, or resolve
`GEOLOCATION_NKEY_SEED` and `NATS_CREDS_FILE` through an injected config
provider so `getNatsOptions` is called only after secrets are available.
In `@apps/oid4vc-verification/src/main.ts`:
- Around line 1-14: Load environment variables before Oid4vpModule is imported,
because `oid4vc-verification.module.ts` reads `process.env` during module
evaluation and `dotenv.config()` in `main.ts` currently runs too late. Move the
dotenv initialization ahead of the `Oid4vpModule` import in `main.ts`, or
refactor the NATS config access used by `getNatsOptions`/`loadConfigSecrets` so
it is deferred until after `.env` is loaded.
In `@libs/common/src/resend-helper-file.ts`:
- Around line 7-17: The getResendClient helper currently fetches secrets and
creates a new Resend instance on every call, which adds unnecessary overhead and
repeats transient secret lookup failures during normal email sends. Update
getResendClient to cache either the initialized Resend client or the
RESEND_API_KEY (with a TTL/refresh strategy for rotation), and have callers
reuse that cached instance instead of rebuilding it each time. Keep the change
localized around getResendClient and the email-sending path that depends on it
so the client is only refreshed when needed.
In `@libs/config/src/secret-storage/openbao.provider.ts`:
- Around line 13-20: In openbao.provider.ts within loadConfigSecrets, validate
BAO_URL and the resolved secretPath before logging or building the request URL
so missing config fails with a clear error. Add an early guard alongside the
existing BAO_ROLE_ID/BAO_SECRET_ID check, and only call this.logger.log after
BAO_URL and secretPath are confirmed present.
- Around line 23-47: Both OpenBao fetch calls in OpenBaoProvider need request
timeouts so bootstrap cannot hang indefinitely. Update the auth request and the
secret-read request to use an abortable timeout mechanism (for example via
AbortController) and ensure both the login call and the secrets fetch fail fast
if OpenBao does not respond. Keep the change localized around the existing
authResponse and response fetch logic so the provider still logs and throws
clear errors on timeout.
In `@libs/config/src/secret-storage/secrets-loader.ts`:
- Around line 45-55: The secret injection loop in the secrets loader is too
permissive because it writes every backend key into process.env except
FORBIDDEN_KEYS. Replace this with an explicit allowlist or key mapping in the
secrets loading logic so only approved secret names are propagated, and keep the
Object.defineProperty-based assignment limited to those vetted keys.
- Around line 33-37: The fallback in secrets-loader’s provider selection should
not silently continue when SECRETS_PROVIDER is explicitly set to an unsupported
value. Update the provider resolution logic in the secrets-loader flow so that
an invalid providerType causes startup to fail fast instead of returning to
local environment variables, and keep the warning only if needed before
throwing/aborting. Use the existing provider selection branch around the
provider lookup in secrets-loader to make the behavior explicit for invalid
remote secret configurations.
In `@libs/storage/src/providers/s3-storage.provider.ts`:
- Around line 39-40: getPublicUrl in S3StorageProvider is using a different
region source than the upload path, which can generate a mismatched URL. Update
getPublicUrl to use the same AWS_PUBLIC_REGION source already used by the upload
logic in S3StorageProvider so both upload and URL generation stay consistent,
and verify the bucket URL is built from that shared region value rather than
process.env directly.
- Around line 56-62: The S3 upload helper in S3StorageProvider is hardcoding the
content type to image/png, which breaks non-PNG uploads. Update the upload logic
to derive ContentType from the file being uploaded instead of using a fixed
value, and pass the correct MIME type through the generic upload path in the
method that calls putObjectAsync.
- Around line 121-127: The S3 upload metadata in the storage provider is
mislabeling JSON as base64 even though the body is created with
Buffer.from(JSON.stringify(body)) in the S3 upload logic. Update the put-object
params in s3-storage.provider.ts to match the actual payload format by removing
the incorrect ContentEncoding value or changing it to the real encoding used,
and keep the ContentType as application/json in the S3 upload path.
- Around line 12-37: The S3 client helpers in S3StorageProvider are re-fetching
OpenBao secrets and rebuilding clients on every call. Add lazy caching or
memoization inside getS3Client, getPublicS3Client, and getStoreObjectS3Client so
each client is created once per process and reused for subsequent storage
operations, while still preserving the existing secret field mappings.
- Line 80: The S3 storage provider is using `AWS.S3.*` type annotations even
though `AWS` is not in scope. Update the type references in `S3StorageProvider`
to use the imported `S3` namespace instead, specifically in the `putObject`,
`getObject`, and `deleteObject` request/response typings such as
`S3.PutObjectRequest`, `S3.GetObjectOutput`, and `S3.DeleteObjectRequest`.
---
Nitpick comments:
In `@apps/agent-provisioning/src/main.ts`:
- Line 12: The import in the agent provisioning entrypoint is reaching into an
internal path instead of using the package boundary. Update the `main.ts` import
to use `loadConfigSecrets` from `@credebl/config`’s public root export rather
than `@credebl/config/secret-storage/secrets-loader`, so the entrypoint depends
only on the supported API.
In `@apps/geo-location/src/main.ts`:
- Line 12: The import in main.ts is still using a deep path for
loadConfigSecrets, which bypasses the new public export and couples the app to
internal config layout. Update the app entrypoint to import loadConfigSecrets
from the `@credebl/config` package root instead of the secrets-loader path,
keeping the existing usage in main() unchanged.
In `@apps/oid4vc-verification/src/main.ts`:
- Line 11: The import in the main bootstrap is using an internal config module
path instead of the package root export. Update the `loadConfigSecrets` import
in `main.ts` to come from the `@credebl/config` root entrypoint, matching the
re-export in `libs/config/src/index.ts`, and apply the same import cleanup in
the sibling bootstraps that use this secret loader.
🪄 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: 74b4a3aa-8a01-4929-bce9-7b0379931aef
📒 Files selected for processing (27)
.env.demoapps/agent-provisioning/src/main.tsapps/agent-service/src/main.tsapps/api-gateway/src/main.tsapps/cloud-wallet/src/main.tsapps/connection/src/main.tsapps/ecosystem/src/main.tsapps/geo-location/src/main.tsapps/issuance/src/main.tsapps/ledger/src/main.tsapps/notification/src/main.tsapps/oid4vc-issuance/src/main.tsapps/oid4vc-verification/src/main.tsapps/organization/src/main.tsapps/user/src/main.tsapps/utility/src/main.tsapps/verification/src/main.tsapps/webhook/src/main.tsapps/x509/src/main.tslibs/common/src/common.constant.tslibs/common/src/resend-helper-file.tslibs/config/src/index.tslibs/config/src/secret-storage/openbao.provider.tslibs/config/src/secret-storage/secret-provider.interface.tslibs/config/src/secret-storage/secrets-loader.tslibs/enum/src/enum.tslibs/storage/src/providers/s3-storage.provider.ts
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com>
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)
libs/storage/src/providers/base-s3-storage.provider.ts (1)
24-38: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMove async client acquisition inside the existing error handling.
get*Client()can now fail before AWS is called, especially when OpenBao fetch/auth fails. Because those awaits happen beforetry, callers get raw errors instead of the existingHttpException/RpcExceptioncontract.Suggested fix
): Promise<string> { - const s4 = await this.getPublicS3Client(); - const timestamp = Date.now(); - const putObjectAsync = promisify(s4.putObject).bind(s4); - const fileKey = `${pathAWS}/${encodeURIComponent(filename)}-${timestamp}.${ext}`; try { + const s4 = await this.getPublicS3Client(); + const timestamp = Date.now(); + const putObjectAsync = promisify(s4.putObject).bind(s4); + const fileKey = `${pathAWS}/${encodeURIComponent(filename)}-${timestamp}.${ext}`; await putObjectAsync({ Bucket: `${bucketName}`, Key: `${pathAWS}/${encodeURIComponent(filename)}-${timestamp}.${ext}`, Body: fileBuffer, ContentEncoding: encoding, ContentType: `image/png` }); return this.getPublicUrl(bucketName, fileKey); @@ async uploadCsvFile(key: string, body: unknown): Promise<void> { - const s3 = await this.getS3Client(); let data: string; @@ }; try { + const s3 = await this.getS3Client(); await s3.upload(params).promise(); @@ async getFile(key: string): Promise<AWS.S3.GetObjectOutput> { - const s3 = await this.getS3Client(); const params: AWS.S3.GetObjectRequest = { Bucket: process.env.FILE_SHARING_BUCKET, Key: key }; try { + const s3 = await this.getS3Client(); return s3.getObject(params).promise(); @@ async deleteFile(key: string): Promise<void> { - const s3 = await this.getS3Client(); const params: AWS.S3.DeleteObjectRequest = { Bucket: process.env.FILE_SHARING_BUCKET, Key: key }; try { + const s3 = await this.getS3Client(); await s3.deleteObject(params).promise(); @@ async storeObject(persistent: boolean, key: string, body: unknown): Promise<S3.ManagedUpload.SendData> { - const s3StoreObject = await this.getStoreObjectS3Client(); const objKey: string = persistent.valueOf() ? `persist/${key}` : `default/${key}`; @@ try { + const s3StoreObject = await this.getStoreObjectS3Client(); return await s3StoreObject.upload(params).promise();Also applies to: 43-62, 65-87, 91-106
🤖 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 `@libs/storage/src/providers/base-s3-storage.provider.ts` around lines 24 - 38, The async client acquisition in BaseS3StorageProvider methods like getPublicS3Client() is happening before the existing try/catch, so failures from get*Client() bypass the current HttpException/RpcException handling. Move the client await calls inside the same try block in the affected upload/download/delete methods (including the ones referenced by the other ranges) so any OpenBao/auth or AWS client setup errors are converted into the existing exception contract instead of leaking raw errors.libs/storage/src/providers/s3-storage.provider.ts (1)
10-34: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winValidate required AWS secret fields before constructing clients.
fetchOpenBaoSecretsonly guarantees an object payload; missing keys becomeundefinedcredentials/regions and fail later during S3 operations. Mirror the Resend helper’s fail-fast key check for each client’s required fields.Suggested fix
export class S3StorageService extends BaseS3StorageService { + private requireSecrets(secrets: Record<string, string>, keys: string[]): void { + const missingKeys = keys.filter((key) => !secrets[key]); + if (missingKeys.length) { + throw new Error(`Missing AWS secret keys: ${missingKeys.join(', ')}`); + } + } + protected async getS3Client(): Promise<S3> { const secrets = await fetchOpenBaoSecrets(CommonConstants.CREDEBL_AWS_KEY_PATH); + this.requireSecrets(secrets, ['AWS_ACCESS_KEY', 'AWS_SECRET_KEY', 'AWS_REGION']); return new S3({ accessKeyId: secrets.AWS_ACCESS_KEY, secretAccessKey: secrets.AWS_SECRET_KEY, region: secrets.AWS_REGION }); } protected async getPublicS3Client(): Promise<S3> { const secrets = await fetchOpenBaoSecrets(CommonConstants.CREDEBL_AWS_KEY_PATH); + this.requireSecrets(secrets, ['AWS_PUBLIC_ACCESS_KEY', 'AWS_PUBLIC_SECRET_KEY', 'AWS_PUBLIC_REGION']); return new S3({ accessKeyId: secrets.AWS_PUBLIC_ACCESS_KEY, secretAccessKey: secrets.AWS_PUBLIC_SECRET_KEY, region: secrets.AWS_PUBLIC_REGION }); } protected async getStoreObjectS3Client(): Promise<S3> { const secrets = await fetchOpenBaoSecrets(CommonConstants.CREDEBL_AWS_KEY_PATH); + this.requireSecrets(secrets, [ + 'AWS_S3_STOREOBJECT_ACCESS_KEY', + 'AWS_S3_STOREOBJECT_SECRET_KEY', + 'AWS_S3_STOREOBJECT_REGION' + ]); return new S3({🤖 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 `@libs/storage/src/providers/s3-storage.provider.ts` around lines 10 - 34, The S3 client factory methods getS3Client, getPublicS3Client, and getStoreObjectS3Client currently pass secret values from fetchOpenBaoSecrets directly into new S3 without verifying that the required AWS_* fields are present. Add a fail-fast validation step in each method to check the specific access key, secret key, and region needed for that client before constructing the S3 instance, and throw or log a clear error if any are missing so undefined credentials are never used.
🤖 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 `@libs/storage/src/providers/base-s3-storage.provider.ts`:
- Around line 24-38: The async client acquisition in BaseS3StorageProvider
methods like getPublicS3Client() is happening before the existing try/catch, so
failures from get*Client() bypass the current HttpException/RpcException
handling. Move the client await calls inside the same try block in the affected
upload/download/delete methods (including the ones referenced by the other
ranges) so any OpenBao/auth or AWS client setup errors are converted into the
existing exception contract instead of leaking raw errors.
In `@libs/storage/src/providers/s3-storage.provider.ts`:
- Around line 10-34: The S3 client factory methods getS3Client,
getPublicS3Client, and getStoreObjectS3Client currently pass secret values from
fetchOpenBaoSecrets directly into new S3 without verifying that the required
AWS_* fields are present. Add a fail-fast validation step in each method to
check the specific access key, secret key, and region needed for that client
before constructing the S3 instance, and throw or log a clear error if any are
missing so undefined credentials are never used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ff370362-0dc3-424d-bf64-2dc9e1e35bb9
📒 Files selected for processing (3)
libs/storage/src/providers/base-s3-storage.provider.tslibs/storage/src/providers/rustfs-storage.provider.tslibs/storage/src/providers/s3-storage.provider.ts
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com>
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com>
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com>
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
libs/common/src/utils/ttl-cache.util.ts (1)
10-18: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winShare the in-flight load on a cold cache miss.
Until
fetchFn()resolves, every caller still seesthis.value === nulland starts its own fetch. InfetchSecrets(), that turns one cold miss into multiple OpenBao auth/read requests for the same path.Suggested change
export class TtlCache<T> { private value: T | null = null; + private inflight: Promise<T> | null = null; private timer: ReturnType<typeof setTimeout> | null = null; private readonly ttlMs: number; constructor(ttlMs: number) { this.ttlMs = ttlMs; } async get(fetchFn: () => Promise<T>): Promise<T> { if (null !== this.value) { this.refreshTimer(); return this.value; } - this.value = await fetchFn(); - this.refreshTimer(); - return this.value; + this.inflight ??= (async (): Promise<T> => { + const value = await fetchFn(); + this.value = value; + this.refreshTimer(); + return value; + })(); + + try { + return await this.inflight; + } finally { + this.inflight = null; + } }🤖 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 `@libs/common/src/utils/ttl-cache.util.ts` around lines 10 - 18, The TTL cache get() method currently triggers multiple concurrent fetchFn calls on a cold miss because this.value stays null until the first request resolves. Update TtlCacheUtil.get to track an in-flight Promise (or similar shared pending state) so concurrent callers reuse the same fetch instead of starting new ones, and clear that pending state once the load completes or fails. Keep the refreshTimer behavior and reference the get method and the cached value handling in TtlCacheUtil.
🤖 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 `@libs/common/src/send-grid-helper-file.ts`:
- Around line 8-23: The initSendgrid helper is pinning the SendGrid API key for
the entire process via sendgridInitPromise, so it never re-reads rotated secrets
or recovers from a bad first value. Update initSendgrid in send-grid-helper-file
to initialize SendGrid from fetchSecrets on each call (or otherwise avoid
caching the promise), while still setting the API key through sendgrid.setApiKey
after resolving the latest secret or env fallback.
In `@libs/common/src/smtp-helper-file.ts`:
- Around line 8-42: The SMTP transporter is being cached forever in
getSmtpTransporter via transporterPromise, which prevents refreshed OpenBao
secrets from taking effect after config changes. Remove the permanent
memoization or make it key off the current secret values/TTL so fetchSecrets()
can drive recreation, and ensure a new nodemailer.createTransport call happens
whenever SMTP_HOST, SMTP_PORT, SMTP_USER, or SMTP_PASS changes.
---
Nitpick comments:
In `@libs/common/src/utils/ttl-cache.util.ts`:
- Around line 10-18: The TTL cache get() method currently triggers multiple
concurrent fetchFn calls on a cold miss because this.value stays null until the
first request resolves. Update TtlCacheUtil.get to track an in-flight Promise
(or similar shared pending state) so concurrent callers reuse the same fetch
instead of starting new ones, and clear that pending state once the load
completes or fails. Keep the refreshTimer behavior and reference the get method
and the cached value handling in TtlCacheUtil.
🪄 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: 41c7054f-72d8-4cf8-82e4-209ec30bb193
📒 Files selected for processing (10)
libs/common/src/common.constant.tslibs/common/src/resend-helper-file.tslibs/common/src/send-grid-helper-file.tslibs/common/src/smtp-helper-file.tslibs/common/src/utils/secretLoader.util.tslibs/common/src/utils/ttl-cache.util.tslibs/config/src/secret-storage/openbao.provider.tslibs/config/src/secret-storage/secret-provider.interface.tslibs/config/src/secret-storage/secrets-loader.tslibs/storage/src/providers/s3-storage.provider.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- libs/config/src/secret-storage/secret-provider.interface.ts
- libs/storage/src/providers/s3-storage.provider.ts
- libs/config/src/secret-storage/openbao.provider.ts
- libs/common/src/resend-helper-file.ts
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com>
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/common/src/send-grid-helper-file.ts (1)
28-33: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winLog SendGrid send failures instead of swallowing them.
The inner
.catch(() => false)prevents Line 33 from logging actual provider send errors. Let the outer catch handle it, or log the error in the inner catch.Proposed fix
- return await sendgrid - .send(msg) - .then(() => true) - .catch(() => false); + await sendgrid.send(msg); + return 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 `@libs/common/src/send-grid-helper-file.ts` around lines 28 - 33, The SendGrid send failure is being swallowed by the inner catch in the send helper, so the outer Logger.error path never sees the real provider error. Update the send logic in sendgrid helper file around the send(msg) flow to either remove the inner .catch(() => false) and let the outer catch handle the thrown error, or log the caught error there and preserve the failure details. Use the sendgrid.send and Logger.error paths to locate the change.
🤖 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 `@libs/config/src/secret-storage/secrets-loader.ts`:
- Around line 61-65: `loadConfigSecrets()` is swallowing secret-loading errors
in its catch block, so bootstrap can continue with invalid credentials; update
the `catch` in `secrets-loader.ts` to rethrow after logging (or otherwise
propagate the failure) so the awaited startup path fails fast. Keep the existing
`logger.error` calls for context, but make sure the caught error from the
provider load path is not converted into a silent success and that callers of
`loadConfigSecrets()` observe the failure.
---
Outside diff comments:
In `@libs/common/src/send-grid-helper-file.ts`:
- Around line 28-33: The SendGrid send failure is being swallowed by the inner
catch in the send helper, so the outer Logger.error path never sees the real
provider error. Update the send logic in sendgrid helper file around the
send(msg) flow to either remove the inner .catch(() => false) and let the outer
catch handle the thrown error, or log the caught error there and preserve the
failure details. Use the sendgrid.send and Logger.error paths to locate the
change.
🪄 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: d5ce336b-3976-4012-82a3-f03fa12eba6b
📒 Files selected for processing (5)
libs/common/src/resend-helper-file.tslibs/common/src/send-grid-helper-file.tslibs/common/src/smtp-helper-file.tslibs/config/src/secret-storage/openbao.provider.tslibs/config/src/secret-storage/secrets-loader.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/config/src/secret-storage/openbao.provider.ts
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
2 participants
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
Summary by CodeRabbit
ldp_vccredential format option.