diff --git a/packages/cli/e2e/__tests__/deploy.spec.ts b/packages/cli/e2e/__tests__/deploy.spec.ts index 70a65f61d..7ff0ad6fb 100644 --- a/packages/cli/e2e/__tests__/deploy.spec.ts +++ b/packages/cli/e2e/__tests__/deploy.spec.ts @@ -8,7 +8,7 @@ import { DateTime, Duration } from 'luxon' import { describe, it, expect, beforeAll, beforeEach, afterAll, afterEach } from 'vitest' import Projects from '../../src/rest/projects' -import CheckTypes from '../../src/constants' +import { CheckTypes } from '../../src/constants' import { FixtureSandbox, RunOptions } from '../../src/testing/fixture-sandbox' import { ExecaError } from 'execa' diff --git a/packages/cli/src/commands/import/plan.ts b/packages/cli/src/commands/import/plan.ts index 0e912fd19..12629626e 100644 --- a/packages/cli/src/commands/import/plan.ts +++ b/packages/cli/src/commands/import/plan.ts @@ -8,6 +8,7 @@ import chalk from 'chalk' import logSymbols from 'log-symbols' import { validate as validateUuid } from 'uuid' +import { LOGICAL_ID_PATTERN } from '../../constants' import * as api from '../../rest/api' import { AuthCommand } from '../authCommand' import commonMessages from '../../messages/common-messages' @@ -526,7 +527,7 @@ ${chalk.cyan('For safety, resources are not deletable until the plan has been co message: 'How would you like your project to be identified?', initial: suggested, validate: input => { - if (!/^[A-Za-z0-9_\-/#.]+$/.test(input)) { + if (!LOGICAL_ID_PATTERN.test(input)) { return `Please only use ASCII letters, numbers, and the ` + `symbols _, -, /, #, and .` } diff --git a/packages/cli/src/constants.ts b/packages/cli/src/constants.ts index 327bf8433..7fcdef2c4 100644 --- a/packages/cli/src/constants.ts +++ b/packages/cli/src/constants.ts @@ -1,8 +1,8 @@ -const CheckTypes = { +export const CheckTypes = { API: 'API', BROWSER: 'BROWSER', HEARTBEAT: 'HEARTBEAT', MULTI_STEP: 'MULTI_STEP', } -export default CheckTypes +export const LOGICAL_ID_PATTERN = /^[A-Za-z0-9_\-/#.]+$/ diff --git a/packages/cli/src/constructs/__tests__/project.spec.ts b/packages/cli/src/constructs/__tests__/project.spec.ts new file mode 100644 index 000000000..e36f3bf68 --- /dev/null +++ b/packages/cli/src/constructs/__tests__/project.spec.ts @@ -0,0 +1,77 @@ +import { describe, it, expect, beforeEach } from 'vitest' + +import { Session } from '../project' +import { Construct } from '../construct' +import { Diagnostics } from '../diagnostics' + +class TestConstruct extends Construct { + constructor (logicalId: string) { + super('test', logicalId) + } + + describe (): string { + return `Test:${this.logicalId}` + } + + synthesize () { + return null + } +} + +describe('Construct logicalId validation', () => { + beforeEach(() => { + Session.reset() + Session.project = { addResource: () => {} } as any + }) + + it('should pass validation for valid logicalIds', async () => { + const validIds = ['my-check', 'my_check', 'myCheck123', 'my/check', 'my#check', 'my.check'] + for (const id of validIds) { + const construct = new TestConstruct(id) + const diagnostics = new Diagnostics() + await construct.validate(diagnostics) + expect(diagnostics.isFatal(), `Expected "${id}" to be valid`).toBe(false) + } + }) + + it('should store the original logicalId without modification', () => { + const construct = new TestConstruct('my check') + expect(construct.logicalId).toBe('my check') + }) + + it('should produce an error diagnostic for logicalIds with spaces', async () => { + const construct = new TestConstruct('my check') + const diagnostics = new Diagnostics() + await construct.validate(diagnostics) + expect(diagnostics.isFatal()).toBe(true) + expect(diagnostics.observations).toEqual(expect.arrayContaining([ + expect.objectContaining({ + message: expect.stringContaining('contains invalid characters'), + }), + ])) + }) + + it('should produce an error diagnostic for logicalIds with special characters', async () => { + const construct = new TestConstruct('my@check!') + const diagnostics = new Diagnostics() + await construct.validate(diagnostics) + expect(diagnostics.isFatal()).toBe(true) + expect(diagnostics.observations).toEqual(expect.arrayContaining([ + expect.objectContaining({ + message: expect.stringContaining('contains invalid characters'), + }), + ])) + }) + + it('should produce an error diagnostic for empty logicalIds', async () => { + const construct = new TestConstruct('') + const diagnostics = new Diagnostics() + await construct.validate(diagnostics) + expect(diagnostics.isFatal()).toBe(true) + expect(diagnostics.observations).toEqual(expect.arrayContaining([ + expect.objectContaining({ + message: expect.stringContaining('contains invalid characters'), + }), + ])) + }) +}) diff --git a/packages/cli/src/constructs/check-group-v1.ts b/packages/cli/src/constructs/check-group-v1.ts index 0e4862abf..63e06d3d1 100644 --- a/packages/cli/src/constructs/check-group-v1.ts +++ b/packages/cli/src/constructs/check-group-v1.ts @@ -15,7 +15,7 @@ import { import { AlertEscalation } from './alert-escalation-policy' import { Diagnostics } from './diagnostics' import { DeprecatedConstructDiagnostic, DeprecatedPropertyDiagnostic, InvalidPropertyValueDiagnostic } from './construct-diagnostics' -import CheckTypes from '../constants' +import { CheckTypes } from '../constants' import { CheckConfigDefaults } from '../services/checkly-config-loader' import { AlertChannelSubscription } from './alert-channel-subscription' import { BrowserCheck } from './browser-check' diff --git a/packages/cli/src/constructs/construct.ts b/packages/cli/src/constructs/construct.ts index 5e466ba33..48b1be679 100644 --- a/packages/cli/src/constructs/construct.ts +++ b/packages/cli/src/constructs/construct.ts @@ -1,5 +1,7 @@ import path from 'node:path' +import { LOGICAL_ID_PATTERN } from '../constants' +import { InvalidPropertyValueDiagnostic } from './construct-diagnostics' import { Diagnostics } from './diagnostics' import { Session } from './project' import { Ref } from './ref' @@ -123,9 +125,14 @@ export abstract class Construct implements Validate, Bundle { * @param diagnostics The Diagnostics instance that any issues should be added to. * @returns A Promise that resolves when validation is complete. */ - // eslint-disable-next-line @typescript-eslint/no-unused-vars, require-await + // eslint-disable-next-line require-await async validate (diagnostics: Diagnostics): Promise { - return + if (!LOGICAL_ID_PATTERN.test(this.logicalId)) { + diagnostics.add(new InvalidPropertyValueDiagnostic( + 'logicalId', + new Error(`"${this.logicalId}" contains invalid characters. Only A-Z, a-z, 0-9, _, -, /, #, and . are allowed.`), + )) + } } /** diff --git a/packages/cli/src/constructs/heartbeat-monitor.ts b/packages/cli/src/constructs/heartbeat-monitor.ts index 951a6b569..08b8c0f9a 100644 --- a/packages/cli/src/constructs/heartbeat-monitor.ts +++ b/packages/cli/src/constructs/heartbeat-monitor.ts @@ -1,7 +1,7 @@ import { Monitor, MonitorProps } from './monitor' import { Session } from './project' import { DateTime } from 'luxon' -import CheckTypes from '../constants' +import { CheckTypes } from '../constants' type TimeUnits = 'seconds' | 'minutes' | 'hours' | 'days' diff --git a/packages/cli/src/constructs/multi-step-check.ts b/packages/cli/src/constructs/multi-step-check.ts index cb2066de8..7791c8260 100644 --- a/packages/cli/src/constructs/multi-step-check.ts +++ b/packages/cli/src/constructs/multi-step-check.ts @@ -3,7 +3,7 @@ import fs from 'node:fs/promises' import { CheckProps, RuntimeCheck, RuntimeCheckProps } from './check' import { Session, SharedFileRef } from './project' import { Content, Entrypoint, isContent, isEntrypoint } from './construct' -import CheckTypes from '../constants' +import { CheckTypes } from '../constants' import { PlaywrightConfig } from './playwright-config' import { Diagnostics } from './diagnostics' import { InvalidPropertyValueDiagnostic, UnsupportedRuntimeFeatureDiagnostic } from './construct-diagnostics' diff --git a/packages/cli/src/constructs/project.ts b/packages/cli/src/constructs/project.ts index fb7d624ca..8cf1f8e34 100644 --- a/packages/cli/src/constructs/project.ts +++ b/packages/cli/src/constructs/project.ts @@ -328,10 +328,6 @@ export class Session { throw new ValidationError(`The "logicalId" of a ${construct.type} construct must be a string (logicalId=${construct.logicalId} [${typeof construct.logicalId}])`) } - if (!/^[A-Za-z0-9_\-/#.]+$/.test(construct.logicalId)) { - throw new ValidationError(`The "logicalId" can only include the following characters: [A-Za-z0-9_-/#.]. (logicalId='${construct.logicalId}')`) - } - if (construct.type === Project.__checklyType) { // Creating the construct is allowed - We're creating the project. } else if (Session.project) { diff --git a/packages/cli/src/services/project-parser.ts b/packages/cli/src/services/project-parser.ts index e20180cfb..14ce3c474 100644 --- a/packages/cli/src/services/project-parser.ts +++ b/packages/cli/src/services/project-parser.ts @@ -149,6 +149,7 @@ export async function parseProject (opts: ProjectParseOpts): Promise { currentCommand, enableWorkspaces = true, } = opts + const project = new Project(projectLogicalId, { name: projectName, repoUrl,