diff --git a/packages/eslint-plugin-boxel/eslint.config.js b/packages/eslint-plugin-boxel/eslint.config.js index e028f80a311..0fa7f9edc7d 100644 --- a/packages/eslint-plugin-boxel/eslint.config.js +++ b/packages/eslint-plugin-boxel/eslint.config.js @@ -12,6 +12,9 @@ module.exports = [ 'dist/**', 'tmp/**', 'compiled/**', + // Type-aware rule fixtures: these are inputs to a TypeScript program the + // rule tester builds, not sources this package compiles or ships. + 'tests/lib/rules/fixtures/**', ], }, diff --git a/packages/eslint-plugin-boxel/lib/rules/no-url-from-realm-identifier.js b/packages/eslint-plugin-boxel/lib/rules/no-url-from-realm-identifier.js new file mode 100644 index 00000000000..2c5b391206f --- /dev/null +++ b/packages/eslint-plugin-boxel/lib/rules/no-url-from-realm-identifier.js @@ -0,0 +1,97 @@ +'use strict'; + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +// `RealmIdentifier` and `RealmResourceIdentifier` are branded strings: +// +// type RealmResourceIdentifier = string & { __rriBrand: unknown }; +// +// so `new URL(identifier)` typechecks and throws only at runtime, and only for +// the canonical prefix form. The compiler cannot see the defect; a type-aware +// rule can, because the brand is a type. +const BRANDS = { + __riBrand: 'RealmIdentifier', + __rriBrand: 'RealmResourceIdentifier', +}; + +// Which brand, if any, a type carries. A branded string is an intersection, and +// a union can carry one in a single member, so both are walked. +// `getApparentType` covers a type parameter constrained to a branded string. +function brandOf(type, checker, seen = new Set()) { + if (!type || seen.has(type)) { + return undefined; + } + seen.add(type); + for (const brand of Object.keys(BRANDS)) { + if (type.getProperty ? type.getProperty(brand) : false) { + return brand; + } + } + for (const part of type.types || []) { + const found = brandOf(part, checker, seen); + if (found) { + return found; + } + } + const apparent = checker.getApparentType(type); + if (apparent !== type) { + return brandOf(apparent, checker, seen); + } + return undefined; +} + +module.exports = { + meta: { + type: 'problem', + docs: { + description: + 'Disallow constructing a URL from a realm identifier, which throws for the canonical prefix form', + category: 'Possible Errors', + recommended: false, + }, + schema: [], + messages: { + urlFromIdentifier: + '`new URL()` on a {{brand}} throws for a prefix-form identifier. Use `new RealmPaths(ri(x))` for path work, `virtualNetwork.toURL(x)` at a genuine network boundary, or ask the realm server which realm a URL belongs to. If this really is the boundary, disable this rule on the line with a reason.', + }, + }, + + create(context) { + // Type information is only present where the config sets + // `parserOptions.project`. Everywhere else this rule is inert rather than + // wrong, so it can be enabled repo-wide and tightened per package. + const services = context.sourceCode + ? context.sourceCode.parserServices || context.parserServices + : context.parserServices; + if (!services || !services.program || !services.esTreeNodeToTSNodeMap) { + return {}; + } + const checker = services.program.getTypeChecker(); + + return { + NewExpression(node) { + if (node.callee.type !== 'Identifier' || node.callee.name !== 'URL') { + return; + } + const arg = node.arguments[0]; + if (!arg || arg.type === 'SpreadElement') { + return; + } + const tsNode = services.esTreeNodeToTSNodeMap.get(arg); + if (!tsNode) { + return; + } + const brand = brandOf(checker.getTypeAtLocation(tsNode), checker); + if (brand) { + context.report({ + node, + messageId: 'urlFromIdentifier', + data: { brand: BRANDS[brand] }, + }); + } + }, + }; + }, +}; diff --git a/packages/eslint-plugin-boxel/tests/lib/rules/fixtures/identifiers.ts b/packages/eslint-plugin-boxel/tests/lib/rules/fixtures/identifiers.ts new file mode 100644 index 00000000000..42f1c9088f8 --- /dev/null +++ b/packages/eslint-plugin-boxel/tests/lib/rules/fixtures/identifiers.ts @@ -0,0 +1,11 @@ +// Mirrors the brands in @cardstack/runtime-common so the rule can be exercised +// without depending on that package from the plugin's tests. +export type RealmResourceIdentifier = string & { __rriBrand: unknown }; +export type RealmIdentifier = string & { __riBrand: unknown }; + +export function rri(s: string): RealmResourceIdentifier { + return s as RealmResourceIdentifier; +} +export function ri(s: string): RealmIdentifier { + return s as RealmIdentifier; +} diff --git a/packages/eslint-plugin-boxel/tests/lib/rules/fixtures/subject.ts b/packages/eslint-plugin-boxel/tests/lib/rules/fixtures/subject.ts new file mode 100644 index 00000000000..03b505eab52 --- /dev/null +++ b/packages/eslint-plugin-boxel/tests/lib/rules/fixtures/subject.ts @@ -0,0 +1,4 @@ +// Placeholder so `parserOptions.project` includes this path. The rule tester +// supplies the actual source for each case; only the path has to be part of +// the fixture program. +export {}; diff --git a/packages/eslint-plugin-boxel/tests/lib/rules/fixtures/tsconfig.json b/packages/eslint-plugin-boxel/tests/lib/rules/fixtures/tsconfig.json new file mode 100644 index 00000000000..9d84e42f7d8 --- /dev/null +++ b/packages/eslint-plugin-boxel/tests/lib/rules/fixtures/tsconfig.json @@ -0,0 +1,11 @@ +{ + "compilerOptions": { + "target": "ES2022", + "module": "ESNext", + "moduleResolution": "bundler", + "strict": true, + "noEmit": true, + "lib": ["ES2022", "DOM"] + }, + "include": ["**/*.ts"] +} diff --git a/packages/eslint-plugin-boxel/tests/lib/rules/no-url-from-realm-identifier-test.js b/packages/eslint-plugin-boxel/tests/lib/rules/no-url-from-realm-identifier-test.js new file mode 100644 index 00000000000..4b8cc12c470 --- /dev/null +++ b/packages/eslint-plugin-boxel/tests/lib/rules/no-url-from-realm-identifier-test.js @@ -0,0 +1,73 @@ +'use strict'; + +const path = require('path'); +const rule = require('../../../lib/rules/no-url-from-realm-identifier'); +const RuleTester = require('eslint').RuleTester; + +const fixtures = path.join(__dirname, 'fixtures'); + +// This rule reads types, so the tester needs a real TypeScript program. +const ruleTester = new RuleTester({ + parser: require.resolve('@typescript-eslint/parser'), + parserOptions: { + ecmaVersion: 2022, + sourceType: 'module', + project: path.join(fixtures, 'tsconfig.json'), + tsconfigRootDir: fixtures, + }, +}); + +const IMPORT = `import { rri, ri } from './identifiers';\n`; +const filename = path.join(fixtures, 'subject.ts'); + +ruleTester.run('no-url-from-realm-identifier', rule, { + valid: [ + // A plain string is not an identifier — the rule is about the brand, not + // about calling `new URL` at all. + { + code: `let u = new URL('https://example.com/base/');`, + filename, + }, + // Resolving through the VirtualNetwork is the sanctioned boundary. + { + code: `${IMPORT}declare const vn: { toURL(x: string): URL };\nlet u = vn.toURL(rri('@cardstack/base/card-api'));`, + filename, + }, + // Reading a URL's own href back is unrelated to the identifier types. + { + code: `let u = new URL(new URL('https://example.com/').href);`, + filename, + }, + ], + + invalid: [ + { + code: `${IMPORT}let u = new URL(rri('@cardstack/base/card-api'));`, + filename, + errors: [{ messageId: 'urlFromIdentifier' }], + }, + { + code: `${IMPORT}let u = new URL(ri('@cardstack/base/'));`, + filename, + errors: [{ messageId: 'urlFromIdentifier' }], + }, + // Through a variable, which is where the compiler's silence really bites. + { + code: `${IMPORT}let id = rri('@cardstack/catalog/Author/mango');\nlet u = new URL(id);`, + filename, + errors: [{ messageId: 'urlFromIdentifier' }], + }, + // A second argument does not make the first one safe. + { + code: `${IMPORT}let u = new URL(rri('@cardstack/base/x'), 'https://example.com/');`, + filename, + errors: [{ messageId: 'urlFromIdentifier' }], + }, + // An optional identifier still carries the brand in its union. + { + code: `${IMPORT}declare const maybe: ReturnType | undefined;\nlet u = new URL(maybe as ReturnType);`, + filename, + errors: [{ messageId: 'urlFromIdentifier' }], + }, + ], +}); diff --git a/packages/runtime-common/.eslintrc.cjs b/packages/runtime-common/.eslintrc.cjs new file mode 100644 index 00000000000..7228ed6fe6e --- /dev/null +++ b/packages/runtime-common/.eslintrc.cjs @@ -0,0 +1,31 @@ +'use strict'; + +// Type-aware linting, scoped to this package. +// +// `no-url-from-realm-identifier` asks whether a value carries one of the +// realm-identifier brands. Those exist only in the type system, so the rule +// needs a TypeScript program — without `parserOptions.project` it is silently +// inert. Building that program makes this package's lint slower, which is why +// it is turned on here rather than repo-wide. +// +// Reporting as a warning to start: the existing sites need triage into "really +// is the network boundary" and "should never have parsed this", and a warning +// surfaces them without blocking anyone mid-triage. It becomes an error once +// that list is empty. +module.exports = { + overrides: [ + { + files: ['**/*.ts'], + excludedFiles: ['**/*.d.ts'], + parser: '@typescript-eslint/parser', + parserOptions: { + project: './tsconfig.json', + tsconfigRootDir: __dirname, + }, + plugins: ['@cardstack/boxel'], + rules: { + '@cardstack/boxel/no-url-from-realm-identifier': 'warn', + }, + }, + ], +}; diff --git a/scripts/find-url-on-realm-identifier.mjs b/scripts/find-url-on-realm-identifier.mjs new file mode 100644 index 00000000000..081bea11ae0 --- /dev/null +++ b/scripts/find-url-on-realm-identifier.mjs @@ -0,0 +1,88 @@ +// Reports every `new URL(x)` whose argument is typed as a realm identifier. +// +// `RealmIdentifier` and `RealmResourceIdentifier` are branded strings, so +// `new URL(identifier)` typechecks and throws only at runtime, and only for the +// canonical form — the compiler is structurally unable to flag it. This walks +// the same type information the compiler has and asks the question directly. +// +// Usage: node scripts/find-url-on-realm-identifier.mjs [...] +import ts from 'typescript'; +import { relative } from 'path'; + +const BRANDS = ['__riBrand', '__rriBrand']; + +function brandOf(type, checker) { + const seen = new Set(); + const walk = (t) => { + if (!t || seen.has(t)) return undefined; + seen.add(t); + for (const brand of BRANDS) { + if (t.getProperty?.(brand)) return brand; + } + // A branded string is an intersection; a union may carry one in a member. + for (const part of t.types ?? []) { + const found = walk(part); + if (found) return found; + } + const apparent = checker.getApparentType(t); + if (apparent !== t) return walk(apparent); + return undefined; + }; + return walk(type); +} + +const configs = process.argv.slice(2); +if (configs.length === 0) { + console.error( + 'usage: node scripts/find-url-on-realm-identifier.mjs ...', + ); + process.exit(2); +} + +let total = 0; +for (const configPath of configs) { + const parsed = ts.getParsedCommandLineOfConfigFile( + configPath, + {}, + { + ...ts.sys, + onUnRecoverableConfigFileDiagnostic: (d) => + console.error(ts.flattenDiagnosticMessageText(d.messageText, '\n')), + }, + ); + if (!parsed) continue; + + const program = ts.createProgram(parsed.fileNames, parsed.options); + const checker = program.getTypeChecker(); + + for (const sf of program.getSourceFiles()) { + if (sf.isDeclarationFile || sf.fileName.includes('/node_modules/')) + continue; + + const visit = (node) => { + if (ts.isNewExpression(node) && node.expression.getText(sf) === 'URL') { + const arg = node.arguments?.[0]; + if (arg) { + const brand = brandOf(checker.getTypeAtLocation(arg), checker); + if (brand) { + const { line } = sf.getLineAndCharacterOfPosition( + node.getStart(sf), + ); + const kind = + brand === '__riBrand' + ? 'RealmIdentifier' + : 'RealmResourceIdentifier'; + console.log( + `${relative(process.cwd(), sf.fileName)}:${line + 1} ${kind} new URL(${arg.getText(sf).slice(0, 60)})`, + ); + total++; + } + } + } + ts.forEachChild(node, visit); + }; + visit(sf); + } +} + +console.log(`\n${total} site(s) constructing a URL from a realm identifier.`);