refactor secrets (drop the lib)#385
Conversation
Reviewer's GuideRefactors secret handling by removing the @flyxc/secrets library and replacing it with a global SECRETS object injected at build time from secrets.env.local via webpack, updating all consumers, build configs, and tests accordingly. Sequence diagram for webpack secrets loading and SECRETS definitionsequenceDiagram
actor Dev
participant Nx as Nx
participant Webpack as Webpack
participant Config as WebpackConfig_js
participant Fs as NodeFs
participant Dotenvx as DotenvxParse
participant DefinePlugin as webpack_DefinePlugin
participant Bundle as CompiledBundle
participant Runtime as NodeRuntime
Dev->>Nx: run build app
Nx->>Webpack: invoke with config
Webpack->>Config: evaluate composePlugins(withNx, config)
Config->>Fs: existsSync(secrets.env.local)
Fs-->>Config: true
Config->>Fs: readFileSync(secrets.env.local)
Fs-->>Config: fileContents
Config->>Dotenvx: parse(fileContents)
Dotenvx-->>Config: envConfig key/value map
Config->>Config: build secrets map SECRETS.KEY = JSON.stringify(value)
Config->>DefinePlugin: new DefinePlugin(secrets)
DefinePlugin-->>Webpack: plugin instance
Webpack->>Bundle: replace SECRETS.* at compile time
Dev->>Runtime: run compiled app
Runtime->>Bundle: execute code
Bundle-->>Runtime: SECRETS global values in place of literals
Class diagram for FlyMe validation refactor using injected tokenclassDiagram
class TrackerEntity {
+flyme any
+inreach any
+skylines any
}
class TrackerModel {
+flyme any
+inreach any
+skylines any
}
class FlyMeValidator {
-currentEnabled boolean
-currentAccount string
-token string
+constructor(flyme TrackerEntity, token string)
+validate(tracker TrackerModel) boolean
+message string
}
class InreachValidator {
-currentEnabled boolean
-currentAccount string
+constructor(inreach TrackerEntity)
+validate(tracker TrackerModel) boolean
+message string
}
class FlyMeApi {
+getFlyMeId(username string, token string) Promise~string|undefined~
}
class Binder {
+model TrackerModel
+for(field any) ValidatorChain
}
class ValidatorChain {
+addValidator(validator any) void
}
class LiveTrackRoute {
+createOrUpdateLiveTrack(req Request, res Response, email string, googleId string, redis Redis) Promise~void~
}
FlyMeValidator ..> FlyMeApi : uses
LiveTrackRoute ..> FlyMeValidator : creates with SECRETS.FLYME_TOKEN
LiveTrackRoute ..> InreachValidator : creates
LiveTrackRoute --> Binder : configures validators
Binder --> TrackerModel
TrackerEntity <.. LiveTrackRoute : loads existing entity
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThis PR removes the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The logic to load
secrets.env.localand build theDefinePluginmap is duplicated across every app’swebpack.config.js; consider extracting this into a shared helper (e.g., in a small util module) so the path, parsing, and error behavior stay consistent and easier to change in one place. - Throwing an error when
secrets.env.localis missing in every webpack config will cause all builds/tests to fail even in contexts that may not require secrets (e.g., CI or local utilities); you might want to make the path configurable or allow a more graceful fallback for non‑prod environments.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The logic to load `secrets.env.local` and build the `DefinePlugin` map is duplicated across every app’s `webpack.config.js`; consider extracting this into a shared helper (e.g., in a small util module) so the path, parsing, and error behavior stay consistent and easier to change in one place.
- Throwing an error when `secrets.env.local` is missing in every webpack config will cause all builds/tests to fail even in contexts that may not require secrets (e.g., CI or local utilities); you might want to make the path configurable or allow a more graceful fallback for non‑prod environments.
## Individual Comments
### Comment 1
<location> `apps/fxc-tiles/webpack.config.js:8-17` </location>
<code_context>
module.exports = composePlugins(withNx(), (config) => {
- // Note: This was added by an Nx migration. Webpack builds are required to have a corresponding Webpack config file.
- // See: https://nx.dev/recipes/webpack/webpack-config-setup
+ const secretsPath = 'secrets.env.local';
+ /** @type {Record<string, string>} */
+ const secrets = {};
+
+ if (!fs.existsSync(secretsPath)) {
+ throw new Error(`Secrets file not found at path: ${secretsPath}`);
+ }
+
+ const envConfig = parse(fs.readFileSync(secretsPath, 'utf-8'));
+
+ for (const [key, value] of Object.entries(envConfig)) {
+ secrets[`SECRETS.${key}`] = JSON.stringify(value);
+ }
+
+ config.plugins = config.plugins || [];
+ config.plugins.push(new webpack.DefinePlugin(secrets));
return config;
});
</code_context>
<issue_to_address>
**suggestion:** The secrets loading + DefinePlugin wiring is duplicated across multiple webpack configs and could be centralized.
This secrets loading + `DefinePlugin` setup is repeated in each app’s webpack config, which makes it easy for behavior to drift between apps (different paths, parsing rules, or keys). Consider extracting it into a shared helper (e.g. `createSecretsDefinePlugin()` in a common build util) or a small custom webpack plugin so the behavior is defined once and reused.
Suggested implementation:
```javascript
const { composePlugins, withNx } = require('@nx/webpack');
const { createSecretsDefinePlugin } = require('../../tools/webpack/create-secrets-define-plugin');
// Nx plugins for webpack.
module.exports = composePlugins(withNx(), (config) => {
config.plugins = config.plugins || [];
config.plugins.push(createSecretsDefinePlugin());
return config;
});
```
To fully implement the centralization and remove duplication, you should add a shared helper (and then migrate other apps to use it):
1. Create `tools/webpack/create-secrets-define-plugin.js` (or adjust the path to match your repo conventions) with something like:
```js
const fs = require('node:fs');
const webpack = require('webpack');
const { parse } = require('@dotenvx/dotenvx');
/**
* Creates a DefinePlugin instance that exposes secrets from a dotenv-style file.
*
* @param {object} options
* @param {string} [options.path='secrets.env.local'] - Path to the secrets file, relative to workspace root.
* @returns {import('webpack').DefinePlugin}
*/
function createSecretsDefinePlugin(options = {}) {
const secretsPath = options.path ?? 'secrets.env.local';
/** @type {Record<string, string>} */
const secrets = {};
if (!fs.existsSync(secretsPath)) {
throw new Error(`Secrets file not found at path: ${secretsPath}`);
}
const envConfig = parse(fs.readFileSync(secretsPath, 'utf-8'));
for (const [key, value] of Object.entries(envConfig)) {
secrets[`SECRETS.${key}`] = JSON.stringify(value);
}
return new webpack.DefinePlugin(secrets);
}
module.exports = { createSecretsDefinePlugin };
```
2. Update any other app-specific `webpack.config.js` files that currently inline the same secrets + `DefinePlugin` logic to instead:
- Remove their direct `fs`, `webpack`, and `@dotenvx/dotenvx` imports.
- Import `createSecretsDefinePlugin` from the shared helper.
- Push `createSecretsDefinePlugin()` into `config.plugins` just like in `apps/fxc-tiles/webpack.config.js`.
3. If different apps need different secrets file paths, pass `{ path: 'custom/path.env' }` to `createSecretsDefinePlugin` where appropriate, keeping the logic centralized while allowing per-app configuration.
</issue_to_address>
### Comment 2
<location> `secrets.d.ts:1-10` </location>
<code_context>
+declare const SECRETS: {
+ GEONAMES: string;
+ REDIS_URL: string;
+ GOOGLE_OAUTH_ID: string;
+ GOOGLE_OAUTH_SECRET: string;
+ SESSION_SECRET: string;
+ FLYME_TOKEN: string;
+ ADMINS: string;
+ PROXY_KEY: string;
+ OPENAIP_KEY: string;
+ AVIANT_URL: string;
+ APRS_USER: string;
+ APRS_PASSWORD: string;
+ ZOLEO_UNLINK_URL: string;
+ ZOLEO_UNLINK_API_KEY: string;
+ ZOLEO_PUSH_USER: string;
+ ZOLEO_PUSH_PWD: string;
+ XCONTEST_JWT: string;
+ BUY_ME_A_COFFEE_TOKEN: string;
+ FLYMASTER_GROUP_ID: string;
+ FLYMASTER_GROUP_TOKEN: string;
+ MAILERSEND_TOKEN: string;
+ MESHBIR_AUTH_TOKEN: string;
+};
</code_context>
<issue_to_address>
**issue (bug_risk):** Typing all `SECRETS` fields as required can hide missing values at runtime, especially in tests.
In tests, only some of these fields are set on `SECRETS` (e.g. only `ADMINS` in `fxc-server`, `{}` in `fetcher`), so other properties will be `undefined` at runtime while typed as `string`. To avoid this mismatch, either (a) make these properties optional and enforce a safe access pattern, (b) ensure test globals stub all keys, or (c) narrow the `SECRETS` type for test builds to just the fields they actually provide.
</issue_to_address>
### Comment 3
<location> `apps/fxc-server/jest.config.ts:4-7` </location>
<code_context>
displayName: 'fetcher',
preset: '../../jest.preset.js',
- globals: {},
+ globals: {
+ SECRETS: {},
+ },
</code_context>
<issue_to_address>
**suggestion (testing):** Jest `SECRETS` stub only defines `ADMINS`, which may lead to runtime `undefined` for other secrets accessed in tests.
Several runtime paths in `fxc-server` (Redis URL, OAuth, Zoleo, etc.) now read from `SECRETS`, so in tests these will see `undefined` despite the type being `string`. Please either populate all currently used secrets with dummy values here or add a small helper for safe secret access in tests (e.g., throwing if an expected secret is missing).
Suggested implementation:
```typescript
globals: {
// Defined secrets used during tests.
// NOTE: Keep this in sync with all secrets accessed via SECRETS in fxc-server runtime code.
SECRETS: {
// existing
ADMINS: 'admin',
// Redis / cache
REDIS_URL: 'redis://localhost:6379/0',
// OAuth / Auth0 / OIDC (adjust keys to match actual usage)
OAUTH_CLIENT_ID: 'test-oauth-client-id',
OAUTH_CLIENT_SECRET: 'test-oauth-client-secret',
OAUTH_ISSUER: 'https://example.test-issuer/',
OAUTH_AUDIENCE: 'test-audience',
// Zoleo or other 3rd-party integrations
ZOLEO_API_KEY: 'test-zoleo-api-key',
ZOLEO_API_BASE_URL: 'https://api.zoleo.test',
// Miscellaneous commonly-used secrets (add/remove to match real keys)
SESSION_SECRET: 'test-session-secret',
JWT_SECRET: 'test-jwt-secret',
},
},
```
I only see the Jest config, not the full runtime code that reads from `SECRETS`. To fully implement the intent:
1. Enumerate all keys actually read from `SECRETS` in `fxc-server` (e.g., a `secrets.ts` module or any `SECRETS.X` usages).
2. Replace or augment the placeholder keys I added (`REDIS_URL`, `OAUTH_*`, `ZOLEO_*`, `SESSION_SECRET`, `JWT_SECRET`) so that this Jest stub exactly matches those keys.
3. Optionally, add a small helper in your secrets-access layer used by tests (e.g., `getSecret(key: keyof typeof SECRETS)`) which throws if a required secret is missing; this would enforce that the Jest `SECRETS` object stays in sync with runtime expectations.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const secretsPath = 'secrets.env.local'; | ||
| /** @type {Record<string, string>} */ | ||
| const secrets = {}; | ||
|
|
||
| if (!fs.existsSync(secretsPath)) { | ||
| throw new Error(`Secrets file not found at path: ${secretsPath}`); | ||
| } | ||
|
|
||
| const envConfig = parse(fs.readFileSync(secretsPath, 'utf-8')); | ||
|
|
There was a problem hiding this comment.
suggestion: The secrets loading + DefinePlugin wiring is duplicated across multiple webpack configs and could be centralized.
This secrets loading + DefinePlugin setup is repeated in each app’s webpack config, which makes it easy for behavior to drift between apps (different paths, parsing rules, or keys). Consider extracting it into a shared helper (e.g. createSecretsDefinePlugin() in a common build util) or a small custom webpack plugin so the behavior is defined once and reused.
Suggested implementation:
const { composePlugins, withNx } = require('@nx/webpack');
const { createSecretsDefinePlugin } = require('../../tools/webpack/create-secrets-define-plugin');
// Nx plugins for webpack.
module.exports = composePlugins(withNx(), (config) => {
config.plugins = config.plugins || [];
config.plugins.push(createSecretsDefinePlugin());
return config;
});To fully implement the centralization and remove duplication, you should add a shared helper (and then migrate other apps to use it):
- Create
tools/webpack/create-secrets-define-plugin.js(or adjust the path to match your repo conventions) with something like:
const fs = require('node:fs');
const webpack = require('webpack');
const { parse } = require('@dotenvx/dotenvx');
/**
* Creates a DefinePlugin instance that exposes secrets from a dotenv-style file.
*
* @param {object} options
* @param {string} [options.path='secrets.env.local'] - Path to the secrets file, relative to workspace root.
* @returns {import('webpack').DefinePlugin}
*/
function createSecretsDefinePlugin(options = {}) {
const secretsPath = options.path ?? 'secrets.env.local';
/** @type {Record<string, string>} */
const secrets = {};
if (!fs.existsSync(secretsPath)) {
throw new Error(`Secrets file not found at path: ${secretsPath}`);
}
const envConfig = parse(fs.readFileSync(secretsPath, 'utf-8'));
for (const [key, value] of Object.entries(envConfig)) {
secrets[`SECRETS.${key}`] = JSON.stringify(value);
}
return new webpack.DefinePlugin(secrets);
}
module.exports = { createSecretsDefinePlugin };-
Update any other app-specific
webpack.config.jsfiles that currently inline the same secrets +DefinePluginlogic to instead:- Remove their direct
fs,webpack, and@dotenvx/dotenvximports. - Import
createSecretsDefinePluginfrom the shared helper. - Push
createSecretsDefinePlugin()intoconfig.pluginsjust like inapps/fxc-tiles/webpack.config.js.
- Remove their direct
-
If different apps need different secrets file paths, pass
{ path: 'custom/path.env' }tocreateSecretsDefinePluginwhere appropriate, keeping the logic centralized while allowing per-app configuration.
| declare const SECRETS: { | ||
| GEONAMES: string; | ||
| REDIS_URL: string; | ||
| GOOGLE_OAUTH_ID: string; | ||
| GOOGLE_OAUTH_SECRET: string; | ||
| SESSION_SECRET: string; | ||
| FLYME_TOKEN: string; | ||
| ADMINS: string; | ||
| PROXY_KEY: string; | ||
| OPENAIP_KEY: string; |
There was a problem hiding this comment.
issue (bug_risk): Typing all SECRETS fields as required can hide missing values at runtime, especially in tests.
In tests, only some of these fields are set on SECRETS (e.g. only ADMINS in fxc-server, {} in fetcher), so other properties will be undefined at runtime while typed as string. To avoid this mismatch, either (a) make these properties optional and enforce a safe access pattern, (b) ensure test globals stub all keys, or (c) narrow the SECRETS type for test builds to just the fields they actually provide.
| globals: { | ||
| // Defined secrets used during tests. | ||
| SECRETS: { | ||
| ADMINS: 'admin', |
There was a problem hiding this comment.
suggestion (testing): Jest SECRETS stub only defines ADMINS, which may lead to runtime undefined for other secrets accessed in tests.
Several runtime paths in fxc-server (Redis URL, OAuth, Zoleo, etc.) now read from SECRETS, so in tests these will see undefined despite the type being string. Please either populate all currently used secrets with dummy values here or add a small helper for safe secret access in tests (e.g., throwing if an expected secret is missing).
Suggested implementation:
globals: {
// Defined secrets used during tests.
// NOTE: Keep this in sync with all secrets accessed via SECRETS in fxc-server runtime code.
SECRETS: {
// existing
ADMINS: 'admin',
// Redis / cache
REDIS_URL: 'redis://localhost:6379/0',
// OAuth / Auth0 / OIDC (adjust keys to match actual usage)
OAUTH_CLIENT_ID: 'test-oauth-client-id',
OAUTH_CLIENT_SECRET: 'test-oauth-client-secret',
OAUTH_ISSUER: 'https://example.test-issuer/',
OAUTH_AUDIENCE: 'test-audience',
// Zoleo or other 3rd-party integrations
ZOLEO_API_KEY: 'test-zoleo-api-key',
ZOLEO_API_BASE_URL: 'https://api.zoleo.test',
// Miscellaneous commonly-used secrets (add/remove to match real keys)
SESSION_SECRET: 'test-session-secret',
JWT_SECRET: 'test-jwt-secret',
},
},I only see the Jest config, not the full runtime code that reads from SECRETS. To fully implement the intent:
- Enumerate all keys actually read from
SECRETSinfxc-server(e.g., asecrets.tsmodule or anySECRETS.Xusages). - Replace or augment the placeholder keys I added (
REDIS_URL,OAUTH_*,ZOLEO_*,SESSION_SECRET,JWT_SECRET) so that this Jest stub exactly matches those keys. - Optionally, add a small helper in your secrets-access layer used by tests (e.g.,
getSecret(key: keyof typeof SECRETS)) which throws if a required secret is missing; this would enforce that the JestSECRETSobject stays in sync with runtime expectations.
Deploying flyxc with
|
| Latest commit: |
1e73a07
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b46e1654.flyxc.pages.dev |
| Branch Preview URL: | https://vicb-secrets.flyxc.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
apps/run/webpack.config.js (1)
8-23: Same issues as apps/proxy/webpack.config.js.This file contains identical secret-loading logic with the same issues flagged in
apps/proxy/webpack.config.js:
- Critical: Relative path issue on Line 8 - needs absolute path resolution
- Essential refactor: Code duplication across multiple webpack configs
- Recommended: Missing validation of required secret keys
- Major: Security implications of embedding secrets in bundles
Please refer to the detailed comments and proposed fixes in
apps/proxy/webpack.config.js.apps/fxc-tiles/webpack.config.js (1)
8-23: Same issues as apps/proxy/webpack.config.js.This file contains identical secret-loading logic with the same issues flagged in
apps/proxy/webpack.config.js:
- Critical: Relative path issue on Line 8 - needs absolute path resolution
- Essential refactor: Code duplication across multiple webpack configs
- Recommended: Missing validation of required secret keys
- Major: Security implications of embedding secrets in bundles
Please refer to the detailed comments and proposed fixes in
apps/proxy/webpack.config.js.
🧹 Nitpick comments (6)
apps/misc/webpack.config.js (1)
8-8: Consider using an explicit workspace-relative path.The relative path
'secrets.env.local'assumes execution from the workspace root. While this works with Nx's webpack execution context, consider usingpath.resolvefor clarity:const path = require('node:path'); const secretsPath = path.resolve(__dirname, '../../secrets.env.local');This makes the path resolution explicit and more maintainable.
apps/fxc-server/webpack.config.js (1)
9-9: Consider using an explicit workspace-relative path.The relative path
'secrets.env.local'assumes execution from the workspace root. While this works with Nx's webpack execution context, consider usingpath.resolvefor clarity:const path = require('node:path'); const secretsPath = path.resolve(__dirname, '../../secrets.env.local');This makes the path resolution explicit and more maintainable.
apps/proxy/webpack.config.js (1)
16-20: Consider validating that required secret keys are present.The code parses the secrets file but doesn't validate that expected keys exist. If a required secret is missing, the error will only surface at runtime when the code tries to access
SECRETS.SOME_KEY.🔎 Optional validation logic
const envConfig = parse(fs.readFileSync(secretsPath, 'utf-8')); +// Validate required secrets for this app +const requiredSecrets = ['ZOLEO_PUSH_USER', 'ZOLEO_PUSH_PWD', 'ZOLEO_UNLINK_URL', 'ZOLEO_UNLINK_API_KEY']; +const missingSecrets = requiredSecrets.filter(key => !(key in envConfig)); +if (missingSecrets.length > 0) { + throw new Error(`Missing required secrets: ${missingSecrets.join(', ')}`); +} + for (const [key, value] of Object.entries(envConfig)) { secrets[`SECRETS.${key}`] = JSON.stringify(value); }Note: The required secrets list would need to be customized per app or made configurable.
apps/fetcher/webpack.config.js (2)
8-14: Consider using an absolute path for reliability.The relative path
secrets.env.localdepends on the current working directory when webpack runs. If the build is invoked from a different directory, the file won't be found.🔎 Suggested improvement
+const path = require('node:path'); + const { composePlugins, withNx } = require('@nx/webpack'); const webpack = require('webpack'); const fs = require('node:fs'); const { parse } = require('@dotenvx/dotenvx'); // Nx plugins for webpack. module.exports = composePlugins(withNx(), (config) => { - const secretsPath = 'secrets.env.local'; + const secretsPath = path.resolve(__dirname, '../../secrets.env.local');
16-20: Add error handling for parse failures.If
secrets.env.localcontains malformed content,parse()may throw or return unexpected results. Consider wrapping in try-catch with a descriptive error message.🔎 Suggested improvement
- const envConfig = parse(fs.readFileSync(secretsPath, 'utf-8')); - - for (const [key, value] of Object.entries(envConfig)) { - secrets[`SECRETS.${key}`] = JSON.stringify(value); - } + let envConfig; + try { + envConfig = parse(fs.readFileSync(secretsPath, 'utf-8')); + } catch (e) { + throw new Error(`Failed to parse secrets file at ${secretsPath}: ${e.message}`); + } + + for (const [key, value] of Object.entries(envConfig)) { + secrets[`SECRETS.${key}`] = JSON.stringify(value); + }secrets.d.ts (1)
1-24: Type declaration looks good. Ensure build-time validation covers all keys.The declaration requires all 22 secrets to be present at runtime. If any key is missing from
secrets.env.local, TypeScript won't catch it at build time—you'll getundefinedat runtime.Consider adding build-time validation in the webpack config to ensure all expected keys are present:
🔎 Optional validation in webpack config
const REQUIRED_SECRETS = [ 'GEONAMES', 'REDIS_URL', 'GOOGLE_OAUTH_ID', 'GOOGLE_OAUTH_SECRET', 'SESSION_SECRET', 'FLYME_TOKEN', 'ADMINS', 'PROXY_KEY', 'OPENAIP_KEY', // ... etc ]; const missingSecrets = REQUIRED_SECRETS.filter(key => !(key in envConfig)); if (missingSecrets.length > 0) { throw new Error(`Missing required secrets: ${missingSecrets.join(', ')}`); }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
libs/secrets/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (55)
.gitignoreapps/fetcher/jest.config.tsapps/fetcher/project.jsonapps/fetcher/src/app/trackers/flymaster.tsapps/fetcher/src/app/trackers/flyme.tsapps/fetcher/src/app/trackers/inreach.tsapps/fetcher/src/app/trackers/refresh.tsapps/fetcher/src/app/trackers/xcontest.tsapps/fetcher/src/app/ufos/aviant.tsapps/fetcher/src/fetcher.tsapps/fetcher/tsconfig.app.jsonapps/fetcher/tsconfig.spec.jsonapps/fetcher/webpack.config.jsapps/fxc-server/jest.config.tsapps/fxc-server/project.jsonapps/fxc-server/src/app/routes/live-track.tsapps/fxc-server/src/app/routes/meshbir.tsapps/fxc-server/src/app/routes/session.tsapps/fxc-server/src/app/routes/zoleo.tsapps/fxc-server/src/main.tsapps/fxc-server/tsconfig.app.jsonapps/fxc-server/tsconfig.spec.jsonapps/fxc-server/webpack.config.jsapps/fxc-tiles/project.jsonapps/fxc-tiles/src/app/airspaces/download-openaip.tsapps/fxc-tiles/tsconfig.app.jsonapps/fxc-tiles/webpack.config.jsapps/misc/project.jsonapps/misc/src/app/email_inreach.tsapps/misc/tsconfig.app.jsonapps/misc/webpack.config.jsapps/proxy/project.jsonapps/proxy/src/index.tsapps/proxy/tsconfig.app.jsonapps/proxy/webpack.config.jsapps/run/project.jsonapps/run/src/app/process.tsapps/run/tsconfig.app.jsonapps/run/webpack.config.jslibs/common-node/src/lib/validators.tslibs/secrets/.gitignorelibs/secrets/README.mdlibs/secrets/eslint.config.jslibs/secrets/package.jsonlibs/secrets/project.jsonlibs/secrets/src/index.tslibs/secrets/src/lib/secrets.tslibs/secrets/tsconfig.jsonlibs/secrets/tsconfig.lib.jsonlibs/secrets/webpack.config.jsnx.jsonpackage.jsonrollup.build.csssecrets.d.tssecrets.env
💤 Files with no reviewable changes (16)
- libs/secrets/.gitignore
- apps/run/project.json
- rollup.build.css
- apps/fetcher/project.json
- apps/misc/project.json
- libs/secrets/eslint.config.js
- libs/secrets/package.json
- libs/secrets/src/index.ts
- libs/secrets/README.md
- libs/secrets/src/lib/secrets.ts
- libs/secrets/webpack.config.js
- libs/secrets/project.json
- apps/fxc-tiles/project.json
- apps/fxc-server/project.json
- libs/secrets/tsconfig.lib.json
- libs/secrets/tsconfig.json
🧰 Additional context used
🧬 Code graph analysis (7)
apps/proxy/webpack.config.js (1)
apps/fetcher/webpack.config.js (7)
webpack(2-2)require(1-1)require(4-4)fs(3-3)secretsPath(8-8)secrets(10-10)envConfig(16-16)
apps/fxc-server/src/main.ts (1)
libs/common-node/src/lib/redis.ts (1)
getRedisClient(7-13)
apps/fetcher/src/app/ufos/aviant.ts (1)
libs/common/src/lib/fetch-timeout.ts (1)
fetchResponse(20-89)
apps/fetcher/src/app/trackers/refresh.ts (1)
apps/fetcher/src/app/trackers/ogn-client.ts (3)
OgnClient(22-152)OGN_HOST(19-19)OGN_PORT(20-20)
libs/common-node/src/lib/validators.ts (2)
apps/fxc-front/src/app/components/2d/topo-elements.ts (1)
url(34-40)libs/common/src/lib/live-track-entity.ts (1)
TrackerEntity(3-11)
apps/fetcher/src/fetcher.ts (1)
libs/common-node/src/lib/redis.ts (1)
getRedisClient(7-13)
apps/fxc-server/src/app/routes/live-track.ts (3)
libs/common-node/src/lib/validators.ts (1)
FlyMeValidator(75-107)libs/common-node/src/index.ts (1)
FlyMeValidator(7-7)libs/common-node/src/lib/live-track-entity.ts (1)
updateLiveTrackEntityFromModel(27-65)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Sourcery review
- GitHub Check: Analyze (javascript)
- GitHub Check: build (22.x)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (42)
apps/fetcher/tsconfig.spec.json (1)
9-9: LGTM!Including
secrets.d.tsin the test configuration ensures type safety for theSECRETSglobal in test files..gitignore (1)
51-51: LGTM!Adding
*.env.localto.gitignoreis essential to prevent accidental commits of local secret files, which is critical for the new build-time secret injection approach.apps/misc/tsconfig.app.json (1)
9-9: LGTM!Including
secrets.d.tsensures TypeScript type checking for theSECRETSglobal in application code.apps/proxy/tsconfig.app.json (1)
9-9: LGTM!Including
secrets.d.tsprovides type safety for theSECRETSglobal in the proxy app.apps/fetcher/jest.config.ts (1)
4-6: No action needed on SECRETS configuration.The tests only import and test pure parsing functions (
parseLiveTrack,parseLiveUsers,parse) with JSON/KML fixtures. These functions don't access SECRETS. The modules that do access SECRETS properties (e.g., in the fetching/network logic) are not imported by tests, so the empty SECRETS object has no impact on test behavior or failures.Likely an incorrect or invalid review comment.
nx.json (1)
52-52: LGTM! Proper cache invalidation for secrets.Adding
secrets.env.localtosharedGlobalsensures that changes to the secrets file will properly invalidate the Nx cache and trigger rebuilds. This is essential for the build-time secret injection approach.apps/fetcher/tsconfig.app.json (1)
9-9: LGTM! Consistent TypeScript configuration for SECRETS.The addition of
../../secrets.d.tsto the include array is consistent with the refactoring pattern across the codebase, enabling TypeScript to recognize the globally injected SECRETS constant in application code.package.json (2)
6-6: LGTM! Aligns with secrets library removal.Removing the
nx build secrets &&prefix from the dev script is correct, as the PR removes the standalone secrets library in favor of build-time secret injection via Webpack DefinePlugin.
98-98: No action needed.@dotenvx/dotenvxappears only independencies(version ^1.51.4) and is not duplicated indevDependencies.Likely an incorrect or invalid review comment.
apps/fxc-server/tsconfig.spec.json (1)
8-8: File exists with proper TypeScript declarations.The
secrets.d.tsfile exists at the workspace root and contains comprehensive type declarations for the SECRETS constant, including all required environment variables (GEONAMES, REDIS_URL, GOOGLE_OAUTH_ID, GOOGLE_OAUTH_SECRET, SESSION_SECRET, and others). The reference intsconfig.spec.jsonis correctly configured.apps/proxy/project.json (1)
15-15: Entry point migration verified and correct.The change from
apps/proxy/src/main.tstoapps/proxy/src/index.tshas been properly executed:
index.tsexists with complete server implementationmain.tshas been removedproject.jsoncorrectly configured with the new entry pointapps/run/tsconfig.app.json (1)
9-9: LGTM: TypeScript configuration updated for SECRETS global.Adding
../../secrets.d.tsto the include array enables TypeScript to recognize theSECRETSglobal type, which is essential for type safety with the new secrets pattern.secrets.env (1)
17-17: Typo fix confirmed: XCONTEXT_JWT → XCONTEST_JWTThe rename correctly reflects "XContest" (the paragliding competition platform). Verification confirms no lingering references to the old misspelling exist and the new naming is consistently used in the type definitions (
secrets.d.ts:18) and API calls (xcontest.ts:53, 105).apps/fetcher/src/app/trackers/flyme.ts (1)
28-28: LGTM: Token source migrated to SECRETS global.The change from
Secrets.FLYME_TOKENtoSECRETS.FLYME_TOKENis consistent with the PR objective to remove the secrets library.SECRETS.FLYME_TOKENis properly declared insecrets.d.tsand typed as a string.apps/proxy/src/index.ts (1)
10-10: LGTM: Proxy key migrated to SECRETS global.The change from
Secrets.PROXY_KEYtoSECRETS.PROXY_KEYis correct and consistent with the codebase architecture:
SECRETS.PROXY_KEYis properly declared insecrets.d.tsas a string property- Build-time injection is correctly configured in
apps/proxy/webpack.config.jsvia webpack's DefinePlugin, which reads fromsecrets.env.localand injects all environment variables asSECRETS.*global constantsapps/fetcher/src/app/trackers/refresh.ts (1)
29-29: LGTM: OGN credentials migrated to SECRETS global.The change from
Secrets.APRS_USERandSecrets.APRS_PASSWORDtoSECRETS.APRS_USERandSECRETS.APRS_PASSWORDis correct. Both properties are properly declared insecrets.d.tsand are injected at build time via webpack configuration.apps/fxc-server/src/app/routes/meshbir.ts (1)
15-15: Verify authentication still works after migration.The migration from
Secrets.MESHBIR_AUTH_TOKENtoSECRETS.MESHBIR_AUTH_TOKENis reflected at line 15. Ensure all old import references have been removed across the codebase and bearer token authentication is tested.apps/fxc-server/src/app/routes/session.ts (1)
3-3: Migration to SECRETS.ADMINS is complete and properly configured.The refactoring from old imports to
SECRETS.ADMINShas been successfully applied. Type definitions include theADMINSfield, no legacy code remains, and the build-time configuration is in place.apps/fetcher/src/app/ufos/aviant.ts (1)
19-19: LGTM! SECRETS configuration verified.The migration from
Secrets.AVIANT_URLtoSECRETS.AVIANT_URLis complete. No remaining old-style imports from@flyxc/secretsare present, andAVIANT_URLis properly defined in the type declarations as part of the globally injectedSECRETSobject.apps/fetcher/src/app/trackers/inreach.ts (1)
77-77: LGTM! Migration to globally injected SECRETS is complete and correct.The migration from
Secrets.PROXY_KEYtoSECRETS.PROXY_KEYat line 77 is consistent with the PR's objective to use globally injected secrets. The globalSECRETStype is properly defined insecrets.d.tswithPROXY_KEY: string. No remaining instances of the oldSecrets.PROXY_KEYpattern or@flyxc/secretsimports exist in the codebase. The proxy authentication logic remains unchanged.apps/fxc-tiles/src/app/airspaces/download-openaip.ts (1)
16-16: LGTM! SECRETS properly injected via webpack DefinePlugin.The migration from
Secrets.OPENAIP_KEYtoSECRETS.OPENAIP_KEYis correctly implemented. SECRETS is properly defined insecrets.d.ts(includingOPENAIP_KEY: string) and injected via webpack's DefinePlugin in the fxc-tiles build configuration, which reads fromsecrets.env.localat build time. No legacy references remain in the codebase.apps/fxc-server/src/main.ts (3)
51-51: LGTM!The migration to
SECRETS.SESSION_SECRETis correct and consistent with the refactoring approach.
78-79: LGTM!The Google OAuth credentials migration is correct and follows the same pattern as other changes in this file.
19-19: SECRETS.REDIS_URL is properly typed and injected.All verifications confirm the migration from
Secrets.REDIS_URLtoSECRETS.REDIS_URLis correctly implemented:
- The
SECRETStype definition includesREDIS_URL: stringinsecrets.d.ts- The webpack DefinePlugin in
fxc-server/webpack.config.jsproperly injects secrets fromsecrets.env.localwith the formatSECRETS.${key}- The
getRedisClientfunction expects astringparameter, which matches the injected value- No old Secrets imports remain in the file
The code change requires no modifications.
apps/fxc-server/tsconfig.app.json (1)
9-9: LGTM!Adding
secrets.d.tsto the TypeScript include array is necessary for the compiler to recognize the globalSECRETStype declaration. The path../../secrets.d.tscorrectly points to the root-level type definition file.apps/fxc-tiles/tsconfig.app.json (1)
9-9: LGTM!The addition of
secrets.d.tsto the include array is correct and consistent with the other TypeScript configuration changes in this PR.apps/fetcher/src/app/trackers/flymaster.ts (1)
54-56: Flymaster credentials properly defined in secrets type.SECRETS.FLYMASTER_GROUP_ID and SECRETS.FLYMASTER_GROUP_TOKEN are correctly defined in secrets.d.ts (lines 20-21) and the migration to the SECRETS constant is complete in this file. No issues found.
apps/misc/src/app/email_inreach.ts (1)
17-17: Code change is correct.The migration is properly done.
SECRETS.MAILERSEND_TOKENis correctly defined insecrets.d.ts(line 22) and used on line 17 without any old Secrets imports.apps/fetcher/src/app/trackers/xcontest.ts (1)
53-53: LGTM!The migration from
Secrets.XCONTEST_JWTtoSECRETS.XCONTEST_JWTis correct. Both Authorization headers now reference the globally injected secret.Also applies to: 105-105
apps/misc/webpack.config.js (2)
12-14: LGTM!Good error handling—throwing a descriptive error when the secrets file is missing ensures that builds fail fast with a clear message.
18-20: LGTM!Correctly constructs the DefinePlugin replacements using
JSON.stringify, which ensures proper string escaping for the injected values.apps/fetcher/src/fetcher.ts (1)
34-34: LGTM!The migration from
Secrets.REDIS_URLtoSECRETS.REDIS_URLcorrectly references the globally injected secret for Redis client initialization.apps/run/src/app/process.ts (1)
81-81: LGTM!The migration from
Secrets.GEONAMEStoSECRETS.GEONAMEScorrectly references the globally injected secret for the Geonames API username.apps/fxc-server/webpack.config.js (3)
13-15: LGTM!Good error handling—throwing a descriptive error when the secrets file is missing ensures that builds fail fast with a clear message.
19-21: LGTM!Correctly constructs the DefinePlugin replacements using
JSON.stringify, which ensures proper string escaping for the injected values.
26-36: LGTM!The existing TerserPlugin optimization configuration is correctly preserved, and the new DefinePlugin is added without interfering with the minimization logic.
apps/fxc-server/src/app/routes/zoleo.ts (1)
22-22: LGTM - Zoleo secrets migration is complete and correct.All Zoleo-related keys are properly defined in the SECRETS type (ZOLEO_PUSH_USER, ZOLEO_PUSH_PWD, ZOLEO_UNLINK_URL, ZOLEO_UNLINK_API_KEY). The old Secrets import has been removed, and SECRETS is used consistently across authentication (line 22), unlink URL formation (line 109), and API key header (line 113). No old Secrets usage remains.
apps/fxc-server/jest.config.ts (1)
4-9: LGTM - SECRETS.ADMINS test value is correctly configured.Verification confirms that
SECRETS.ADMINSis used inapps/fxc-server/src/app/routes/session.tswhere it's split as a comma-separated string. The Jest globals setting of'admin'aligns with the type definition insecrets.d.tsand works correctly for testing purposes.apps/proxy/webpack.config.js (1)
2-23: This concern does not apply to Node.js server-side applications.All apps using DefinePlugin to embed secrets (apps/proxy, apps/run, apps/fetcher, apps/fxc-server, apps/misc, and apps/fxc-tiles) are configured with
"target": "node"and are Node.js servers, not client-side bundles served to browsers. The secrets embedded in these server-side bundles are not exposed to end users. Additionally, secret files are properly excluded from version control via the.gitignorerule*.env.local.Likely an incorrect or invalid review comment.
libs/common-node/src/lib/validators.ts (1)
50-51: Good refactoring to dependency injection pattern.Passing the token as a constructor parameter instead of importing from a global makes the validator more testable and decouples it from the secrets infrastructure. The implementation correctly threads the token through
getFlyMeId.Also applies to: 80-80, 100-100
apps/fxc-server/src/app/routes/live-track.ts (2)
139-139: Good parameter rename for clarity.Renaming
tokentogoogleIdmakes the intent clearer and distinguishes it from API tokens likeFLYME_TOKEN. The change aligns with the parameter name inupdateLiveTrackEntityFromModel.Also applies to: 166-166
32-32: No action needed. The TypeScript configuration for fxc-server already explicitly includes../../secrets.d.tsin bothtsconfig.app.jsonandtsconfig.spec.json, ensuring the ambient declaration is properly available.Likely an incorrect or invalid review comment.
| const secretsPath = 'secrets.env.local'; | ||
| /** @type {Record<string, string>} */ | ||
| const secrets = {}; | ||
|
|
||
| if (!fs.existsSync(secretsPath)) { | ||
| throw new Error(`Secrets file not found at path: ${secretsPath}`); | ||
| } | ||
|
|
||
| const envConfig = parse(fs.readFileSync(secretsPath, 'utf-8')); | ||
|
|
||
| for (const [key, value] of Object.entries(envConfig)) { | ||
| secrets[`SECRETS.${key}`] = JSON.stringify(value); | ||
| } | ||
|
|
||
| config.plugins = config.plugins || []; | ||
| config.plugins.push(new webpack.DefinePlugin(secrets)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract duplicated secret-loading logic into a shared utility.
This exact secret-loading pattern (20+ lines) is duplicated across multiple webpack configs (apps/proxy, apps/run, apps/fxc-tiles, and based on the AI summary, also apps/fetcher, apps/fxc-server, apps/misc). This violates the DRY principle and makes maintenance difficult.
🔎 Suggested refactor
Create a shared utility file (e.g., tools/webpack/secrets-loader.js):
const webpack = require('webpack');
const fs = require('node:fs');
const path = require('node:path');
const { parse } = require('@dotenvx/dotenvx');
/**
* Loads secrets from secrets.env.local and returns a DefinePlugin configured with them.
* @param {string} rootDir - The repository root directory
* @returns {webpack.DefinePlugin}
*/
function createSecretsPlugin(rootDir) {
const secretsPath = path.join(rootDir, 'secrets.env.local');
if (!fs.existsSync(secretsPath)) {
throw new Error(`Secrets file not found at path: ${secretsPath}`);
}
const envConfig = parse(fs.readFileSync(secretsPath, 'utf-8'));
const secrets = {};
for (const [key, value] of Object.entries(envConfig)) {
secrets[`SECRETS.${key}`] = JSON.stringify(value);
}
return new webpack.DefinePlugin(secrets);
}
module.exports = { createSecretsPlugin };Then update this config to:
+const path = require('node:path');
const { composePlugins, withNx } = require('@nx/webpack');
-const webpack = require('webpack');
-const fs = require('node:fs');
-const { parse } = require('@dotenvx/dotenvx');
+const { createSecretsPlugin } = require('../../tools/webpack/secrets-loader');
// Nx plugins for webpack.
module.exports = composePlugins(withNx(), (config) => {
- const secretsPath = 'secrets.env.local';
- /** @type {Record<string, string>} */
- const secrets = {};
-
- if (!fs.existsSync(secretsPath)) {
- throw new Error(`Secrets file not found at path: ${secretsPath}`);
- }
-
- const envConfig = parse(fs.readFileSync(secretsPath, 'utf-8'));
-
- for (const [key, value] of Object.entries(envConfig)) {
- secrets[`SECRETS.${key}`] = JSON.stringify(value);
- }
-
config.plugins = config.plugins || [];
- config.plugins.push(new webpack.DefinePlugin(secrets));
+ config.plugins.push(createSecretsPlugin(path.join(__dirname, '..', '..')));
return config;
});Committable suggestion skipped: line range outside the PR's diff.
| module.exports = composePlugins(withNx(), (config) => { | ||
| // Note: This was added by an Nx migration. Webpack builds are required to have a corresponding Webpack config file. | ||
| // See: https://nx.dev/recipes/webpack/webpack-config-setup | ||
| const secretsPath = 'secrets.env.local'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify the secrets file path relative to webpack configs
# Find all webpack configs that reference secrets.env.local
echo "=== Webpack configs referencing secrets.env.local ==="
rg -n "secrets\.env\.local" --type=js -g "*webpack.config.js"
# Check if secrets.env.local exists at repository root
echo -e "\n=== Checking if secrets.env.local exists at root ==="
if [ -f "secrets.env.local" ]; then
echo "✓ secrets.env.local exists at repository root"
else
echo "✗ secrets.env.local NOT found at repository root"
fi
# Check the location of webpack config files
echo -e "\n=== Webpack config locations ==="
fd "webpack.config.js" --type f --exec echo {}Repository: vicb/flyXC
Length of output: 890
🏁 Script executed:
cat -n apps/proxy/webpack.config.jsRepository: vicb/flyXC
Length of output: 1009
🏁 Script executed:
head -60 package.jsonRepository: vicb/flyXC
Length of output: 1966
🏁 Script executed:
# Search for any secrets or env files
echo "=== Looking for secrets or env files ==="
find . -name "*secrets*" -o -name "*.env*" | head -20
echo -e "\n=== Checking Nx project configuration for proxy app ==="
if [ -f "apps/proxy/project.json" ]; then
cat apps/proxy/project.json
else
echo "No project.json found in apps/proxy"
fiRepository: vicb/flyXC
Length of output: 1905
🏁 Script executed:
echo "=== Checking .gitignore for secrets files ==="
rg "secrets" .gitignore
echo -e "\n=== Contents of secrets.env (first 20 lines) ==="
head -20 secrets.env 2>/dev/null || echo "secrets.env not readable or empty"
echo -e "\n=== Checking if secrets.env.local should be created ==="
rg -i "secrets\.env\.local" --type=md --type=txt
echo -e "\n=== Check for documentation about setup ==="
ls -la | grep -i readme
cat README.md | head -100 2>/dev/null | grep -i secret || echo "No secrets reference in README"Repository: vicb/flyXC
Length of output: 795
🏁 Script executed:
echo "=== Check all webpack configs for secrets.env handling ==="
for file in $(fd "webpack.config.js" --type f); do
echo -e "\n=== $file ==="
sed -n '8,20p' "$file"
done
echo -e "\n=== Check if there's .env.local anywhere ==="
find . -name ".env.local" 2>/dev/null
echo -e "\n=== Check git log for secrets.env.local references ==="
git log --oneline --all --grep="secrets" -i 2>/dev/null | head -5 || echo "Git history not available"
echo -e "\n=== Check if this is actually used in tests/CI ==="
rg "secrets\.env" --type=yml --type=yaml -A 2 -B 2Repository: vicb/flyXC
Length of output: 3057
🏁 Script executed:
echo "=== Git history of webpack configs ==="
git log --oneline -10 -- apps/proxy/webpack.config.js 2>/dev/null | head -10 || echo "Git history unavailable"
echo -e "\n=== Check if build actually requires the file or has optional logic ==="
rg "try.*fs.existsSync|catch.*secretsPath" -A 3 -B 3
echo -e "\n=== Look at the actual error message when file is missing ==="
rg "Secrets file not found" -B 5 -A 2
echo -e "\n=== Check if webpack build skips or makes secrets optional ==="
rg "secretsPath\|secrets\[" apps/proxy/webpack.config.js -B 2 -A 2
echo -e "\n=== Check if there's any documentation about setup in docs or .github ==="
find . -path ./.git -prune -o -type f \( -name "*.md" -o -name "*.txt" \) -print | xargs grep -l "secrets.env" 2>/dev/null | head -5Repository: vicb/flyXC
Length of output: 3192
🏁 Script executed:
echo "=== Check for CI/build workflows ==="
find . -path ./.git -prune -o -name "*.yaml" -o -name "*.yml" | grep -E "(github|gitlab|ci)" | head -10
echo -e "\n=== Check root directory structure ==="
ls -la | grep -E "^-" | awk '{print $9}'
echo -e "\n=== Check if secrets.env.local is supposed to exist ==="
git ls-files "*secrets.env*" 2>/dev/null
echo -e "\n=== Check the actual git commit that changed this ==="
git show 1e73a07 --stat 2>/dev/null | head -30Repository: vicb/flyXC
Length of output: 2333
🏁 Script executed:
echo "=== Full diff of the secrets refactor commit ==="
git show 1e73a07 -- "*webpack.config.js" 2>/dev/null | grep -A 20 -B 5 "secretsPath" | head -80
echo -e "\n=== Check if the commit mentions .local vs .env ==="
git log -1 --format="%B" 1e73a07 2>/dev/null
echo -e "\n=== Check if there's any document explaining the secrets setup ==="
find . -type f \( -name "CONTRIBUTING.md" -o -name "SETUP.md" -o -name "DEVELOPMENT.md" \) -exec grep -l "secret" {} \;Repository: vicb/flyXC
Length of output: 2744
🏁 Script executed:
echo "=== Check Nx configuration for working directory ==="
cat nx.json | grep -A 10 -B 2 "workspaceLayout\|tasksRunnerOptions" || echo "No explicit cwd config in nx.json"
echo -e "\n=== Check if secrets.env.local should be created as a setup step ==="
cat .github/workflows/node.js.yml
echo -e "\n=== Summary: Check what files actually exist at repo root ==="
ls -la secrets* 2>/dev/null || echo "No secrets files found at root"
echo -e "\n=== Verify the issue: path.join vs relative path behavior ==="
python3 << 'EOF'
import os
# Simulate the two approaches
print("=== Scenario 1: Using relative path 'secrets.env.local' ===")
print(f"If cwd is /repo:")
print(f" fs.readFileSync('secrets.env.local') -> /repo/secrets.env.local")
print(f"If cwd is /repo/apps/proxy:")
print(f" fs.readFileSync('secrets.env.local') -> /repo/apps/proxy/secrets.env.local")
print()
print("=== Scenario 2: Using path.join(__dirname, '..', '..', 'secrets.env.local') ===")
dirname = "/repo/apps/proxy"
resolved = os.path.join(dirname, '..', '..', 'secrets.env.local')
normalized = os.path.normpath(resolved)
print(f"If __dirname is {dirname}:")
print(f" path.join result -> {normalized}")
EOFRepository: vicb/flyXC
Length of output: 140
🏁 Script executed:
head -100 .github/workflows/node.js.ymlRepository: vicb/flyXC
Length of output: 784
🏁 Script executed:
echo "=== Check if build is tested in CI ==="
grep -r "build\|webpack" .github/workflows/node.js.yml
echo -e "\n=== Check if any app actually uses these webpack configs ==="
rg "webpackConfig" apps/ --type json
echo -e "\n=== Confirm: secrets.env.local should exist ==="
echo "File status:"
test -f secrets.env.local && echo "✓ secrets.env.local EXISTS" || echo "✗ secrets.env.local DOES NOT EXIST"
test -f secrets.env && echo "✓ secrets.env EXISTS" || echo "✗ secrets.env DOES NOT EXIST"Repository: vicb/flyXC
Length of output: 778
🏁 Script executed:
cat CONTRIBUTING.md | grep -A 10 -B 5 -i "secret\|env\|setup" || echo "No setup instructions found"Repository: vicb/flyXC
Length of output: 931
Use path.join(__dirname, ...) to make the secrets file path independent of the working directory.
The hardcoded relative path 'secrets.env.local' is resolved relative to the current working directory when webpack runs, not relative to this config file's location. If webpack is invoked from a different directory (e.g., from a parent workspace or CI environment), the path lookup will fail.
Recommended fix
+const path = require('node:path');
const { composePlugins, withNx } = require('@nx/webpack');
const webpack = require('webpack');
const fs = require('node:fs');
const { parse } = require('@dotenvx/dotenvx');
// Nx plugins for webpack.
module.exports = composePlugins(withNx(), (config) => {
- const secretsPath = 'secrets.env.local';
+ const secretsPath = path.join(__dirname, '..', '..', 'secrets.env.local');
/** @type {Record<string, string>} */
const secrets = {};Note: This pattern exists in all 6 webpack configs (proxy, fxc-server, fxc-tiles, fetcher, misc, run).
🤖 Prompt for AI Agents
In apps/proxy/webpack.config.js around line 8 the secrets path is hardcoded as
'secrets.env.local', which is resolved relative to the current working directory
and can fail in CI or when webpack is invoked from another directory; replace
the hardcoded string with a path constructed using path.join(__dirname,
'secrets.env.local') (ensure you require/import Node's path at the top if not
present) so the config always resolves the file relative to this config file's
location; apply the same change to the other webpack configs (fxc-server,
fxc-tiles, fetcher, misc, run).
| "@vitejs/plugin-react": "^4.3.4", | ||
| "@vitest/coverage-v8": "^1.0.4", | ||
| "@vitest/ui": "^1.3.1", | ||
| "baseline-browser-mapping": "^2.9.11", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
baseline-browser-mapping npm package
💡 Result:
baseline-browser-mapping — brief summary
- What it is: An npm package (by the W3C WebDX Community Group) that provides Baseline "widely available" and Baseline-year browser-version mappings and exports getCompatibleVersions() and getAllVersions() (also a CLI). (npmjs.com)
- Install: npm i --save-dev baseline-browser-mapping (or npm i baseline-browser-mapping). (npmjs.com)
- Key API:
- getCompatibleVersions(options) — returns minimum browser versions for Baseline Widely available / target year.
- getAllVersions(options) — returns data for all browser versions (array/object/csv). (npmjs.com)
- Latest published info (from npm at time of lookup): version 2.7.4, last published 3 days ago; weekly downloads ~12k. (npmjs.com)
- Source / docs: GitHub repo web-platform-dx/baseline-browser-mapping. (github.com)
If you want, I can show example code using getCompatibleVersions() or fetch the package JSON/README for a specific version.
Citations:
- 1: https://www.npmjs.com/package/baseline-browser-mapping?utm_source=openai
- 2: https://www.npmjs.com/package/baseline-browser-mapping?utm_source=openai
- 3: https://www.npmjs.com/package/baseline-browser-mapping?utm_source=openai
- 4: https://www.npmjs.com/package/baseline-browser-mapping?utm_source=openai
- 5: https://github.com/web-platform-dx/baseline-browser-mapping?utm_source=openai
🏁 Script executed:
# Search for usage of baseline-browser-mapping in the codebase
rg "baseline-browser-mapping" --type json --type js --type ts --type tsx
# Check git history for this dependency if available
git log -p --follow -S "baseline-browser-mapping" -- package.json 2>/dev/null | head -100Repository: vicb/flyXC
Length of output: 81
🏁 Script executed:
# Search for baseline-browser-mapping usage across all files
rg "baseline-browser-mapping" --type-add 'json:*.json' -t json -t js -t ts
# Also check for any imports or requires
rg "from.*baseline-browser-mapping|require.*baseline-browser-mapping" --type-add 'json:*.json' -t js -t ts
# Get context around the package.json entry
cat -n package.json | sed -n '50,70p'Repository: vicb/flyXC
Length of output: 1404
🏁 Script executed:
# Search for actual imports or usage of baseline-browser-mapping in source code
rg "baseline-browser-mapping" --type-add 'json:*.json' -t js -t ts --exclude-dir=node_modules --exclude=package.json --exclude=package-lock.json
# Also check if it's used as a script or referenced in config files
rg "baseline-browser-mapping" --exclude-dir=node_modules
# Check if there's a build script or npm script using it
cat -n package.json | grep -A 5 -B 5 "scripts"Repository: vicb/flyXC
Length of output: 1331
🏁 Script executed:
# Search for baseline-browser-mapping usage across the codebase (excluding node_modules)
rg "baseline-browser-mapping" --glob "!node_modules" --glob "!package-lock.json"
# Check if there's a script or build configuration that uses it
fd "baseline" --type fRepository: vicb/flyXC
Length of output: 109
Remove baseline-browser-mapping or clarify its intended purpose.
The baseline-browser-mapping package (v2.9.11) is added as a devDependency but is not imported or used anywhere in the codebase. If this dependency is not needed, remove it to reduce bundle bloat. If it is intentional (e.g., for future use or tooling), add a comment in the PR explaining its purpose.
🤖 Prompt for AI Agents
In package.json around line 58, the devDependency "baseline-browser-mapping":
"^2.9.11" is present but not used; either remove the entry from devDependencies
or annotate the PR explaining why it must remain (e.g., future tooling, pinned
baseline for browser tests). To fix: if unused, delete the package.json
devDependency line and run npm/yarn install to update lockfile; if intentional,
keep the entry and add a brief PR comment describing its purpose and where/when
it will be used.
1 participant
Summary by Sourcery
Inline environment secret handling across apps using a shared global SECRETS definition and remove the dedicated secrets library and its Nx dependencies.
Enhancements:
Build:
Chores:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.