From abc12cd8e174b6a2a40a98e54b53822a597fa19e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 17 Mar 2026 10:54:11 +0100 Subject: [PATCH] fix: address review feedback for actions CLI module Fix argument parsing bugs in the execute command where actionId at index 0 was incorrectly skipped when --instance was absent, and flag values matching the actionId string were erroneously removed. Add --help support to the execute command for CLI report generation. Add missing bin script and cli-report.md for cli-module-actions. Add resolveAuth tests. Bump backend-defaults changeset to minor for the security-relevant auth change. Replace cross-fetch with native fetch in cli-module-auth to avoid punycode deprecation warnings during CLI report generation. Signed-off-by: Patrik Oldsberg Made-with: Cursor --- .changeset/allow-user-action-invocation.md | 2 +- .../bin/backstage-cli-module-actions | 32 +++++ packages/cli-module-actions/cli-report.md | 94 +++++++++++++ packages/cli-module-actions/package.json | 4 +- .../src/commands/execute.ts | 41 +++++- .../src/lib/resolveAuth.test.ts | 124 ++++++++++++++++++ packages/cli-module-auth/package.json | 1 - packages/cli-module-auth/src/lib/http.test.ts | 7 +- packages/cli-module-auth/src/lib/http.ts | 1 - packages/cli/cli-report.md | 78 +++++++++++ yarn.lock | 3 +- 11 files changed, 375 insertions(+), 12 deletions(-) create mode 100755 packages/cli-module-actions/bin/backstage-cli-module-actions create mode 100644 packages/cli-module-actions/cli-report.md create mode 100644 packages/cli-module-actions/src/lib/resolveAuth.test.ts diff --git a/.changeset/allow-user-action-invocation.md b/.changeset/allow-user-action-invocation.md index 299c38b41b..d9ac46a60e 100644 --- a/.changeset/allow-user-action-invocation.md +++ b/.changeset/allow-user-action-invocation.md @@ -1,5 +1,5 @@ --- -'@backstage/backend-defaults': patch +'@backstage/backend-defaults': minor --- The actions registry invoke endpoint now accepts direct user credentials in addition to service principals, enabling CLI and other direct user clients to invoke actions. diff --git a/packages/cli-module-actions/bin/backstage-cli-module-actions b/packages/cli-module-actions/bin/backstage-cli-module-actions new file mode 100755 index 0000000000..59f8267233 --- /dev/null +++ b/packages/cli-module-actions/bin/backstage-cli-module-actions @@ -0,0 +1,32 @@ +#!/usr/bin/env node +/* + * Copyright 2025 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +const path = require('node:path'); + +/* eslint-disable-next-line no-restricted-syntax */ +const isLocal = require('node:fs').existsSync( + path.resolve(__dirname, '../src'), +); + +if (isLocal) { + require('@backstage/cli-node/config/nodeTransform.cjs'); +} + +const { runCliModule } = require('@backstage/cli-node'); +const cliModule = require(isLocal ? '../src/index' : '..').default; +const pkg = require('../package.json'); +runCliModule({ module: cliModule, name: pkg.name, version: pkg.version }); diff --git a/packages/cli-module-actions/cli-report.md b/packages/cli-module-actions/cli-report.md new file mode 100644 index 0000000000..bbbab62b3f --- /dev/null +++ b/packages/cli-module-actions/cli-report.md @@ -0,0 +1,94 @@ +## CLI Report file for "@backstage/cli-module-actions" + +> Do not edit this file. It is a report generated by `yarn build:api-reports` + +### `backstage-cli-module-actions` + +``` +Usage: @backstage/cli-module-actions [options] [command] + +Options: + -V, --version + -h, --help + +Commands: + actions [command] + help [command] +``` + +### `backstage-cli-module-actions actions` + +``` +Usage: @backstage/cli-module-actions actions [options] [command] [command] + +Options: + -h, --help + +Commands: + execute + help [command] + list + sources [command] +``` + +### `backstage-cli-module-actions actions execute` + +``` +Usage: @backstage/cli-module-actions actions execute + +Options: + --instance + -h, --help +``` + +### `backstage-cli-module-actions actions list` + +``` +Usage: @backstage/cli-module-actions actions list + +Options: + --instance + -h, --help +``` + +### `backstage-cli-module-actions actions sources` + +``` +Usage: @backstage/cli-module-actions actions sources [options] [command] [command] + +Options: + -h, --help + +Commands: + add + help [command] + list + remove +``` + +### `backstage-cli-module-actions actions sources add` + +``` +Usage: @backstage/cli-module-actions actions sources add + +Options: + -h, --help +``` + +### `backstage-cli-module-actions actions sources list` + +``` +Usage: @backstage/cli-module-actions actions sources list + +Options: + -h, --help +``` + +### `backstage-cli-module-actions actions sources remove` + +``` +Usage: @backstage/cli-module-actions actions sources remove + +Options: + -h, --help +``` diff --git a/packages/cli-module-actions/package.json b/packages/cli-module-actions/package.json index ca3c423999..fe19a8a90d 100644 --- a/packages/cli-module-actions/package.json +++ b/packages/cli-module-actions/package.json @@ -20,8 +20,10 @@ "main": "src/index.ts", "types": "src/index.ts", "files": [ - "dist" + "dist", + "bin" ], + "bin": "bin/backstage-cli-module-actions", "scripts": { "build": "backstage-cli package build", "clean": "backstage-cli package clean", diff --git a/packages/cli-module-actions/src/commands/execute.ts b/packages/cli-module-actions/src/commands/execute.ts index fccb1e1595..4ff955096b 100644 --- a/packages/cli-module-actions/src/commands/execute.ts +++ b/packages/cli-module-actions/src/commands/execute.ts @@ -21,12 +21,45 @@ import { schemaToFlags } from '../lib/schemaToFlags'; import { resolveAuth } from '../lib/resolveAuth'; export default async ({ args, info }: CliCommandContext) => { + if (args.includes('--help') || args.includes('-h')) { + cli( + { + help: info, + parameters: [''], + flags: { + instance: { + type: String, + description: 'Name of the instance to use', + }, + }, + }, + undefined, + args, + ); + return; + } + const instanceIdx = args.indexOf('--instance'); const instanceFlag = instanceIdx !== -1 ? args[instanceIdx + 1] : undefined; - const actionId = args.find( - (a, i) => !a.startsWith('-') && i !== instanceIdx + 1, - ); + // Skip flag names, flag values (the argument after a known flag), and + // the --instance value position so we only pick up positional arguments. + const skipIndices = new Set(); + if (instanceIdx !== -1) { + skipIndices.add(instanceIdx); + skipIndices.add(instanceIdx + 1); + } + + let actionId: string | undefined; + let actionIdIdx = -1; + for (let i = 0; i < args.length; i++) { + if (!skipIndices.has(i) && !args[i].startsWith('-')) { + actionId = args[i]; + actionIdIdx = i; + break; + } + } + if (!actionId) { process.stderr.write('Usage: actions execute [flags]\n'); process.exit(1); @@ -46,7 +79,7 @@ export default async ({ args, info }: CliCommandContext) => { const schemaFlags = schemaToFlags(action.schema.input as any); - const flagArgs = args.filter(a => a !== actionId); + const flagArgs = args.filter((_, i) => i !== actionIdIdx); const { flags } = cli( { diff --git a/packages/cli-module-actions/src/lib/resolveAuth.test.ts b/packages/cli-module-actions/src/lib/resolveAuth.test.ts new file mode 100644 index 0000000000..f2702cf757 --- /dev/null +++ b/packages/cli-module-actions/src/lib/resolveAuth.test.ts @@ -0,0 +1,124 @@ +/* + * Copyright 2025 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { resolveAuth } from './resolveAuth'; +import { + getSelectedInstance, + getInstanceConfig, + accessTokenNeedsRefresh, + refreshAccessToken, + getSecretStore, + type StoredInstance, +} from '@backstage/cli-module-auth'; + +jest.mock('@backstage/cli-module-auth', () => ({ + getSelectedInstance: jest.fn(), + getInstanceConfig: jest.fn(), + accessTokenNeedsRefresh: jest.fn(), + refreshAccessToken: jest.fn(), + getSecretStore: jest.fn(), +})); + +const mockGetSelectedInstance = getSelectedInstance as jest.MockedFunction< + typeof getSelectedInstance +>; +const mockGetInstanceConfig = getInstanceConfig as jest.MockedFunction< + typeof getInstanceConfig +>; +const mockAccessTokenNeedsRefresh = + accessTokenNeedsRefresh as jest.MockedFunction< + typeof accessTokenNeedsRefresh + >; +const mockRefreshAccessToken = refreshAccessToken as jest.MockedFunction< + typeof refreshAccessToken +>; +const mockGetSecretStore = getSecretStore as jest.MockedFunction< + typeof getSecretStore +>; + +describe('resolveAuth', () => { + const mockInstance: StoredInstance = { + name: 'production', + baseUrl: 'https://backstage.example.com', + clientId: 'my-client', + issuedAt: Date.now(), + accessTokenExpiresAt: Date.now() + 3600_000, + }; + + const mockSecretStore = { + get: jest.fn(), + set: jest.fn(), + delete: jest.fn(), + }; + + beforeEach(() => { + jest.clearAllMocks(); + mockGetSelectedInstance.mockResolvedValue(mockInstance); + mockAccessTokenNeedsRefresh.mockReturnValue(false); + mockGetSecretStore.mockResolvedValue(mockSecretStore); + mockSecretStore.get.mockResolvedValue('test-access-token'); + mockGetInstanceConfig.mockResolvedValue(['catalog', 'scaffolder']); + }); + + it('resolves auth with the selected instance and stored token', async () => { + const result = await resolveAuth(); + + expect(mockGetSelectedInstance).toHaveBeenCalledWith(undefined); + expect(mockAccessTokenNeedsRefresh).toHaveBeenCalledWith(mockInstance); + expect(mockRefreshAccessToken).not.toHaveBeenCalled(); + expect(result).toEqual({ + instance: mockInstance, + accessToken: 'test-access-token', + pluginSources: ['catalog', 'scaffolder'], + }); + }); + + it('passes instance name flag to getSelectedInstance', async () => { + await resolveAuth('staging'); + + expect(mockGetSelectedInstance).toHaveBeenCalledWith('staging'); + }); + + it('refreshes the access token when it is about to expire', async () => { + const refreshedInstance = { + ...mockInstance, + accessTokenExpiresAt: Date.now() + 7200_000, + }; + mockAccessTokenNeedsRefresh.mockReturnValue(true); + mockRefreshAccessToken.mockResolvedValue(refreshedInstance); + + const result = await resolveAuth(); + + expect(mockRefreshAccessToken).toHaveBeenCalledWith('production'); + expect(result.instance).toBe(refreshedInstance); + }); + + it('throws when no access token is stored', async () => { + mockSecretStore.get.mockResolvedValue(undefined); + + await expect(resolveAuth()).rejects.toThrow( + 'No access token found. Run "auth login" to authenticate.', + ); + }); + + it('returns empty plugin sources when none are configured', async () => { + mockGetInstanceConfig.mockResolvedValue(undefined); + + const result = await resolveAuth(); + + expect(result.pluginSources).toEqual([]); + }); +}); diff --git a/packages/cli-module-auth/package.json b/packages/cli-module-auth/package.json index 86bb491f7a..94169e998e 100644 --- a/packages/cli-module-auth/package.json +++ b/packages/cli-module-auth/package.json @@ -35,7 +35,6 @@ "@backstage/cli-node": "workspace:^", "@backstage/errors": "workspace:^", "cleye": "^2.3.0", - "cross-fetch": "^4.0.0", "fs-extra": "^11.2.0", "glob": "^7.1.7", "inquirer": "^8.2.0", diff --git a/packages/cli-module-auth/src/lib/http.test.ts b/packages/cli-module-auth/src/lib/http.test.ts index 74416a0dee..40ab7009b1 100644 --- a/packages/cli-module-auth/src/lib/http.test.ts +++ b/packages/cli-module-auth/src/lib/http.test.ts @@ -14,12 +14,13 @@ * limitations under the License. */ -import fetch from 'cross-fetch'; import { httpJson } from './http'; -jest.mock('cross-fetch'); +const mockFetch = jest.fn() as jest.MockedFunction; -const mockFetch = fetch as jest.MockedFunction; +beforeEach(() => { + global.fetch = mockFetch; +}); describe('http', () => { beforeEach(() => { diff --git a/packages/cli-module-auth/src/lib/http.ts b/packages/cli-module-auth/src/lib/http.ts index 789c627606..4861a07722 100644 --- a/packages/cli-module-auth/src/lib/http.ts +++ b/packages/cli-module-auth/src/lib/http.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import fetch from 'cross-fetch'; import { ResponseError } from '@backstage/errors'; /** @public */ diff --git a/packages/cli/cli-report.md b/packages/cli/cli-report.md index bb22326786..4408c6e969 100644 --- a/packages/cli/cli-report.md +++ b/packages/cli/cli-report.md @@ -12,6 +12,7 @@ Options: -h, --help Commands: + actions [command] auth [command] build-workspace config [command] @@ -31,6 +32,83 @@ Commands: versions:migrate ``` +### `backstage-cli actions` + +``` +Usage: backstage-cli actions [options] [command] [command] + +Options: + -h, --help + +Commands: + execute + help [command] + list + sources [command] +``` + +### `backstage-cli actions execute` + +``` +Usage: backstage-cli actions execute + +Options: + --instance + -h, --help +``` + +### `backstage-cli actions list` + +``` +Usage: backstage-cli actions list + +Options: + --instance + -h, --help +``` + +### `backstage-cli actions sources` + +``` +Usage: backstage-cli actions sources [options] [command] [command] + +Options: + -h, --help + +Commands: + add + help [command] + list + remove +``` + +### `backstage-cli actions sources add` + +``` +Usage: backstage-cli actions sources add + +Options: + -h, --help +``` + +### `backstage-cli actions sources list` + +``` +Usage: backstage-cli actions sources list + +Options: + -h, --help +``` + +### `backstage-cli actions sources remove` + +``` +Usage: backstage-cli actions sources remove + +Options: + -h, --help +``` + ### `backstage-cli auth` ``` diff --git a/yarn.lock b/yarn.lock index daff483d5a..a01ca993a3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2830,6 +2830,8 @@ __metadata: "@backstage/cli-module-auth": "workspace:^" "@backstage/cli-node": "workspace:^" cleye: "npm:^2.3.0" + bin: + cli-module-actions: bin/backstage-cli-module-actions languageName: unknown linkType: soft @@ -2844,7 +2846,6 @@ __metadata: "@types/fs-extra": "npm:^11.0.0" "@types/proper-lockfile": "npm:^4" cleye: "npm:^2.3.0" - cross-fetch: "npm:^4.0.0" fs-extra: "npm:^11.2.0" glob: "npm:^7.1.7" inquirer: "npm:^8.2.0"