Skip to content
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
6 changes: 5 additions & 1 deletion tools/nx-npm-trust/src/check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,11 @@ if (report.checks.exists === true && name) {
const setMfa = npm(['access', 'set', `mfa=${mfaTarget}`, name]);
if (setMfa.code === 0) {
report.fixes.push(`npm access set mfa=${mfaTarget} ${name}`);
report.checks.mfa = mfaTarget;
// Stored as `mfaPolicy` (not `mfa`) to reflect that the value is the
// configured publish policy (`none` / `automatic`), not a user secret
// or authentication factor. Keeping the name `mfa` triggered CodeQL's
// `js/clear-text-logging` heuristic on the JSON report log below.
report.checks.mfaPolicy = mfaTarget;
} else {
report.problems.push(
`npm access set mfa=${mfaTarget} failed: ${firstErrorLine(setMfa.stderr)}`,
Expand Down
51 changes: 45 additions & 6 deletions tools/nx-npm-trust/src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,58 @@
}

/**
* Parse `<owner>/<repo>` from a git remote URL (ssh/https/git protocol).
* Returns null for non-GitHub remotes.
* Strict allow-list for GitHub `<owner>/<repo>` path segments. This is the
* final validation gate before the value flows into a shell command (see
* `--trust-repo=${trustRepo}` below), so characters outside `[A-Za-z0-9._-]`
* must be rejected even if the URL parser accepted them.
*/
const OWNER_REPO = /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/;

/** Strip leading slashes and a trailing `.git`, then validate against
* {@link OWNER_REPO}. Returns the normalised `owner/repo` or `null`. */
function parseOwnerRepo(path: string): string | null {
const normalised = path.replace(/^\/+|\.git$/g, '');

Check warning on line 71 in tools/nx-npm-trust/src/plugin.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Prefer `String#replaceAll()` over `String#replace()`.

See more on https://sonarcloud.io/project/issues?id=abapify_adt-cli&issues=AZ2vTqL0Xk8djZPTLuQ6&open=AZ2vTqL0Xk8djZPTLuQ6&pullRequest=115
return OWNER_REPO.test(normalised) ? normalised : null;
}

/**
* Parse `<owner>/<repo>` from a git remote URL (https, http, git, ssh,
* git+ssh, or SCP-style `git@host:owner/repo`). Returns null for non-GitHub
* remotes.
*
* The host check is anchored via `URL.hostname` (URL-form) or the explicit
* SCP pattern (for `user@host:path` form), so substrings like
* `github.com.attacker.com` or `evil/github.com/...` cannot match. Fixes
* CodeQL `js/incomplete-url-substring-sanitization`.
*
* The SCP branch is skipped when the input contains `://`: proper URL-form
* remotes with a `user@` and a `:port` (e.g. `ssh://git@github.com:22/…`)
* would otherwise be mis-parsed as SCP.
*/
function detectGithubRepo(): string | null {
const res = spawnSync('git', ['remote', 'get-url', 'origin'], {
cwd: workspaceRoot,
encoding: 'utf-8',
});
if (res.status !== 0) return null;
const url = res.stdout.trim();
if (!url.includes('github.com')) return null;
const m = url.match(/[:/]([^/]+\/[^/.]+?)(\.git)?$/);
return m ? m[1] : null;
const raw = res.stdout.trim();

// SCP-style: `user@host:path`. Must be handled manually because `new URL()`
// does not accept it. Guarded by the `://` check so URL-form remotes with
// `user@host:port` fall through to the URL parser below.
const scp = !raw.includes('://') && raw.match(/^[^@\s]+@([^:]+):(.+)$/);
if (scp) {
return scp[1] === 'github.com' ? parseOwnerRepo(scp[2]) : null;
}

// Any proper URL scheme (http, https, git, ssh, git+ssh, …).
try {
const u = new URL(raw);
if (u.hostname !== 'github.com') return null;
return parseOwnerRepo(u.pathname);
} catch {
return null;
}
}

export const createNodesV2: CreateNodesV2<NxNpmTrustOptions> = [
Expand Down
Loading