Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add INVALID_REPOSITORY_VALUE rule #106

Merged
merged 20 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions pkg/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
/**
Expand Down
61 changes: 60 additions & 1 deletion pkg/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ import {
getPkgPathValue,
replaceLast,
isRelativePath,
isAbsolutePath
isAbsolutePath,
isGitUrl,
isShorthandRepositoryUrl,
isShorthandGitHubOrGitLabUrl,
isDeprecatedGitHubGitUrl
} from './utils.js'

/**
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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, string> | 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
*/
Expand Down
26 changes: 26 additions & 0 deletions pkg/src/message.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
54 changes: 54 additions & 0 deletions pkg/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { lintableFileExtensions } from './constants.js'
* main: string,
* module: string,
* exports: Record<string, string>,
* repository: Record<string, string> | string,
* type: 'module' | 'commonjs'
* }} Pkg
*/
Expand Down Expand Up @@ -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}
Expand Down
3 changes: 2 additions & 1 deletion pkg/tests/fixtures/invalid-field-types/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@
"type": "commonjs",
"main": 0,
"module": true,
"jsnext:main": false
"jsnext:main": false,
"repository": 123
}
Original file line number Diff line number Diff line change
@@ -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"
}
}
Original file line number Diff line number Diff line change
@@ -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/"
}
}
Original file line number Diff line number Diff line change
@@ -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"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "publint-invalid-repository-value-shorthand",
"version": "0.0.1",
"type": "commonjs",
"repository": "npm/npm"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "publint-invalid-repository-value-string-not-url",
"version": "0.0.1",
"type": "commonjs",
"repository": "not_an_url"
}
31 changes: 31 additions & 0 deletions pkg/tests/playground.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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']
Expand Down
62 changes: 62 additions & 0 deletions pkg/tests/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions site/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
```
Loading