Avoid hardcoding the product name in scripts and other source files#2478
Merged
brionmario merged 1 commit intoasgardeo:mainfrom Apr 28, 2026
Merged
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (36)
📝 WalkthroughWalkthroughThis pull request refactors the codebase to generalize Thunder-specific naming and environment variables into configurable, product-agnostic alternatives. It renames API helper functions (e.g., Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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 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 |
brionmario
added
the
skip-changelog
brionmario
force-pushed
the
refactor-thunder-rebrand
branch
2 times, most recently
from
8720c40 to
c05c1be
Compare
![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
🧹 Nitpick comments (1)
backend/cmd/server/bootstrap/common.sh (1)
23-24: Consider exportingPRODUCT_NAMEfor external use.The variable is defined but not exported. Scripts sourcing this file will have access to it in the current shell context, but if any child processes need it, consider adding
export:-PRODUCT_NAME="Thunder" +export PRODUCT_NAME="Thunder"This aligns with how
API_BASEis expected to be available from the environment (exported bysetup.sh).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/cmd/server/bootstrap/common.sh` around lines 23 - 24, The PRODUCT_NAME variable is set but not exported, so child processes won't see it; update the assignment of PRODUCT_NAME to be exported (e.g., make PRODUCT_NAME available to the environment the same way API_BASE is exported by setup.sh) by exporting PRODUCT_NAME so downstream scripts/processes can access it; target the PRODUCT_NAME definition in this file (and mirror the export pattern used for API_BASE in setup.sh).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/cmd/server/repository/conf/deployment.yaml`:
- Around line 54-60: The current origin validation uses substring checks
(strings.Contains) and allows crafted hosts like
"https://localhost:5173.attacker.tld" to pass; update the validator in
serverutil.go (the function handling origin checks, e.g.,
IsOriginAllowed/ValidateOrigin) to parse the incoming origin using url.Parse,
normalize it to "scheme://host[:port]" and compare for exact equality against
the allow-list entries (also normalized) instead of using strings.Contains;
ensure nil/parse errors are rejected and treat default ports consistently when
comparing.
In `@build.ps1`:
- Around line 60-62: PRODUCT_NAME is a UI/display string but you're deriving
PRODUCT_NAME_LOWERCASE from it which can produce invalid slugs; introduce an
explicit BINARY_NAME (a slug) and replace uses of PRODUCT_NAME_LOWERCASE with
BINARY_NAME (including the other occurrences around PRODUCT_NAME at lines
referenced 159-164) so builds and artifacts use the stable slug while
PRODUCT_NAME remains the human-friendly display name.
- Around line 1575-1577: The code temporarily sets $env:SKIP_SECURITY and
possibly API_BASE before calling Run-Backend; wrap that override block in a
try/finally so you always restore $env:SKIP_SECURITY (from
$script:ORIGINAL_SKIP_SECURITY) and any temporary API_BASE value even if
Run-Backend throws or an exit is triggered; locate the override and call to
Run-Backend and move the environment assignments into a try { ... Run-Backend
-ShowFinalOutput $false } finally { restore $env:SKIP_SECURITY =
$script:ORIGINAL_SKIP_SECURITY; restore/clear API_BASE as originally saved } to
ensure no insecure state persists.
In `@build.sh`:
- Around line 96-103: The build uses PRODUCT_NAME and derives BINARY_NAME by
lowercasing PRODUCT_NAME which breaks when the display name contains spaces or
punctuation; introduce an explicit slug variable (e.g., BINARY_SLUG or
PACKAGE_SLUG) and use that for all filesystem/artifact names instead of
PRODUCT_NAME_LOWERCASE or BINARY_NAME, update PRODUCT_FOLDER to use the slug,
and ensure start.sh and setup.sh reference the new slug variable (not
PRODUCT_NAME) so display label and executable/artifact name remain independent
and stable.
---
Nitpick comments:
In `@backend/cmd/server/bootstrap/common.sh`:
- Around line 23-24: The PRODUCT_NAME variable is set but not exported, so child
processes won't see it; update the assignment of PRODUCT_NAME to be exported
(e.g., make PRODUCT_NAME available to the environment the same way API_BASE is
exported by setup.sh) by exporting PRODUCT_NAME so downstream scripts/processes
can access it; target the PRODUCT_NAME definition in this file (and mirror the
export pattern used for API_BASE in setup.sh).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 43cc065b-b16e-4112-aa16-e54f47ed1907
📒 Files selected for processing (27)
DockerfileMakefilebackend/README.mdbackend/cmd/server/bootstrap/01-default-resources.ps1backend/cmd/server/bootstrap/01-default-resources.shbackend/cmd/server/bootstrap/02-sample-resources.ps1backend/cmd/server/bootstrap/02-sample-resources.shbackend/cmd/server/bootstrap/common.ps1backend/cmd/server/bootstrap/common.shbackend/cmd/server/repository/conf/deployment.yamlbuild.ps1build.shfrontend/README.mdfrontend/pnpm-workspace.yamlsetup.ps1setup.shstart.ps1start.shtests/e2e/configs/routes/console-routes.tstests/e2e/tests/accessibility/dashboard-views.a11y.spec.tstests/e2e/tests/accessibility/sign-in-page.a11y.spec.tstests/e2e/tests/sample-app-authentication/.env.exampletests/e2e/tests/sample-app-authentication/sample-app-mfa-login.spec.tstests/e2e/utils/accessibility/index.tstests/e2e/utils/authentication/console-admin-auth-utils.tstests/e2e/utils/mock-sms-server.tstests/e2e/utils/thunder-setup/mfa-setup.ts
brionmario
force-pushed
the
refactor-thunder-rebrand
branch
from
c05c1be to
e244377
Compare
brionmario
requested review from
DonOmalVindula,
ayeshajay,
rajithacharith and
senthalan
as code owners
brionmario
force-pushed
the
refactor-thunder-rebrand
branch
from
e244377 to
1fe8883
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2478 +/- ##
=======================================
Coverage 88.79% 88.79%
=======================================
Files 872 872
Lines 60306 60306
=======================================
Hits 53550 53550
Misses 4944 4944
Partials 1812 1812
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
DonOmalVindula
approved these changes
Apr 28, 2026
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.
Purpose
This pull request focuses on standardizing the use of the product name "Thunder" across documentation, scripts, and build tooling, as well as improving maintainability by parameterizing product references. It also refactors PowerShell bootstrap scripts to use a more generic API invocation function, making the codebase more consistent and easier to update in the future.
Approach
Product Naming Standardization and Build Improvements:
DockerfileandMakefileto use variables for the product and binary names, replacing hardcoded references to "Thunder" withPRODUCT_NAMEandBINARY_NAMEwhere appropriate for easier future renaming and consistency. [1] [2] [3] [4] [5]Documentation Updates:
backend/README.mdheader and description to clearly state the backend's purpose and highlight the product branding.PowerShell Bootstrap Script Refactor:
$PRODUCT_NAMEvariable inbackend/cmd/server/bootstrap/01-default-resources.ps1and replaced all hardcoded "Thunder" references with this variable, ensuring easier rebranding and consistent messaging throughout the script. [1] [2] [3]Invoke-Apifunction instead of the product-specificInvoke-ThunderApi, making the script more maintainable and decoupled from the product name. This change was applied to all API calls in the script. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17] [18] [19] [20]Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
New Features
Documentation
Chores