diff --git a/packages/cli/README.md b/packages/cli/README.md index e26be7b2e..e7bf30b39 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -1283,7 +1283,8 @@ repo show ID PROJECT [--json]`. `mosaic org project repo set ID PROJECT (--local-only --accept-local-loss-risk | --remote URL) --if-revision N [--json]` changes a project's repository later. A remote is https or ssh; its checkout's `origin` is set and checked once with `git ls-remote`, without credentials, and -the receipt says `reachable`, `needs-credential` or `unreachable`. A copy +the receipt says `reachable`, `needs-credential` or `unreachable`. An ssh check +does not verify the host key and says so. A copy credential binding supplies credentials for https Gitea remotes only. See `packages/organization/README.md` for the URL rules, layout and scope. On existing CLI commands, `--org ID` is an alias for `--business ID`; if both are supplied they must name the same ID. diff --git a/packages/organization/README.md b/packages/organization/README.md index 9c6c3748c..0c5febd11 100644 --- a/packages/organization/README.md +++ b/packages/organization/README.md @@ -584,7 +584,9 @@ credential helper, no askpass, no terminal prompt, no redirects, `GIT_ALLOW_PROTOCOL=https:ssh`, no git or ssh config files, no ssh agent or key, and a 15 second limit. The receipt's `check.result` is `reachable`, `needs-credential` (the check sends none) or `unreachable` with git's error -summarized; `check.url` names the URL it ran against, and `repo show` reports a +summarized. The ssh check accepts any host key, so an ssh answer shows only that +something at that address speaks git; its message adds "the ssh host key was not +verified". `check.url` names the URL it ran against, and `repo show` reports a check only for the URL recorded now. The check never refuses the setup, and nothing is pushed. A copy credential binding (CB lanes) supplies credentials for https Gitea remotes only; an ssh or other https remote that needs a diff --git a/packages/organization/src/repo-remote.mjs b/packages/organization/src/repo-remote.mjs index f65ed32e7..c3ef7cea1 100644 --- a/packages/organization/src/repo-remote.mjs +++ b/packages/organization/src/repo-remote.mjs @@ -83,6 +83,11 @@ function lsRemote(url, { protocols, timeoutMs }) { let override = null; +// The ssh check accepts any host key (nothing secret is sent to it), so an answer over ssh shows +// that something at that address speaks git, not that it is the named host. Every ssh outcome that +// came from an answer says so; https verifies the host's certificate as usual. +export const SSH_HOST_UNVERIFIED = 'the ssh host key was not verified'; + // The check for a URL parseRepoRemote accepted: https and ssh only, 15 second limit. Returns // {result: 'reachable'|'needs-credential'|'unreachable', message, url}. A URL that does not parse // is refused by the caller first; here it is reported unreachable without running git. @@ -90,7 +95,8 @@ export async function checkRemote(url) { const parsed = parseRepoRemote(url); if (!parsed.ok) return { result: 'unreachable', message: parsed.message, url }; const outcome = override ? await override(parsed.url) : await lsRemote(parsed.url, PRODUCTION); - return { result: outcome.result, message: outcome.message, url: parsed.url }; + const message = parsed.kind === 'ssh' && outcome.result !== 'unreachable' ? `${outcome.message}; ${SSH_HOST_UNVERIFIED}` : outcome.message; + return { result: outcome.result, message, url: parsed.url }; } // TEST SEAM. Tests reach a local bare repository through this and nothing else: lsRemote with a diff --git a/packages/organization/tests/repo-remote.test.mjs b/packages/organization/tests/repo-remote.test.mjs index a21e76f5d..c0f40c2fc 100644 --- a/packages/organization/tests/repo-remote.test.mjs +++ b/packages/organization/tests/repo-remote.test.mjs @@ -8,7 +8,7 @@ import { chmodSync, mkdtempSync, mkdirSync, readdirSync, readFileSync, writeFile import { dirname, join, resolve } from 'node:path'; import { tmpdir } from 'node:os'; import { fileURLToPath } from 'node:url'; -import { parseRepoRemote, checkRemote, classifyCheck, summarizeGitError, CHECK_SSH_COMMAND, NEEDS_CREDENTIAL_MESSAGE, repoRemoteTestSeam } from '../src/repo-remote.mjs'; +import { parseRepoRemote, checkRemote, classifyCheck, summarizeGitError, CHECK_SSH_COMMAND, NEEDS_CREDENTIAL_MESSAGE, SSH_HOST_UNVERIFIED, repoRemoteTestSeam } from '../src/repo-remote.mjs'; import { createOrg, templateRender, projectRepoInit, projectRepoShow, projectRepoSet, queueInit, showOrg, updateOrg } from '../src/org-service.mjs'; import { loadBusiness } from '../src/organization-config.mjs'; import { REPO_ROLES } from './helpers.mjs'; @@ -132,6 +132,25 @@ test('no production source reaches the test seam', () => { assert.deepEqual(offenders, []); }); +// #1873 security review P2: the ssh check accepts any host key, so an ssh answer cannot say the +// named host answered. Every ssh outcome that came from an answer says the key was not verified; +// https and an unreachable ssh host do not carry the note. +test('an ssh answer says the host key was not verified; https does not', async () => { + const answer = { reachable: { result: 'reachable', message: 'the repository answered' }, needs: { result: 'needs-credential', message: NEEDS_CREDENTIAL_MESSAGE }, down: { result: 'unreachable', message: 'ssh: Could not resolve hostname h.invalid' } }; + try { + for (const url of ['git@git.example.com:team/work.git', 'ssh://git@git.example.com:2222/team/work.git']) { + seam.useCheck(async () => answer.reachable); + assert.deepEqual(await checkRemote(url), { result: 'reachable', message: `the repository answered; ${SSH_HOST_UNVERIFIED}`, url }); + seam.useCheck(async () => answer.needs); + assert.equal((await checkRemote(url)).message, `${NEEDS_CREDENTIAL_MESSAGE}; ${SSH_HOST_UNVERIFIED}`); + seam.useCheck(async () => answer.down); + assert.equal((await checkRemote(url)).message, answer.down.message); + } + seam.useCheck(async () => answer.reachable); + assert.equal((await checkRemote('https://git.example.com/team/work.git')).message, 'the repository answered'); + } finally { seam.useCheck(null); } +}); + async function orgWithRepo(c, opts = { mode: 'local-only', acceptLocalLossRisk: true }) { await createOrg(c, 'alpha', await templateRender('alpha')); return projectRepoInit(c, 'alpha', 'work', opts); } diff --git a/packages/webui/tests/project-repo-browser.test.mjs b/packages/webui/tests/project-repo-browser.test.mjs index dc5e7af5b..ce981fd7e 100644 --- a/packages/webui/tests/project-repo-browser.test.mjs +++ b/packages/webui/tests/project-repo-browser.test.mjs @@ -153,7 +153,7 @@ test('the owner edits the repository on Settings: remote, refusals, local only a // origin). The page says so; saving the recorded URL again repairs it. p.git('remote', 'set-url', 'origin', 'https://elsewhere.invalid/x/y'); await reread(p); - assert.deepEqual(await facts(b), ['Remote', scp, 'https://elsewhere.invalid/x/y', 'Needs a credential: the repository needs a credential; this check uses none.']); + assert.deepEqual(await facts(b), ['Remote', scp, 'https://elsewhere.invalid/x/y', 'Needs a credential: the repository needs a credential; this check uses none; the ssh host key was not verified.']); assert.equal(await text(b, '[data-repo-drift]'), 'Git origin differs from the recorded repository; save it again to repair.'); await save(p, '/Saved/'); assert.equal(p.remotes(), `origin\t${scp} (fetch)\norigin\t${scp} (push)`);