diff --git a/pkg/index.d.ts b/pkg/index.d.ts index 2be4cce..c8b5b3f 100644 --- a/pkg/index.d.ts +++ b/pkg/index.d.ts @@ -109,6 +109,16 @@ export type Message = } > | BaseMessage<'DEPRECATED_FIELD_JSNEXT'> + | BaseMessage< + 'INVALID_REPOSITORY_VALUE', + { + type: + | 'invalid-string-shorthand' + | 'invalid-git-url' + | 'deprecated-github-git-protocol' + | 'shorthand-git-sites' + } + > export interface Options { /** diff --git a/pkg/src/index.js b/pkg/src/index.js index b78765f..6e5462e 100755 --- a/pkg/src/index.js +++ b/pkg/src/index.js @@ -22,7 +22,11 @@ import { getPkgPathValue, replaceLast, isRelativePath, - isAbsolutePath + isAbsolutePath, + isGitUrl, + isShorthandRepositoryUrl, + isShorthandGitHubOrGitLabUrl, + isDeprecatedGitHubGitUrl } from './utils.js' /** @@ -234,6 +238,12 @@ export async function publint({ pkgDir, vfs, level, strict, _packedFiles }) { } } + // if `repository` field exist, check if the value is valid + // `repository` might be a shorthand string of URL or an object + if ('repository' in rootPkg) { + promiseQueue.push(() => checkRepositoryField(rootPkg.repository)) + } + // check file existence for other known package fields const knownFields = [ 'types', @@ -436,6 +446,55 @@ export async function publint({ pkgDir, vfs, level, strict, _packedFiles }) { } } + // https://docs.npmjs.com/cli/v10/configuring-npm/package-json#repository + /** + * @param {Record | string} repository + */ + async function checkRepositoryField(repository) { + if (!ensureTypeOfField(repository, ['string', 'object'], ['repository'])) + return + + if (typeof repository === 'string') { + // the string field accepts shorthands only. if this doesn't look like a shorthand, + // and looks like a git URL, recommend using the object form. + if (!isShorthandRepositoryUrl(repository)) { + messages.push({ + code: 'INVALID_REPOSITORY_VALUE', + args: { type: 'invalid-string-shorthand' }, + path: ['repository'], + type: 'warning' + }) + } + } else if ( + typeof repository === 'object' && + repository.url && + repository.type === 'git' + ) { + if (!isGitUrl(repository.url)) { + messages.push({ + code: 'INVALID_REPOSITORY_VALUE', + args: { type: 'invalid-git-url' }, + path: ['repository', 'url'], + type: 'warning' + }) + } else if (isDeprecatedGitHubGitUrl(repository.url)) { + messages.push({ + code: 'INVALID_REPOSITORY_VALUE', + args: { type: 'deprecated-github-git-protocol' }, + path: ['repository', 'url'], + type: 'suggestion' + }) + } else if (isShorthandGitHubOrGitLabUrl(repository.url)) { + messages.push({ + code: 'INVALID_REPOSITORY_VALUE', + args: { type: 'shorthand-git-sites' }, + path: ['repository', 'url'], + type: 'suggestion' + }) + } + } + } + /** * @param {string[]} pkgPath */ diff --git a/pkg/src/message.js b/pkg/src/message.js index 704c561..e0eb81b 100644 --- a/pkg/src/message.js +++ b/pkg/src/message.js @@ -158,6 +158,32 @@ export function formatMessage(m, pkg) { case 'DEPRECATED_FIELD_JSNEXT': // prettier-ignore return `${c.bold(fp(m.path))} is deprecated. ${c.bold('pkg.module')} should be used instead.` + case 'INVALID_REPOSITORY_VALUE': + switch (m.args.type) { + case 'invalid-string-shorthand': + // prettier-ignore + return `${c.bold(fp(m.path))} is ${c.bold(pv(m.path))} which isn't a valid shorthand value supported by npm. Consider using an object that references a repository.` + case 'invalid-git-url': + // prettier-ignore + return `${c.bold(fp(m.path))} is ${c.bold(pv(m.path))} which isn't a valid git URL. A valid git URL is usually in the form of "${c.bold('git+https://example.com/user/repo.git')}".` + case 'deprecated-github-git-protocol': + // prettier-ignore + return `${c.bold(fp(m.path))} is ${c.bold(pv(m.path))} which uses the git:// protocol that is deprecated by GitHub due to security concerns. Consider replacing the protocol with https://.` + case 'shorthand-git-sites': { + let fullUrl = pv(m.path) + if (fullUrl[fullUrl.length - 1] === '/') { + fullUrl = fullUrl.slice(0, -1) + } + if (!fullUrl.startsWith('git+')) { + fullUrl = 'git+' + fullUrl + } + if (!fullUrl.endsWith('.git')) { + fullUrl += '.git' + } + // prettier-ignore + return `${c.bold(fp(m.path))} is ${c.bold(pv(m.path))} but could be a full git URL like "${c.bold(fullUrl)}".` + } + } default: return } diff --git a/pkg/src/utils.js b/pkg/src/utils.js index 64d15e7..39c9913 100644 --- a/pkg/src/utils.js +++ b/pkg/src/utils.js @@ -6,6 +6,7 @@ import { lintableFileExtensions } from './constants.js' * main: string, * module: string, * exports: Record, + * repository: Record | string, * type: 'module' | 'commonjs' * }} Pkg */ @@ -45,6 +46,59 @@ export function stripComments(code) { .replace(SINGLELINE_COMMENTS_RE, '') } +// Reference: https://git-scm.com/docs/git-clone#_git_urls and https://github.com/npm/hosted-git-info +const GIT_URL_RE = + /^(git\+https?|git\+ssh|https?|ssh|git):\/\/(?:[\w._-]+@)?([\w.-]+)(?::([\w\d-]+))?(\/[\w._/-]+)\/?$/ +/** + * @param {string} url + */ +export function isGitUrl(url) { + return GIT_URL_RE.test(url) +} +/** + * @param {string} url + */ +export function isShorthandGitHubOrGitLabUrl(url) { + const tokens = url.match(GIT_URL_RE) + if (tokens) { + const host = tokens[2] + const path = tokens[4] + + if (/(github|gitlab)/.test(host)) { + return !url.startsWith('git+') || !path.endsWith('.git') + } + } + + return false +} +/** + * Reference: https://github.blog/security/application-security/improving-git-protocol-security-github/ + * @param {string} url + */ +export function isDeprecatedGitHubGitUrl(url) { + const tokens = url.match(GIT_URL_RE) + if (tokens) { + const protocol = tokens[1] + const host = tokens[2] + + if (/github/.test(host) && protocol === 'git') { + return true + } + } + + return false +} + +// Reference: https://docs.npmjs.com/cli/v10/configuring-npm/package-json#repository +const SHORTHAND_REPOSITORY_URL_RE = + /^(?:(?:github|bitbucket|gitlab):[\w\-]+\/[\w\-]+|gist:\w+|[\w\-]+\/[\w\-]+)$/ +/** + * @param {string} url + */ +export function isShorthandRepositoryUrl(url) { + return SHORTHAND_REPOSITORY_URL_RE.test(url) +} + /** * @param {string} code * @returns {CodeFormat} diff --git a/pkg/tests/fixtures/invalid-field-types/package.json b/pkg/tests/fixtures/invalid-field-types/package.json index aaa9be7..4a34416 100644 --- a/pkg/tests/fixtures/invalid-field-types/package.json +++ b/pkg/tests/fixtures/invalid-field-types/package.json @@ -5,5 +5,6 @@ "type": "commonjs", "main": 0, "module": true, - "jsnext:main": false + "jsnext:main": false, + "repository": 123 } \ No newline at end of file diff --git a/pkg/tests/fixtures/invalid-repository-value-object-deprecated/package.json b/pkg/tests/fixtures/invalid-repository-value-object-deprecated/package.json new file mode 100644 index 0000000..6d2b8ee --- /dev/null +++ b/pkg/tests/fixtures/invalid-repository-value-object-deprecated/package.json @@ -0,0 +1,9 @@ +{ + "name": "publint-invalid-repository-value-object-deprecatec", + "version": "0.0.1", + "type": "commonjs", + "repository": { + "type": "git", + "url": "git://www.github.com/bluwy/publint" + } + } diff --git a/pkg/tests/fixtures/invalid-repository-value-object-not-git-url/package.json b/pkg/tests/fixtures/invalid-repository-value-object-not-git-url/package.json new file mode 100644 index 0000000..a0b4b73 --- /dev/null +++ b/pkg/tests/fixtures/invalid-repository-value-object-not-git-url/package.json @@ -0,0 +1,9 @@ +{ + "name": "publint-invalid-repository-value-object-not-git-url", + "version": "0.0.1", + "type": "commonjs", + "repository": { + "type": "git", + "url": "imap://fake.com/" + } + } diff --git a/pkg/tests/fixtures/invalid-repository-value-object-shorthand-site/package.json b/pkg/tests/fixtures/invalid-repository-value-object-shorthand-site/package.json new file mode 100644 index 0000000..7d300b4 --- /dev/null +++ b/pkg/tests/fixtures/invalid-repository-value-object-shorthand-site/package.json @@ -0,0 +1,9 @@ +{ + "name": "publint-invalid-repository-value-object-shorthand-site", + "version": "0.0.1", + "type": "commonjs", + "repository": { + "type": "git", + "url": "https://www.github.com/bluwy/publint" + } + } diff --git a/pkg/tests/fixtures/invalid-repository-value-shorthand/package.json b/pkg/tests/fixtures/invalid-repository-value-shorthand/package.json new file mode 100644 index 0000000..f4c83a4 --- /dev/null +++ b/pkg/tests/fixtures/invalid-repository-value-shorthand/package.json @@ -0,0 +1,6 @@ +{ + "name": "publint-invalid-repository-value-shorthand", + "version": "0.0.1", + "type": "commonjs", + "repository": "npm/npm" + } diff --git a/pkg/tests/fixtures/invalid-repository-value-string-not-url/package.json b/pkg/tests/fixtures/invalid-repository-value-string-not-url/package.json new file mode 100644 index 0000000..c1ae5db --- /dev/null +++ b/pkg/tests/fixtures/invalid-repository-value-string-not-url/package.json @@ -0,0 +1,6 @@ +{ + "name": "publint-invalid-repository-value-string-not-url", + "version": "0.0.1", + "type": "commonjs", + "repository": "not_an_url" + } diff --git a/pkg/tests/playground.js b/pkg/tests/playground.js index ee5a093..a343e3d 100644 --- a/pkg/tests/playground.js +++ b/pkg/tests/playground.js @@ -26,6 +26,7 @@ testFixture('glob-deprecated', [ ]) testFixture('invalid-field-types', [ + 'FIELD_INVALID_VALUE_TYPE', 'FIELD_INVALID_VALUE_TYPE', 'FIELD_INVALID_VALUE_TYPE', 'FIELD_INVALID_VALUE_TYPE' @@ -144,6 +145,36 @@ testFixture('deprecated-fields', [ 'USE_TYPE' ]) +testFixture('invalid-repository-value-string-not-url', [ + { + code: 'INVALID_REPOSITORY_VALUE', + type: 'warning' + } +]) + +testFixture('invalid-repository-value-shorthand', []) + +testFixture('invalid-repository-value-object-not-git-url', [ + { + code: 'INVALID_REPOSITORY_VALUE', + type: 'warning' + } +]) + +testFixture('invalid-repository-value-object-shorthand-site', [ + { + code: 'INVALID_REPOSITORY_VALUE', + type: 'suggestion' + } +]) + +testFixture('invalid-repository-value-object-deprecated', [ + { + code: 'INVALID_REPOSITORY_VALUE', + type: 'suggestion' + } +]) + /** * @typedef {{ * level?: import('../index.d.ts').Options['level'] diff --git a/pkg/tests/utils.js b/pkg/tests/utils.js index 46fe53c..a381c46 100644 --- a/pkg/tests/utils.js +++ b/pkg/tests/utils.js @@ -7,8 +7,12 @@ import { getCodeFormat, isCodeCjs, isCodeEsm, + isDeprecatedGitHubGitUrl, isFileContentLintable, isFilePathLintable, + isGitUrl, + isShorthandGitHubOrGitLabUrl, + isShorthandRepositoryUrl, stripComments } from '../src/utils.js' import { createNodeVfs } from '../src/vfs.js' @@ -104,6 +108,64 @@ test('getCodeFormat', () => { } }) +test('isGitUrl', () => { + equal(isGitUrl('https://host.xz/path/to/repo.git/'), true) + equal(isGitUrl('http://host.xz/path/to/repo.git/'), true) + equal(isGitUrl('https://host.xz/path/to/repo.git'), true) + equal(isGitUrl('https://subdomain.host.xz/path/to/repo.git'), true) + equal(isGitUrl('https://192.168.0.1/path/to/repo.git'), true) + equal(isGitUrl('https://192.168.0.1:1234/path/to/repo.git'), true) + equal(isGitUrl('http://host.xz/path/to/repo.git'), true) + equal(isGitUrl('git+https://host.xz/path/to/repo.git'), true) + equal(isGitUrl('git+https://host.xz/path/to/repo'), true) + equal(isGitUrl('https://host.xz/path/to/repo.git'), true) + equal(isGitUrl('git+ssh://git@host.xz/path/to/repo.git'), true) + equal(isGitUrl('git+ssh://git@host.xz/path/to/repo'), true) + equal(isGitUrl('git+ssh://git@host.xz:1234/path/to/repo'), true) + equal(isGitUrl('git+ssh://host.xz:1234/path/to/repo'), true) + equal(isGitUrl('git://host.xz/path/to/repo.git'), true) + equal(isGitUrl('git://host.xz/path/to/repo'), true) + equal(isGitUrl('ssh://git@host.xz/user/project.git'), true) + equal(isGitUrl('ssh://git@host.xz:user/project.git'), true) + equal(isGitUrl('git+ssh://git@host.xz/user/project.git'), true) + equal(isGitUrl('git+ssh://git@host.xz:user/project.git'), true) + // NOTE: this technically works, but it's quite an edge case and technically not a URL, + // so maybe better to skip this and encourage proper URL format instead + // equal(isGitUrl('git@github.com:react-component/tooltip.git'), true) + + equal(isGitUrl('file://host.xz/path/to/repo'), false) + equal(isGitUrl('/User/foo/bar'), false) +}) + +test('isDeprecatedGitHubGitUrl', () => { + equal(isDeprecatedGitHubGitUrl('git://github.com/user/project.git'), true) + equal(isDeprecatedGitHubGitUrl('https://github.com/user/project'), false) +}) + +test('isShorthandGitHubOrGitLabUrl', () => { + const f = isShorthandGitHubOrGitLabUrl // shorten to please prettier + equal(f('https://github.com/user/project'), true) + equal(f('git+https://github.com/user/project'), true) + equal(f('https://github.com/user/project.git'), true) + equal(f('https://gitlab.com/user/project'), true) + equal(f('git+https://gitlab.com/user/project'), true) + equal(f('https://gitlab.com/user/project.git'), true) + + equal(f('https://bitbucket.com/user/project'), false) + equal(f('https://example.com'), false) +}) + +test('isShortHandRepositoryUrl', () => { + equal(isShorthandRepositoryUrl('user/project'), true) + equal(isShorthandRepositoryUrl('github:user/project'), true) + equal(isShorthandRepositoryUrl('gist:11081aaa281'), true) + equal(isShorthandRepositoryUrl('bitbucket:user/project'), true) + equal(isShorthandRepositoryUrl('gitlab:user/project'), true) + + equal(isShorthandRepositoryUrl('foobar'), false) + equal(isShorthandRepositoryUrl('https://github.com/user/project'), false) +}) + test('stripComments', () => { const result = stripComments(` // hello world diff --git a/site/rules.md b/site/rules.md index 9df1696..a0ce1d3 100644 --- a/site/rules.md +++ b/site/rules.md @@ -238,3 +238,23 @@ To fix this, you can rename `"./lib.server.js"` to `"./lib.worker.js"` for examp ## `DEPRECATED_FIELD_JSNEXT` The `"jsnext:main"` and `"jsnext"` fields are deprecated. The `"module"` field should be used instead. See [this issue](https://github.com/jsforum/jsforum/issues/5) for more information. + +## `INVALID_REPOSITORY_VALUE` + +`publint` is able to detect some cases where the `"repository"` field is not a valid and may not properly display on https://npmjs.com. The sub-rules below are mostly based on the [`"repository"` field docs](https://docs.npmjs.com/cli/v10/configuring-npm/package-json#repository). + +- If `"repository"` is a string, it must be one of the supported shorthand strings from the docs. +- If `"repository"` is an object with `"type": "git"`, the `"url"` must be a valid [git URL](https://git-scm.com/docs/git-clone#_git_urls) and can be [parsed by npm](https://github.com/npm/hosted-git-info). +- The `git://` protocol for GitHub repos should not be used due [security concerns](https://github.blog/security/application-security/improving-git-protocol-security-github/). +- GitHub or GitLab links should be prefixed with `git+` and postfixed with `.git`. (This is also warned by npm when publishing a package). + +An example config that meets these criterias may look like this: + +```json +{ + "repository": { + "type": "git", + "url": "git+https://github.com/bluwy/publint.git" + } +} +``` diff --git a/site/src/utils/message.js b/site/src/utils/message.js index d22784b..ab2916d 100644 --- a/site/src/utils/message.js +++ b/site/src/utils/message.js @@ -100,8 +100,8 @@ function messageToString(m, pkg) { if (m.args.actualExtension && m.args.expectExtension) { // prettier-ignore return `The types is not exported. Consider adding ${bold(fp(m.path) + '.types')} to be compatible with TypeScript's ${bold('"moduleResolution": "bundler"')} compiler option. ` - + `Note that you cannot use "${bold(typesFilePath)}" because it has a mismatching format. Instead, you can duplicate the file and use the ${bold(m.args.expectExtension)} extension, e.g. ` - + `${bold(fp(m.path) + '.types')}: "${bold(replaceLast(typesFilePath,m.args.actualExtension, m.args.expectExtension))}"` + + `Note that you cannot use "${bold(typesFilePath)}" because it has a mismatching format. Instead, you can duplicate the file and use the ${bold(m.args.expectExtension)} extension, e.g. ` + + `${bold(fp(m.path) + '.types')}: "${bold(replaceLast(typesFilePath, m.args.actualExtension, m.args.expectExtension))}"` } else { // prettier-ignore return `The types is not exported. Consider adding ${bold(fp(m.path) + '.types')}: "${bold(typesFilePath)}" to be compatible with TypeScript's ${bold('"moduleResolution": "bundler"')} compiler option.` @@ -152,6 +152,32 @@ function messageToString(m, pkg) { case 'DEPRECATED_FIELD_JSNEXT': // prettier-ignore return `${bold(fp(m.path))} is deprecated. ${bold('pkg.module')} should be used instead.` + case 'INVALID_REPOSITORY_VALUE': + switch (m.args.type) { + case 'invalid-string-shorthand': + // prettier-ignore + return `The field value isn't a valid shorthand value supported by npm. Consider using an object that references a repository.` + case 'invalid-git-url': + // prettier-ignore + return `The field value isn't a valid git URL. A valid git URL is usually in the form of "${bold('git+https://example.com/user/repo.git')}".` + case 'deprecated-github-git-protocol': + // prettier-ignore + return `The field value uses the git:// protocol that is deprecated by GitHub due to security concerns. Consider replacing the protocol with https://.` + case 'shorthand-git-sites': { + let fullUrl = pv(m.path) + if (fullUrl[fullUrl.length - 1] === '/') { + fullUrl = fullUrl.slice(0, -1) + } + if (!fullUrl.startsWith('git+')) { + fullUrl = 'git+' + fullUrl + } + if (!fullUrl.endsWith('.git')) { + fullUrl += '.git' + } + // prettier-ignore + return `The field value could be a full git URL like "${bold(fullUrl)}".` + } + } default: return }