Skip to content

Commit

Permalink
ESLint rule: process-env-computed (#8612)
Browse files Browse the repository at this point in the history
* eslint-plugin-redwood

* Better typechecking

* Shorter plugin name

* Change what node we look at

* Move to TS

* pin package versions

* dedupe

* Add missing file

* WIP: tests

* remove babel from build

* style: clarify comment on metafile

* Add testing

* Much improved test output

* update lockfile

* eslint-config: Add rwjs/eslint-plugin as dep

* Avoid default exports

* Fix process.env accessors

* Disable rule in tests

* Update esbuild

---------

Co-authored-by: Dominic Saadi <[email protected]>
  • Loading branch information
Tobbe and jtoar authored Jun 15, 2023
1 parent 79447d0 commit 7413755
Show file tree
Hide file tree
Showing 15 changed files with 245 additions and 16 deletions.
11 changes: 11 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,5 +171,16 @@ module.exports = {
],
},
},
// Allow computed member access on process.env in NodeJS contexts and tests
{
files: [
'packages/core/config/webpack.common.js',
'packages/testing/**',
'packages/vite/src/index.ts',
],
rules: {
'@redwoodjs/process-env-computed': 'off',
},
},
],
}
2 changes: 1 addition & 1 deletion packages/api/src/auth/verifiers/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export type SupportedVerifiers =

export type SupportedVerifierTypes = keyof typeof verifierLookup

export const DEFAULT_WEBHOOK_SECRET = process.env['WEBHOOK_SECRET'] ?? ''
export const DEFAULT_WEBHOOK_SECRET = process.env.WEBHOOK_SECRET ?? ''

export const VERIFICATION_ERROR_MESSAGE =
"You don't have access to invoke this function."
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/commands/deploy/__tests__/baremetal.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,8 @@ describe('parseConfig', () => {
// No substitution should work
expect(server.privateKeyPath).toEqual('/Users/me/.ssh/id_rsa')

delete process.env['TEST_VAR_HOST']
delete process.env['TEST_VAR_REPO']
delete process.env.TEST_VAR_HOST
delete process.env.TEST_VAR_REPO
})
})

Expand Down
2 changes: 1 addition & 1 deletion packages/codemods/src/testUtils/matchFolderTransform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,5 +116,5 @@ export const matchFolderTransform: MatchFolderTransformFunction = async (
expect(actualPath).toMatchFileContents(expectedPath, { removeWhitespace })
})

delete process.env['RWJS_CWD']
delete process.env.RWJS_CWD
}
10 changes: 5 additions & 5 deletions packages/core/config/__tests__/webpack.common.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,18 @@ describe('getEnvVars', () => {
beforeEach(() => {})

it('REDWOOD_ENV_ is filtered and transformed', () => {
process.env['REDWOOD_ENV_TEST'] = 1234
process.env['REDWOOD_X'] = false
process.env.REDWOOD_ENV_TEST = 1234
process.env.REDWOOD_X = false
expect(getEnvVars()).toEqual({ 'process.env.REDWOOD_ENV_TEST': '"1234"' })

delete process.env.REDWOOD_ENV_TEST
delete process.env.REDWOOD_X
})

it('transforms and passes env vars defined in `redwood.toml`', () => {
process.env['API_KEY'] = 'dog'
process.env['API_SECRET'] = 'cat'
process.env['API_SECRET2'] = 'chicken'
process.env.API_KEY = 'dog'
process.env.API_SECRET = 'cat'
process.env.API_SECRET2 = 'chicken'
expect(getEnvVars()).toEqual({
'process.env.API_KEY': '"dog"',
'process.env.API_SECRET': '"cat"',
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-config/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"@babel/core": "7.22.5",
"@babel/eslint-parser": "7.22.5",
"@babel/eslint-plugin": "7.22.5",
"@redwoodjs/eslint-plugin": "5.0.0",
"@redwoodjs/internal": "5.0.0",
"@redwoodjs/project-config": "5.0.0",
"@typescript-eslint/eslint-plugin": "5.59.9",
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-config/shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ module.exports = {
'react',
'react-hooks',
'jest-dom',
'@redwoodjs',
],
ignorePatterns: ['node_modules', 'dist'],
settings: {
Expand All @@ -45,6 +46,7 @@ module.exports = {
'import/internal-regex': '^src/',
},
rules: {
'@redwoodjs/process-env-computed': 'error',
'prettier/prettier': 'warn',
'no-console': 'off',
'prefer-object-spread': 'warn',
Expand Down
23 changes: 23 additions & 0 deletions packages/eslint-plugin/build.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import fs from 'node:fs'

import * as esbuild from 'esbuild'
import fg from 'fast-glob'

const sourceFiles = fg.sync(['./src/**/*.ts'], { ignore: ['./src/__tests__'] })

const result = await esbuild.build({
entryPoints: sourceFiles,
outdir: 'dist',

format: 'cjs',
platform: 'node',
target: ['node18'],

logLevel: 'info',

// For visualizing dist.
// See https://esbuild.github.io/api/#metafile and https://esbuild.github.io/analyze/.
metafile: true,
})

fs.writeFileSync('meta.json', JSON.stringify(result.metafile, null, 2))
35 changes: 35 additions & 0 deletions packages/eslint-plugin/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"name": "@redwoodjs/eslint-plugin",
"version": "5.0.0",
"repository": {
"type": "git",
"url": "https://github.com/redwoodjs/redwood.git",
"directory": "packages/eslint-plugin"
},
"license": "MIT",
"main": "./dist/index.js",
"types": "./dist/index.d.ts",
"files": [
"dist"
],
"scripts": {
"build": "yarn node ./build.mjs && yarn build:types",
"build:types": "tsc --build --verbose",
"build:watch": "nodemon --watch src --ext \"js,jsx,ts,tsx\" --ignore dist --exec \"yarn build\"",
"prepublishOnly": "NODE_ENV=production yarn build",
"test": "glob './src/**/__tests__/*.test.ts' --cmd='node --loader tsx --no-warnings --test' && echo",
"test:watch": "glob './src/**/__tests__/*.test.ts' --cmd='node --loader tsx --no-warnings --test --watch'"
},
"dependencies": {
"eslint": "8.42.0"
},
"devDependencies": {
"@types/eslint": "8",
"@types/estree": "1.0.1",
"esbuild": "0.18.2",
"fast-glob": "3.2.12",
"glob": "10.2.7",
"typescript": "5.1.3"
},
"gitHead": "3905ed045508b861b495f8d5630d76c7a157d8f1"
}
50 changes: 50 additions & 0 deletions packages/eslint-plugin/src/__tests__/process-env-computed.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { describe, it } from 'node:test'

import { RuleTester } from 'eslint'

import { processEnvComputedRule } from '../process-env-computed.js'

// @ts-expect-error - Types are wrong
RuleTester.describe = describe
// @ts-expect-error - Types are wrong
RuleTester.it = it

const ruleTester = new RuleTester()

ruleTester.run('process-env-computed', processEnvComputedRule, {
valid: [
{
code: 'process.env.foo',
},
{
code: 'process.env.BAR',
},
{
code: 'process.env.REDWOOD_ENV_FOOBAR',
},
{
filename: 'packages/testing/src/api/__tests__/directUrlHelpers.test.ts',
code: 'expect(process.env[directUrlEnvVar]).toBe(defaultDb)',
},
],
invalid: [
{
code: 'process.env[foo]',
errors: [
{
message:
'Computed member access on process.env does not work in production environments.',
},
],
},
{
code: "process.env['BAR']",
errors: [
{
message:
'Computed member access on process.env does not work in production environments.',
},
],
},
],
})
5 changes: 5 additions & 0 deletions packages/eslint-plugin/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { processEnvComputedRule } from './process-env-computed.js'

export const rules = {
'process-env-computed': processEnvComputedRule,
}
76 changes: 76 additions & 0 deletions packages/eslint-plugin/src/process-env-computed.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { Rule } from 'eslint'
import { Identifier, MemberExpression } from 'estree'

function isProcessEnv(node: unknown) {
return (
isMemberExpression(node) &&
hasName(node.object, 'process') &&
hasName(node.property, 'env')
)
}

function isIdentifier(node: unknown): node is Identifier {
return (
typeof node !== 'undefined' && (node as Identifier).type === 'Identifier'
)
}

function isMemberExpression(node: unknown): node is MemberExpression {
return (
typeof node !== 'undefined' &&
(node as MemberExpression).type === 'MemberExpression'
)
}

function hasName(node: unknown, name: string) {
return isIdentifier(node) && node.name === name
}

function isTestFile(filename: string) {
return (
filename.endsWith('.test.ts') ||
filename.endsWith('.test.js') ||
filename.endsWith('.test.tsx') ||
filename.endsWith('.test.jsx') ||
filename.endsWith('.test.mts') ||
filename.endsWith('.test.mjs') ||
filename.endsWith('.test.cjs') ||
filename.endsWith('.spec.ts') ||
filename.endsWith('.spec.js') ||
filename.endsWith('.spec.tsx') ||
filename.endsWith('.spec.jsx') ||
filename.endsWith('.spec.mts') ||
filename.endsWith('.spec.mjs') ||
filename.endsWith('.spec.cjs') ||
filename.includes('/__tests__/')
)
}

export const processEnvComputedRule: Rule.RuleModule = {
meta: {
type: 'problem',
docs: {
description: 'Find computed member access on process.env',
},
// fixable: 'code',
schema: [], // No additional configuration needed
},
create(context) {
return {
MemberExpression: function (node) {
if (
isProcessEnv(node.object) &&
node.computed &&
!isTestFile(context.filename)
) {
context.report({
message:
'Computed member access on process.env does not work in production environments.',
node,
// fix(fixer) {},
})
}
},
}
},
}
11 changes: 11 additions & 0 deletions packages/eslint-plugin/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"extends": "../../tsconfig.compilerOption.json",
"compilerOptions": {
"strict": true,
"baseUrl": ".",
"rootDir": "src",
"tsBuildInfoFile": "dist/tsconfig.tsbuildinfo",
"outDir": "dist",
},
"include": ["src"],
}
4 changes: 2 additions & 2 deletions packages/project-config/src/__tests__/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ describe('getConfig', () => {
expect(config.web.apiUrl).toBe('/bazinga')
expect(config.web.title).toBe('App running on staging')

delete process.env['API_URL']
delete process.env['APP_ENV']
delete process.env.API_URL
delete process.env.APP_ENV
})
})
25 changes: 20 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7838,6 +7838,7 @@ __metadata:
"@babel/core": 7.22.5
"@babel/eslint-parser": 7.22.5
"@babel/eslint-plugin": 7.22.5
"@redwoodjs/eslint-plugin": 5.0.0
"@redwoodjs/internal": 5.0.0
"@redwoodjs/project-config": 5.0.0
"@typescript-eslint/eslint-plugin": 5.59.9
Expand All @@ -7858,6 +7859,20 @@ __metadata:
languageName: unknown
linkType: soft

"@redwoodjs/[email protected], @redwoodjs/eslint-plugin@workspace:packages/eslint-plugin":
version: 0.0.0-use.local
resolution: "@redwoodjs/eslint-plugin@workspace:packages/eslint-plugin"
dependencies:
"@types/eslint": 8
"@types/estree": 1.0.1
esbuild: 0.18.2
eslint: 8.42.0
fast-glob: 3.2.12
glob: 10.2.7
typescript: 5.1.3
languageName: unknown
linkType: soft

"@redwoodjs/[email protected], @redwoodjs/fastify@workspace:packages/fastify":
version: 0.0.0-use.local
resolution: "@redwoodjs/fastify@workspace:packages/fastify"
Expand Down Expand Up @@ -9932,17 +9947,17 @@ __metadata:
languageName: node
linkType: hard

"@types/eslint@npm:*":
version: 8.40.0
resolution: "@types/eslint@npm:8.40.0"
"@types/eslint@npm:*, @types/eslint@npm:8":
version: 8.40.2
resolution: "@types/eslint@npm:8.40.2"
dependencies:
"@types/estree": "*"
"@types/json-schema": "*"
checksum: 9b9ea412985339cab15339a751c3249634bb52d9e29b35688395ee32b46e2fa0609ec3b092772909af6d4b9d267bd0eafc69f39cc3a08dac4dc8911b0fc93c95
checksum: 5797dce7805f601ee34b2f63d6a80dba21302e2fe2614c7990eca7a22472f9e0c386d56d82fe79a7cdede57c8dcc1e0f9b1e5dc384adf736833b901ffcc29628
languageName: node
linkType: hard

"@types/estree@npm:*, @types/estree@npm:^1.0.0":
"@types/estree@npm:*, @types/estree@npm:1.0.1, @types/estree@npm:^1.0.0":
version: 1.0.1
resolution: "@types/estree@npm:1.0.1"
checksum: b4022067f834d86766f23074a1a7ac6c460e823b00cd8fe94c997bc491e7794615facd3e1520a934c42bd8c0689dbff81e5c643b01f1dee143fc758cac19669e
Expand Down

0 comments on commit 7413755

Please sign in to comment.