fix(sign-plugin): prevent symlink escape check bypass and add test coverage

packages/sign-plugin/package.json

@@ -33,7 +33,9 @@
     "@libs/find-up": "^1.0.0",
     "@libs/output": "^1.0.3",
     "@libs/version": "^1.0.2",
-    "@types/minimist": "^1.2.5"
+    "@types/minimist": "^1.2.5",
+    "@types/tmp": "0.2.6",
+    "tmp": "0.2.7"
   },
   "engines": {
     "node": ">=20"

packages/sign-plugin/src/commands/sign.command.test.ts

@@ -0,0 +1,194 @@
+import minimist from 'minimist';
+import { existsSync, readFileSync, rmSync } from 'node:fs';
+import path from 'node:path';
+import { MockInstance, vi } from 'vitest';
+import { sign } from './sign.command.js';
+import { ManifestInfo } from '../utils/manifest.js';
+import { createPluginDistDir, createTempDir, writeFiles } from '../utils/tests/fixtures.js';
+
+const mocks = vi.hoisted(() => {
+  return {
+    postData: vi.fn(),
+  };
+});
+
+vi.mock('../utils/request.js', () => {
+  return { postData: mocks.postData };
+});
+
+const PROCESS_EXIT_MESSAGE = 'process.exit called';
+
+function argvFor(args: Record<string, unknown> = {}): minimist.ParsedArgs {
+  return { _: [], ...args } as minimist.ParsedArgs;
+}
+
+function getPostedManifest(): ManifestInfo {
+  return mocks.postData.mock.calls[0][1] as ManifestInfo;
+}
+
+describe('sign command', () => {
+  const tempDirs: string[] = [];
+  let stdoutSpy: MockInstance<typeof process.stdout.write>;
+  let exitSpy: MockInstance<typeof process.exit>;
+
+  function pluginDistDir(files: Record<string, string> = {}) {
+    const dir = createPluginDistDir(files);
+    tempDirs.push(dir);
+    return dir;
+  }
+
+  beforeEach(() => {
+    exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => {
+      throw new Error(PROCESS_EXIT_MESSAGE);
+    });
+    stdoutSpy = vi.spyOn(process.stdout, 'write').mockImplementation(() => true);
+    vi.stubEnv('GRAFANA_ACCESS_POLICY_TOKEN', 'test-token');
+    vi.stubEnv('GRAFANA_API_KEY', undefined);
+    vi.stubEnv('GRAFANA_COM_URL', undefined);
+    mocks.postData.mockReset().mockResolvedValue({ status: 200, data: 'SIGNED-MANIFEST' });
+  });
+
+  afterEach(() => {
+    vi.unstubAllEnvs();
+    vi.restoreAllMocks();
+  });
+
+  afterAll(() => {
+    tempDirs.forEach((dir) => rmSync(dir, { recursive: true, force: true }));
+  });
+
+  it('should exit when the plugin dist directory does not exist', async () => {
+    await expect(sign(argvFor({ distDir: '/path/that/does/not/exist' }))).rejects.toThrow(PROCESS_EXIT_MESSAGE);
+
+    expect(exitSpy).toHaveBeenCalledWith(1);
+    expect(mocks.postData).not.toHaveBeenCalled();
+  });
+
+  it('should post the manifest to the Grafana API and save the signed response', async () => {
+    const dir = pluginDistDir({ 'module.js': 'export {};' });
+
+    await sign(argvFor({ distDir: dir }));
+
+    const [url, manifest, headers] = mocks.postData.mock.calls[0];
+    expect(url).toBe('https://grafana.com/api/plugins/ci/sign');
+    expect(headers).toEqual({ Authorization: 'Bearer test-token' });
+    expect(manifest).toMatchObject({
+      plugin: 'grafana-test-app',
+      version: '1.0.0',
+      signPlugin: { version: expect.any(String) },
+    });
+    expect(readFileSync(path.join(dir, 'MANIFEST.txt'), 'utf-8')).toBe('SIGNED-MANIFEST');
+  });
+
+  it('should respect the GRAFANA_COM_URL environment variable', async () => {
+    vi.stubEnv('GRAFANA_COM_URL', 'https://grafana-dev.com/api');
+    const dir = pluginDistDir();
+
+    await sign(argvFor({ distDir: dir }));
+
+    expect(mocks.postData.mock.calls[0][0]).toBe('https://grafana-dev.com/api/plugins/ci/sign');
+  });
+
+  it('should pass signatureType and rootUrls to the manifest', async () => {
+    const dir = pluginDistDir();
+
+    await sign(
+      argvFor({
+        distDir: dir,
+        signatureType: 'private',
+        rootUrls: 'https://example.com/grafana,https://grafana.example.com',
+      })
+    );
+
+    expect(getPostedManifest()).toMatchObject({
+      signatureType: 'private',
+      rootUrls: ['https://example.com/grafana', 'https://grafana.example.com'],
+    });
+  });
+
+  it('should exit without calling the API when a root URL is invalid', async () => {
+    const dir = pluginDistDir();
+
+    await expect(sign(argvFor({ distDir: dir, rootUrls: 'not-a-valid-url' }))).rejects.toThrow(PROCESS_EXIT_MESSAGE);
+
+    expect(exitSpy).toHaveBeenCalledWith(1);
+    expect(mocks.postData).not.toHaveBeenCalled();
+    expect(existsSync(path.join(dir, 'MANIFEST.txt'))).toBe(false);
+  });
+
+  it('should exit without calling the API when no token is configured', async () => {
+    vi.stubEnv('GRAFANA_ACCESS_POLICY_TOKEN', undefined);
+    const dir = pluginDistDir();
+
+    await expect(sign(argvFor({ distDir: dir }))).rejects.toThrow(PROCESS_EXIT_MESSAGE);
+
+    expect(exitSpy).toHaveBeenCalledWith(1);
+    expect(mocks.postData).not.toHaveBeenCalled();
+  });
+
+  it('should report an invalid root URL before a missing token', async () => {
+    vi.stubEnv('GRAFANA_ACCESS_POLICY_TOKEN', undefined);
+    const dir = pluginDistDir();
+
+    await expect(sign(argvFor({ distDir: dir, rootUrls: 'not-a-valid-url' }))).rejects.toThrow(PROCESS_EXIT_MESSAGE);
+
+    const stdout = stdoutSpy.mock.calls.map((call) => String(call[0])).join('');
+    expect(stdout).toContain('not-a-valid-url is not a valid URL');
+    expect(stdout).not.toContain('Missing GRAFANA_ACCESS_POLICY_TOKEN');
+  });
+
+  it('should prefer GRAFANA_ACCESS_POLICY_TOKEN over GRAFANA_API_KEY but still warn about deprecation', async () => {
+    vi.stubEnv('GRAFANA_API_KEY', 'legacy-api-key');
+    const dir = pluginDistDir();
+
+    await sign(argvFor({ distDir: dir }));
+
+    const stdout = stdoutSpy.mock.calls.map((call) => String(call[0])).join('');
+    expect(stdout).toContain('deprecated');
+    expect(mocks.postData.mock.calls[0][2]).toEqual({ Authorization: 'Bearer test-token' });
+  });
+
+  it('should warn about deprecation but proceed when only GRAFANA_API_KEY is set', async () => {
+    vi.stubEnv('GRAFANA_ACCESS_POLICY_TOKEN', undefined);
+    vi.stubEnv('GRAFANA_API_KEY', 'legacy-api-key');
+    const dir = pluginDistDir();
+
+    await sign(argvFor({ distDir: dir }));
+
+    const stdout = stdoutSpy.mock.calls.map((call) => String(call[0])).join('');
+    expect(stdout).toContain('deprecated');
+    expect(mocks.postData.mock.calls[0][2]).toEqual({ Authorization: 'Bearer legacy-api-key' });
+  });
+
+  it('should exit without saving the manifest when the API responds with an error', async () => {
+    mocks.postData.mockResolvedValue({ status: 400, data: JSON.stringify({ message: 'invalid plugin' }) });
+    const dir = pluginDistDir();
+
+    await expect(sign(argvFor({ distDir: dir }))).rejects.toThrow(PROCESS_EXIT_MESSAGE);
+
+    expect(exitSpy).toHaveBeenCalledWith(1);
+    expect(existsSync(path.join(dir, 'MANIFEST.txt'))).toBe(false);
+  });
+
+  it('should exit when the API request fails', async () => {
+    mocks.postData.mockRejectedValue(new Error('socket hang up'));
+    const dir = pluginDistDir();
+
+    await expect(sign(argvFor({ distDir: dir }))).rejects.toThrow(PROCESS_EXIT_MESSAGE);
+
+    expect(exitSpy).toHaveBeenCalledWith(1);
+    expect(existsSync(path.join(dir, 'MANIFEST.txt'))).toBe(false);
+  });
+
+  it('should add the create-plugin version from .config/.cprc.json to the manifest', async () => {
+    const cwdDir = createTempDir();
+    tempDirs.push(cwdDir);
+    writeFiles(cwdDir, { '.config/.cprc.json': JSON.stringify({ version: '5.0.0' }) });
+    vi.spyOn(process, 'cwd').mockReturnValue(cwdDir);
+    const dir = pluginDistDir();
+
+    await sign(argvFor({ distDir: dir }));
+
+    expect(getPostedManifest().createPlugin).toEqual({ version: '5.0.0' });
+  });
+});

packages/sign-plugin/src/commands/sign.command.ts

@@ -46,7 +46,8 @@ export const sign = async (argv: minimist.ParsedArgs) => {
       manifest.createPlugin = { version: createPluginVersion };
     }
 
-    const signedManifest = await signManifest(manifest);
+    const token = getSigningToken();
+    const signedManifest = await signManifest(manifest, token);
 
     saveManifest(pluginDistDir, signedManifest);
     output.success({
@@ -63,3 +64,27 @@ export const sign = async (argv: minimist.ParsedArgs) => {
     process.exit(1);
   }
 };
+
+function getSigningToken(): string {
+  const GRAFANA_API_KEY = process.env.GRAFANA_API_KEY;
+  const GRAFANA_ACCESS_POLICY_TOKEN = process.env.GRAFANA_ACCESS_POLICY_TOKEN;
+  const token = GRAFANA_ACCESS_POLICY_TOKEN ? GRAFANA_ACCESS_POLICY_TOKEN : GRAFANA_API_KEY;
+
+  if (!token) {
+    output.error({
+      title: 'Missing GRAFANA_ACCESS_POLICY_TOKEN.',
+      body: ['You must create a GRAFANA_ACCESS_POLICY_TOKEN env variable to sign plugins.'],
+      link: 'https://grafana.com/developers/plugin-tools/publish-a-plugin/sign-a-plugin#generate-an-access-policy-token',
+    });
+    process.exit(1);
+  }
+  if (GRAFANA_API_KEY) {
+    output.warning({
+      title: 'Usage of GRAFANA_API_KEY is deprecated.',
+      body: ['Please migrate to using a GRAFANA_ACCESS_POLICY_TOKEN instead.'],
+      link: 'https://grafana.com/developers/plugin-tools/publish-a-plugin/sign-a-plugin',
+    });
+  }
+
+  return token;
+}

packages/sign-plugin/src/utils/getCreatePluginVersion.test.ts

@@ -0,0 +1,41 @@
+import { rmSync } from 'node:fs';
+import { vi } from 'vitest';
+import { getCreatePluginVersion } from './getCreatePluginVersion.js';
+import { createTempDir, writeFiles } from './tests/fixtures.js';
+
+describe('getCreatePluginVersion', () => {
+  const tempDirs: string[] = [];
+
+  function mockCwd(files: Record<string, string> = {}) {
+    const dir = createTempDir();
+    tempDirs.push(dir);
+    writeFiles(dir, files);
+    vi.spyOn(process, 'cwd').mockReturnValue(dir);
+  }
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  afterAll(() => {
+    tempDirs.forEach((dir) => rmSync(dir, { recursive: true, force: true }));
+  });
+
+  it('should return the version from .config/.cprc.json', () => {
+    mockCwd({ '.config/.cprc.json': JSON.stringify({ version: '5.0.0' }) });
+
+    expect(getCreatePluginVersion()).toBe('5.0.0');
+  });
+
+  it('should return null when .config/.cprc.json does not exist', () => {
+    mockCwd();
+
+    expect(getCreatePluginVersion()).toBeNull();
+  });
+
+  it('should return null when .config/.cprc.json is malformed', () => {
+    mockCwd({ '.config/.cprc.json': 'not-valid-json' });
+
+    expect(getCreatePluginVersion()).toBeNull();
+  });
+});