Skip to content

[9.3] Loosen apiClient ESLint rule on Security (#252212) #252665

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
steliosmavro merged 2 commits into from
Feb 11, 2026
Merged

[9.3] Loosen apiClient ESLint rule on Security (#252212) #252665

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14392ea
Loosen `apiClient` ESLint rule on Security (#252212)
steliosmavro Feb 11, 2026
e1ed61c
fix
steliosmavro Feb 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2703,8 +2703,9 @@ module.exports = {
],
},
},
// Custom rules for scout tests
{
// Custom rules for scout tests
// Platform & Solutions
files: [
'src/platform/plugins/**/test/{scout,scout_*}/**/*.ts',
'x-pack/platform/**/plugins/**/test/{scout,scout_*}/**/*.ts',
Expand All @@ -2714,13 +2715,26 @@ module.exports = {
'@kbn/eslint/scout_no_describe_configure': 'error',
'@kbn/eslint/scout_max_one_describe': 'error',
'@kbn/eslint/scout_test_file_naming': 'error',
'@kbn/eslint/scout_require_api_client_in_api_test': 'error',
'@kbn/eslint/scout_require_global_setup_hook_in_parallel_tests': 'error',
'@kbn/eslint/scout_no_es_archiver_in_parallel_tests': 'error',
'@kbn/eslint/scout_expect_import': 'error',
'@kbn/eslint/require_include_in_check_a11y': 'warn',
},
},
{
// Platform & Solutions API Tests
files: [
'src/platform/plugins/**/test/{scout,scout_*}/api/**/*.ts',
'x-pack/platform/**/plugins/**/test/{scout,scout_*}/api/**/*.ts',
'x-pack/solutions/**/plugins/**/test/{scout,scout_*}/api/**/*.ts',
],
rules: {
'@kbn/eslint/scout_require_api_client_in_api_test': [
'error',
{ alternativeFixtures: ['esClient'] },
],
},
},
{
// Deployment-agnostic test files must use proper context and services
files: [
Expand Down
76 changes: 53 additions & 23 deletions packages/kbn-eslint-plugin-eslint/rules/scout_require_api_client_in_api_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,23 @@
/** @typedef {import("@typescript-eslint/typescript-estree").TSESTree.CallExpression} CallExpression */
/** @typedef {import("@typescript-eslint/typescript-estree").TSESTree.Identifier} Identifier */

const ERROR_MSG =
'The `apiClient` fixture should be used in `apiTest` to call an endpoint and later verify response code and body.';

const traverse = require('eslint-traverse');

const isApiTestCall = (node) => node.callee.type === 'Identifier' && node.callee.name === 'apiTest';

/** Get local param name for `apiClient` (supports `{ apiClient: alias }`). */
const getApiClientLocalName = (fnNode) => {
/** Get local param name for a fixture (supports `{ fixtureName: alias }`). */
const getFixtureLocalName = (fnNode, fixtureName) => {
const firstParam = fnNode.params[0];
if (!firstParam || firstParam.type !== 'ObjectPattern') {
return 'apiClient';
return fixtureName;
}

const apiClientProp = firstParam.properties.find((prop) => {
const fixtureProp = firstParam.properties.find((prop) => {
if (prop.type === 'RestElement') return false;
return prop.key && prop.key.name === 'apiClient';
return prop.key && prop.key.name === fixtureName;
});

return apiClientProp?.value?.name || 'apiClient';
return fixtureProp?.value?.name || fixtureName;
};

/** Helper: Check if an identifier is used anywhere in a function body (simple recursion). */
Expand Down Expand Up @@ -65,18 +62,18 @@ const paramUsesIdentifierInBody = (node, paramName) => {
};

/**
* Determine whether `fnNode` uses `apiClient`.
* Determine whether `fnNode` uses the given `fixtureName`.
*
* Strategy: Single-pass traversal that:
* 1. Collects variable aliases pointing to apiClient (e.g., `const client = apiClient`)
* 1. Collects variable aliases pointing to the fixture (e.g., `const client = apiClient`)
* 2. Collects local function definitions and their parameter usage
* 3. Detects member access on apiClient or calls passing apiClient to functions that use it
* 3. Detects member access on the fixture or calls passing it to functions that use it
*
* We avoid the ESLint scope manager to maintain consistency with other rules in this package.
*/
const functionUsesApiClient = (fnNode, context) => {
const apiClientName = getApiClientLocalName(fnNode);
const variableAliases = new Set([apiClientName]);
const functionUsesFixture = (fnNode, context, fixtureName) => {
const localName = getFixtureLocalName(fnNode, fixtureName);
const variableAliases = new Set([localName]);
const localFnUsesParam = new Map();
let found = false;

Expand All @@ -95,7 +92,7 @@ const functionUsesApiClient = (fnNode, context) => {
// Pre-compute parameter usage for local function expressions
if (node.init.type === 'FunctionExpression' || node.init.type === 'ArrowFunctionExpression') {
const fnName = node.id.name;
const paramName = getApiClientLocalName(node.init);
const paramName = getFixtureLocalName(node.init, fixtureName);
localFnUsesParam.set(fnName, paramUsesIdentifierInBody(node.init.body, paramName));
// Skip traversing into nested function bodies to avoid false positives
return traverse.SKIP;
Expand All @@ -105,13 +102,13 @@ const functionUsesApiClient = (fnNode, context) => {
// Collect function declarations and pre-compute parameter usage
if (node.type === 'FunctionDeclaration' && node.id?.type === 'Identifier') {
const fnName = node.id.name;
const paramName = getApiClientLocalName(node);
const paramName = getFixtureLocalName(node, fixtureName);
localFnUsesParam.set(fnName, paramUsesIdentifierInBody(node.body, paramName));
// Skip traversing into nested function bodies to avoid false positives
return traverse.SKIP;
}

// Detect member access on apiClient (e.g., `apiClient.get(...)`)
// Detect member access on the fixture (e.g., `apiClient.get(...)`)
if (
node.type === 'MemberExpression' &&
node.object?.type === 'Identifier' &&
Expand All @@ -121,7 +118,7 @@ const functionUsesApiClient = (fnNode, context) => {
return traverse.SKIP;
}

// Detect calls with apiClient passed as argument
// Detect calls with the fixture passed as argument
if (node.type === 'CallExpression' && node.arguments) {
for (const arg of node.arguments) {
if (arg.type === 'Identifier' && variableAliases.has(arg.name)) {
Expand All @@ -140,7 +137,7 @@ const functionUsesApiClient = (fnNode, context) => {
}
}

// Also check if a local function is called that uses apiClient from closure (no args needed)
// Also check if a local function is called that uses the fixture from closure (no args needed)
const callee = node.callee;
if (callee.type === 'Identifier' && localFnUsesParam.has(callee.name)) {
if (localFnUsesParam.get(callee.name)) {
Expand All @@ -154,6 +151,18 @@ const functionUsesApiClient = (fnNode, context) => {
return found;
};

/** Build a human-readable error message listing all accepted fixtures. */
const buildErrorMsg = (acceptedFixtures) => {
if (acceptedFixtures.length === 1) {
return `The \`${acceptedFixtures[0]}\` fixture should be used in \`apiTest\` to call an endpoint and later verify response code and body.`;
}
const formatted = acceptedFixtures.map((f) => `\`${f}\``);
const last = formatted.pop();
return `One of ${formatted.join(
', '
)} or ${last} fixtures should be used in \`apiTest\` to interact with an endpoint and later verify the response.`;
};

/** @type {Rule} */
module.exports = {
meta: {
Expand All @@ -163,10 +172,27 @@ module.exports = {
category: 'Best Practices',
},
fixable: null,
schema: [],
schema: [
{
type: 'object',
properties: {
alternativeFixtures: {
type: 'array',
items: { type: 'string' },
uniqueItems: true,
},
},
additionalProperties: false,
},
],
},

create(context) {
const options = context.options[0] || {};
const alternativeFixtures = options.alternativeFixtures || [];
const acceptedFixtures = ['apiClient', ...alternativeFixtures];
const errorMsg = buildErrorMsg(acceptedFixtures);

return {
CallExpression(node) {
if (!isApiTestCall(node)) return;
Expand All @@ -193,10 +219,14 @@ module.exports = {
return;
}

if (!functionUsesApiClient(callbackArg, context)) {
const usesAcceptedFixture = acceptedFixtures.some((fixture) =>
functionUsesFixture(callbackArg, context, fixture)
);

if (!usesAcceptedFixture) {
context.report({
node,
message: ERROR_MSG,
message: errorMsg,
});
}
},
Expand Down
61 changes: 53 additions & 8 deletions packages/kbn-eslint-plugin-eslint/rules/scout_require_api_client_in_api_test.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ const { RuleTester } = require('eslint');
const rule = require('./scout_require_api_client_in_api_test');
const dedent = require('dedent');

const ERROR_MSG =
const DEFAULT_ERROR_MSG =
'The `apiClient` fixture should be used in `apiTest` to call an endpoint and later verify response code and body.';
const ALT_ERROR_MSG =
'One of `apiClient` or `esClient` fixtures should be used in `apiTest` to interact with an endpoint and later verify the response.';
const ruleTester = new RuleTester({
parser: require.resolve('@typescript-eslint/parser'),
parserOptions: {
Expand Down Expand Up @@ -161,7 +163,7 @@ ruleTester.run('@kbn/eslint/scout_require_api_client_in_api_test', rule, {
console.log(other);
});
`,
errors: [{ message: ERROR_MSG }],
errors: [{ message: DEFAULT_ERROR_MSG }],
},
// Nested apiTest inside apiTest.describe missing apiClient
{
Expand All @@ -170,7 +172,7 @@ ruleTester.run('@kbn/eslint/scout_require_api_client_in_api_test', rule, {
apiTest('inner missing', () => {});
});
`,
errors: [{ message: ERROR_MSG }],
errors: [{ message: DEFAULT_ERROR_MSG }],
},
// Nested blocks missing apiClient
{
Expand All @@ -181,7 +183,7 @@ ruleTester.run('@kbn/eslint/scout_require_api_client_in_api_test', rule, {
}
});
`,
errors: [{ message: ERROR_MSG }],
errors: [{ message: DEFAULT_ERROR_MSG }],
},
// Nested helper function missing apiClient
{
Expand All @@ -193,7 +195,7 @@ ruleTester.run('@kbn/eslint/scout_require_api_client_in_api_test', rule, {
helper(apiClient);
});
`,
errors: [{ message: ERROR_MSG }],
errors: [{ message: DEFAULT_ERROR_MSG }],
},
// Arrow function inside test
{
Expand All @@ -202,7 +204,7 @@ ruleTester.run('@kbn/eslint/scout_require_api_client_in_api_test', rule, {
const callApi = () => apiClient.get('/bar');
});
`,
errors: [{ message: ERROR_MSG }],
errors: [{ message: DEFAULT_ERROR_MSG }],
},
// External helper without apiClient usage
{
Expand All @@ -214,7 +216,7 @@ ruleTester.run('@kbn/eslint/scout_require_api_client_in_api_test', rule, {
externalHelper();
});
`,
errors: [{ message: ERROR_MSG }],
errors: [{ message: DEFAULT_ERROR_MSG }],
},
// Passing apiClient to a variable but never using it
{
Expand All @@ -224,7 +226,50 @@ ruleTester.run('@kbn/eslint/scout_require_api_client_in_api_test', rule, {
fn(apiClient);
});
`,
errors: [{ message: ERROR_MSG }],
errors: [{ message: DEFAULT_ERROR_MSG }],
},
],
});

// --- Tests with alternativeFixtures option ---
const altOptions = [{ alternativeFixtures: ['esClient'] }];

ruleTester.run(
'@kbn/eslint/scout_require_api_client_in_api_test (with alternativeFixtures)',
rule,
{
valid: [
// apiClient still accepted when alternativeFixtures is configured
{
code: dedent`
apiTest('uses apiClient', async ({ apiClient }) => {
await apiClient.get('/foo');
});
`,
options: altOptions,
},
// Alternative fixture used instead of apiClient
{
code: dedent`
apiTest('uses esClient', async ({ esClient }) => {
await esClient.load('path/to/archive');
});
`,
options: altOptions,
},
],

invalid: [
// Neither apiClient nor alternative fixture used
{
code: dedent`
apiTest('uses neither', async () => {
console.log('no fixture');
});
`,
options: altOptions,
errors: [{ message: ALT_ERROR_MSG }],
},
],
}
);