From 2b3859692fc4765de1d33ae5d16137d6cc7145b2 Mon Sep 17 00:00:00 2001 From: Cristopher Date: Tue, 6 Aug 2024 21:09:41 +0700 Subject: [PATCH 01/20] feat: add INVALID_REPOSITORY_VALUE rule --- pkg/index.d.ts | 3 + pkg/src/index.js | 61 ++++++++++++++++++- pkg/src/message.js | 12 ++++ pkg/src/utils.js | 10 +++ .../invalid-repository-value/index.js | 0 .../invalid-repository-value/package.json | 7 +++ pkg/tests/playground.js | 2 + 7 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 pkg/tests/fixtures/invalid-repository-value/index.js create mode 100644 pkg/tests/fixtures/invalid-repository-value/package.json diff --git a/pkg/index.d.ts b/pkg/index.d.ts index 2be4cce..fff9cd3 100644 --- a/pkg/index.d.ts +++ b/pkg/index.d.ts @@ -109,6 +109,9 @@ export type Message = } > | BaseMessage<'DEPRECATED_FIELD_JSNEXT'> + | BaseMessage<'INVALID_REPOSITORY_VALUE', { + directory?: string; + }>; export interface Options { /** diff --git a/pkg/src/index.js b/pkg/src/index.js index b78765f..3b9f8c3 100755 --- a/pkg/src/index.js +++ b/pkg/src/index.js @@ -22,7 +22,8 @@ import { getPkgPathValue, replaceLast, isRelativePath, - isAbsolutePath + isAbsolutePath, + isRepositoryUrl } from './utils.js' /** @@ -234,6 +235,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 +443,58 @@ export async function publint({ pkgDir, vfs, level, strict, _packedFiles }) { } } + /** + * @param {Record | string} repositoryField + */ + async function checkRepositoryField(repositoryField) { + if (typeof repositoryField === 'string') { + if (!isRepositoryUrl(repositoryField)) { + messages.push({ + code: 'INVALID_REPOSITORY_VALUE', + args: {}, + path: ['repository'], + type: 'warning', + }); + } + + return; + } else if (typeof repositoryField === 'object') { + if (repositoryField.url && !isRepositoryUrl(repositoryField.url)) { + messages.push({ + code: 'INVALID_REPOSITORY_VALUE', + args: {}, + path: ['repository', 'url'], + type: 'warning', + }); + } + + if (repositoryField.directory) { + const altRootPath = vfs.pathJoin(rootPkgPath, repositoryField.directory); + const isAltRootExist = await vfs.isPathExist(altRootPath); + + if (!isAltRootExist) { + messages.push({ + code: 'INVALID_REPOSITORY_VALUE', + args: { + directory: repositoryField.directory, + }, + path: ['repository', 'directory'], + type: 'error', + }); + } + } + + return; + } + + messages.push({ + code: 'INVALID_REPOSITORY_VALUE', + args: {}, + path: ['repository'], + type: 'warning', + }) + } + /** * @param {string[]} pkgPath */ diff --git a/pkg/src/message.js b/pkg/src/message.js index 704c561..13b68d9 100644 --- a/pkg/src/message.js +++ b/pkg/src/message.js @@ -158,6 +158,18 @@ 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': + if (m.path.length === 1) { + return `${c.bold(fp(m.path))} must be a valid GitHub URL or an object that contains repository metadata.`; + } + + if (m.path[m.path.length - 1] === 'url') { + return `${c.bold(fp(m.path))} must be a valid GitHub URL.`; + } + + const dir = m.args.directory || ''; + + return `Cannot find package.json in ${c.bold(dir)}. Please ensure that package.json exist in the provided path.` default: return } diff --git a/pkg/src/utils.js b/pkg/src/utils.js index 64d15e7..5b0e56d 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,15 @@ export function stripComments(code) { .replace(SINGLELINE_COMMENTS_RE, '') } +// Reference: https://github.com/JoshuaKGoldberg/eslint-plugin-package-json/blob/main/src/rules/repository-shorthand.ts +const REPOSITORY_URL = /^(?:git\+)?(?:ssh:\/\/git@|http?s:\/\/)?(?:www\.)?github\.com\// +/** + * @param {string} url + */ +export function isRepositoryUrl(url) { + return REPOSITORY_URL.test(url); +} + /** * @param {string} code * @returns {CodeFormat} diff --git a/pkg/tests/fixtures/invalid-repository-value/index.js b/pkg/tests/fixtures/invalid-repository-value/index.js new file mode 100644 index 0000000..e69de29 diff --git a/pkg/tests/fixtures/invalid-repository-value/package.json b/pkg/tests/fixtures/invalid-repository-value/package.json new file mode 100644 index 0000000..25934aa --- /dev/null +++ b/pkg/tests/fixtures/invalid-repository-value/package.json @@ -0,0 +1,7 @@ +{ + "name": "publint-invalid-repository-value", + "version": "0.0.1", + "type": "commonjs", + "main": "./index.js", + "repository": "https://www.google.com" + } diff --git a/pkg/tests/playground.js b/pkg/tests/playground.js index ee5a093..72a750b 100644 --- a/pkg/tests/playground.js +++ b/pkg/tests/playground.js @@ -144,6 +144,8 @@ testFixture('deprecated-fields', [ 'USE_TYPE' ]) +testFixture('invalid-repository-value', ['INVALID_REPOSITORY_VALUE']) + /** * @typedef {{ * level?: import('../index.d.ts').Options['level'] From 6e3b0408e685f10f1601cf8addfb54be70beb1de Mon Sep 17 00:00:00 2001 From: Cristopher Date: Wed, 7 Aug 2024 20:10:07 +0700 Subject: [PATCH 02/20] feat: implement INVALID_REPOSITORY_VALUE on site --- site/src/utils/message.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/site/src/utils/message.js b/site/src/utils/message.js index d22784b..ecc3cf1 100644 --- a/site/src/utils/message.js +++ b/site/src/utils/message.js @@ -152,6 +152,18 @@ 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': + if (m.path.length === 1) { + return `${bold(fp(m.path))} must be a valid GitHub URL or an object that contains repository metadata.`; + } + + if (m.path[m.path.length - 1] === 'url') { + return `${bold(fp(m.path))} must be a valid GitHub URL.`; + } + + const dir = m.args.directory || ''; + + return `Cannot find package.json in ${bold(dir)}. Please ensure that package.json exist in the provided path.` default: return } From 91bf8bb5bb3a6256d7afadfea82eb0b4257fc531 Mon Sep 17 00:00:00 2001 From: Cristopher Date: Wed, 7 Aug 2024 21:39:16 +0700 Subject: [PATCH 03/20] feat: improve regex to be more generic --- pkg/src/utils.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/src/utils.js b/pkg/src/utils.js index 5b0e56d..f79d505 100644 --- a/pkg/src/utils.js +++ b/pkg/src/utils.js @@ -46,8 +46,8 @@ export function stripComments(code) { .replace(SINGLELINE_COMMENTS_RE, '') } -// Reference: https://github.com/JoshuaKGoldberg/eslint-plugin-package-json/blob/main/src/rules/repository-shorthand.ts -const REPOSITORY_URL = /^(?:git\+)?(?:ssh:\/\/git@|http?s:\/\/)?(?:www\.)?github\.com\// +// Reference: https://www.debuggex.com/r/cleXSxwdY3n9BaMV +const REPOSITORY_URL = /((git|ssh|http(s)?)|(git@[\w\.]+)|([\w\.]+@[\w\.]+))(:(\/\/)?|:)([\w\.@\:/\-~]+)(\.git)(\/)?/; /** * @param {string} url */ From d9773aa84cd60053773f5c31bd041fec0d0b223d Mon Sep 17 00:00:00 2001 From: Cristopher Date: Wed, 7 Aug 2024 21:42:02 +0700 Subject: [PATCH 04/20] fix: replace rootPkgPath with pkgDir as intended --- pkg/src/index.js | 18 +++++++----------- pkg/src/message.js | 2 +- site/src/utils/message.js | 2 +- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/pkg/src/index.js b/pkg/src/index.js index 3b9f8c3..3c883b4 100755 --- a/pkg/src/index.js +++ b/pkg/src/index.js @@ -448,16 +448,12 @@ export async function publint({ pkgDir, vfs, level, strict, _packedFiles }) { */ async function checkRepositoryField(repositoryField) { if (typeof repositoryField === 'string') { - if (!isRepositoryUrl(repositoryField)) { - messages.push({ - code: 'INVALID_REPOSITORY_VALUE', - args: {}, - path: ['repository'], - type: 'warning', - }); - } - - return; + messages.push({ + code: 'INVALID_REPOSITORY_VALUE', + args: {}, + path: ['repository'], + type: 'suggestion', + }); } else if (typeof repositoryField === 'object') { if (repositoryField.url && !isRepositoryUrl(repositoryField.url)) { messages.push({ @@ -469,7 +465,7 @@ export async function publint({ pkgDir, vfs, level, strict, _packedFiles }) { } if (repositoryField.directory) { - const altRootPath = vfs.pathJoin(rootPkgPath, repositoryField.directory); + const altRootPath = vfs.pathJoin(pkgDir, repositoryField.directory); const isAltRootExist = await vfs.isPathExist(altRootPath); if (!isAltRootExist) { diff --git a/pkg/src/message.js b/pkg/src/message.js index 13b68d9..5f87433 100644 --- a/pkg/src/message.js +++ b/pkg/src/message.js @@ -160,7 +160,7 @@ export function formatMessage(m, pkg) { return `${c.bold(fp(m.path))} is deprecated. ${c.bold('pkg.module')} should be used instead.` case 'INVALID_REPOSITORY_VALUE': if (m.path.length === 1) { - return `${c.bold(fp(m.path))} must be a valid GitHub URL or an object that contains repository metadata.`; + return `Consider using an object for to represent ${c.bold(fp(m.path))}.`; } if (m.path[m.path.length - 1] === 'url') { diff --git a/site/src/utils/message.js b/site/src/utils/message.js index ecc3cf1..9406c4d 100644 --- a/site/src/utils/message.js +++ b/site/src/utils/message.js @@ -154,7 +154,7 @@ function messageToString(m, pkg) { return `${bold(fp(m.path))} is deprecated. ${bold('pkg.module')} should be used instead.` case 'INVALID_REPOSITORY_VALUE': if (m.path.length === 1) { - return `${bold(fp(m.path))} must be a valid GitHub URL or an object that contains repository metadata.`; + return `Consider using an object for to represent ${bold(fp(m.path))}.`; } if (m.path[m.path.length - 1] === 'url') { From 039c505e153747b331a0e7c6176d1228958a18cd Mon Sep 17 00:00:00 2001 From: Cristopher Date: Wed, 7 Aug 2024 21:46:06 +0700 Subject: [PATCH 05/20] fix: only check for URL if type is .git --- pkg/src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/src/index.js b/pkg/src/index.js index 3c883b4..ab6fbba 100755 --- a/pkg/src/index.js +++ b/pkg/src/index.js @@ -455,7 +455,7 @@ export async function publint({ pkgDir, vfs, level, strict, _packedFiles }) { type: 'suggestion', }); } else if (typeof repositoryField === 'object') { - if (repositoryField.url && !isRepositoryUrl(repositoryField.url)) { + if (repositoryField.url && repositoryField.type === 'git' && !isRepositoryUrl(repositoryField.url)) { messages.push({ code: 'INVALID_REPOSITORY_VALUE', args: {}, From 2814291e51ff6d5b6f02d026c16e66f7f78677c7 Mon Sep 17 00:00:00 2001 From: Cristopher Date: Fri, 9 Aug 2024 22:45:13 +0700 Subject: [PATCH 06/20] fix: revise code based on review --- pkg/index.d.ts | 4 ++- pkg/src/index.js | 64 +++++++++++++++++++--------------------------- pkg/src/message.js | 16 +++++++----- pkg/src/utils.js | 33 +++++++++++++++++++++--- 4 files changed, 68 insertions(+), 49 deletions(-) diff --git a/pkg/index.d.ts b/pkg/index.d.ts index fff9cd3..c20361f 100644 --- a/pkg/index.d.ts +++ b/pkg/index.d.ts @@ -110,7 +110,9 @@ export type Message = > | BaseMessage<'DEPRECATED_FIELD_JSNEXT'> | BaseMessage<'INVALID_REPOSITORY_VALUE', { - directory?: string; + type: 'short' | 'long' + normal: boolean + valid: boolean }>; export interface Options { diff --git a/pkg/src/index.js b/pkg/src/index.js index ab6fbba..ae9594d 100755 --- a/pkg/src/index.js +++ b/pkg/src/index.js @@ -23,7 +23,9 @@ import { replaceLast, isRelativePath, isAbsolutePath, - isRepositoryUrl + isGitUrl, + isShorthandRepositoryUrl, + isNormalizedGitUrl } from './utils.js' /** @@ -447,48 +449,36 @@ export async function publint({ pkgDir, vfs, level, strict, _packedFiles }) { * @param {Record | string} repositoryField */ async function checkRepositoryField(repositoryField) { - if (typeof repositoryField === 'string') { + /** + * @param {boolean} valid + * @param {boolean} normal + * @param {'short' | 'long'} type + * @param {string[]} path + */ + const addMessage = (valid, normal, type, path) => { messages.push({ code: 'INVALID_REPOSITORY_VALUE', - args: {}, - path: ['repository'], - type: 'suggestion', + args: { valid, normal, type }, + path, + type: valid ? 'suggestion' : 'warning', }); - } else if (typeof repositoryField === 'object') { - if (repositoryField.url && repositoryField.type === 'git' && !isRepositoryUrl(repositoryField.url)) { - messages.push({ - code: 'INVALID_REPOSITORY_VALUE', - args: {}, - path: ['repository', 'url'], - type: 'warning', - }); + }; + + if (typeof repositoryField === 'string') { + if (isShorthandRepositoryUrl(repositoryField)) { + addMessage(true, true, 'short', ['repository']); + } else { + addMessage(false, false, 'long', ['repository']); } - - if (repositoryField.directory) { - const altRootPath = vfs.pathJoin(pkgDir, repositoryField.directory); - const isAltRootExist = await vfs.isPathExist(altRootPath); - - if (!isAltRootExist) { - messages.push({ - code: 'INVALID_REPOSITORY_VALUE', - args: { - directory: repositoryField.directory, - }, - path: ['repository', 'directory'], - type: 'error', - }); - } + } else if (typeof repositoryField === 'object' && repositoryField.url && repositoryField.type === 'git') { + if (!isGitUrl(repositoryField.url)) { + addMessage(false, false, 'long', ['repository', 'url']); + } else if (!isNormalizedGitUrl(repositoryField.url)) { + addMessage(true, false, 'long', ['repository', 'url']); } - - return; + } else { + addMessage(false, false, 'long', ['repository']); } - - messages.push({ - code: 'INVALID_REPOSITORY_VALUE', - args: {}, - path: ['repository'], - type: 'warning', - }) } /** diff --git a/pkg/src/message.js b/pkg/src/message.js index 5f87433..38e1556 100644 --- a/pkg/src/message.js +++ b/pkg/src/message.js @@ -159,17 +159,19 @@ export function formatMessage(m, pkg) { // prettier-ignore return `${c.bold(fp(m.path))} is deprecated. ${c.bold('pkg.module')} should be used instead.` case 'INVALID_REPOSITORY_VALUE': - if (m.path.length === 1) { - return `Consider using an object for to represent ${c.bold(fp(m.path))}.`; - } + if (!m.args.valid) { + if (m.path.length === 2) { + return `${c.bold(fp(m.path))} must be a valid URL.` + } - if (m.path[m.path.length - 1] === 'url') { - return `${c.bold(fp(m.path))} must be a valid GitHub URL.`; + return `${c.bold(fp(m.path))} must be an object that references a repository.` } - const dir = m.args.directory || ''; + if (m.args.type === 'short') { + return `Consider using an object to represent ${c.bold(fp(m.path))}.` + } - return `Cannot find package.json in ${c.bold(dir)}. Please ensure that package.json exist in the provided path.` + return `${c.bold(fp(m.path))} should start with \`git+\` and ends with \`.git\`` default: return } diff --git a/pkg/src/utils.js b/pkg/src/utils.js index f79d505..350feaa 100644 --- a/pkg/src/utils.js +++ b/pkg/src/utils.js @@ -46,13 +46,38 @@ export function stripComments(code) { .replace(SINGLELINE_COMMENTS_RE, '') } -// Reference: https://www.debuggex.com/r/cleXSxwdY3n9BaMV -const REPOSITORY_URL = /((git|ssh|http(s)?)|(git@[\w\.]+)|([\w\.]+@[\w\.]+))(:(\/\/)?|:)([\w\.@\:/\-~]+)(\.git)(\/)?/; +// Reference: https://git-scm.com/docs/git-clone#_git_urls +const GIT_URL = /^((?:git(?:\+(?:https?|file))?|https?|ftps?|file|ssh):\/\/)?(?:[\w._-]+@)?([\w.-]+)(?::(\d+))?\/([\w._/-]+(?:\.git)?)(?:\/|\?.*)?$/; /** * @param {string} url */ -export function isRepositoryUrl(url) { - return REPOSITORY_URL.test(url); +export function isGitUrl(url) { + return GIT_URL.test(url); +} +/** + * @param {string} url + */ +export function isNormalizedGitUrl(url) { + const tokens = url.match(GIT_URL) + if (tokens) { + const host = tokens[1] + const path = tokens[3] + + if (/(github|gitlab)/.test(host)) { + return url.startsWith('git+') && path.endsWith('.git') + } + } + + return true +} + +// Reference: https://docs.npmjs.com/cli/v10/configuring-npm/package-json#repository +const SHORTHAND_REPOSITORY_URL = /((github|gist|bitbucket|gitlab):)?[\w\-]+(\/[\w\-]+)?/ +/** + * @param {string} url + */ +export function isShorthandRepositoryUrl(url) { + return SHORTHAND_REPOSITORY_URL.test(url) } /** From 6de6925f7f7b7b770456ac038459c32de21d7a51 Mon Sep 17 00:00:00 2001 From: Cristopher Date: Fri, 9 Aug 2024 22:47:26 +0700 Subject: [PATCH 07/20] feat: include message handler on site --- site/src/utils/message.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/site/src/utils/message.js b/site/src/utils/message.js index 9406c4d..de0340b 100644 --- a/site/src/utils/message.js +++ b/site/src/utils/message.js @@ -153,17 +153,19 @@ function messageToString(m, pkg) { // prettier-ignore return `${bold(fp(m.path))} is deprecated. ${bold('pkg.module')} should be used instead.` case 'INVALID_REPOSITORY_VALUE': - if (m.path.length === 1) { - return `Consider using an object for to represent ${bold(fp(m.path))}.`; - } + if (!m.args.valid) { + if (m.path.length === 2) { + return `${bold(fp(m.path))} must be a valid URL.` + } - if (m.path[m.path.length - 1] === 'url') { - return `${bold(fp(m.path))} must be a valid GitHub URL.`; + return `${bold(fp(m.path))} must be an object that references a repository.` } - const dir = m.args.directory || ''; + if (m.args.type === 'short') { + return `Consider using an object to represent ${bold(fp(m.path))}.` + } - return `Cannot find package.json in ${bold(dir)}. Please ensure that package.json exist in the provided path.` + return `${bold(fp(m.path))} should start with \`git+\` and ends with \`.git\`` default: return } From d859f098ba2fe092d6ecb4a46c933aae16bb4185 Mon Sep 17 00:00:00 2001 From: Cristopher Date: Sat, 10 Aug 2024 11:05:34 +0700 Subject: [PATCH 08/20] test: add more test for repository rule --- pkg/src/utils.js | 4 +-- .../index.js | 0 .../package.json | 7 ++++++ .../index.js | 0 .../package.json | 10 ++++++++ .../index.js | 0 .../package.json | 10 ++++++++ .../index.js | 0 .../package.json | 7 ++++++ .../index.js | 0 .../package.json | 7 ++++++ .../invalid-repository-value/package.json | 7 ------ pkg/tests/playground.js | 25 ++++++++++++++++++- 13 files changed, 67 insertions(+), 10 deletions(-) rename pkg/tests/fixtures/{invalid-repository-value => invalid-repository-value-not-string}/index.js (100%) create mode 100644 pkg/tests/fixtures/invalid-repository-value-not-string/package.json create mode 100644 pkg/tests/fixtures/invalid-repository-value-object-not-git-url/index.js create mode 100644 pkg/tests/fixtures/invalid-repository-value-object-not-git-url/package.json create mode 100644 pkg/tests/fixtures/invalid-repository-value-object-not-normalized/index.js create mode 100644 pkg/tests/fixtures/invalid-repository-value-object-not-normalized/package.json create mode 100644 pkg/tests/fixtures/invalid-repository-value-shorthand/index.js create mode 100644 pkg/tests/fixtures/invalid-repository-value-shorthand/package.json create mode 100644 pkg/tests/fixtures/invalid-repository-value-string-not-url/index.js create mode 100644 pkg/tests/fixtures/invalid-repository-value-string-not-url/package.json delete mode 100644 pkg/tests/fixtures/invalid-repository-value/package.json diff --git a/pkg/src/utils.js b/pkg/src/utils.js index 350feaa..5a5e36a 100644 --- a/pkg/src/utils.js +++ b/pkg/src/utils.js @@ -60,8 +60,8 @@ export function isGitUrl(url) { export function isNormalizedGitUrl(url) { const tokens = url.match(GIT_URL) if (tokens) { - const host = tokens[1] - const path = tokens[3] + const host = tokens[2] + const path = tokens[4] if (/(github|gitlab)/.test(host)) { return url.startsWith('git+') && path.endsWith('.git') diff --git a/pkg/tests/fixtures/invalid-repository-value/index.js b/pkg/tests/fixtures/invalid-repository-value-not-string/index.js similarity index 100% rename from pkg/tests/fixtures/invalid-repository-value/index.js rename to pkg/tests/fixtures/invalid-repository-value-not-string/index.js diff --git a/pkg/tests/fixtures/invalid-repository-value-not-string/package.json b/pkg/tests/fixtures/invalid-repository-value-not-string/package.json new file mode 100644 index 0000000..38861e6 --- /dev/null +++ b/pkg/tests/fixtures/invalid-repository-value-not-string/package.json @@ -0,0 +1,7 @@ +{ + "name": "publint-invalid-repository-value-not-string", + "version": "0.0.1", + "type": "commonjs", + "main": "./index.js", + "repository": 123 + } diff --git a/pkg/tests/fixtures/invalid-repository-value-object-not-git-url/index.js b/pkg/tests/fixtures/invalid-repository-value-object-not-git-url/index.js new file mode 100644 index 0000000..e69de29 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..25b174f --- /dev/null +++ b/pkg/tests/fixtures/invalid-repository-value-object-not-git-url/package.json @@ -0,0 +1,10 @@ +{ + "name": "publint-invalid-repository-value-object-not-git-url", + "version": "0.0.1", + "type": "commonjs", + "main": "./index.js", + "repository": { + "type": "git", + "url": "imap://fake.com/" + } + } diff --git a/pkg/tests/fixtures/invalid-repository-value-object-not-normalized/index.js b/pkg/tests/fixtures/invalid-repository-value-object-not-normalized/index.js new file mode 100644 index 0000000..e69de29 diff --git a/pkg/tests/fixtures/invalid-repository-value-object-not-normalized/package.json b/pkg/tests/fixtures/invalid-repository-value-object-not-normalized/package.json new file mode 100644 index 0000000..0eb1a16 --- /dev/null +++ b/pkg/tests/fixtures/invalid-repository-value-object-not-normalized/package.json @@ -0,0 +1,10 @@ +{ + "name": "publint-invalid-repository-value-object-not-normalized", + "version": "0.0.1", + "type": "commonjs", + "main": "./index.js", + "repository": { + "type": "git", + "url": "https://www.github.com/bluwy/publint" + } + } diff --git a/pkg/tests/fixtures/invalid-repository-value-shorthand/index.js b/pkg/tests/fixtures/invalid-repository-value-shorthand/index.js new file mode 100644 index 0000000..e69de29 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..287bfff --- /dev/null +++ b/pkg/tests/fixtures/invalid-repository-value-shorthand/package.json @@ -0,0 +1,7 @@ +{ + "name": "publint-invalid-repository-value-shorthand", + "version": "0.0.1", + "type": "commonjs", + "main": "./index.js", + "repository": "npm/npm" + } diff --git a/pkg/tests/fixtures/invalid-repository-value-string-not-url/index.js b/pkg/tests/fixtures/invalid-repository-value-string-not-url/index.js new file mode 100644 index 0000000..e69de29 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..0be783a --- /dev/null +++ b/pkg/tests/fixtures/invalid-repository-value-string-not-url/package.json @@ -0,0 +1,7 @@ +{ + "name": "publint-invalid-repository-value-not-string", + "version": "0.0.1", + "type": "commonjs", + "main": "./index.js", + "repository": "not_an_url" + } diff --git a/pkg/tests/fixtures/invalid-repository-value/package.json b/pkg/tests/fixtures/invalid-repository-value/package.json deleted file mode 100644 index 25934aa..0000000 --- a/pkg/tests/fixtures/invalid-repository-value/package.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "name": "publint-invalid-repository-value", - "version": "0.0.1", - "type": "commonjs", - "main": "./index.js", - "repository": "https://www.google.com" - } diff --git a/pkg/tests/playground.js b/pkg/tests/playground.js index 72a750b..1a45523 100644 --- a/pkg/tests/playground.js +++ b/pkg/tests/playground.js @@ -144,7 +144,30 @@ testFixture('deprecated-fields', [ 'USE_TYPE' ]) -testFixture('invalid-repository-value', ['INVALID_REPOSITORY_VALUE']) +testFixture('invalid-repository-value-not-string', [{ + code: 'INVALID_REPOSITORY_VALUE', + type: 'warning', +}]) + +testFixture('invalid-repository-value-string-not-url', [{ + code: 'INVALID_REPOSITORY_VALUE', + type: 'suggestion', +}]) + +testFixture('invalid-repository-value-shorthand', [{ + code: 'INVALID_REPOSITORY_VALUE', + type: 'suggestion', +}]) + +testFixture('invalid-repository-value-object-not-git-url', [{ + code: 'INVALID_REPOSITORY_VALUE', + type: 'warning', +}]) + +testFixture('invalid-repository-value-object-not-normalized', [{ + code: 'INVALID_REPOSITORY_VALUE', + type: 'suggestion', +}]) /** * @typedef {{ From 67df03734c61bbd66d2fd238afea2270e5f6b4c6 Mon Sep 17 00:00:00 2001 From: Cristopher Date: Sat, 10 Aug 2024 11:24:54 +0700 Subject: [PATCH 09/20] feat: add git:// deprecation check for GitHub URLs --- pkg/index.d.ts | 1 + pkg/src/index.js | 28 +++++++++++-------- pkg/src/message.js | 6 +++- pkg/src/utils.js | 18 ++++++++++++ .../index.js | 0 .../package.json | 10 +++++++ pkg/tests/playground.js | 5 ++++ site/src/utils/message.js | 10 +++++-- 8 files changed, 62 insertions(+), 16 deletions(-) create mode 100644 pkg/tests/fixtures/invalid-repository-value-object-deprecated/index.js create mode 100644 pkg/tests/fixtures/invalid-repository-value-object-deprecated/package.json diff --git a/pkg/index.d.ts b/pkg/index.d.ts index c20361f..928a10f 100644 --- a/pkg/index.d.ts +++ b/pkg/index.d.ts @@ -112,6 +112,7 @@ export type Message = | BaseMessage<'INVALID_REPOSITORY_VALUE', { type: 'short' | 'long' normal: boolean + deprecated: boolean valid: boolean }>; diff --git a/pkg/src/index.js b/pkg/src/index.js index ae9594d..3fc8112 100755 --- a/pkg/src/index.js +++ b/pkg/src/index.js @@ -25,7 +25,8 @@ import { isAbsolutePath, isGitUrl, isShorthandRepositoryUrl, - isNormalizedGitUrl + isNormalizedGitUrl, + isDeprecatedUrl } from './utils.js' /** @@ -452,32 +453,35 @@ export async function publint({ pkgDir, vfs, level, strict, _packedFiles }) { /** * @param {boolean} valid * @param {boolean} normal + * @param {boolean} deprecated * @param {'short' | 'long'} type * @param {string[]} path */ - const addMessage = (valid, normal, type, path) => { + const addMessage = (valid, normal, deprecated, type, path) => { messages.push({ code: 'INVALID_REPOSITORY_VALUE', - args: { valid, normal, type }, + args: { valid, normal, deprecated, type }, path, - type: valid ? 'suggestion' : 'warning', - }); - }; + type: valid && !deprecated ? 'suggestion' : 'warning', + }) + } if (typeof repositoryField === 'string') { if (isShorthandRepositoryUrl(repositoryField)) { - addMessage(true, true, 'short', ['repository']); + addMessage(true, true, false, 'short', ['repository']) } else { - addMessage(false, false, 'long', ['repository']); + addMessage(false, false, false, 'long', ['repository']) } } else if (typeof repositoryField === 'object' && repositoryField.url && repositoryField.type === 'git') { if (!isGitUrl(repositoryField.url)) { - addMessage(false, false, 'long', ['repository', 'url']); + addMessage(false, false, false, 'long', ['repository', 'url']) + } else if (isDeprecatedUrl(repositoryField.url)) { + addMessage(true, true, true, 'long', ['repository', 'url']) } else if (!isNormalizedGitUrl(repositoryField.url)) { - addMessage(true, false, 'long', ['repository', 'url']); - } + addMessage(true, false, false, 'long', ['repository', 'url']) + } } else { - addMessage(false, false, 'long', ['repository']); + addMessage(false, false, false, 'long', ['repository']); } } diff --git a/pkg/src/message.js b/pkg/src/message.js index 38e1556..9892166 100644 --- a/pkg/src/message.js +++ b/pkg/src/message.js @@ -171,7 +171,11 @@ export function formatMessage(m, pkg) { return `Consider using an object to represent ${c.bold(fp(m.path))}.` } - return `${c.bold(fp(m.path))} should start with \`git+\` and ends with \`.git\`` + if (m.args.deprecated) { + return `The git:// protocol in ${c.bold(fp(m.path))} is already deprecated by GitHub due to security concerns. Consider replacing the protocol with https://.` + } + + return `${c.bold(fp(m.path))} should start with \`git+\` and ends with \`.git\`.` default: return } diff --git a/pkg/src/utils.js b/pkg/src/utils.js index 5a5e36a..10b058a 100644 --- a/pkg/src/utils.js +++ b/pkg/src/utils.js @@ -70,6 +70,24 @@ export function isNormalizedGitUrl(url) { return true } +/** + * Reference: https://github.blog/security/application-security/improving-git-protocol-security-github/ + * + * @param {string} url + */ +export function isDeprecatedUrl(url) { + const tokens = url.match(GIT_URL) + 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 = /((github|gist|bitbucket|gitlab):)?[\w\-]+(\/[\w\-]+)?/ diff --git a/pkg/tests/fixtures/invalid-repository-value-object-deprecated/index.js b/pkg/tests/fixtures/invalid-repository-value-object-deprecated/index.js new file mode 100644 index 0000000..e69de29 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..a8211cd --- /dev/null +++ b/pkg/tests/fixtures/invalid-repository-value-object-deprecated/package.json @@ -0,0 +1,10 @@ +{ + "name": "publint-invalid-repository-value-object-not-normalized", + "version": "0.0.1", + "type": "commonjs", + "main": "./index.js", + "repository": { + "type": "git", + "url": "git://www.github.com/bluwy/publint" + } + } diff --git a/pkg/tests/playground.js b/pkg/tests/playground.js index 1a45523..dfa2177 100644 --- a/pkg/tests/playground.js +++ b/pkg/tests/playground.js @@ -169,6 +169,11 @@ testFixture('invalid-repository-value-object-not-normalized', [{ type: 'suggestion', }]) +testFixture('invalid-repository-value-object-deprecated', [{ + code: 'INVALID_REPOSITORY_VALUE', + type: 'warning', +}]) + /** * @typedef {{ * level?: import('../index.d.ts').Options['level'] diff --git a/site/src/utils/message.js b/site/src/utils/message.js index de0340b..ee18cf1 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.` @@ -165,7 +165,11 @@ function messageToString(m, pkg) { return `Consider using an object to represent ${bold(fp(m.path))}.` } - return `${bold(fp(m.path))} should start with \`git+\` and ends with \`.git\`` + if (m.args.deprecated) { + return `The git:// protocol in ${bold(fp(m.path))} is already deprecated by GitHub due to security concerns. Consider replacing the protocol with https://.` + } + + return `${bold(fp(m.path))} should start with \`git+\` and ends with \`.git\`.` default: return } From 0c4d4e9ffae49c6db25ae718b897ac5c30fee618 Mon Sep 17 00:00:00 2001 From: bluwy Date: Mon, 12 Aug 2024 21:19:28 +0800 Subject: [PATCH 10/20] chore: remove unused fields --- pkg/tests/fixtures/invalid-repository-value-not-string/index.js | 0 .../fixtures/invalid-repository-value-not-string/package.json | 1 - .../fixtures/invalid-repository-value-object-deprecated/index.js | 0 .../invalid-repository-value-object-deprecated/package.json | 1 - .../invalid-repository-value-object-not-git-url/index.js | 0 .../invalid-repository-value-object-not-git-url/package.json | 1 - .../invalid-repository-value-object-not-normalized/index.js | 0 .../invalid-repository-value-object-not-normalized/package.json | 1 - pkg/tests/fixtures/invalid-repository-value-shorthand/index.js | 0 .../fixtures/invalid-repository-value-shorthand/package.json | 1 - .../fixtures/invalid-repository-value-string-not-url/index.js | 0 .../invalid-repository-value-string-not-url/package.json | 1 - 12 files changed, 6 deletions(-) delete mode 100644 pkg/tests/fixtures/invalid-repository-value-not-string/index.js delete mode 100644 pkg/tests/fixtures/invalid-repository-value-object-deprecated/index.js delete mode 100644 pkg/tests/fixtures/invalid-repository-value-object-not-git-url/index.js delete mode 100644 pkg/tests/fixtures/invalid-repository-value-object-not-normalized/index.js delete mode 100644 pkg/tests/fixtures/invalid-repository-value-shorthand/index.js delete mode 100644 pkg/tests/fixtures/invalid-repository-value-string-not-url/index.js diff --git a/pkg/tests/fixtures/invalid-repository-value-not-string/index.js b/pkg/tests/fixtures/invalid-repository-value-not-string/index.js deleted file mode 100644 index e69de29..0000000 diff --git a/pkg/tests/fixtures/invalid-repository-value-not-string/package.json b/pkg/tests/fixtures/invalid-repository-value-not-string/package.json index 38861e6..e708ffb 100644 --- a/pkg/tests/fixtures/invalid-repository-value-not-string/package.json +++ b/pkg/tests/fixtures/invalid-repository-value-not-string/package.json @@ -2,6 +2,5 @@ "name": "publint-invalid-repository-value-not-string", "version": "0.0.1", "type": "commonjs", - "main": "./index.js", "repository": 123 } diff --git a/pkg/tests/fixtures/invalid-repository-value-object-deprecated/index.js b/pkg/tests/fixtures/invalid-repository-value-object-deprecated/index.js deleted file mode 100644 index e69de29..0000000 diff --git a/pkg/tests/fixtures/invalid-repository-value-object-deprecated/package.json b/pkg/tests/fixtures/invalid-repository-value-object-deprecated/package.json index a8211cd..e224d70 100644 --- a/pkg/tests/fixtures/invalid-repository-value-object-deprecated/package.json +++ b/pkg/tests/fixtures/invalid-repository-value-object-deprecated/package.json @@ -2,7 +2,6 @@ "name": "publint-invalid-repository-value-object-not-normalized", "version": "0.0.1", "type": "commonjs", - "main": "./index.js", "repository": { "type": "git", "url": "git://www.github.com/bluwy/publint" diff --git a/pkg/tests/fixtures/invalid-repository-value-object-not-git-url/index.js b/pkg/tests/fixtures/invalid-repository-value-object-not-git-url/index.js deleted file mode 100644 index e69de29..0000000 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 index 25b174f..a0b4b73 100644 --- 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 @@ -2,7 +2,6 @@ "name": "publint-invalid-repository-value-object-not-git-url", "version": "0.0.1", "type": "commonjs", - "main": "./index.js", "repository": { "type": "git", "url": "imap://fake.com/" diff --git a/pkg/tests/fixtures/invalid-repository-value-object-not-normalized/index.js b/pkg/tests/fixtures/invalid-repository-value-object-not-normalized/index.js deleted file mode 100644 index e69de29..0000000 diff --git a/pkg/tests/fixtures/invalid-repository-value-object-not-normalized/package.json b/pkg/tests/fixtures/invalid-repository-value-object-not-normalized/package.json index 0eb1a16..8d8bed0 100644 --- a/pkg/tests/fixtures/invalid-repository-value-object-not-normalized/package.json +++ b/pkg/tests/fixtures/invalid-repository-value-object-not-normalized/package.json @@ -2,7 +2,6 @@ "name": "publint-invalid-repository-value-object-not-normalized", "version": "0.0.1", "type": "commonjs", - "main": "./index.js", "repository": { "type": "git", "url": "https://www.github.com/bluwy/publint" diff --git a/pkg/tests/fixtures/invalid-repository-value-shorthand/index.js b/pkg/tests/fixtures/invalid-repository-value-shorthand/index.js deleted file mode 100644 index e69de29..0000000 diff --git a/pkg/tests/fixtures/invalid-repository-value-shorthand/package.json b/pkg/tests/fixtures/invalid-repository-value-shorthand/package.json index 287bfff..f4c83a4 100644 --- a/pkg/tests/fixtures/invalid-repository-value-shorthand/package.json +++ b/pkg/tests/fixtures/invalid-repository-value-shorthand/package.json @@ -2,6 +2,5 @@ "name": "publint-invalid-repository-value-shorthand", "version": "0.0.1", "type": "commonjs", - "main": "./index.js", "repository": "npm/npm" } diff --git a/pkg/tests/fixtures/invalid-repository-value-string-not-url/index.js b/pkg/tests/fixtures/invalid-repository-value-string-not-url/index.js deleted file mode 100644 index e69de29..0000000 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 index 0be783a..797e8a1 100644 --- a/pkg/tests/fixtures/invalid-repository-value-string-not-url/package.json +++ b/pkg/tests/fixtures/invalid-repository-value-string-not-url/package.json @@ -2,6 +2,5 @@ "name": "publint-invalid-repository-value-not-string", "version": "0.0.1", "type": "commonjs", - "main": "./index.js", "repository": "not_an_url" } From 773cb51d685cba68a7d31fa81e847c3e0df3da3c Mon Sep 17 00:00:00 2001 From: bluwy Date: Mon, 12 Aug 2024 21:19:58 +0800 Subject: [PATCH 11/20] style: format --- pkg/index.d.ts | 15 +++++---- pkg/src/index.js | 24 ++++++++------ pkg/src/utils.js | 18 ++++++----- pkg/tests/playground.js | 70 ++++++++++++++++++++++++----------------- 4 files changed, 74 insertions(+), 53 deletions(-) diff --git a/pkg/index.d.ts b/pkg/index.d.ts index 928a10f..851e8f1 100644 --- a/pkg/index.d.ts +++ b/pkg/index.d.ts @@ -109,12 +109,15 @@ export type Message = } > | BaseMessage<'DEPRECATED_FIELD_JSNEXT'> - | BaseMessage<'INVALID_REPOSITORY_VALUE', { - type: 'short' | 'long' - normal: boolean - deprecated: boolean - valid: boolean - }>; + | BaseMessage< + 'INVALID_REPOSITORY_VALUE', + { + type: 'short' | 'long' + normal: boolean + deprecated: boolean + valid: boolean + } + > export interface Options { /** diff --git a/pkg/src/index.js b/pkg/src/index.js index 3fc8112..b1315df 100755 --- a/pkg/src/index.js +++ b/pkg/src/index.js @@ -241,7 +241,7 @@ 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)); + promiseQueue.push(() => checkRepositoryField(rootPkg.repository)) } // check file existence for other known package fields @@ -451,37 +451,41 @@ export async function publint({ pkgDir, vfs, level, strict, _packedFiles }) { */ async function checkRepositoryField(repositoryField) { /** - * @param {boolean} valid - * @param {boolean} normal + * @param {boolean} valid + * @param {boolean} normal * @param {boolean} deprecated - * @param {'short' | 'long'} type - * @param {string[]} path + * @param {'short' | 'long'} type + * @param {string[]} path */ const addMessage = (valid, normal, deprecated, type, path) => { messages.push({ code: 'INVALID_REPOSITORY_VALUE', args: { valid, normal, deprecated, type }, path, - type: valid && !deprecated ? 'suggestion' : 'warning', + type: valid && !deprecated ? 'suggestion' : 'warning' }) } - + if (typeof repositoryField === 'string') { if (isShorthandRepositoryUrl(repositoryField)) { addMessage(true, true, false, 'short', ['repository']) } else { addMessage(false, false, false, 'long', ['repository']) } - } else if (typeof repositoryField === 'object' && repositoryField.url && repositoryField.type === 'git') { + } else if ( + typeof repositoryField === 'object' && + repositoryField.url && + repositoryField.type === 'git' + ) { if (!isGitUrl(repositoryField.url)) { addMessage(false, false, false, 'long', ['repository', 'url']) } else if (isDeprecatedUrl(repositoryField.url)) { addMessage(true, true, true, 'long', ['repository', 'url']) } else if (!isNormalizedGitUrl(repositoryField.url)) { addMessage(true, false, false, 'long', ['repository', 'url']) - } + } } else { - addMessage(false, false, false, 'long', ['repository']); + addMessage(false, false, false, 'long', ['repository']) } } diff --git a/pkg/src/utils.js b/pkg/src/utils.js index 10b058a..db48050 100644 --- a/pkg/src/utils.js +++ b/pkg/src/utils.js @@ -47,15 +47,16 @@ export function stripComments(code) { } // Reference: https://git-scm.com/docs/git-clone#_git_urls -const GIT_URL = /^((?:git(?:\+(?:https?|file))?|https?|ftps?|file|ssh):\/\/)?(?:[\w._-]+@)?([\w.-]+)(?::(\d+))?\/([\w._/-]+(?:\.git)?)(?:\/|\?.*)?$/; +const GIT_URL = + /^((?:git(?:\+(?:https?|file))?|https?|ftps?|file|ssh):\/\/)?(?:[\w._-]+@)?([\w.-]+)(?::(\d+))?\/([\w._/-]+(?:\.git)?)(?:\/|\?.*)?$/ /** - * @param {string} url + * @param {string} url */ export function isGitUrl(url) { - return GIT_URL.test(url); + return GIT_URL.test(url) } /** - * @param {string} url + * @param {string} url */ export function isNormalizedGitUrl(url) { const tokens = url.match(GIT_URL) @@ -72,8 +73,8 @@ export function isNormalizedGitUrl(url) { } /** * Reference: https://github.blog/security/application-security/improving-git-protocol-security-github/ - * - * @param {string} url + * + * @param {string} url */ export function isDeprecatedUrl(url) { const tokens = url.match(GIT_URL) @@ -90,9 +91,10 @@ export function isDeprecatedUrl(url) { } // Reference: https://docs.npmjs.com/cli/v10/configuring-npm/package-json#repository -const SHORTHAND_REPOSITORY_URL = /((github|gist|bitbucket|gitlab):)?[\w\-]+(\/[\w\-]+)?/ +const SHORTHAND_REPOSITORY_URL = + /((github|gist|bitbucket|gitlab):)?[\w\-]+(\/[\w\-]+)?/ /** - * @param {string} url + * @param {string} url */ export function isShorthandRepositoryUrl(url) { return SHORTHAND_REPOSITORY_URL.test(url) diff --git a/pkg/tests/playground.js b/pkg/tests/playground.js index dfa2177..29a7a64 100644 --- a/pkg/tests/playground.js +++ b/pkg/tests/playground.js @@ -144,35 +144,47 @@ testFixture('deprecated-fields', [ 'USE_TYPE' ]) -testFixture('invalid-repository-value-not-string', [{ - code: 'INVALID_REPOSITORY_VALUE', - type: 'warning', -}]) - -testFixture('invalid-repository-value-string-not-url', [{ - code: 'INVALID_REPOSITORY_VALUE', - type: 'suggestion', -}]) - -testFixture('invalid-repository-value-shorthand', [{ - code: 'INVALID_REPOSITORY_VALUE', - type: 'suggestion', -}]) - -testFixture('invalid-repository-value-object-not-git-url', [{ - code: 'INVALID_REPOSITORY_VALUE', - type: 'warning', -}]) - -testFixture('invalid-repository-value-object-not-normalized', [{ - code: 'INVALID_REPOSITORY_VALUE', - type: 'suggestion', -}]) - -testFixture('invalid-repository-value-object-deprecated', [{ - code: 'INVALID_REPOSITORY_VALUE', - type: 'warning', -}]) +testFixture('invalid-repository-value-not-string', [ + { + code: 'INVALID_REPOSITORY_VALUE', + type: 'warning' + } +]) + +testFixture('invalid-repository-value-string-not-url', [ + { + code: 'INVALID_REPOSITORY_VALUE', + type: 'suggestion' + } +]) + +testFixture('invalid-repository-value-shorthand', [ + { + code: 'INVALID_REPOSITORY_VALUE', + type: 'suggestion' + } +]) + +testFixture('invalid-repository-value-object-not-git-url', [ + { + code: 'INVALID_REPOSITORY_VALUE', + type: 'warning' + } +]) + +testFixture('invalid-repository-value-object-not-normalized', [ + { + code: 'INVALID_REPOSITORY_VALUE', + type: 'suggestion' + } +]) + +testFixture('invalid-repository-value-object-deprecated', [ + { + code: 'INVALID_REPOSITORY_VALUE', + type: 'warning' + } +]) /** * @typedef {{ From f687cd669d19f4644a6c81ebe67c5a1b2b9fc8be Mon Sep 17 00:00:00 2001 From: bluwy Date: Tue, 13 Aug 2024 00:27:43 +0800 Subject: [PATCH 12/20] refactor: update rule and message --- pkg/index.d.ts | 9 ++- pkg/src/index.js | 78 ++++++++++--------- pkg/src/message.js | 35 +++++---- pkg/src/utils.js | 23 +++--- .../package.json | 2 +- .../package.json | 2 +- .../package.json | 2 +- pkg/tests/playground.js | 15 ++-- site/src/utils/message.js | 35 +++++---- 9 files changed, 106 insertions(+), 95 deletions(-) rename pkg/tests/fixtures/{invalid-repository-value-object-not-normalized => invalid-repository-value-object-shorthand-site}/package.json (68%) diff --git a/pkg/index.d.ts b/pkg/index.d.ts index 851e8f1..c8b5b3f 100644 --- a/pkg/index.d.ts +++ b/pkg/index.d.ts @@ -112,10 +112,11 @@ export type Message = | BaseMessage< 'INVALID_REPOSITORY_VALUE', { - type: 'short' | 'long' - normal: boolean - deprecated: boolean - valid: boolean + type: + | 'invalid-string-shorthand' + | 'invalid-git-url' + | 'deprecated-github-git-protocol' + | 'shorthand-git-sites' } > diff --git a/pkg/src/index.js b/pkg/src/index.js index b1315df..f39d0d5 100755 --- a/pkg/src/index.js +++ b/pkg/src/index.js @@ -25,8 +25,8 @@ import { isAbsolutePath, isGitUrl, isShorthandRepositoryUrl, - isNormalizedGitUrl, - isDeprecatedUrl + isShorthandGitHubOrGitLabUrl, + isDeprecatedGitHubGitUrl } from './utils.js' /** @@ -446,46 +446,52 @@ export async function publint({ pkgDir, vfs, level, strict, _packedFiles }) { } } + // https://docs.npmjs.com/cli/v10/configuring-npm/package-json#repository /** - * @param {Record | string} repositoryField + * @param {Record | string} repository */ - async function checkRepositoryField(repositoryField) { - /** - * @param {boolean} valid - * @param {boolean} normal - * @param {boolean} deprecated - * @param {'short' | 'long'} type - * @param {string[]} path - */ - const addMessage = (valid, normal, deprecated, type, path) => { - messages.push({ - code: 'INVALID_REPOSITORY_VALUE', - args: { valid, normal, deprecated, type }, - path, - type: valid && !deprecated ? 'suggestion' : 'warning' - }) - } - - if (typeof repositoryField === 'string') { - if (isShorthandRepositoryUrl(repositoryField)) { - addMessage(true, true, false, 'short', ['repository']) - } else { - addMessage(false, false, false, 'long', ['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 repositoryField === 'object' && - repositoryField.url && - repositoryField.type === 'git' + typeof repository === 'object' && + repository.url && + repository.type === 'git' ) { - if (!isGitUrl(repositoryField.url)) { - addMessage(false, false, false, 'long', ['repository', 'url']) - } else if (isDeprecatedUrl(repositoryField.url)) { - addMessage(true, true, true, 'long', ['repository', 'url']) - } else if (!isNormalizedGitUrl(repositoryField.url)) { - addMessage(true, false, false, 'long', ['repository', 'url']) + 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: 'warning' + }) + } else if (isShorthandGitHubOrGitLabUrl(repository.url)) { + messages.push({ + code: 'INVALID_REPOSITORY_VALUE', + args: { type: 'shorthand-git-sites' }, + path: ['repository', 'url'], + type: 'suggestion' + }) } - } else { - addMessage(false, false, false, 'long', ['repository']) } } diff --git a/pkg/src/message.js b/pkg/src/message.js index 9892166..47be43e 100644 --- a/pkg/src/message.js +++ b/pkg/src/message.js @@ -159,23 +159,28 @@ export function formatMessage(m, pkg) { // prettier-ignore return `${c.bold(fp(m.path))} is deprecated. ${c.bold('pkg.module')} should be used instead.` case 'INVALID_REPOSITORY_VALUE': - if (!m.args.valid) { - if (m.path.length === 2) { - return `${c.bold(fp(m.path))} must be a valid URL.` + 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 "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.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)}".` } - - return `${c.bold(fp(m.path))} must be an object that references a repository.` - } - - if (m.args.type === 'short') { - return `Consider using an object to represent ${c.bold(fp(m.path))}.` } - - if (m.args.deprecated) { - return `The git:// protocol in ${c.bold(fp(m.path))} is already deprecated by GitHub due to security concerns. Consider replacing the protocol with https://.` - } - - return `${c.bold(fp(m.path))} should start with \`git+\` and ends with \`.git\`.` default: return } diff --git a/pkg/src/utils.js b/pkg/src/utils.js index db48050..938a631 100644 --- a/pkg/src/utils.js +++ b/pkg/src/utils.js @@ -47,37 +47,36 @@ export function stripComments(code) { } // Reference: https://git-scm.com/docs/git-clone#_git_urls -const GIT_URL = +const GIT_URL_RE = /^((?:git(?:\+(?:https?|file))?|https?|ftps?|file|ssh):\/\/)?(?:[\w._-]+@)?([\w.-]+)(?::(\d+))?\/([\w._/-]+(?:\.git)?)(?:\/|\?.*)?$/ /** * @param {string} url */ export function isGitUrl(url) { - return GIT_URL.test(url) + return GIT_URL_RE.test(url) } /** * @param {string} url */ -export function isNormalizedGitUrl(url) { - const tokens = url.match(GIT_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 !url.startsWith('git+') || !path.endsWith('.git') } } - return true + return false } /** * Reference: https://github.blog/security/application-security/improving-git-protocol-security-github/ - * * @param {string} url */ -export function isDeprecatedUrl(url) { - const tokens = url.match(GIT_URL) +export function isDeprecatedGitHubGitUrl(url) { + const tokens = url.match(GIT_URL_RE) if (tokens) { const protocol = tokens[1] const host = tokens[2] @@ -91,13 +90,13 @@ export function isDeprecatedUrl(url) { } // Reference: https://docs.npmjs.com/cli/v10/configuring-npm/package-json#repository -const SHORTHAND_REPOSITORY_URL = - /((github|gist|bitbucket|gitlab):)?[\w\-]+(\/[\w\-]+)?/ +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.test(url) + return SHORTHAND_REPOSITORY_URL_RE.test(url) } /** diff --git a/pkg/tests/fixtures/invalid-repository-value-object-deprecated/package.json b/pkg/tests/fixtures/invalid-repository-value-object-deprecated/package.json index e224d70..6d2b8ee 100644 --- a/pkg/tests/fixtures/invalid-repository-value-object-deprecated/package.json +++ b/pkg/tests/fixtures/invalid-repository-value-object-deprecated/package.json @@ -1,5 +1,5 @@ { - "name": "publint-invalid-repository-value-object-not-normalized", + "name": "publint-invalid-repository-value-object-deprecatec", "version": "0.0.1", "type": "commonjs", "repository": { diff --git a/pkg/tests/fixtures/invalid-repository-value-object-not-normalized/package.json b/pkg/tests/fixtures/invalid-repository-value-object-shorthand-site/package.json similarity index 68% rename from pkg/tests/fixtures/invalid-repository-value-object-not-normalized/package.json rename to pkg/tests/fixtures/invalid-repository-value-object-shorthand-site/package.json index 8d8bed0..7d300b4 100644 --- a/pkg/tests/fixtures/invalid-repository-value-object-not-normalized/package.json +++ b/pkg/tests/fixtures/invalid-repository-value-object-shorthand-site/package.json @@ -1,5 +1,5 @@ { - "name": "publint-invalid-repository-value-object-not-normalized", + "name": "publint-invalid-repository-value-object-shorthand-site", "version": "0.0.1", "type": "commonjs", "repository": { 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 index 797e8a1..c1ae5db 100644 --- a/pkg/tests/fixtures/invalid-repository-value-string-not-url/package.json +++ b/pkg/tests/fixtures/invalid-repository-value-string-not-url/package.json @@ -1,5 +1,5 @@ { - "name": "publint-invalid-repository-value-not-string", + "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 29a7a64..eef469e 100644 --- a/pkg/tests/playground.js +++ b/pkg/tests/playground.js @@ -146,24 +146,19 @@ testFixture('deprecated-fields', [ testFixture('invalid-repository-value-not-string', [ { - code: 'INVALID_REPOSITORY_VALUE', - type: 'warning' + code: 'FIELD_INVALID_VALUE_TYPE', + type: 'error' } ]) testFixture('invalid-repository-value-string-not-url', [ { code: 'INVALID_REPOSITORY_VALUE', - type: 'suggestion' + type: 'warning' } ]) -testFixture('invalid-repository-value-shorthand', [ - { - code: 'INVALID_REPOSITORY_VALUE', - type: 'suggestion' - } -]) +testFixture('invalid-repository-value-shorthand', []) testFixture('invalid-repository-value-object-not-git-url', [ { @@ -172,7 +167,7 @@ testFixture('invalid-repository-value-object-not-git-url', [ } ]) -testFixture('invalid-repository-value-object-not-normalized', [ +testFixture('invalid-repository-value-object-shorthand-site', [ { code: 'INVALID_REPOSITORY_VALUE', type: 'suggestion' diff --git a/site/src/utils/message.js b/site/src/utils/message.js index ee18cf1..eaf0a65 100644 --- a/site/src/utils/message.js +++ b/site/src/utils/message.js @@ -153,23 +153,28 @@ function messageToString(m, pkg) { // prettier-ignore return `${bold(fp(m.path))} is deprecated. ${bold('pkg.module')} should be used instead.` case 'INVALID_REPOSITORY_VALUE': - if (!m.args.valid) { - if (m.path.length === 2) { - return `${bold(fp(m.path))} must be a valid URL.` + 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 "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.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)}".` } - - return `${bold(fp(m.path))} must be an object that references a repository.` - } - - if (m.args.type === 'short') { - return `Consider using an object to represent ${bold(fp(m.path))}.` } - - if (m.args.deprecated) { - return `The git:// protocol in ${bold(fp(m.path))} is already deprecated by GitHub due to security concerns. Consider replacing the protocol with https://.` - } - - return `${bold(fp(m.path))} should start with \`git+\` and ends with \`.git\`.` default: return } From d7fa64d755023a79578226958d2daa8713fcb1f9 Mon Sep 17 00:00:00 2001 From: bluwy Date: Tue, 13 Aug 2024 00:38:08 +0800 Subject: [PATCH 13/20] docs: add rule docs --- site/rules.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/site/rules.md b/site/rules.md index 9df1696..57815a6 100644 --- a/site/rules.md +++ b/site/rules.md @@ -238,3 +238,12 @@ 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 ba a valid [git URL](https://git-scm.com/docs/git-clone#_git_urls). +- 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). From cb4ee836763a3ee9239ed58d3dbc620796e0e825 Mon Sep 17 00:00:00 2001 From: bluwy Date: Tue, 13 Aug 2024 00:39:07 +0800 Subject: [PATCH 14/20] chore: simplify test --- pkg/src/index.js | 2 +- pkg/tests/fixtures/invalid-field-types/package.json | 3 ++- .../invalid-repository-value-not-string/package.json | 6 ------ pkg/tests/playground.js | 10 ++-------- 4 files changed, 5 insertions(+), 16 deletions(-) delete mode 100644 pkg/tests/fixtures/invalid-repository-value-not-string/package.json diff --git a/pkg/src/index.js b/pkg/src/index.js index f39d0d5..5e9f2f6 100755 --- a/pkg/src/index.js +++ b/pkg/src/index.js @@ -489,7 +489,7 @@ export async function publint({ pkgDir, vfs, level, strict, _packedFiles }) { code: 'INVALID_REPOSITORY_VALUE', args: { type: 'shorthand-git-sites' }, path: ['repository', 'url'], - type: 'suggestion' + type: 'warning' }) } } 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-not-string/package.json b/pkg/tests/fixtures/invalid-repository-value-not-string/package.json deleted file mode 100644 index e708ffb..0000000 --- a/pkg/tests/fixtures/invalid-repository-value-not-string/package.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "name": "publint-invalid-repository-value-not-string", - "version": "0.0.1", - "type": "commonjs", - "repository": 123 - } diff --git a/pkg/tests/playground.js b/pkg/tests/playground.js index eef469e..771ae82 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,13 +145,6 @@ testFixture('deprecated-fields', [ 'USE_TYPE' ]) -testFixture('invalid-repository-value-not-string', [ - { - code: 'FIELD_INVALID_VALUE_TYPE', - type: 'error' - } -]) - testFixture('invalid-repository-value-string-not-url', [ { code: 'INVALID_REPOSITORY_VALUE', @@ -170,7 +164,7 @@ testFixture('invalid-repository-value-object-not-git-url', [ testFixture('invalid-repository-value-object-shorthand-site', [ { code: 'INVALID_REPOSITORY_VALUE', - type: 'suggestion' + type: 'warning' } ]) From d170d67ba4565741d8df4ec99190637ef5659760 Mon Sep 17 00:00:00 2001 From: bluwy Date: Tue, 13 Aug 2024 12:34:52 +0800 Subject: [PATCH 15/20] chore: add test --- pkg/tests/utils.js | 64 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/pkg/tests/utils.js b/pkg/tests/utils.js index 46fe53c..486ecb1 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,66 @@ test('getCodeFormat', () => { } }) +test.skip('isGitUrl', () => { + equal(isGitUrl('git@github.com:user/project.git'), true) + equal(isGitUrl('https://github.com/user/project.git'), true) + equal(isGitUrl('http://github.com/user/project.git'), true) + equal(isGitUrl('git@192.168.101.127:user/project.git'), true) + equal(isGitUrl('https://192.168.101.127/user/project.git'), true) + equal(isGitUrl('http://192.168.101.127/user/project.git'), true) + equal(isGitUrl('ssh://user@host.xz:port/path/to/repo.git/'), true) + equal(isGitUrl('ssh://user@host.xz/path/to/repo.git/'), true) + equal(isGitUrl('ssh://host.xz:1234/path/to/repo.git/'), true) + equal(isGitUrl('ssh://host.xz/path/to/repo.git/'), true) + equal(isGitUrl('ssh://user@host.xz/path/to/repo.git/'), true) + equal(isGitUrl('ssh://host.xz/path/to/repo.git/'), true) + equal(isGitUrl('ssh://user@host.xz/~user/path/to/repo.git/'), true) + equal(isGitUrl('ssh://host.xz/~user/path/to/repo.git/'), true) + equal(isGitUrl('ssh://user@host.xz/~/path/to/repo.git'), true) + equal(isGitUrl('ssh://host.xz/~/path/to/repo.git'), true) + equal(isGitUrl('git://host.xz/path/to/repo.git/'), true) + equal(isGitUrl('git://host.xz/~user/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('git+https://github.com/user/repo.git'), true) + equal(isGitUrl('user@server:project.git'), true) + equal(isGitUrl('/path/to/repo.git/'), true) + equal(isGitUrl('path/to/repo.git/'), true) + equal(isGitUrl('~/path/to/repo.git'), true) + equal(isGitUrl('file:///path/to/repo.git/'), true) + equal(isGitUrl('file://~/path/to/repo.git/'), true) + equal(isGitUrl('host.xz:/path/to/repo.git/'), true) +}) + +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 From b413c1de702e9998d1925a290e40fe53431f1d9d Mon Sep 17 00:00:00 2001 From: bluwy Date: Tue, 13 Aug 2024 17:22:00 +0800 Subject: [PATCH 16/20] fix: update git url regex --- pkg/src/utils.js | 6 +++--- pkg/tests/utils.js | 51 +++++++++++++++++++++------------------------- 2 files changed, 26 insertions(+), 31 deletions(-) diff --git a/pkg/src/utils.js b/pkg/src/utils.js index 938a631..39c9913 100644 --- a/pkg/src/utils.js +++ b/pkg/src/utils.js @@ -46,9 +46,9 @@ export function stripComments(code) { .replace(SINGLELINE_COMMENTS_RE, '') } -// Reference: https://git-scm.com/docs/git-clone#_git_urls +// Reference: https://git-scm.com/docs/git-clone#_git_urls and https://github.com/npm/hosted-git-info const GIT_URL_RE = - /^((?:git(?:\+(?:https?|file))?|https?|ftps?|file|ssh):\/\/)?(?:[\w._-]+@)?([\w.-]+)(?::(\d+))?\/([\w._/-]+(?:\.git)?)(?:\/|\?.*)?$/ + /^(git\+https?|git\+ssh|https?|ssh|git):\/\/(?:[\w._-]+@)?([\w.-]+)(?::([\w\d-]+))?(\/[\w._/-]+)\/?$/ /** * @param {string} url */ @@ -81,7 +81,7 @@ export function isDeprecatedGitHubGitUrl(url) { const protocol = tokens[1] const host = tokens[2] - if (/github/.test(host) && protocol === 'git://') { + if (/github/.test(host) && protocol === 'git') { return true } } diff --git a/pkg/tests/utils.js b/pkg/tests/utils.js index 486ecb1..480d02b 100644 --- a/pkg/tests/utils.js +++ b/pkg/tests/utils.js @@ -108,35 +108,30 @@ test('getCodeFormat', () => { } }) -test.skip('isGitUrl', () => { - equal(isGitUrl('git@github.com:user/project.git'), true) - equal(isGitUrl('https://github.com/user/project.git'), true) - equal(isGitUrl('http://github.com/user/project.git'), true) - equal(isGitUrl('git@192.168.101.127:user/project.git'), true) - equal(isGitUrl('https://192.168.101.127/user/project.git'), true) - equal(isGitUrl('http://192.168.101.127/user/project.git'), true) - equal(isGitUrl('ssh://user@host.xz:port/path/to/repo.git/'), true) - equal(isGitUrl('ssh://user@host.xz/path/to/repo.git/'), true) - equal(isGitUrl('ssh://host.xz:1234/path/to/repo.git/'), true) - equal(isGitUrl('ssh://host.xz/path/to/repo.git/'), true) - equal(isGitUrl('ssh://user@host.xz/path/to/repo.git/'), true) - equal(isGitUrl('ssh://host.xz/path/to/repo.git/'), true) - equal(isGitUrl('ssh://user@host.xz/~user/path/to/repo.git/'), true) - equal(isGitUrl('ssh://host.xz/~user/path/to/repo.git/'), true) - equal(isGitUrl('ssh://user@host.xz/~/path/to/repo.git'), true) - equal(isGitUrl('ssh://host.xz/~/path/to/repo.git'), true) - equal(isGitUrl('git://host.xz/path/to/repo.git/'), true) - equal(isGitUrl('git://host.xz/~user/path/to/repo.git/'), true) - equal(isGitUrl('http://host.xz/path/to/repo.git/'), true) +test('isGitUrl', () => { equal(isGitUrl('https://host.xz/path/to/repo.git/'), true) - equal(isGitUrl('git+https://github.com/user/repo.git'), true) - equal(isGitUrl('user@server:project.git'), true) - equal(isGitUrl('/path/to/repo.git/'), true) - equal(isGitUrl('path/to/repo.git/'), true) - equal(isGitUrl('~/path/to/repo.git'), true) - equal(isGitUrl('file:///path/to/repo.git/'), true) - equal(isGitUrl('file://~/path/to/repo.git/'), true) - equal(isGitUrl('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) + + equal(isGitUrl('file://host.xz/path/to/repo'), false) + equal(isGitUrl('/User/foo/bar'), false) }) test('isDeprecatedGitHubGitUrl', () => { From 9f9408a73db4399a1d2242485efe9dca8d78313b Mon Sep 17 00:00:00 2001 From: bluwy Date: Tue, 13 Aug 2024 17:30:10 +0800 Subject: [PATCH 17/20] chore: note one case --- pkg/tests/utils.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/tests/utils.js b/pkg/tests/utils.js index 480d02b..a381c46 100644 --- a/pkg/tests/utils.js +++ b/pkg/tests/utils.js @@ -129,6 +129,9 @@ test('isGitUrl', () => { 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) From bafb639395716ad77feb738298653555887c2669 Mon Sep 17 00:00:00 2001 From: bluwy Date: Tue, 13 Aug 2024 17:34:20 +0800 Subject: [PATCH 18/20] docs: update --- site/rules.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/rules.md b/site/rules.md index 57815a6..896c9b2 100644 --- a/site/rules.md +++ b/site/rules.md @@ -244,6 +244,6 @@ The `"jsnext:main"` and `"jsnext"` fields are deprecated. The `"module"` field s `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 ba a valid [git URL](https://git-scm.com/docs/git-clone#_git_urls). +- 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). From 6af7d023050f61453db9f68e2e0b855a492f2189 Mon Sep 17 00:00:00 2001 From: bluwy Date: Thu, 15 Aug 2024 15:36:36 +0800 Subject: [PATCH 19/20] chore: update level --- pkg/src/index.js | 4 ++-- pkg/src/message.js | 5 ++++- site/rules.md | 11 +++++++++++ site/src/utils/message.js | 5 ++++- 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/pkg/src/index.js b/pkg/src/index.js index 5e9f2f6..6e5462e 100755 --- a/pkg/src/index.js +++ b/pkg/src/index.js @@ -482,14 +482,14 @@ export async function publint({ pkgDir, vfs, level, strict, _packedFiles }) { code: 'INVALID_REPOSITORY_VALUE', args: { type: 'deprecated-github-git-protocol' }, path: ['repository', 'url'], - type: 'warning' + type: 'suggestion' }) } else if (isShorthandGitHubOrGitLabUrl(repository.url)) { messages.push({ code: 'INVALID_REPOSITORY_VALUE', args: { type: 'shorthand-git-sites' }, path: ['repository', 'url'], - type: 'warning' + type: 'suggestion' }) } } diff --git a/pkg/src/message.js b/pkg/src/message.js index 47be43e..e0eb81b 100644 --- a/pkg/src/message.js +++ b/pkg/src/message.js @@ -165,12 +165,15 @@ export function formatMessage(m, pkg) { 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 "git+https://example.com/user/repo.git".` + 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 } diff --git a/site/rules.md b/site/rules.md index 896c9b2..a0ce1d3 100644 --- a/site/rules.md +++ b/site/rules.md @@ -247,3 +247,14 @@ The `"jsnext:main"` and `"jsnext"` fields are deprecated. The `"module"` field s - 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 eaf0a65..ab2916d 100644 --- a/site/src/utils/message.js +++ b/site/src/utils/message.js @@ -159,12 +159,15 @@ function messageToString(m, pkg) { 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 "git+https://example.com/user/repo.git".` + 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 } From a6d505b0cff50b6c184242bcbe905c7d5dee0de1 Mon Sep 17 00:00:00 2001 From: bluwy Date: Thu, 15 Aug 2024 15:37:57 +0800 Subject: [PATCH 20/20] chore: my bad --- pkg/tests/playground.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/tests/playground.js b/pkg/tests/playground.js index 771ae82..a343e3d 100644 --- a/pkg/tests/playground.js +++ b/pkg/tests/playground.js @@ -164,14 +164,14 @@ testFixture('invalid-repository-value-object-not-git-url', [ testFixture('invalid-repository-value-object-shorthand-site', [ { code: 'INVALID_REPOSITORY_VALUE', - type: 'warning' + type: 'suggestion' } ]) testFixture('invalid-repository-value-object-deprecated', [ { code: 'INVALID_REPOSITORY_VALUE', - type: 'warning' + type: 'suggestion' } ])