Skip to content

Commit

Permalink
feat: add INVALID_REPOSITORY_VALUE rule (#106)
Browse files Browse the repository at this point in the history
Co-authored-by: bluwy <[email protected]>
  • Loading branch information
Namchee and bluwy authored Aug 15, 2024
1 parent 17464b8 commit a94f663
Show file tree
Hide file tree
Showing 14 changed files with 332 additions and 4 deletions.
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://[email protected]/path/to/repo.git'), true)
equal(isGitUrl('git+ssh://[email protected]/path/to/repo'), true)
equal(isGitUrl('git+ssh://[email protected]: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://[email protected]/user/project.git'), true)
equal(isGitUrl('ssh://[email protected]:user/project.git'), true)
equal(isGitUrl('git+ssh://[email protected]/user/project.git'), true)
equal(isGitUrl('git+ssh://[email protected]: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('[email protected]: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

0 comments on commit a94f663

Please sign in to comment.