From e4daad520546a6f3439dd4a38a939a427f28593c Mon Sep 17 00:00:00 2001 From: Alejandro Rizzo Date: Tue, 11 Aug 2026 10:35:56 +0100 Subject: [PATCH 1/5] feat: Add repository (project) token support OD-489 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Connecting CI or the auto-configuration agent to Codacy required a personal account API token that acts as the full user — every organization, every repository, and no expiry. Repository tokens now work on the operations the API whitelists for them, so CI can authenticate with a credential whose blast radius is one repository. Adds `--repository-token ` (and `CODACY_PROJECT_TOKEN`) on every command, sent as the `project-token` header. Precedence matches the Codacy Analysis CLI exactly: flag > CODACY_PROJECT_TOKEN > CODACY_API_TOKEN > stored login. Auth is now a `RemoteAuth` discriminated union carrying both the token kind and its source, so "exactly one token, and we know which" is a compile-time property and refusals can name where the token came from. Codacy honours repository tokens on only 13 operations, so commands whose endpoints are outside that set refuse up front — before any request, and before the git auto-detection line — instead of surfacing a bare `Unauthorized`. `codacy repository` skips its two non-whitelisted calls and marks them in JSON as `unavailable`, keeping `pullRequests` an iterable empty array so existing `jq` consumers keep working; under an account token the payload is byte-identical to before. Also fixes two pre-existing issues found along the way: the dashboard lost entirely when the pull-request lookup failed (its three sibling calls were already guarded), and `login` reporting a repository token as "invalid" when it is rejected by /user by design. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/repository-token-support.md | 29 ++ AGENTS.md | 25 +- README.md | 39 ++- SPECS/README.md | 3 + SPECS/missing-endpoints.md | 48 +++ SPECS/repository-tokens.md | 120 +++++++ src/commands/AGENTS.md | 2 + src/commands/directories.ts | 5 +- src/commands/finding.ts | 5 +- src/commands/findings.ts | 5 +- src/commands/info.ts | 5 +- src/commands/issue.ts | 5 +- src/commands/issues.test.ts | 64 ++++ src/commands/issues.ts | 33 +- src/commands/login.test.ts | 56 ++++ src/commands/login.ts | 35 +- src/commands/logout.test.ts | 31 ++ src/commands/logout.ts | 11 +- src/commands/ls.ts | 5 +- src/commands/pattern.test.ts | 20 ++ src/commands/pattern.ts | 5 +- src/commands/patterns.test.ts | 16 + src/commands/patterns.ts | 5 +- src/commands/pull-request.ts | 5 +- src/commands/pull-requests.ts | 5 +- src/commands/repositories.ts | 5 +- .../repository-token-refusals.test.ts | 183 +++++++++++ src/commands/repository.test.ts | 180 ++++++++++ src/commands/repository.ts | 115 ++++++- src/commands/tool.test.ts | 21 ++ src/commands/tool.ts | 5 +- src/commands/tools.test.ts | 133 ++++++++ src/commands/tools.ts | 54 ++- src/index.ts | 10 +- src/utils/auth.test.ts | 307 +++++++++++++++++- src/utils/auth.ts | 232 ++++++++++++- src/utils/import-config.ts | 12 +- vitest.config.mts | 6 + 38 files changed, 1752 insertions(+), 93 deletions(-) create mode 100644 .changeset/repository-token-support.md create mode 100644 SPECS/missing-endpoints.md create mode 100644 SPECS/repository-tokens.md create mode 100644 src/commands/repository-token-refusals.test.ts diff --git a/.changeset/repository-token-support.md b/.changeset/repository-token-support.md new file mode 100644 index 0000000..abeb7bd --- /dev/null +++ b/.changeset/repository-token-support.md @@ -0,0 +1,29 @@ +--- +"@codacy/codacy-cloud-cli": minor +--- + +Add repository (project) token support + +You can now authenticate with a **repository token** — scoped to a single repository — instead of a personal account API token that reaches every organization and repository you can see. This is the right credential for CI and for the auto-configuration agent: if it leaks, the blast radius is one repository. + +```bash +codacy tools --repository-token +# or, for a whole CI job: +export CODACY_PROJECT_TOKEN= +``` + +Get one from **Codacy > Repository > Settings > Integrations > Project API token**. The new `--repository-token ` flag is accepted by every command, and `CODACY_PROJECT_TOKEN` is picked up automatically. + +**Token precedence** (identical to the Codacy Analysis CLI): `--repository-token` > `CODACY_PROJECT_TOKEN` > `CODACY_API_TOKEN` > stored `codacy login`. An explicit `--repository-token` wins outright, so a deliberately scoped run is never silently widened. Note that `CODACY_PROJECT_TOKEN` outranks `CODACY_API_TOKEN` — unset it if you want your account token used. + +**Not every command accepts a repository token**, because Codacy only honours them on a limited set of repository-scoped operations: + +- **Fully supported:** `tools`, `tool`, `patterns`, `pattern`, `issues` (including `--overview`), `tools --import`, `repository --reanalyze` / `--reanalyze-and-wait`. +- **Partially supported:** `repository` works but omits the pull request and coverage sections. In `--output json`, `pullRequests` stays an empty array and a new `unavailable: ["pullRequests"]` field marks what couldn't be fetched. Output under an account token is unchanged. +- **Account token required:** `info`, `repositories`, `ls`, `directories`, `pull-request`, `pull-requests`, `issue`, `findings`, `finding`, `issues --ignore`/`--ignored`, `tools --import --force`, and `repository`'s `--add`/`--remove`/`--follow`/`--unfollow`/`--link-standard`/`--unlink-standard`. + +Unsupported combinations now fail immediately with a message naming the operation, why a repository token can't perform it, and which token is in use — instead of sending a request that comes back as a bare `Unauthorized`. + +`codacy login` continues to store account tokens only; repository tokens are passed per command or via the environment. + +Also fixed: `codacy repository` no longer loses the entire dashboard when the pull request lookup fails, and `codacy login` no longer reports a repository token as "invalid" when it is rejected for being the wrong kind of token. diff --git a/AGENTS.md b/AGENTS.md index e16f440..6bc9e35 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -86,9 +86,16 @@ codacy-cloud-cli/ - Prefer that over calling `setTimeout`/`sleep` directly in a command, unless you have a clear reason not to. - Default cadence is `POLL_INTERVAL_MS` (10s), capped at `MAX_WAIT_MS` (20min). - **Error handling:** Use `try/catch` with the shared `handleError()` from `src/utils/error.ts` -- **Authentication:** All commands that call the API must call `checkApiToken()` from `src/utils/auth.ts` before making requests - **API base URL:** `https://app.codacy.com/api/v3` (configured in `src/index.ts` via `OpenAPI.BASE`) -- **Auth mechanism:** `CODACY_API_TOKEN` environment variable, sent as `api-token` header +- **Authentication — two token kinds.** Read `SPECS/repository-tokens.md` before touching auth or adding a command. + - An **account token** (`api-token` header) reaches everything its owner can see. A **repository token** (`project-token` header) is scoped to one repository and is accepted only on a fixed whitelist of 13 operations; everywhere else Codacy rejects it as if no token had been sent. + - Every command that calls the API resolves auth first, via `resolveAuth(this)` from `src/utils/auth.ts` (returns a `RemoteAuth` discriminated union), and declares `.addOption(repositoryTokenOption())` so `--repository-token` parses. + - **New commands must decide their token scope**, using the whitelist in `SPECS/repository-tokens.md`: + - account-only end to end → `resolveAccountAuth(this, "")` + - fully whitelisted → `resolveAuth(this)` + - mixed → `resolveAuth(this)` plus `requireAccountToken(auth, "", "")` per unsupported flag, or `fetchIfAccountToken(...)` to skip an unsupported sub-call + - **Guards must run before any request**, and before `resolveRepoArgs()` — that shells out to git and prints an auto-detection line, which is misleading ahead of a refusal. + - The whitelist is hardcoded in these guards. **Re-verify it after every `npm run update-api`.** ### Command Pattern @@ -98,7 +105,7 @@ Every command file follows this structure: // src/commands/.ts import { Command } from "commander"; import ora from "ora"; -import { checkApiToken } from "../utils/auth"; +import { repositoryTokenOption, resolveAuth } from "../utils/auth"; import { handleError } from "../utils/error"; // Import relevant API service(s) @@ -108,9 +115,14 @@ export function registerCommand(program: Command) { .description("Clear description of what this command does") .argument("[args]", "Description of arguments") .option("--flag ", "Description of options") - .action(async (args, options) => { + // Declared per command (not only in index.ts) so `--repository-token` parses + // in the test harnesses, which each build a bare `new Command()`. + .addOption(repositoryTokenOption()) + .action(async function (this: Command, args, options) { try { - checkApiToken(); + // Or resolveAccountAuth(this, "") for an account-only command — + // see the Authentication bullet above. + const auth = resolveAuth(this); const spinner = ora("Loading...").start(); // Call API service // Format and display output @@ -216,7 +228,8 @@ When completing work, agents **must** update relevant documentation: | Variable | Required | Description | |---|---|---| -| `CODACY_API_TOKEN` | Yes | API token for authenticating with Codacy. Get it from Codacy > Account > API Tokens | +| `CODACY_API_TOKEN` | One of the two | Account API token. Get it from Codacy > Account > API Tokens | +| `CODACY_PROJECT_TOKEN` | One of the two | Repository (project) token, scoped to one repository. Get it from Codacy > Repository > Settings > Integrations > Project API token. **Outranks `CODACY_API_TOKEN`** — see `SPECS/repository-tokens.md` | ## Useful Context diff --git a/README.md b/README.md index 6459f6b..8e6cddc 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,11 @@ npm link ## Authentication -Log in interactively (recommended): +The CLI accepts two kinds of token. + +### Account API token + +Reaches every organization and repository your account can see. Log in interactively (recommended): ```bash codacy login @@ -38,6 +42,38 @@ You can get a token from **Codacy > My Account > Access Management > API Tokens* The `login` command stores the token encrypted at `~/.codacy/credentials`. The environment variable takes precedence over stored credentials when both are present. +### Repository (project) token + +Scoped to a single repository — the right choice for CI, since a leaked token can't reach anything else. Get one from **Codacy > Repository > Settings > Integrations > Project API token**. + +```bash +codacy tools --repository-token your-repository-token +# or, for a whole CI job: +export CODACY_PROJECT_TOKEN=your-repository-token +``` + +Codacy accepts repository tokens on a **limited set of repository-scoped operations**, so some commands require an account token and say so explicitly rather than failing with a generic authorization error: + +| Works with a repository token | Requires an account token | +|---|---| +| `tools`, `tool`, `patterns`, `pattern` | `info`, `repositories` | +| `issues` (including `--overview`) | `issues --ignore`, `issues --ignored`, `issue` | +| `repository`, `repository --reanalyze` | `repository --add`/`--remove`/`--follow`/`--unfollow`/`--link-standard`/`--unlink-standard` | +| | `pull-request`, `pull-requests`, `ls`, `directories`, `findings`, `finding` | + +`codacy repository` works, but omits the pull request and coverage sections — those endpoints don't accept repository tokens. In `--output json` it marks them as `"unavailable": ["pullRequests"]`. + +`codacy login` stores account tokens only; pass repository tokens per command or via `CODACY_PROJECT_TOKEN`. + +### Token precedence + +1. `--repository-token ` +2. `CODACY_PROJECT_TOKEN` +3. `CODACY_API_TOKEN` +4. Stored credentials from `codacy login` + +An explicit `--repository-token` wins outright, so a deliberately scoped run is never silently widened by an environment variable or a stale login. Note that `CODACY_PROJECT_TOKEN` outranks `CODACY_API_TOKEN` (matching the [Codacy Analysis CLI](https://github.com/codacy/analysis-cli)) — unset it if you want your account token used. + ## Usage ```bash @@ -50,6 +86,7 @@ codacy --help # Detailed usage for any command | Option | Description | |---|---| | `-o, --output ` | Output format: `table` (default) or `json` | +| `--repository-token ` | Repository (project) token, scoped to one repository (env: `CODACY_PROJECT_TOKEN`) | | `-V, --version` | Show version | | `-h, --help` | Show help | diff --git a/SPECS/README.md b/SPECS/README.md index a8dba29..31469e4 100644 --- a/SPECS/README.md +++ b/SPECS/README.md @@ -37,6 +37,8 @@ _No pending tasks._ All commands implemented. - [setup.md](setup.md) — test framework, build, CI/CD setup - [deployment.md](deployment.md) — npm publishing, brew formula +- [repository-tokens.md](repository-tokens.md) — **read before touching auth or adding a command**: the two token kinds, precedence, the 13-operation backend whitelist, and the per-command support matrix +- [missing-endpoints.md](missing-endpoints.md) — API v3 operations that don't accept repository tokens yet, ranked; candidate Linear tasks ## Changelog @@ -83,3 +85,4 @@ _No pending tasks._ All commands implemented. | 2026-07-28 | (OD-296, findings side) `SrmItem` gained its own `advisoryInformation` field server-side (bumped pinned API `57.3.0` → `57.3.9`), closing the gap noted on 2026-07-24. `findings` (list) now shows the same compact "Vulnerable functions: fn1, fn2 (+N more)" line as `issues`, via the newly-exported `summarizeFunctions`. `finding` (detail) shows the full `printAdvisoryBlock` — but only when there's no linked Codacy issue, since `printIssueCodeContext` already renders the equivalent block from `issue.advisoryInformation` in that case; this is what makes vulnerable functions visible for SCA/dependency findings (and any other non-Codacy-source finding) that have no linked issue to borrow it from at all. Added to both commands' JSON `pickDeep` whitelists (6 new tests, 494 total) | | 2026-07-28 | (OD-378) New `pull-requests` (`prs`) command — the plural counterpart to `pull-request`, listing PRs for a repository with the same analysis-gated table columns as `repository`'s "Open Pull Requests" section (reuses `buildGateStatus`/`formatStandards`/`formatPrIssues`/`formatPrCoverage`/`formatDelta`). `--search-text`/`-q` and `--branch`/`-b` map to the API's `textQuery`/`targetBranch` params added in OD-376; the classification param (`search`, Merged vs. last-updated) is deliberately not exposed — different axis, out of scope. `[provider] [org] [repo]` auto-detect via `resolveRepoArgs`, paginate-to-`--limit` loop matching `findings`. Registered in `src/index.ts` (10 new tests, 516 total) | | 2026-07-30 | (OD-378, review follow-up) `pull-requests` table polish + a real data bug. **Bug:** Complexity rendered as "no data" on every PR because the API omits the flat top-level `deltaComplexity` and only returns `quality.deltaComplexity` (while still sending a top-level `deltaClonesCount`) — new shared `prQualityMetric(pr, key)` in `utils/formatting.ts` reads the nested `quality` value first and falls back to the flat field; also applied to `repository`'s Open PR table and `pull-request`'s Analysis section, which had the same bug. **Layout:** `✓` moved to the first column; metric order now matches `repositories` (issues → complexity → duplication → coverage); the Coverage column is dropped entirely when no listed PR has a coverage value (new `hasAnyPrCoverage()` — repos without coverage return `diffCoverage.cause` and no numbers on any PR); missing metric values now render as a dim `-` instead of `N/A` in `formatDelta`/`formatPrCoverage`/`formatPrIssues`, matching `formatStandards`/`formatCountCell`/`formatCoverageCell`; and a zero issue count renders as a bare `0` rather than `+0`/`-0` (`-0` read as a negative), matching what `pull-request`'s Files table and `formatDelta` already did. **JSON:** added `quality.resultReasons`/`coverage.resultReasons` (Codacy review suggestion — they drive the per-metric gate coloring, so consumers need them to see which gates passed/failed) plus the `quality.*` metric mirrors the table actually renders (23 new tests, 544 total) | +| 2026-08-11 | (OD-489) Repository (project) token support. New `--repository-token ` on every command (plus `CODACY_PROJECT_TOKEN`), sent as the `project-token` header; account tokens keep `api-token`. `src/utils/auth.ts` rewritten around a `RemoteAuth` discriminated union carrying both kind and source, replacing `checkApiToken()` with `resolveAuth(this)` / `resolveAccountAuth(this, why)` / `requireAccountToken(...)` / `fetchIfAccountToken(...)`. Precedence matches `codacy-analysis` exactly — flag > `CODACY_PROJECT_TOKEN` > `CODACY_API_TOKEN` > stored login — so `vitest.config.mts` now blanks `CODACY_PROJECT_TOKEN` (it outranks the account token and is exported job-wide by the coverage reporter, so tests would otherwise depend on the developer's shell). Codacy whitelists only 13 operations for repository tokens, so `tool`/`patterns`/`pattern` work unchanged, `issues` (incl. `--overview`) and `tools --import` work, and the 9 account-only commands plus `repository`'s 6 management flags, `issues --ignore`/`--ignored`, and `tools --import --force` (only when standards exist) **fail fast before any request** with a message naming the operation, the reason, and where the token came from. `repository`'s dashboard skips the two non-whitelisted calls: the table keeps the "Open Pull Requests" header with an explanatory line, and JSON keeps `pullRequests: []` (so `jq '.pullRequests[]'` still works) plus an additive `unavailable: ["pullRequests"]` — under an account token the payload is byte-identical. Also added the long-missing `.catch()` on the PR call so an account token lacking PR access degrades instead of losing the whole dashboard, and fixed `login`'s 401 message, which told repository-token users their token was "invalid" when it is rejected by `/user` by design. New `SPECS/repository-tokens.md` (whitelist + matrix, re-verify on every `npm run update-api`) and `SPECS/missing-endpoints.md` (ranked gaps for follow-up Linear tasks) (40 new tests, 606 total) | diff --git a/SPECS/missing-endpoints.md b/SPECS/missing-endpoints.md new file mode 100644 index 0000000..acd92da --- /dev/null +++ b/SPECS/missing-endpoints.md @@ -0,0 +1,48 @@ +# Missing repository-token endpoints + +Backlog of API v3 operations that **don't** accept a repository (project) token +but would unlock Cloud CLI functionality if they did. Found while implementing +[OD-489](https://linear.app/codacy/issue/OD-489/cloud-cli-add-support-for-project-tokens); +each row is a candidate Linear task against the backend whitelist (see +[repository-tokens.md](repository-tokens.md) for the current 13). + +Nothing here blocks the `configure-codacy-cloud` skill — it is fully supported +today. These are quality-of-life gaps that force users onto an account token for +otherwise repository-scoped work. + +## Ranked by value + +| # | operationId | Method | Unblocks | Why it matters | +|---|---|---|---|---| +| 1 | `listRepositoryPullRequests` | GET | `codacy pull-requests`, and the only reason `codacy repository` degrades at all | The single highest-value gap. Adding it makes the `repository` dashboard complete under a repository token and removes the whole skip/`unavailable` code path. Also unblocks a repository-scoped command users reach for constantly. | +| 2 | `getIssue` | GET | `codacy issue ` | Odd asymmetry today: `codacy issues` lists issues fine, but drilling into one is refused. Wants `getFileContent` (below) alongside it to render the code context. | +| 3 | `getFileContent` | GET | `codacy issue`, `codacy finding` code context | Only useful paired with #2. | +| 4 | `searchRepositoryIgnoredIssues` | POST | `codacy issues --ignored` | Read-only, and a natural sibling of the already-whitelisted `searchRepositoryIssues`. | +| 5 | `bulkIgnoreIssues` | POST | `codacy issues --ignore` | Write. Would let the auto-configuration flow ignore noisy issues instead of only disabling patterns. Note: a future read-only repository token must block this by operationId. | +| 6 | `updateIssueState` | PATCH | `codacy issue --ignore/--unignore` | Same category as #5, single-issue. | +| 7 | `listCoverageReports` | GET | The coverage-expectation suffix on `codacy repository`'s Analysis row | Lowest value of the reads: it affects one optional suffix and has **zero** JSON impact (no coverage key is projected). Listed for completeness. | + +## Deliberately out of scope + +Per the parent project's "Out of scope" section, these are intended to keep +refusing repository tokens — documented rather than fixed: + +- `AccountService.getUser` / `listUserOrganizations` — account-level by + definition (`codacy info`). +- `listOrganizationRepositoriesWithAnalysis` — organization-level + (`codacy repositories`). +- `SecurityService.*` (`searchSecurityItems`, `getSecurityItem`, + `ignoreSecurityItem`, `unignoreSecurityItem`) — security findings are + organization-scoped (`codacy findings` / `finding`). +- `addRepository`, `deleteRepository`, `followAddedRepository`, + `unfollowRepository` — account-level repository management. +- `applyCodingStandardToRepositories` — coding standards are organization-level + (`codacy repository --link-standard`, `codacy tools --import --force`). + +## Unclear / needs a decision + +- `RepositoryService.listFiles` / `listDirectories` — back `codacy ls` and + `codacy directories`. Both are plainly repository-scoped reads, so there's no + obvious reason to refuse them, but they weren't part of the CI-setup or + auto-configuration use cases the whitelist was drawn around. Worth asking + whether the omission was deliberate. diff --git a/SPECS/repository-tokens.md b/SPECS/repository-tokens.md new file mode 100644 index 0000000..b652633 --- /dev/null +++ b/SPECS/repository-tokens.md @@ -0,0 +1,120 @@ +# Repository (project) token support + +Status: ✅ Done (2026-08-11) — [OD-489](https://linear.app/codacy/issue/OD-489/cloud-cli-add-support-for-project-tokens) + +A **repository token** (also called a project token) is scoped to a single +repository, unlike an **account token**, which reaches everything its owner can +see. It lets CI and the auto-configuration agent authenticate without a personal +all-access token. + +## How it reaches the API + +| | Account token | Repository token | +|---|---|---| +| Header | `api-token` | `project-token` | +| Sources | `CODACY_API_TOKEN`, `codacy login` | `--repository-token`, `CODACY_PROJECT_TOKEN` | + +Resolution order (`pickAuth` in `src/utils/auth.ts`), **identical to +`codacy-analysis`** so both CLIs document the same rule: + +1. `--repository-token ` → repository token +2. `CODACY_PROJECT_TOKEN` → repository token +3. `CODACY_API_TOKEN` → account token +4. stored credentials (`codacy login`) → account token +5. otherwise: error + +An explicit `--repository-token` wins outright — it never even looks for an +account token, so a deliberately-scoped run can't be silently widened by an +ambient env var or a stale login. + +> ⚠️ Because `CODACY_PROJECT_TOKEN` outranks `CODACY_API_TOKEN`, and it is the +> variable the Codacy coverage reporter reads (so it is routinely exported +> job-wide in CI), `vitest.config.mts` blanks it via `test.env` — otherwise token +> resolution under test would depend on the developer's shell. + +## The backend whitelist + +> ⚠️ **Re-verify this list after every `npm run update-api`.** The CLI's guards +> hardcode it; if the backend adds an operation, a guard here will still refuse +> it. Source: Linear project *"Project token works in selected API v3 +> (+expiration)"*. + +Codacy accepts a repository token on **exactly** these 13 operations. Everywhere +else it is rejected as if no token had been sent. + +| operationId | Method | Used by this CLI | +|---|---|---| +| `listRepositoryTools` | GET | `tools`, `tool`, `patterns`, `pattern`, `issues -O` | +| `listRepositoryToolPatterns` | GET | `patterns`, `pattern`, `issues -O` | +| `getRepositoryWithAnalysis` | GET | `repository`, `tools --import` | +| `issuesOverview` | POST | `repository`, `issues -O` | +| `searchRepositoryIssues` | POST | `issues` | +| `toolPatternsOverview` | GET | `patterns --enable-all/--disable-all` | +| `listRepositoryCommits` | GET | `repository`, `repository --reanalyze*` | +| `configureTool` | PATCH | `tool`, `pattern`, `tools --import` | +| `updateRepositoryToolPatterns` | PATCH | `patterns`, `tools --import` | +| `reanalyzeCommitById` | POST | `repository --reanalyze*` | +| `getRepositoryLanguages` | GET | — | +| `getRepository` | GET | — | +| `listIgnoredFiles` | GET | — | + +`ToolsService.listTools` / `listPatterns` / `getPattern` are declared +`security: []` in the spec — unauthenticated, so they work with any token or +none. `issues -T ` and the `issues -O` noise suggestions rely on this. + +## Support matrix + +| Command | Under a repository token | +|---|---| +| `tool`, `patterns`, `pattern` | ✅ all modes | +| `issues` (list, all filters, `-O`) | ✅ | +| `issues --ignore` / `--ignored` | ❌ refused | +| `tools` (list, `--import`) | ✅ | +| `tools --import --force` | ❌ refused **when standards exist**; warns and continues when there are none (`--force` is then a no-op) | +| `repository` (dashboard) | ⚠️ partial — pull requests + coverage reports skipped | +| `repository --reanalyze` / `--reanalyze-and-wait` | ✅ | +| `repository --add/--remove/--follow/--unfollow/--link-standard/--unlink-standard` | ❌ refused | +| `info`, `repositories`, `ls`, `directories`, `pull-request`, `pull-requests`, `issue`, `findings`, `finding` | ❌ refused | +| `login`, `logout` | Warn that the flag is ignored | + +Every invocation in the `configure-codacy-cloud` skill lands in ✅ or the +partial-but-sufficient `repository` dashboard. + +## Implementation notes + +- **Fail fast, never a bare 401.** Guards run *before* any request — and before + `resolveRepoArgs`, which shells out to git and prints an auto-detection line + that would be misleading ahead of a refusal. Refusal messages name the + operation, the reason, **and where the token came from** (`--repository-token` + vs `CODACY_PROJECT_TOKEN`), which is what makes a surprising refusal + debuggable in one read. +- **`--repository-token` has no short flag.** The short-flag space collides per + command (`-r` is `repository --remove`, `-R` is `--reanalyze`, `-t` is + `--tags` on `issues`/`patterns` and `--token` on `login`), so any letter would + mean different things in different commands. `codacy-analysis` dropped its + short flag for the same reason. This is a deliberate exception to the + "every option needs a short flag" rule in `src/commands/AGENTS.md`. +- **Declared per command, plus once on the root program**, via + `.addOption(repositoryTokenOption())`. Declaring it only in `index.ts` would + make it invisible to the test harnesses, which each build a bare + `new Command()`. `repositoryTokenFlag()` reads the command's own value before + the inherited one, so the nearest wins. +- **`repository` dashboard degradation.** `listRepositoryPullRequests` and + `listCoverageReports` are skipped rather than attempted. The table keeps the + `Open Pull Requests` header with an explanatory line — a vanishing section + reads as a bug, and `printPullRequests([])` would claim "No open pull + requests", a different and false statement. In JSON, `pullRequests` stays `[]` + (so `jq '.pullRequests[]'` and `| length` keep working) and an additive + `unavailable: ["pullRequests"]` distinguishes "none" from "couldn't look". + Under an account token the payload is byte-identical to before this change. +- **`login` is account-only.** It validates against `/user`, which a repository + token can never reach, and the credentials store holds a single bare token + with no record of its kind. Its 401 message names the repository-token case + explicitly, since that request fails by design rather than because the token + is bad. +- **The "flag ignored" warning keys on the explicit flag only**, never on an + ambient `CODACY_PROJECT_TOKEN` — warning on that would fire on every + unrelated invocation in CI and train users to ignore warnings. + +See [missing-endpoints.md](missing-endpoints.md) for the whitelist gaps worth +closing. diff --git a/src/commands/AGENTS.md b/src/commands/AGENTS.md index ed66911..3152905 100644 --- a/src/commands/AGENTS.md +++ b/src/commands/AGENTS.md @@ -42,6 +42,8 @@ Every command must declare a short alias via `.alias()`. Keep aliases short (2 Every command option must have both a short flag and a long flag: `-X, --long-name `. Pick single letters that are intuitive and don't conflict with Commander's built-in flags (`-V/--version`, `-h/--help`) or the global `-o/--output` option. When the natural letter is already taken, use uppercase (e.g. `-O, --overview` instead of `-o`). +**Exception — cross-command options are long-only.** An option declared on *every* command (currently `--repository-token`, `--no-update-notifier`) can't safely take a short flag: the short-flag space is already contested per command, so any letter would mean different things in different places (`-r` is `repository --remove`, `-R` is `--reanalyze`, `-t` is `--tags` on `issues`/`patterns` but `--token` on `login`). `codacy-analysis` dropped its `--repository-token` short flag for the same reason. Do not add short flags to these. + ## Option Naming: Singular vs Plural Use a **singular** option name when the parameter accepts a single value, and a **plural** name when it accepts a comma-separated list: diff --git a/src/commands/directories.ts b/src/commands/directories.ts index 756cb88..23a4f2b 100644 --- a/src/commands/directories.ts +++ b/src/commands/directories.ts @@ -2,7 +2,7 @@ import { Command } from "commander"; import ora from "ora"; import ansis from "ansis"; import pluralize from "pluralize"; -import { checkApiToken } from "../utils/auth"; +import { repositoryTokenOption, resolveAccountAuth } from "../utils/auth"; import { handleError } from "../utils/error"; import { createTable, getOutputFormat, printJson } from "../utils/output"; import { sanitizeText } from "../utils/sanitize"; @@ -191,6 +191,7 @@ export function registerDirectoriesCommand(program: Command) { "-d, --direction ", "sort direction: asc (ascending) or desc (descending)", ) + .addOption(repositoryTokenOption()) .addHelpText( "after", ` @@ -210,7 +211,7 @@ Examples: options: DirOptions, ) { try { - checkApiToken(); + resolveAccountAuth(this, "Codacy does not accept repository tokens on the directory listing endpoints"); const format = getOutputFormat(this); const ctx = resolveDirContext( providerArg, diff --git a/src/commands/finding.ts b/src/commands/finding.ts index 8c3ca2a..1e67c99 100644 --- a/src/commands/finding.ts +++ b/src/commands/finding.ts @@ -1,7 +1,7 @@ import { Command } from "commander"; import ora from "ora"; import ansis from "ansis"; -import { checkApiToken } from "../utils/auth"; +import { repositoryTokenOption, resolveAccountAuth } from "../utils/auth"; import { handleError } from "../utils/error"; import { getOutputFormat, pickDeep, printJson } from "../utils/output"; import { @@ -173,6 +173,7 @@ export function registerFindingCommand(program: Command) { ) .option("-m, --ignore-comment ", "optional comment for the ignore action", "") .option("-U, --unignore", "unignore this finding") + .addOption(repositoryTokenOption()) .addHelpText( "after", ` @@ -190,7 +191,7 @@ Examples: findingId: string, ) { try { - checkApiToken(); + resolveAccountAuth(this, "Codacy does not accept repository tokens on the security findings endpoints"); const format = getOutputFormat(this); const shouldIgnore: boolean = !!this.opts().ignore; const shouldUnignore: boolean = !!this.opts().unignore; diff --git a/src/commands/findings.ts b/src/commands/findings.ts index 613cd95..a8504b4 100644 --- a/src/commands/findings.ts +++ b/src/commands/findings.ts @@ -1,7 +1,7 @@ import { Command } from "commander"; import ora from "ora"; import ansis from "ansis"; -import { checkApiToken } from "../utils/auth"; +import { repositoryTokenOption, resolveAccountAuth } from "../utils/auth"; import { handleError } from "../utils/error"; import { detectRepoContext } from "../utils/git-remote"; import { @@ -214,6 +214,7 @@ export function registerFindingsCommand(program: Command) { ) .option("-n, --limit ", "maximum number of findings to return (default: 100, max: 1000)", "100") .option("-d, --dast-targets ", "comma-separated DAST target URLs") + .addOption(repositoryTokenOption()) .addHelpText( "after", ` @@ -233,7 +234,7 @@ Examples: repositoryArg?: string, ) { try { - checkApiToken(); + resolveAccountAuth(this, "Codacy does not accept repository tokens on the security findings endpoints"); const argCount = [providerArg, organizationArg, repositoryArg].filter( (v) => v !== undefined, diff --git a/src/commands/info.ts b/src/commands/info.ts index 33a9324..9a84fc8 100644 --- a/src/commands/info.ts +++ b/src/commands/info.ts @@ -1,7 +1,7 @@ import { Command } from "commander"; import ora from "ora"; import ansis from "ansis"; -import { checkApiToken } from "../utils/auth"; +import { repositoryTokenOption, resolveAccountAuth } from "../utils/auth"; import { handleError } from "../utils/error"; import { createTable, @@ -18,6 +18,7 @@ export function registerInfoCommand(program: Command) { .command("info") .alias("i") .description("Show authenticated user information and organizations") + .addOption(repositoryTokenOption()) .addHelpText( "after", ` @@ -27,7 +28,7 @@ Examples: ) .action(async function (this: Command) { try { - checkApiToken(); + resolveAccountAuth(this, "it reads account-level data (your profile and organizations)"); const format = getOutputFormat(this); const spinner = ora("Fetching user info...").start(); diff --git a/src/commands/issue.ts b/src/commands/issue.ts index a303d53..28a6a13 100644 --- a/src/commands/issue.ts +++ b/src/commands/issue.ts @@ -1,7 +1,7 @@ import { Command } from "commander"; import ora from "ora"; import ansis from "ansis"; -import { checkApiToken } from "../utils/auth"; +import { repositoryTokenOption, resolveAccountAuth } from "../utils/auth"; import { handleError } from "../utils/error"; import { resolveRepoArgs } from "../utils/resolve-repo-args"; import { getOutputFormat, pickDeep, printJson } from "../utils/output"; @@ -29,6 +29,7 @@ export function registerIssueCommand(program: Command) { ) .option("-m, --ignore-comment ", "optional comment for the ignore action", "") .option("-U, --unignore", "unignore this issue") + .addOption(repositoryTokenOption()) .addHelpText( "after", ` @@ -48,7 +49,7 @@ Examples: issueIdArg?: string, ) { try { - checkApiToken(); + resolveAccountAuth(this, "Codacy does not accept repository tokens on the single-issue endpoint; run 'codacy issues' to list issues with a repository token"); const { provider, organization, repository, trailingArgs } = resolveRepoArgs( [providerArg, organizationArg, repositoryArg, issueIdArg], diff --git a/src/commands/issues.test.ts b/src/commands/issues.test.ts index 1b044a0..a0a425c 100644 --- a/src/commands/issues.test.ts +++ b/src/commands/issues.test.ts @@ -1982,4 +1982,68 @@ describe("issues command", () => { ).not.toHaveBeenCalled(); }); }); + + describe("with a repository token", () => { + function expectRefusal(argv: string[]) { + vi.spyOn(console, "error").mockImplementation(() => {}); + vi.spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit called"); + }); + + const program = createProgram(); + return expect( + program.parseAsync([ + "node", "test", "issues", "gh", "test-org", "test-repo", + ...argv, "--repository-token", "rt", + ]), + ).rejects.toThrow("process.exit called"); + } + + it("lists issues — searchRepositoryIssues accepts a repository token", async () => { + vi.mocked(AnalysisService.searchRepositoryIssues).mockResolvedValue({ + data: mockIssues as any, + pagination: undefined, + }); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "issues", "gh", "test-org", "test-repo", + "--repository-token", "rt", + ]); + + expect(AnalysisService.searchRepositoryIssues).toHaveBeenCalled(); + }); + + it("shows the overview — issuesOverview accepts a repository token", async () => { + vi.mocked(AnalysisService.issuesOverview).mockResolvedValue( + mockOverview as any, + ); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "issues", "gh", "test-org", "test-repo", + "--overview", "--repository-token", "rt", + ]); + + expect(AnalysisService.issuesOverview).toHaveBeenCalled(); + }); + + it("refuses --ignored without querying the ignored-issues endpoint", async () => { + await expectRefusal(["--ignored"]); + + expect( + AnalysisService.searchRepositoryIgnoredIssues, + ).not.toHaveBeenCalled(); + }); + + it("refuses --ignore before fetching anything to ignore", async () => { + await expectRefusal(["--ignore", "-y"]); + + // The refusal must land before the fetch-all sweep, not just before the + // bulk call — otherwise a repository token pages the whole issue list + // only to be rejected at the end. + expect(AnalysisService.searchRepositoryIssues).not.toHaveBeenCalled(); + expect(AnalysisService.bulkIgnoreIssues).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/commands/issues.ts b/src/commands/issues.ts index 7d6e90b..1491f65 100644 --- a/src/commands/issues.ts +++ b/src/commands/issues.ts @@ -1,7 +1,11 @@ import { Command } from "commander"; import ora from "ora"; import ansis from "ansis"; -import { checkApiToken } from "../utils/auth"; +import { + repositoryTokenOption, + requireAccountToken, + resolveAuth, +} from "../utils/auth"; import { handleError } from "../utils/error"; import { resolveRepoArgs } from "../utils/resolve-repo-args"; import { confirmAction } from "../utils/prompt"; @@ -658,6 +662,7 @@ export function registerIssuesCommand(program: Command) { "-y, --skip-confirmation", "skip the confirmation prompt when using --ignore (for CI/scripts)", ) + .addOption(repositoryTokenOption()) .addHelpText( "after", ` @@ -685,14 +690,36 @@ Examples: repositoryArg?: string, ) { try { - checkApiToken(); + const auth = resolveAuth(this); + const opts = this.opts(); + + // Searching and the overview both accept a repository token; the ignore + // endpoints don't. Refuse before `buildFilterBody` (which can hit the + // network to resolve tool names) and before the fetch-all + confirmation + // prompt inside `executeBulkIgnore`. This also lands ahead of the + // flag-combination checks below, which is the right precedence: "your + // token can't do this at all" beats "these two flags conflict". + if (opts.ignored) { + requireAccountToken( + auth, + "codacy issues --ignored", + "listing ignored issues is not available to repository tokens", + ); + } + if (opts.ignore) { + requireAccountToken( + auth, + "codacy issues --ignore", + "ignoring issues is not available to repository tokens", + ); + } + const { provider, organization, repository } = resolveRepoArgs( [providerArg, organizationArg, repositoryArg], 0, "issues", [], ); - const opts = this.opts(); const format = getOutputFormat(this); const isOverview = !!opts.overview; const listIgnored = !!opts.ignored; diff --git a/src/commands/login.test.ts b/src/commands/login.test.ts index 9f23c6c..5f75c22 100644 --- a/src/commands/login.test.ts +++ b/src/commands/login.test.ts @@ -112,4 +112,60 @@ describe("login command", () => { expect(saveCredentials).not.toHaveBeenCalled(); mockExit.mockRestore(); }); + + describe("--repository-token", () => { + function warnings(): string { + return (console.error as ReturnType).mock.calls + .flat() + .join("\n"); + } + + it("warns that the flag is ignored, but still stores the account token", async () => { + vi.mocked(AccountService.getUser).mockResolvedValue({ data: mockUser } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "login", "--token", "account-token", + "--repository-token", "rt", + ]); + + expect(warnings()).toContain("--repository-token is ignored by"); + // The flag is inert here, not fatal — the login itself must still work. + expect(saveCredentials).toHaveBeenCalledWith("account-token"); + }); + + it("stays silent when only CODACY_PROJECT_TOKEN is set", async () => { + // That variable is exported job-wide in CI, so warning on it would fire on + // every unrelated login and train users to ignore warnings. + process.env.CODACY_PROJECT_TOKEN = "env-project-token"; + vi.mocked(AccountService.getUser).mockResolvedValue({ data: mockUser } as any); + + const program = createProgram(); + await program.parseAsync(["node", "test", "login", "--token", "account-token"]); + + expect(warnings()).not.toContain("--repository-token"); + delete process.env.CODACY_PROJECT_TOKEN; + }); + + it("explains that a repository token cannot be used to log in", async () => { + vi.mocked(AccountService.getUser).mockRejectedValue( + Object.assign(new Error("Unauthorized"), { status: 401 }), + ); + const mockExit = vi.spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit called"); + }); + + const program = createProgram(); + await expect( + program.parseAsync(["node", "test", "login", "--token", "a-repo-token"]), + ).rejects.toThrow("process.exit called"); + + // A repository token is rejected by /user by design, so the 401 message has + // to name that case instead of only implying the token is invalid. + expect(warnings()).toContain("repository (project) token"); + expect(warnings()).toContain("--repository-token"); + expect(saveCredentials).not.toHaveBeenCalled(); + mockExit.mockRestore(); + }); + }); }); diff --git a/src/commands/login.ts b/src/commands/login.ts index ad8584b..a6a9c45 100644 --- a/src/commands/login.ts +++ b/src/commands/login.ts @@ -3,7 +3,11 @@ import ansis from "ansis"; import ora from "ora"; import { AccountService } from "../api/client/services/AccountService"; import { handleError } from "../utils/error"; -import { updateApiHeaders } from "../utils/auth"; +import { + applyAccountToken, + repositoryTokenOption, + warnUnusedRepositoryToken, +} from "../utils/auth"; import { saveCredentials, getCredentialsPath, @@ -14,19 +18,32 @@ export function registerLoginCommand(program: Command) { program .command("login") .description("Authenticate with Codacy by storing your API token") - .option("-t, --token ", "API token (skips interactive prompt)") + .option("-t, --token ", "account API token (skips interactive prompt)") + .addOption(repositoryTokenOption()) .addHelpText( "after", ` Examples: $ codacy login - $ codacy login --token + $ codacy login --token Get your token at: https://app.codacy.com/account/access-management - My Account > Access Management > API Tokens`, + My Account > Access Management > API Tokens + +login stores an account API token. Repository (project) tokens are not stored — +pass them per command with --repository-token, or set CODACY_PROJECT_TOKEN.`, ) - .action(async (options) => { + .action(async function (this: Command, options) { try { + // login stores account tokens only: it validates against /user, which a + // repository token can never reach, and the credentials store holds a + // single bare token with no record of its kind. + warnUnusedRepositoryToken( + this, + "`codacy login`, which stores an account API token. " + + "Pass a repository token per command with --repository-token, or set CODACY_PROJECT_TOKEN.", + ); + let token: string; if (options.token) { @@ -56,7 +73,7 @@ Get your token at: https://app.codacy.com/account/access-management const spinner = ora("Validating token...").start(); - updateApiHeaders(token); + applyAccountToken(token); let userName: string; let userEmail: string; @@ -67,8 +84,12 @@ Get your token at: https://app.codacy.com/account/access-management } catch (apiErr: any) { spinner.fail("Authentication failed."); if (apiErr?.status === 401) { + // A repository token lands here too — it is rejected by /user by + // design — so name that case rather than only implying a bad token. throw new Error( - "Invalid API token. Check that it is correct and not expired.", + "Invalid account API token. Check that it is correct and not expired. " + + "If this is a repository (project) token, it can't be used to log in — " + + "pass it per command with --repository-token, or set CODACY_PROJECT_TOKEN.", ); } if (typeof apiErr?.status === "number") { diff --git a/src/commands/logout.test.ts b/src/commands/logout.test.ts index b3c9c15..7b065ba 100644 --- a/src/commands/logout.test.ts +++ b/src/commands/logout.test.ts @@ -43,4 +43,35 @@ describe("logout command", () => { expect(deleteCredentials).toHaveBeenCalledOnce(); expect(console.log).toHaveBeenCalledWith("No stored credentials found."); }); + + describe("--repository-token", () => { + it("warns that the flag is ignored, but still logs out", async () => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + vi.mocked(deleteCredentials).mockReturnValue(true); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "logout", "--repository-token", "rt", + ]); + + expect(errorSpy.mock.calls.flat().join("\n")).toContain( + "--repository-token is ignored by", + ); + expect(deleteCredentials).toHaveBeenCalledOnce(); + }); + + it("stays silent when only CODACY_PROJECT_TOKEN is set", async () => { + process.env.CODACY_PROJECT_TOKEN = "env-project-token"; + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + vi.mocked(deleteCredentials).mockReturnValue(true); + + const program = createProgram(); + await program.parseAsync(["node", "test", "logout"]); + + expect(errorSpy.mock.calls.flat().join("\n")).not.toContain( + "--repository-token", + ); + delete process.env.CODACY_PROJECT_TOKEN; + }); + }); }); diff --git a/src/commands/logout.ts b/src/commands/logout.ts index 1260f0c..b9e58a1 100644 --- a/src/commands/logout.ts +++ b/src/commands/logout.ts @@ -5,19 +5,28 @@ import { getCredentialsPath, } from "../utils/credentials"; import { handleError } from "../utils/error"; +import { repositoryTokenOption, warnUnusedRepositoryToken } from "../utils/auth"; export function registerLogoutCommand(program: Command) { program .command("logout") .description("Remove stored Codacy API token") + .addOption(repositoryTokenOption()) .addHelpText( "after", ` Examples: $ codacy logout`, ) - .action(() => { + .action(function (this: Command) { try { + // Nothing to log out of for a repository token: login never stores one, + // so there is no stored copy for this command to remove. + warnUnusedRepositoryToken( + this, + "`codacy logout`, which only removes the locally stored account API token.", + ); + const deleted = deleteCredentials(); if (deleted) { console.log( diff --git a/src/commands/ls.ts b/src/commands/ls.ts index bfd8834..051c93d 100644 --- a/src/commands/ls.ts +++ b/src/commands/ls.ts @@ -2,7 +2,7 @@ import { Command } from "commander"; import ora from "ora"; import ansis from "ansis"; import pluralize from "pluralize"; -import { checkApiToken } from "../utils/auth"; +import { repositoryTokenOption, resolveAccountAuth } from "../utils/auth"; import { handleError } from "../utils/error"; import { createTable, getOutputFormat, printJson } from "../utils/output"; import { sanitizeText } from "../utils/sanitize"; @@ -222,6 +222,7 @@ export function registerLsCommand(program: Command) { "-d, --direction ", "sort direction: asc (ascending) or desc (descending)", ) + .addOption(repositoryTokenOption()) .addHelpText( "after", ` @@ -242,7 +243,7 @@ Examples: options: LsOptions, ) { try { - checkApiToken(); + resolveAccountAuth(this, "Codacy does not accept repository tokens on the file and directory listing endpoints"); const format = getOutputFormat(this); const ctx = resolveLsContext( providerArg, diff --git a/src/commands/pattern.test.ts b/src/commands/pattern.test.ts index 779a3a5..a437b44 100644 --- a/src/commands/pattern.test.ts +++ b/src/commands/pattern.test.ts @@ -557,4 +557,24 @@ describe("pattern command", () => { ); }); }); + + // configureTool and the two reads this command makes are all on the + // repository-token whitelist, so no guard is needed and no account token either. + it("modifies a pattern with only a repository token", async () => { + delete process.env.CODACY_API_TOKEN; + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "pattern", "gh", "test-org", "test-repo", "eslint", + "no-unused-vars", "--enable", "--repository-token", "rt", + ]); + + expect(AnalysisService.configureTool).toHaveBeenCalledWith( + "gh", + "test-org", + "test-repo", + "uuid-eslint", + { patterns: [{ id: "no-unused-vars", enabled: true }] }, + ); + }); }); diff --git a/src/commands/pattern.ts b/src/commands/pattern.ts index 870c680..53990dc 100644 --- a/src/commands/pattern.ts +++ b/src/commands/pattern.ts @@ -1,7 +1,7 @@ import { Command } from "commander"; import ora from "ora"; import ansis from "ansis"; -import { checkApiToken } from "../utils/auth"; +import { repositoryTokenOption, resolveAuth } from "../utils/auth"; import { handleError } from "../utils/error"; import { resolveRepoArgs } from "../utils/resolve-repo-args"; import { getOutputFormat, pickDeep, printJson } from "../utils/output"; @@ -40,6 +40,7 @@ export function registerPatternCommand(program: Command) { (val: string, acc: string[]) => [...acc, val], [] as string[], ) + .addOption(repositoryTokenOption()) .addHelpText( "after", ` @@ -60,7 +61,7 @@ Examples: patternIdArg?: string, ) { try { - checkApiToken(); + resolveAuth(this); const { provider, organization, repository, trailingArgs } = resolveRepoArgs( [providerArg, organizationArg, repositoryArg, toolNameArg, patternIdArg], diff --git a/src/commands/patterns.test.ts b/src/commands/patterns.test.ts index 0064099..7bd3a9f 100644 --- a/src/commands/patterns.test.ts +++ b/src/commands/patterns.test.ts @@ -844,4 +844,20 @@ describe("patterns command", () => { ); }); }); + + // Every endpoint this command uses is on the repository-token whitelist + // (listRepositoryTools, listRepositoryToolPatterns, updateRepositoryToolPatterns, + // toolPatternsOverview), so no guard is needed and no account token either. + it("lists patterns with only a repository token", async () => { + delete process.env.CODACY_API_TOKEN; + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "patterns", "gh", "test-org", "test-repo", "eslint", + "--repository-token", "rt", + ]); + + expect(AnalysisService.listRepositoryTools).toHaveBeenCalled(); + expect(AnalysisService.listRepositoryToolPatterns).toHaveBeenCalled(); + }); }); diff --git a/src/commands/patterns.ts b/src/commands/patterns.ts index 61cb02b..a12cdd2 100644 --- a/src/commands/patterns.ts +++ b/src/commands/patterns.ts @@ -1,7 +1,7 @@ import { Command } from "commander"; import ora from "ora"; import ansis from "ansis"; -import { checkApiToken } from "../utils/auth"; +import { repositoryTokenOption, resolveAuth } from "../utils/auth"; import { handleError } from "../utils/error"; import { resolveRepoArgs } from "../utils/resolve-repo-args"; import { @@ -179,6 +179,7 @@ export function registerPatternsCommand(program: Command) { .option("-r, --recommended", "show only recommended patterns") .option("-E, --enable-all", "bulk enable matching patterns") .option("-X, --disable-all", "bulk disable matching patterns") + .addOption(repositoryTokenOption()) .addHelpText( "after", ` @@ -199,7 +200,7 @@ Examples: toolNameArg?: string, ) { try { - checkApiToken(); + resolveAuth(this); const { provider, organization, repository, trailingArgs } = resolveRepoArgs( [providerArg, organizationArg, repositoryArg, toolNameArg], diff --git a/src/commands/pull-request.ts b/src/commands/pull-request.ts index 5a2bf82..170ad3b 100644 --- a/src/commands/pull-request.ts +++ b/src/commands/pull-request.ts @@ -1,7 +1,7 @@ import { Command } from "commander"; import ora from "ora"; import ansis from "ansis"; -import { checkApiToken } from "../utils/auth"; +import { repositoryTokenOption, resolveAccountAuth } from "../utils/auth"; import { handleError } from "../utils/error"; import { resolveRepoArgs } from "../utils/resolve-repo-args"; import { @@ -711,6 +711,7 @@ export function registerPullRequestCommand(program: Command) { "-w, --reanalyze-and-wait", "request reanalysis of this pull request, wait for it to finish, then show what changed", ) + .addOption(repositoryTokenOption()) .addHelpText( "after", ` @@ -735,7 +736,7 @@ Examples: prNumberArg?: string, ) { try { - checkApiToken(); + resolveAccountAuth(this, "Codacy does not accept repository tokens on the pull request endpoints"); const { provider, organization, repository, trailingArgs } = resolveRepoArgs( [providerArg, organizationArg, repositoryArg, prNumberArg], diff --git a/src/commands/pull-requests.ts b/src/commands/pull-requests.ts index 0e653d2..7b0d521 100644 --- a/src/commands/pull-requests.ts +++ b/src/commands/pull-requests.ts @@ -1,7 +1,7 @@ import { Command } from "commander"; import ora from "ora"; import ansis from "ansis"; -import { checkApiToken } from "../utils/auth"; +import { repositoryTokenOption, resolveAccountAuth } from "../utils/auth"; import { handleError } from "../utils/error"; import { resolveRepoArgs } from "../utils/resolve-repo-args"; import { @@ -167,6 +167,7 @@ export function registerPullRequestsCommand(program: Command) { "maximum number of pull requests to return (default: 100, max: 1000)", "100", ) + .addOption(repositoryTokenOption()) .addHelpText( "after", ` @@ -185,7 +186,7 @@ Examples: repositoryArg?: string, ) { try { - checkApiToken(); + resolveAccountAuth(this, "Codacy does not accept repository tokens on the pull request endpoints"); const { provider, organization, repository } = resolveRepoArgs( [providerArg, organizationArg, repositoryArg], diff --git a/src/commands/repositories.ts b/src/commands/repositories.ts index 788e471..a2a23fc 100644 --- a/src/commands/repositories.ts +++ b/src/commands/repositories.ts @@ -1,7 +1,7 @@ import { Command } from "commander"; import ora from "ora"; import ansis from "ansis"; -import { checkApiToken } from "../utils/auth"; +import { repositoryTokenOption, resolveAccountAuth } from "../utils/auth"; import { handleError } from "../utils/error"; import { createTable, @@ -43,6 +43,7 @@ export function registerRepositoriesCommand(program: Command) { .argument("", "git provider (gh, gl, or bb)") .argument("", "organization name") .option("-s, --search ", "filter repositories by name") + .addOption(repositoryTokenOption()) .addHelpText( "after", ` @@ -58,7 +59,7 @@ Examples: options: { search?: string }, ) { try { - checkApiToken(); + resolveAccountAuth(this, "it reads organization-level data (every repository in the organization)"); const format = getOutputFormat(this); const spinner = ora("Fetching repositories...").start(); diff --git a/src/commands/repository-token-refusals.test.ts b/src/commands/repository-token-refusals.test.ts new file mode 100644 index 0000000..97e9c37 --- /dev/null +++ b/src/commands/repository-token-refusals.test.ts @@ -0,0 +1,183 @@ +/** + * Cross-cutting coverage for the commands Codacy does not accept a repository + * (project) token on. Kept in one file rather than spread across nine command + * suites: the guarantee under test is a single rule applied uniformly ("refuse + * before any request"), and asserting it in one place is what makes a newly + * added account-only command's missing guard obvious. + * + * Per-command behaviour that *does* work under a repository token lives in each + * command's own suite. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { Command } from "commander"; +import { registerInfoCommand } from "./info"; +import { registerRepositoriesCommand } from "./repositories"; +import { registerLsCommand } from "./ls"; +import { registerDirectoriesCommand } from "./directories"; +import { registerPullRequestCommand } from "./pull-request"; +import { registerPullRequestsCommand } from "./pull-requests"; +import { registerIssueCommand } from "./issue"; +import { registerFindingsCommand } from "./findings"; +import { registerFindingCommand } from "./finding"; +import { AccountService } from "../api/client/services/AccountService"; +import { AnalysisService } from "../api/client/services/AnalysisService"; +import { RepositoryService } from "../api/client/services/RepositoryService"; +import { SecurityService } from "../api/client/services/SecurityService"; + +vi.mock("../api/client/services/AccountService"); +vi.mock("../api/client/services/AnalysisService"); +vi.mock("../api/client/services/RepositoryService"); +vi.mock("../api/client/services/SecurityService"); +vi.mock("../api/client/services/CoverageService"); +vi.mock("../api/client/services/FileService"); +vi.mock("../api/client/services/ToolsService"); +vi.mock("../utils/credentials", () => ({ loadCredentials: vi.fn(() => null) })); +vi.mock("../utils/git-remote", () => ({ + detectRepoContext: vi.fn(() => ({ + provider: "gh", + organization: "auto-org", + repository: "auto-repo", + })), +})); + +/** + * Every account-only command, with the argv that invokes it and the service call + * that must never happen. `register` is per-case so each test builds a program + * containing only the command under test. + */ +const ACCOUNT_ONLY_COMMANDS = [ + { + name: "info", + register: registerInfoCommand, + argv: ["info"], + neverCalled: () => AccountService.getUser, + }, + { + name: "repositories", + register: registerRepositoriesCommand, + argv: ["repositories", "gh", "test-org"], + neverCalled: () => AnalysisService.listOrganizationRepositoriesWithAnalysis, + }, + { + name: "ls", + register: registerLsCommand, + argv: ["ls"], + neverCalled: () => RepositoryService.listDirectories, + }, + { + name: "directories", + register: registerDirectoriesCommand, + argv: ["directories"], + neverCalled: () => RepositoryService.listDirectories, + }, + { + name: "pull-request", + register: registerPullRequestCommand, + argv: ["pull-request", "1"], + neverCalled: () => AnalysisService.getRepositoryPullRequest, + }, + { + name: "pull-requests", + register: registerPullRequestsCommand, + argv: ["pull-requests"], + neverCalled: () => AnalysisService.listRepositoryPullRequests, + }, + { + name: "issue", + register: registerIssueCommand, + argv: ["issue", "12345"], + neverCalled: () => AnalysisService.getIssue, + }, + { + name: "findings", + register: registerFindingsCommand, + argv: ["findings"], + neverCalled: () => SecurityService.searchSecurityItems, + }, + { + name: "finding", + register: registerFindingCommand, + argv: ["finding", "gh", "test-org", "00000000-0000-0000-0000-000000000000"], + neverCalled: () => SecurityService.getSecurityItem, + }, +] as const; + +function createProgram(register: (program: Command) => void): Command { + const program = new Command(); + program.option("-o, --output ", "output format", "table"); + register(program); + return program; +} + +describe("account-only commands refuse a repository token", () => { + let errorSpy: ReturnType; + + beforeEach(() => { + vi.clearAllMocks(); + delete process.env.CODACY_API_TOKEN; + delete process.env.CODACY_PROJECT_TOKEN; + vi.spyOn(console, "log").mockImplementation(() => {}); + errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + vi.spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit called"); + }); + }); + + for (const command of ACCOUNT_ONLY_COMMANDS) { + it(`refuses \`codacy ${command.name}\` with --repository-token, before any request`, async () => { + const program = createProgram(command.register); + + await expect( + program.parseAsync([ + "node", + "test", + ...command.argv, + "--repository-token", + "rt", + ]), + ).rejects.toThrow("process.exit called"); + + const output = errorSpy.mock.calls.flat().join("\n"); + expect(output).toContain("requires an account API token"); + expect(output).toContain("run 'codacy login'"); + // The whole point of failing fast: no doomed request is ever sent. + expect(command.neverCalled()).not.toHaveBeenCalled(); + }); + } + + it("refuses a repository token supplied via CODACY_PROJECT_TOKEN", async () => { + process.env.CODACY_PROJECT_TOKEN = "env-project-token"; + const program = createProgram(registerFindingsCommand); + + await expect( + program.parseAsync(["node", "test", "findings"]), + ).rejects.toThrow("process.exit called"); + + const output = errorSpy.mock.calls.flat().join("\n"); + expect(output).toContain("requires an account API token"); + // Names the env var, so the surprise is debuggable without guessing. + expect(output).toContain("from CODACY_PROJECT_TOKEN"); + expect(SecurityService.searchSecurityItems).not.toHaveBeenCalled(); + }); + + it("still works with an account token from CODACY_API_TOKEN", async () => { + process.env.CODACY_API_TOKEN = "account-token"; + vi.mocked(AccountService.getUser).mockResolvedValue({ + data: { + name: "Test User", + mainEmail: "test@example.com", + otherEmails: [], + isAdmin: false, + isActive: true, + }, + } as any); + vi.mocked(AccountService.listUserOrganizations).mockResolvedValue({ + data: [], + } as any); + + const program = createProgram(registerInfoCommand); + await program.parseAsync(["node", "test", "info"]); + + expect(AccountService.getUser).toHaveBeenCalledOnce(); + }); +}); diff --git a/src/commands/repository.test.ts b/src/commands/repository.test.ts index 9204148..2916236 100644 --- a/src/commands/repository.test.ts +++ b/src/commands/repository.test.ts @@ -944,4 +944,184 @@ describe("repository command", () => { expect(parsed.totals).toEqual({ before: 15, after: 17, net: 2 }); }); }); + + describe("with a repository token", () => { + /** Mocks the three dashboard calls a repository token *can* make. */ + function mockWhitelistedDashboardCalls() { + vi.mocked(AnalysisService.getRepositoryWithAnalysis).mockResolvedValue({ + data: mockRepoData as any, + }); + vi.mocked(AnalysisService.issuesOverview).mockResolvedValue({ + data: { counts: mockIssuesCounts }, + }); + } + + function getAllOutput(): string { + return (console.log as ReturnType).mock.calls + .map((c) => c[0]) + .join("\n"); + } + + it("skips the pull request and coverage calls entirely", async () => { + mockWhitelistedDashboardCalls(); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "repository", "gh", "test-org", "test-repo", + "--repository-token", "rt", + ]); + + // Both are outside a repository token's scope: don't even try. + expect(AnalysisService.listRepositoryPullRequests).not.toHaveBeenCalled(); + expect(RepositoryService.listCoverageReports).not.toHaveBeenCalled(); + // The whitelisted calls still run, so the dashboard is still worth showing. + expect(AnalysisService.getRepositoryWithAnalysis).toHaveBeenCalled(); + expect(AnalysisService.issuesOverview).toHaveBeenCalled(); + }); + + it("keeps the pull request section header and explains the omission", async () => { + mockWhitelistedDashboardCalls(); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "repository", "gh", "test-org", "test-repo", + "--repository-token", "rt", + ]); + + const output = getAllOutput(); + expect(output).toContain("Open Pull Requests"); + expect(output).toContain("Not shown with a repository token"); + // "No open pull requests" would be a different, and false, claim. + expect(output).not.toContain("No open pull requests"); + }); + + it("emits pullRequests as an empty array plus an unavailable marker in JSON", async () => { + mockWhitelistedDashboardCalls(); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "repository", "gh", "test-org", "test-repo", + "--repository-token", "rt", "--output", "json", + ]); + + const calls = (console.log as ReturnType).mock.calls; + const parsed = JSON.parse(calls[calls.length - 1][0]); + // Present and iterable, so `jq '.pullRequests[]'` and `| length` still work. + expect(parsed.pullRequests).toEqual([]); + expect(parsed.unavailable).toEqual(["pullRequests"]); + // The fields the auto-configuration skill reads are unaffected. + expect(parsed.repository.fileCount).toBe(83); + expect(parsed.repository.repository.standards).toBeDefined(); + }); + + it("still supports --reanalyze", async () => { + vi.mocked(RepositoryService.reanalyzeCommitById).mockResolvedValue( + undefined as any, + ); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "repository", "gh", "test-org", "test-repo", + "--reanalyze", "--repository-token", "rt", + ]); + + expect(RepositoryService.reanalyzeCommitById).toHaveBeenCalled(); + }); + + it.each([ + ["--add", () => RepositoryService.addRepository], + ["--remove", () => RepositoryService.deleteRepository], + ["--follow", () => RepositoryService.followAddedRepository], + ["--unfollow", () => RepositoryService.unfollowRepository], + ])("refuses %s without calling the API", async (flag, service) => { + vi.spyOn(console, "error").mockImplementation(() => {}); + const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit called"); + }); + + const program = createProgram(); + await expect( + program.parseAsync([ + "node", "test", "repository", "gh", "test-org", "test-repo", + flag, "--repository-token", "rt", + ]), + ).rejects.toThrow("process.exit called"); + + expect(service()).not.toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalled(); + }); + + it.each(["--link-standard", "--unlink-standard"])( + "refuses %s without calling the coding standards API", + async (flag) => { + vi.spyOn(console, "error").mockImplementation(() => {}); + vi.spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit called"); + }); + + const program = createProgram(); + await expect( + program.parseAsync([ + "node", "test", "repository", "gh", "test-org", "test-repo", + flag, "12345", "--repository-token", "rt", + ]), + ).rejects.toThrow("process.exit called"); + + expect( + CodingStandardsService.applyCodingStandardToRepositories, + ).not.toHaveBeenCalled(); + }, + ); + }); + + describe("with an account token", () => { + it("renders the dashboard even when the pull request call fails", async () => { + vi.mocked(AnalysisService.getRepositoryWithAnalysis).mockResolvedValue({ + data: mockRepoData as any, + }); + vi.mocked(AnalysisService.issuesOverview).mockResolvedValue({ + data: { counts: mockIssuesCounts }, + }); + vi.mocked(AnalysisService.listRepositoryPullRequests).mockRejectedValue( + Object.assign(new Error("Forbidden"), { status: 403 }), + ); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "repository", "gh", "test-org", "test-repo", + ]); + + const output = (console.log as ReturnType).mock.calls + .map((c) => c[0]) + .join("\n"); + // The rest of the dashboard survives; the PR section says so plainly, and + // does not blame a repository token that isn't in use. + expect(output).toContain("test-repo"); + expect(output).toContain("Could not load pull requests."); + expect(output).not.toContain("Not shown with a repository token"); + }); + + it("omits the unavailable marker from JSON", async () => { + vi.mocked(AnalysisService.getRepositoryWithAnalysis).mockResolvedValue({ + data: mockRepoData as any, + }); + vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ + data: mockPullRequests as any, + }); + vi.mocked(AnalysisService.issuesOverview).mockResolvedValue({ + data: { counts: mockIssuesCounts }, + }); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "repository", "gh", "test-org", "test-repo", + "--output", "json", + ]); + + const calls = (console.log as ReturnType).mock.calls; + const parsed = JSON.parse(calls[calls.length - 1][0]); + expect(parsed).not.toHaveProperty("unavailable"); + expect(Array.isArray(parsed.pullRequests)).toBe(true); + }); + }); }); diff --git a/src/commands/repository.ts b/src/commands/repository.ts index 9a6ab30..3fc5773 100644 --- a/src/commands/repository.ts +++ b/src/commands/repository.ts @@ -1,7 +1,13 @@ import { Command } from "commander"; import ora from "ora"; import ansis from "ansis"; -import { checkApiToken } from "../utils/auth"; +import { + fetchIfAccountToken, + repositoryTokenOption, + repositoryTokenSkipNote, + requireAccountToken, + resolveAuth, +} from "../utils/auth"; import { handleError } from "../utils/error"; import { resolveRepoArgs } from "../utils/resolve-repo-args"; import { @@ -148,6 +154,21 @@ function printMetrics(data: RepositoryWithAnalysis): void { console.log(table.toString()); } +/** + * Fallbacks for the two dashboard calls a repository token can't make. Used both + * when we deliberately skip them and when they fail for an account token that + * lacks access — the rest of the dashboard is worth rendering either way. The + * explicit `pagination: undefined` keeps `prsResponse.pagination` type-checking + * across the union with the real response. + */ +function noPullRequests(): { data: PullRequestWithAnalysis[]; pagination: undefined } { + return { data: [], pagination: undefined }; +} + +function noCoverageReports(): { data: { hasCoverageOverview: boolean } } { + return { data: { hasCoverageOverview: false } }; +} + function printPullRequests(pullRequests: PullRequestWithAnalysis[]): void { const open = pullRequests.filter( (pr) => @@ -239,6 +260,7 @@ export function registerRepositoryCommand(program: Command) { ) .option("-L, --link-standard ", "link a coding standard to this repository (by standard ID)") .option("-K, --unlink-standard ", "unlink a coding standard from this repository (by standard ID)") + .addOption(repositoryTokenOption()) .addHelpText( "after", ` @@ -262,14 +284,34 @@ Examples: repositoryArg?: string, ) { try { - checkApiToken(); + const auth = resolveAuth(this); + const opts = this.opts(); + + // A repository (project) token is scoped to one repository's analysis + // data; every action below reaches an organization- or account-level + // resource, so Codacy rejects it. Refuse before `resolveRepoArgs`, which + // shells out to git and prints an auto-detection line — misleading ahead + // of a refusal. + const ACCOUNT_ONLY_ACTIONS = [ + { opt: "add", flag: "--add", why: "adding a repository to Codacy is an account-level operation" }, + { opt: "remove", flag: "--remove", why: "removing a repository from Codacy is an account-level operation" }, + { opt: "follow", flag: "--follow", why: "following a repository is tied to your Codacy account" }, + { opt: "unfollow", flag: "--unfollow", why: "following a repository is tied to your Codacy account" }, + { opt: "linkStandard", flag: "--link-standard", why: "coding standards are managed at the organization level" }, + { opt: "unlinkStandard", flag: "--unlink-standard", why: "coding standards are managed at the organization level" }, + ] as const; + for (const action of ACCOUNT_ONLY_ACTIONS) { + if (opts[action.opt]) { + requireAccountToken(auth, `codacy repository ${action.flag}`, action.why); + } + } + const { provider, organization, repository } = resolveRepoArgs( [providerArg, organizationArg, repositoryArg], 0, "repository", [], ); - const opts = this.opts(); // ── Action: add ────────────────────────────────────────────────── if (opts.add) { @@ -490,16 +532,28 @@ Examples: const format = getOutputFormat(this); const spinner = ora("Fetching repository details...").start(); + // Pull requests and coverage reports are outside a repository token's + // scope — Codacy rejects them as if no token had been sent. Skip the + // requests rather than firing two we know will fail, and keep .catch() + // on the pull request call so an account token that lacks access + // degrades the same way instead of losing the whole dashboard (its three + // sibling calls were already guarded). + let prsUnavailable = auth.kind !== "account-token"; const [repoResponse, prsResponse, issuesResponse, commitsResponse, coverageReportsResponse] = await Promise.all([ AnalysisService.getRepositoryWithAnalysis( provider, organization, repository, ), - AnalysisService.listRepositoryPullRequests( - provider, - organization, - repository, + fetchIfAccountToken(auth, noPullRequests(), () => + AnalysisService.listRepositoryPullRequests( + provider, + organization, + repository, + ).catch(() => { + prsUnavailable = true; + return noPullRequests(); + }), ), AnalysisService.issuesOverview(provider, organization, repository), AnalysisService.listRepositoryCommits( @@ -510,12 +564,14 @@ Examples: undefined, 1, ).catch(() => ({ data: [] })), - RepositoryService.listCoverageReports( - provider, - organization, - repository, - 1, - ).catch(() => ({ data: { hasCoverageOverview: false } })), + fetchIfAccountToken(auth, noCoverageReports(), () => + RepositoryService.listCoverageReports( + provider, + organization, + repository, + 1, + ).catch(() => noCoverageReports()), + ), ]); spinner.stop(); @@ -533,8 +589,13 @@ Examples: ...data, fileCount: data.coverage?.numberTotalFiles, }, + // Always an array, never null or absent, so `jq '.pullRequests[]'` + // and `| length` keep working. `unavailable` is what distinguishes + // "no open pull requests" from "couldn't look"; pickDeep drops + // undefined, so it stays absent whenever the data is real. pullRequests, issuesOverview: issuesCounts, + unavailable: prsUnavailable ? ["pullRequests"] : undefined, }, [ // About "repository.repository.provider", @@ -563,6 +624,8 @@ Examples: "pullRequests", // Issues Overview "issuesOverview", + // Sections that couldn't be fetched with the token in use + "unavailable", ])); return; } @@ -570,12 +633,28 @@ Examples: printAbout(data, headCommit, expectsCoverage, hasCoverageData); printSetup(data); printMetrics(data); - printPullRequests(pullRequests); + if (prsUnavailable) { + // Keep the section header: a section that silently vanishes reads as a + // bug, and printPullRequests([]) would claim "No open pull requests", + // which is a different (and false) statement. + printSection("Open Pull Requests"); + console.log( + ansis.dim( + ` ${ + auth.kind === "account-token" + ? "Could not load pull requests." + : repositoryTokenSkipNote("pull requests") + }`, + ), + ); + } else { + printPullRequests(pullRequests); - printPaginationWarning( - prsResponse.pagination, - "Not all pull requests are shown.", - ); + printPaginationWarning( + prsResponse.pagination, + "Not all pull requests are shown.", + ); + } printIssuesOverview(issuesCounts); } catch (err) { diff --git a/src/commands/tool.test.ts b/src/commands/tool.test.ts index 855a7f7..af2c561 100644 --- a/src/commands/tool.test.ts +++ b/src/commands/tool.test.ts @@ -304,4 +304,25 @@ describe("tool command", () => { ); }); }); + + // Both endpoints this command uses are on the repository-token whitelist, so + // every mode works — no guard, and no account token needed at all. + it("configures a tool with only a repository token", async () => { + delete process.env.CODACY_API_TOKEN; + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "tool", "gh", "test-org", "test-repo", "eslint", + "--enable", "--repository-token", "rt", + ]); + + expect(AnalysisService.listRepositoryTools).toHaveBeenCalled(); + expect(AnalysisService.configureTool).toHaveBeenCalledWith( + "gh", + "test-org", + "test-repo", + "uuid-eslint", + { enabled: true }, + ); + }); }); diff --git a/src/commands/tool.ts b/src/commands/tool.ts index 0ce16c3..f99cc64 100644 --- a/src/commands/tool.ts +++ b/src/commands/tool.ts @@ -1,7 +1,7 @@ import { Command } from "commander"; import ora from "ora"; import ansis from "ansis"; -import { checkApiToken } from "../utils/auth"; +import { repositoryTokenOption, resolveAuth } from "../utils/auth"; import { handleError } from "../utils/error"; import { resolveRepoArgs } from "../utils/resolve-repo-args"; import { AnalysisService } from "../api/client/services/AnalysisService"; @@ -26,6 +26,7 @@ export function registerToolCommand(program: Command) { "-c, --configuration-file ", "use a configuration file (true or false)", ) + .addOption(repositoryTokenOption()) .addHelpText( "after", ` @@ -44,7 +45,7 @@ Examples: toolNameArg?: string, ) { try { - checkApiToken(); + resolveAuth(this); const { provider, organization, repository, trailingArgs } = resolveRepoArgs( [providerArg, organizationArg, repositoryArg, toolNameArg], diff --git a/src/commands/tools.test.ts b/src/commands/tools.test.ts index caa0ac0..a7ce1c4 100644 --- a/src/commands/tools.test.ts +++ b/src/commands/tools.test.ts @@ -431,4 +431,137 @@ describe("tools command", () => { ); }); }); + + describe("with a repository token", () => { + const configContent = JSON.stringify({ + version: 1, + metadata: { + repositoryId: null, + repositoryName: null, + createdAt: "2025-01-01", + updatedAt: "2025-01-01", + languages: ["TypeScript"], + }, + tools: [{ toolId: "ESLint", patterns: [{ patternId: "no-unused-vars" }] }], + }); + const tmpConfigPath = "/tmp/test-import-repo-token.json"; + + /** Mocks the import flow, with `standards` controlling the --force path. */ + function setupImport(standards: { id: number; name: string }[]) { + fs.writeFileSync(tmpConfigPath, configContent); + vi.mocked(AnalysisService.updateRepositoryToolPatterns).mockResolvedValue( + undefined as any, + ); + vi.mocked(AnalysisService.configureTool).mockResolvedValue(undefined as any); + vi.spyOn(importConfig, "fetchAllTools").mockResolvedValue([ + { + uuid: "uuid-eslint", + name: "ESLint", + shortName: "eslint", + prefix: "ESLint_", + languages: ["TypeScript"], + clientSide: false, + standalone: false, + configurable: true, + }, + ] as any); + vi.spyOn(importConfig, "getLocalSupportedToolIds").mockResolvedValue([ + "ESLint", + ]); + vi.mocked(AnalysisService.getRepositoryWithAnalysis).mockResolvedValue({ + data: { + repository: { + provider: "gh", + owner: "test-org", + name: "test-repo", + standards, + languages: [], + problems: [], + }, + }, + } as any); + } + + afterEach(() => { + if (fs.existsSync(tmpConfigPath)) fs.unlinkSync(tmpConfigPath); + }); + + it("lists tools — listRepositoryTools accepts a repository token", async () => { + const program = createProgram(); + await program.parseAsync([ + "node", "test", "tools", "gh", "test-org", "test-repo", + "--repository-token", "rt", + ]); + + expect(AnalysisService.listRepositoryTools).toHaveBeenCalled(); + }); + + it("imports a configuration — configureTool accepts a repository token", async () => { + setupImport([]); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "tools", "gh", "test-org", "test-repo", + "--import", tmpConfigPath, "-y", "--repository-token", "rt", + ]); + + expect(getAllOutput()).toContain("imported successfully"); + }); + + it("refuses --force when standards would actually be unlinked", async () => { + setupImport([{ id: 100, name: "Security" }]); + vi.spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit called"); + }); + + const program = createProgram(); + await expect( + program.parseAsync([ + "node", "test", "tools", "gh", "test-org", "test-repo", + "--import", tmpConfigPath, "--force", "-y", "--repository-token", "rt", + ]), + ).rejects.toThrow("process.exit called"); + + expect( + CodingStandardsService.applyCodingStandardToRepositories, + ).not.toHaveBeenCalled(); + // Nothing may be applied: the user must never approve — or half-execute — + // a plan that says "will stop following", since leaving the standard in + // place while reconfiguring tools is the exact state --force prevents. + expect(AnalysisService.configureTool).not.toHaveBeenCalled(); + expect(getAllOutput()).not.toContain("will stop following"); + }); + + it("warns but proceeds when --force has no standards to unlink", async () => { + setupImport([]); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "tools", "gh", "test-org", "test-repo", + "--import", tmpConfigPath, "--force", "-y", "--repository-token", "rt", + ]); + + const warnings = (console.error as ReturnType).mock.calls + .flat() + .join("\n"); + expect(warnings).toContain("--force ignored"); + expect(getAllOutput()).toContain("imported successfully"); + }); + + it("points at Codacy rather than --force in the standards hint", async () => { + setupImport([{ id: 100, name: "Security" }]); + vi.spyOn(prompt, "confirmAction").mockResolvedValue(false); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "tools", "gh", "test-org", "test-repo", + "--import", tmpConfigPath, "--repository-token", "rt", + ]); + + const output = getAllOutput(); + // Both remedies the default hint suggests are themselves refused here. + expect(output).toContain("can't be unlinked with a repository token"); + expect(output).not.toContain("Use --force to unlink them"); + }); + }); }); diff --git a/src/commands/tools.ts b/src/commands/tools.ts index 309fb51..7c7f52a 100644 --- a/src/commands/tools.ts +++ b/src/commands/tools.ts @@ -2,7 +2,12 @@ import * as path from "path"; import { Command } from "commander"; import ora from "ora"; import ansis from "ansis"; -import { checkApiToken } from "../utils/auth"; +import { + RemoteAuth, + repositoryTokenOption, + requireAccountToken, + resolveAuth, +} from "../utils/auth"; import { handleError } from "../utils/error"; import { resolveRepoArgs } from "../utils/resolve-repo-args"; import { createTable, getOutputFormat, pickDeep, printJson } from "../utils/output"; @@ -73,6 +78,44 @@ function printImportErrors(failures: ImportFailure[]): void { console.log(); } +/** + * Enforces that `--force` can actually do what its preview promises. + * + * Unlinking coding standards is organization-level, so `--force` can't run under + * a repository token. This must be called *before* the preview is printed and + * approved: otherwise the user confirms a plan that says "will stop following 1 + * coding standard" and cannot execute it, landing in exactly the state `--force` + * exists to prevent — tools reconfigured while a standard still overrides them. + * + * Only refuses when it would actually do something, though. With no standards to + * unlink, `--force` iterates an empty list and its preview block is skipped + * entirely; refusing a genuine no-op would break anyone with `--force` baked + * into a CI script. + */ +const guardForceUnlink = ( + auth: RemoteAuth, + standardsToUnlink: number, + force: boolean, +): void => { + if (!force || auth.kind === "account-token") return; + + if (standardsToUnlink > 0) { + requireAccountToken( + auth, + "codacy tools --import --force", + "unlinking coding standards is an organization-level operation. " + + "Re-run without --force to import anyway — the coding standard will " + + "keep overriding the imported configuration — or unlink it in Codacy first", + ); + } + + console.error( + ansis.yellow( + "⚠ --force ignored — this repository follows no coding standards to unlink.", + ), + ); +}; + export function registerToolsCommand(program: Command) { program .command("tools") @@ -84,6 +127,7 @@ export function registerToolsCommand(program: Command) { .option("--import [path]", "import tool configuration from a file (default: .codacy/codacy.config.json)") .option("-y, --skip-approval", "skip confirmation prompt during import") .option("--force", "unlink all coding standards before importing") + .addOption(repositoryTokenOption()) .addHelpText( "after", ` @@ -103,7 +147,7 @@ Examples: repositoryArg?: string, ) { try { - checkApiToken(); + const auth = resolveAuth(this); const { provider, organization, repository } = resolveRepoArgs( [providerArg, organizationArg, repositoryArg], 0, @@ -154,7 +198,11 @@ Examples: localToolIds, ); - printImportPreview(preview, repository, Boolean(opts.force)); + guardForceUnlink(auth, preview.standards.length, Boolean(opts.force)); + + printImportPreview(preview, repository, Boolean(opts.force), { + canUnlinkStandards: auth.kind === "account-token", + }); // Confirm if (!opts.skipApproval) { diff --git a/src/index.ts b/src/index.ts index 7b7fa36..bc19ac6 100644 --- a/src/index.ts +++ b/src/index.ts @@ -3,6 +3,7 @@ import { Command } from "commander"; import { OpenAPI } from "./api/client/core/OpenAPI"; import { cliVersion } from "./version"; import { getOutputFormat } from "./utils/output"; +import { repositoryTokenOption } from "./utils/auth"; import { maybeNotifyUpdate } from "./utils/update-check"; import { registerInfoCommand } from "./commands/info"; import { registerRepositoriesCommand } from "./commands/repositories"; @@ -25,8 +26,12 @@ import { registerLogoutCommand } from "./commands/logout"; const program = new Command(); OpenAPI.BASE = (process.env.CODACY_API_BASE_URL || "https://app.codacy.com").replace(/\/$/, "") + "/api/v3"; +// No token here. Which header carries it depends on the token kind, which isn't +// known until a command resolves its auth — every API path installs headers +// first, via `resolveAuth()` in commands or `applyAccountToken()` in `login`. +// Baking in `api-token` would send an empty account header on every +// repository-token run. OpenAPI.HEADERS = { - "api-token": process.env.CODACY_API_TOKEN || "", "X-Codacy-Origin": "cli-cloud-tool", }; @@ -35,6 +40,9 @@ program .description("A CLI tool to interact with the Codacy API") .version(cliVersion) .option("-o, --output ", "output format (table or json)", "table") + // Declared on the root so `codacy --repository-token X tools` parses; each + // command declares its own too, so it also works after the command name. + .addOption(repositoryTokenOption()) // update-notifier reads `--no-update-notifier` straight from argv to opt out. // Declared here (and on every subcommand below) so Commander accepts the flag // instead of failing with "unknown option" when a user passes it. diff --git a/src/utils/auth.test.ts b/src/utils/auth.test.ts index 2077803..0f376d9 100644 --- a/src/utils/auth.test.ts +++ b/src/utils/auth.test.ts @@ -1,5 +1,19 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; -import { checkApiToken } from "./auth"; +import { Command } from "commander"; +import { OpenAPI } from "../api/client/core/OpenAPI"; +import { + NO_TOKEN_MESSAGE, + applyAccountToken, + applyAuthHeaders, + fetchIfAccountToken, + repositoryTokenFlag, + repositoryTokenOption, + requireAccountToken, + resolveAccountAuth, + resolveAuth, + resolveAuthFromToken, + warnUnusedRepositoryToken, +} from "./auth"; vi.mock("./credentials", () => ({ loadCredentials: vi.fn(() => null), @@ -7,32 +21,289 @@ vi.mock("./credentials", () => ({ import { loadCredentials } from "./credentials"; -describe("checkApiToken", () => { - beforeEach(() => { - delete process.env.CODACY_API_TOKEN; - vi.mocked(loadCredentials).mockReturnValue(null); +beforeEach(() => { + delete process.env.CODACY_API_TOKEN; + delete process.env.CODACY_PROJECT_TOKEN; + vi.mocked(loadCredentials).mockReturnValue(null); + // OpenAPI.HEADERS is real module state shared across tests in this file. + OpenAPI.HEADERS = undefined; + vi.restoreAllMocks(); +}); + +describe("resolveAuthFromToken precedence", () => { + it("prefers an explicit --repository-token over every other source", () => { + process.env.CODACY_PROJECT_TOKEN = "env-project"; + process.env.CODACY_API_TOKEN = "env-account"; + vi.mocked(loadCredentials).mockReturnValue("stored"); + + expect(resolveAuthFromToken("flag-token")).toEqual({ + kind: "repository-token", + token: "flag-token", + source: "flag", + }); + // An explicit flag wins outright: no stored login is even loaded. + expect(loadCredentials).not.toHaveBeenCalled(); }); - it("should return the token when CODACY_API_TOKEN is set", () => { - process.env.CODACY_API_TOKEN = "my-token"; - expect(checkApiToken()).toBe("my-token"); + it("prefers CODACY_PROJECT_TOKEN over CODACY_API_TOKEN (matches codacy-analysis)", () => { + process.env.CODACY_PROJECT_TOKEN = "env-project"; + process.env.CODACY_API_TOKEN = "env-account"; + + expect(resolveAuthFromToken()).toEqual({ + kind: "repository-token", + token: "env-project", + source: "CODACY_PROJECT_TOKEN", + }); }); - it("should prefer env var over stored credentials", () => { - process.env.CODACY_API_TOKEN = "env-token"; - vi.mocked(loadCredentials).mockReturnValue("stored-token"); - expect(checkApiToken()).toBe("env-token"); + it("uses CODACY_API_TOKEN when no repository token is available", () => { + process.env.CODACY_API_TOKEN = "env-account"; + vi.mocked(loadCredentials).mockReturnValue("stored"); + + expect(resolveAuthFromToken()).toEqual({ + kind: "account-token", + token: "env-account", + source: "CODACY_API_TOKEN", + }); expect(loadCredentials).not.toHaveBeenCalled(); }); - it("should fall back to stored credentials when env var is not set", () => { - vi.mocked(loadCredentials).mockReturnValue("stored-token"); - expect(checkApiToken()).toBe("stored-token"); + it("falls back to stored credentials last", () => { + vi.mocked(loadCredentials).mockReturnValue("stored"); + + expect(resolveAuthFromToken()).toEqual({ + kind: "account-token", + token: "stored", + source: "credentials", + }); + }); + + it("throws when no token can be resolved", () => { + expect(() => resolveAuthFromToken()).toThrow(NO_TOKEN_MESSAGE); + }); + + it("ignores an empty CODACY_PROJECT_TOKEN", () => { + process.env.CODACY_PROJECT_TOKEN = ""; + process.env.CODACY_API_TOKEN = "env-account"; + + expect(resolveAuthFromToken().kind).toBe("account-token"); + }); +}); + +describe("applyAuthHeaders", () => { + it("sends an account token on the api-token header", () => { + applyAuthHeaders({ kind: "account-token", token: "a", source: "CODACY_API_TOKEN" }); + + expect(OpenAPI.HEADERS).toEqual({ + "api-token": "a", + "X-Codacy-Origin": "cli-cloud-tool", + }); + }); + + it("sends a repository token on the project-token header", () => { + applyAuthHeaders({ kind: "repository-token", token: "r", source: "flag" }); + + expect(OpenAPI.HEADERS).toEqual({ + "project-token": "r", + "X-Codacy-Origin": "cli-cloud-tool", + }); + }); + + it("never leaves the other token's header behind when switching kinds", () => { + applyAuthHeaders({ kind: "account-token", token: "a", source: "CODACY_API_TOKEN" }); + applyAuthHeaders({ kind: "repository-token", token: "r", source: "flag" }); + + expect(OpenAPI.HEADERS).not.toHaveProperty("api-token"); + expect(OpenAPI.HEADERS).toHaveProperty("project-token", "r"); + }); + + it("installs the header as part of resolving", () => { + process.env.CODACY_PROJECT_TOKEN = "env-project"; + resolveAuthFromToken(); + + expect(OpenAPI.HEADERS).toHaveProperty("project-token", "env-project"); + }); + + it("applyAccountToken installs an account header for login", () => { + applyAccountToken("login-token"); + + expect(OpenAPI.HEADERS).toEqual({ + "api-token": "login-token", + "X-Codacy-Origin": "cli-cloud-tool", + }); + }); +}); + +describe("repositoryTokenFlag", () => { + /** Builds a root program plus one subcommand, both declaring the option. */ + function buildProgram(): { program: Command; sub: Command } { + const program = new Command(); + program.addOption(repositoryTokenOption()); + const sub = program.command("thing").addOption(repositoryTokenOption()); + sub.action(() => {}); + return { program, sub }; + } + + it("reads the flag passed after the subcommand", () => { + const { program, sub } = buildProgram(); + program.parse(["node", "test", "thing", "--repository-token", "own"]); + + expect(repositoryTokenFlag(sub)).toBe("own"); + }); + + it("reads the flag passed before the subcommand", () => { + const { program, sub } = buildProgram(); + program.parse(["node", "test", "--repository-token", "global", "thing"]); + + expect(repositoryTokenFlag(sub)).toBe("global"); }); - it("should throw when no env var and no stored credentials", () => { - expect(() => checkApiToken()).toThrow( - "No API token found. Set CODACY_API_TOKEN or run 'codacy login'.", + it("prefers the command's own value over the inherited one", () => { + const { program, sub } = buildProgram(); + program.parse([ + "node", "test", "--repository-token", "global", "thing", "--repository-token", "own", + ]); + + expect(repositoryTokenFlag(sub)).toBe("own"); + }); + + it("returns undefined when the flag is absent", () => { + const { program, sub } = buildProgram(); + program.parse(["node", "test", "thing"]); + + expect(repositoryTokenFlag(sub)).toBeUndefined(); + }); + + it("returns a distinct Option instance per call", () => { + expect(repositoryTokenOption()).not.toBe(repositoryTokenOption()); + }); +}); + +describe("requireAccountToken", () => { + const accountAuth = { + kind: "account-token", + token: "a", + source: "CODACY_API_TOKEN", + } as const; + + it("passes an account token straight through", () => { + expect(requireAccountToken(accountAuth, "codacy info", "why")).toBe(accountAuth); + }); + + it("names the operation, the reason and the flag source", () => { + expect(() => + requireAccountToken( + { kind: "repository-token", token: "r", source: "flag" }, + "codacy info", + "it reads account-level data", + ), + ).toThrow( + "codacy info requires an account API token — it reads account-level data. " + + "The token in use is a repository token (provided with --repository-token). " + + "Set CODACY_API_TOKEN or run 'codacy login'.", + ); + }); + + it("names CODACY_PROJECT_TOKEN when the token came from the environment", () => { + expect(() => + requireAccountToken( + { kind: "repository-token", token: "r", source: "CODACY_PROJECT_TOKEN" }, + "--add", + "adding a repository is an account-level operation", + ), + ).toThrow(/repository token \(from CODACY_PROJECT_TOKEN\)/); + }); +}); + +describe("resolveAccountAuth", () => { + function commandNamed(name: string): Command { + const cmd = new Command(name); + cmd.addOption(repositoryTokenOption()); + return cmd; + } + + it("resolves an account token normally", () => { + process.env.CODACY_API_TOKEN = "env-account"; + + expect(resolveAccountAuth(commandNamed("info"), "why").kind).toBe("account-token"); + }); + + it("refuses a repository token, deriving the command name", () => { + process.env.CODACY_PROJECT_TOKEN = "env-project"; + + expect(() => resolveAccountAuth(commandNamed("info"), "why")).toThrow( + /^codacy info requires an account API token/, ); }); }); + +describe("resolveAuth", () => { + it("resolves from a command's parsed flag", () => { + const program = new Command(); + program.addOption(repositoryTokenOption()); + const sub = program.command("thing").addOption(repositoryTokenOption()); + sub.action(() => {}); + program.parse(["node", "test", "thing", "--repository-token", "rt"]); + + expect(resolveAuth(sub)).toEqual({ + kind: "repository-token", + token: "rt", + source: "flag", + }); + }); +}); + +describe("fetchIfAccountToken", () => { + it("calls fetch under an account token", async () => { + const fetch = vi.fn().mockResolvedValue("real"); + + await expect( + fetchIfAccountToken( + { kind: "account-token", token: "a", source: "CODACY_API_TOKEN" }, + "fallback", + fetch, + ), + ).resolves.toBe("real"); + expect(fetch).toHaveBeenCalled(); + }); + + it("returns the fallback without calling fetch under a repository token", async () => { + const fetch = vi.fn().mockResolvedValue("real"); + + await expect( + fetchIfAccountToken( + { kind: "repository-token", token: "r", source: "flag" }, + "fallback", + fetch, + ), + ).resolves.toBe("fallback"); + expect(fetch).not.toHaveBeenCalled(); + }); +}); + +describe("warnUnusedRepositoryToken", () => { + function parsedCommand(argv: string[]): Command { + const program = new Command(); + const sub = program.command("thing").addOption(repositoryTokenOption()); + sub.action(() => {}); + program.parse(["node", "test", "thing", ...argv]); + return sub; + } + + it("warns when the flag was passed explicitly", () => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + warnUnusedRepositoryToken(parsedCommand(["--repository-token", "rt"]), "`codacy login`"); + + expect(errorSpy.mock.calls.join("\n")).toContain( + "--repository-token is ignored by `codacy login`", + ); + }); + + it("stays silent when only CODACY_PROJECT_TOKEN is set", () => { + process.env.CODACY_PROJECT_TOKEN = "env-project"; + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + warnUnusedRepositoryToken(parsedCommand([]), "`codacy login`"); + + expect(errorSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/src/utils/auth.ts b/src/utils/auth.ts index e1b4f63..3adfd31 100644 --- a/src/utils/auth.ts +++ b/src/utils/auth.ts @@ -1,27 +1,239 @@ +import ansis from "ansis"; +import { Command, Option } from "commander"; import { OpenAPI } from "../api/client/core/OpenAPI"; import { loadCredentials } from "./credentials"; -export function updateApiHeaders(token: string): void { +/** + * Codacy accepts two kinds of token, on two different headers, with two very + * different scopes: + * + * - an **account token** (`api-token` header) reaches anything the account can + * see: every organization, every repository, every endpoint; + * - a **repository token** (`project-token` header, also called a project + * token) is issued for a single repository and is accepted only on a small, + * fixed whitelist of repository-scoped operations. Everywhere else the API + * rejects it exactly as if no token had been sent. + * + * That last point is why the CLI has to know which kind it holds *before* it + * calls: a bare `Error: Unauthorized` tells the user nothing, so commands + * refuse (or skip a section) up front instead. See `SPECS/repository-tokens.md` + * for the whitelist and the per-command support matrix. + * + * The discriminated union makes "exactly one token, and we always know which + * kind and where it came from" a compile-time property: there is no way to + * build a value carrying both, or neither. `source` is carried so refusals can + * name the thing the user actually set — the difference between an explicit + * `--repository-token` and an ambient `CODACY_PROJECT_TOKEN` is exactly what + * makes a surprising refusal debuggable in one read. + */ +export type AccountAuth = { + kind: "account-token"; + token: string; + source: "CODACY_API_TOKEN" | "credentials"; +}; + +export type RepositoryAuth = { + kind: "repository-token"; + token: string; + source: "flag" | "CODACY_PROJECT_TOKEN"; +}; + +export type RemoteAuth = AccountAuth | RepositoryAuth; + +export const NO_TOKEN_MESSAGE = + "No API token found. Provide --repository-token, set CODACY_PROJECT_TOKEN, " + + "set CODACY_API_TOKEN, or run 'codacy login'."; + +const REPOSITORY_TOKEN_FLAGS = "--repository-token "; +const REPOSITORY_TOKEN_DESCRIPTION = + "repository (project) token, scoped to a single repository (env: CODACY_PROJECT_TOKEN)"; + +/** + * The `--repository-token` option, for `.addOption()` on the root program and + * on every command. + * + * Deliberately no short flag. The short-flag space is crowded and collides + * per command (`-r` is `repository --remove`, `-R` is `--reanalyze`, `-t` is + * `--tags` on `issues`/`patterns` and `--token` on `login`), so any single + * letter would either clash or mean something different depending on the + * command. `codacy-analysis` dropped its short flag for the same reason. + * + * Returns a **new** Option per call: Commander stores the instance on the + * command and writes parsed values onto it, so a shared instance would be + * shared mutable state across all 17 commands. + */ +export function repositoryTokenOption(): Option { + return new Option(REPOSITORY_TOKEN_FLAGS, REPOSITORY_TOKEN_DESCRIPTION); +} + +/** + * The `--repository-token` value *as typed by the user*, or undefined. + * + * The option is declared both on the root program (`codacy --repository-token X + * tools`) and on each command (`codacy tools --repository-token X`). Commander's + * `optsWithGlobals()` lets globals overwrite locals, so read the command's own + * value first — nearest wins, which is what users expect. (In practice they + * cannot conflict: the option has no default, so a program that never received + * it has no key to overwrite with.) + */ +export function repositoryTokenFlag(command: Command): string | undefined { + const own = command.opts().repositoryToken; + if (typeof own === "string" && own !== "") return own; + const inherited = command.optsWithGlobals().repositoryToken; + return typeof inherited === "string" && inherited !== "" ? inherited : undefined; +} + +/** + * Point the generated client at a token. Auth is process-global + * (`OpenAPI.HEADERS`) because the generated services never accept per-request + * headers; the header *name* is what selects the token kind server-side. + * + * Always assigns a fresh object — never merges — so switching kinds can never + * leave the other token's header behind. + */ +export function applyAuthHeaders(auth: RemoteAuth): void { + const tokenHeader = auth.kind === "account-token" ? "api-token" : "project-token"; OpenAPI.HEADERS = { - "api-token": token, + [tokenHeader]: auth.token, "X-Codacy-Origin": "cli-cloud-tool", }; } -export function checkApiToken(): string { - const envToken = process.env.CODACY_API_TOKEN; - if (envToken) { - updateApiHeaders(envToken); - return envToken; +/** + * Install an account token directly. For `login`, which validates a token it + * was just handed rather than resolving one — it must not go through + * {@link resolveAuth}, which would pick up an ambient token instead. + */ +export function applyAccountToken(token: string): void { + applyAuthHeaders({ kind: "account-token", token, source: "credentials" }); +} + +/** + * How the token in use was configured, phrased to drop into a sentence. Typed + * as a total Record over the union so adding a source is a compile error until + * it has a description. + */ +const SOURCE_DESCRIPTIONS: Record = { + flag: "provided with --repository-token", + CODACY_PROJECT_TOKEN: "from CODACY_PROJECT_TOKEN", + CODACY_API_TOKEN: "from CODACY_API_TOKEN", + credentials: "from the stored login", +}; + +/** + * Token precedence, identical to `codacy-analysis`: + * + * 1. explicit `--repository-token` → repository token + * 2. CODACY_PROJECT_TOKEN → repository token + * 3. CODACY_API_TOKEN → account token + * 4. stored credentials (`codacy login`) → account token + * 5. throw + * + * An explicit flag wins outright — it never even looks for an account token, so + * a deliberately-scoped run can't be silently widened by an ambient env var or + * a stale login lying around. + */ +function pickAuth(flagToken?: string): RemoteAuth { + if (flagToken) { + return { kind: "repository-token", token: flagToken, source: "flag" }; + } + + const projectEnv = process.env.CODACY_PROJECT_TOKEN; + if (projectEnv) { + return { kind: "repository-token", token: projectEnv, source: "CODACY_PROJECT_TOKEN" }; + } + + const accountEnv = process.env.CODACY_API_TOKEN; + if (accountEnv) { + return { kind: "account-token", token: accountEnv, source: "CODACY_API_TOKEN" }; } const stored = loadCredentials(); if (stored) { - updateApiHeaders(stored); - return stored; + return { kind: "account-token", token: stored, source: "credentials" }; } + throw new Error(NO_TOKEN_MESSAGE); +} + +/** + * Resolve the auth to use and install its header. Exported separately from + * {@link resolveAuth} so unit tests can drive it without a Commander instance. + */ +export function resolveAuthFromToken(flagToken?: string): RemoteAuth { + const auth = pickAuth(flagToken); + applyAuthHeaders(auth); + return auth; +} + +/** + * What every API-calling command calls first, mirroring `getOutputFormat(this)` + * from `utils/output.ts`. Returns the resolved auth; commands that branch on + * the token kind keep the value, the rest ignore it. + */ +export function resolveAuth(command: Command): RemoteAuth { + return resolveAuthFromToken(repositoryTokenFlag(command)); +} + +/** + * Guard for an operation Codacy does not accept a repository token on. Returns + * the narrowed account auth so callers can keep using the value; otherwise + * throws into the command's existing `catch (err) { handleError(err) }`, which + * prints `Error: ` in red and exits 1. + * + * @param operation what the user asked for, e.g. `codacy info` or `--add` + * @param because why a repository token can't do it, as a sentence fragment + */ +export function requireAccountToken( + auth: RemoteAuth, + operation: string, + because: string, +): AccountAuth { + if (auth.kind === "account-token") return auth; throw new Error( - "No API token found. Set CODACY_API_TOKEN or run 'codacy login'.", + `${operation} requires an account API token — ${because}. ` + + `The token in use is a repository token (${SOURCE_DESCRIPTIONS[auth.source]}). ` + + `Set CODACY_API_TOKEN or run 'codacy login'.`, ); } + +/** + * Resolve auth for a command that is account-only end to end, deriving the + * operation name from the command itself. + */ +export function resolveAccountAuth(command: Command, because: string): AccountAuth { + return requireAccountToken(resolveAuth(command), `codacy ${command.name()}`, because); +} + +/** + * Run `fetch` only under an account token; otherwise resolve to `fallback` + * without calling at all. For sections of an otherwise-supported command that + * hang off a non-whitelisted endpoint — see the pull request table in + * `commands/repository.ts`. + */ +export function fetchIfAccountToken( + auth: RemoteAuth, + fallback: F, + fetch: () => Promise, +): Promise { + return auth.kind === "account-token" ? fetch() : Promise.resolve(fallback); +} + +/** Note rendered in place of a section skipped because of the token kind. */ +export function repositoryTokenSkipNote(what: string): string { + return `Not shown with a repository token — ${what} require an account API token.`; +} + +/** + * Warn that an explicitly-passed `--repository-token` is doing nothing here. + * + * Keyed on the **explicit flag only**, never on an ambient + * `CODACY_PROJECT_TOKEN`: that variable is the standard Codacy CI credential + * (the coverage reporter reads it), so it is routinely exported job-wide. + * Warning on it would fire on every unrelated invocation in such a job, which + * trains users to ignore the warning that does matter. + */ +export function warnUnusedRepositoryToken(command: Command, detail: string): void { + if (!repositoryTokenFlag(command)) return; + console.error(ansis.yellow(`Warning: --repository-token is ignored by ${detail}`)); +} diff --git a/src/utils/import-config.ts b/src/utils/import-config.ts index d150301..939088b 100644 --- a/src/utils/import-config.ts +++ b/src/utils/import-config.ts @@ -228,7 +228,15 @@ export function printImportPreview( preview: ImportPreview, repoName: string, force: boolean, + /** + * Whether the token in use can unlink coding standards. False under a + * repository token, where both remedies the default hint suggests (`--force` + * and `codacy repository --unlink-standard`) are themselves refused — so the + * hint has to point somewhere the user can actually go. + */ + options: { canUnlinkStandards?: boolean } = {}, ): void { + const canUnlinkStandards = options.canUnlinkStandards ?? true; console.log(); // Standards @@ -246,7 +254,9 @@ export function printImportPreview( ); console.log( ansis.yellow( - " Standards may override tool configuration. Use --force to unlink them, or --unlink-standard to remove them manually.", + canUnlinkStandards + ? " Standards may override tool configuration. Use --force to unlink them, or --unlink-standard to remove them manually." + : " Standards may override tool configuration. They can't be unlinked with a repository token — unlink them in Codacy (Repository > Settings > Coding standards), or re-run with an account API token.", ), ); } diff --git a/vitest.config.mts b/vitest.config.mts index 3f824fb..6d0bafa 100644 --- a/vitest.config.mts +++ b/vitest.config.mts @@ -4,5 +4,11 @@ export default defineConfig({ test: { globals: true, environment: "node", + // CODACY_PROJECT_TOKEN outranks CODACY_API_TOKEN in the auth precedence, and + // it is the variable the Codacy coverage reporter reads — so it is routinely + // exported job-wide in CI and set in many developers' shells. Neutralize it + // here so token resolution under test never depends on the ambient + // environment. Empty string is falsy for every `if (env)` check. + env: { CODACY_PROJECT_TOKEN: "" }, }, }); From 148baa03478c0d4c784e08a7bad043a8f6e25dcd Mon Sep 17 00:00:00 2001 From: Alejandro Rizzo Date: Tue, 11 Aug 2026 10:57:45 +0100 Subject: [PATCH 2/5] fix: Address Codacy review feedback on OD-489 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract `acquireToken()` and `resolveAndValidateUser()` out of login's action handler, which was doing token acquisition, API validation and three error branches in one function. Brings it to CCN 2 / 18 NLOC (was 9 / 61). The complexity predated this branch but surfaced as new because the PR touched that function. Split two compound bullets in AGENTS.md's authentication section, and give the "guards must run before any request" rule the exception it was missing: commands whose endpoints are all whitelisted need no guard, and an unclear operation scope should be confirmed with the API owners rather than guessed. Add .codacy/instructions/review.md with the project-specific context behind this round's false positives — long test files are deliberate, Lizard's TS parser merges adjacent function spans, the token whitelist mirrors a server-side allowlist and is hardcoded on purpose, and cross-references are often added in the same PR as their target. `.codacy/` was ignored wholesale, so the ignore rule is narrowed to `.codacy/*` to let authored files under `instructions/` be tracked while all local CLI state stays ignored. Co-Authored-By: Claude Opus 5 (1M context) --- .codacy/instructions/review.md | 54 ++++++++++++++++ .gitignore | 8 ++- AGENTS.md | 8 ++- src/commands/login.ts | 110 +++++++++++++++++---------------- 4 files changed, 124 insertions(+), 56 deletions(-) create mode 100644 .codacy/instructions/review.md diff --git a/.codacy/instructions/review.md b/.codacy/instructions/review.md new file mode 100644 index 0000000..18392c0 --- /dev/null +++ b/.codacy/instructions/review.md @@ -0,0 +1,54 @@ +# Codacy AI review instructions + +Project-specific context for reviewing this repository. These notes exist to +prevent recurring false positives — they are not blanket exemptions, so still +flag a finding when it points at a concrete defect. + +## Repository shape + +- Single-package Node.js + TypeScript CLI (`@codacy/codacy-cloud-cli`) wrapping + the Codacy API v3. Commander for the CLI, Vitest for tests. +- `src/api/client/` is **auto-generated** from the OpenAPI spec by + `npm run update-api`. Never flag findings there and never suggest edits to it. +- Conventions live in `AGENTS.md` (root) and `src/commands/AGENTS.md`; specs and + the backlog live in `SPECS/`. + +## Tests + +- Test files are deliberately long and repetitive: fixtures are written out in + full rather than factored into builders, so each test reads standalone. **File-level + length and duplication findings on `*.test.ts` are expected** and should not be + reported. +- Each command test builds its own bare `new Command()` harness rather than + importing `src/index.ts`. That duplication is intentional — it keeps a command's + tests independent of global CLI wiring. + +## Complexity metrics + +- Lizard's TypeScript parser sometimes **merges adjacent function declarations** + into a single span, reporting their combined cyclomatic complexity against the + first function's name. Before reporting a complexity finding, check that the + named function really contains that many branches; if the reported span covers + more than one declaration, the number is a parser artifact. +- Command action handlers are inherently branchy — they dispatch across mutually + exclusive flag modes with early returns. Prefer suggesting extraction of a + cohesive block (validation, rendering) over generic "reduce complexity" advice. + +## Authentication + +- The CLI accepts two token kinds: an **account token** (`api-token` header) and a + **repository/project token** (`project-token` header). See + `SPECS/repository-tokens.md`. +- Codacy honours repository tokens on only a fixed set of operations. That + whitelist is **deliberately hardcoded** in the command guards — it mirrors a + server-side allowlist that the client cannot query, so don't suggest deriving it + dynamically. It carries a "re-verify after every `npm run update-api`" note. +- Guards intentionally refuse **before** issuing any request and before + `resolveRepoArgs()` runs, so an unsupported operation fails fast instead of + returning a bare `Unauthorized`. + +## Documentation + +- Cross-references between `SPECS/*.md`, `AGENTS.md`, and `README.md` are often + added in the **same** pull request as the file they point at. Verify the target + is absent from the PR's own diff before reporting a broken reference. diff --git a/.gitignore b/.gitignore index 89dc384..6f70c98 100644 --- a/.gitignore +++ b/.gitignore @@ -16,8 +16,12 @@ dist/ # Ignore api-v3 api-v3/ -# Ignore .codacy -.codacy/ +# Ignore .codacy local state (config, logs, tool configs, generated files). +# Uses `.codacy/*` rather than `.codacy/` so authored, shareable files below can +# be re-included — git cannot un-ignore anything inside an excluded directory. +.codacy/* +# Instructions for Codacy's AI reviewer are authored and belong in the repo. +!.codacy/instructions/ #Ignore vscode AI rules .github/instructions/codacy.instructions.md diff --git a/AGENTS.md b/AGENTS.md index 6bc9e35..5ee3918 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -88,14 +88,18 @@ codacy-cloud-cli/ - **Error handling:** Use `try/catch` with the shared `handleError()` from `src/utils/error.ts` - **API base URL:** `https://app.codacy.com/api/v3` (configured in `src/index.ts` via `OpenAPI.BASE`) - **Authentication — two token kinds.** Read `SPECS/repository-tokens.md` before touching auth or adding a command. - - An **account token** (`api-token` header) reaches everything its owner can see. A **repository token** (`project-token` header) is scoped to one repository and is accepted only on a fixed whitelist of 13 operations; everywhere else Codacy rejects it as if no token had been sent. + - An **account token** (`api-token` header) reaches everything its owner can see. + - A **repository token** (`project-token` header) is scoped to one repository. It is accepted only on a fixed whitelist of 13 operations; everywhere else Codacy rejects it as if no token had been sent. - Every command that calls the API resolves auth first, via `resolveAuth(this)` from `src/utils/auth.ts` (returns a `RemoteAuth` discriminated union), and declares `.addOption(repositoryTokenOption())` so `--repository-token` parses. - **New commands must decide their token scope**, using the whitelist in `SPECS/repository-tokens.md`: - account-only end to end → `resolveAccountAuth(this, "")` - fully whitelisted → `resolveAuth(this)` - mixed → `resolveAuth(this)` plus `requireAccountToken(auth, "", "")` per unsupported flag, or `fetchIfAccountToken(...)` to skip an unsupported sub-call - **Guards must run before any request**, and before `resolveRepoArgs()` — that shells out to git and prints an auto-detection line, which is misleading ahead of a refusal. - - The whitelist is hardcoded in these guards. **Re-verify it after every `npm run update-api`.** + - Exception: a command whose endpoints are all whitelisted needs no guard at all — `resolveAuth(this)` alone is correct (see `tool`, `patterns`, `pattern`). + - If an operation's scope is genuinely unclear, don't guess a guard: confirm the whitelist against the API owners, and record the answer in `SPECS/repository-tokens.md`. + - The whitelist is hardcoded in these guards. + - **Re-verify the whitelist after every `npm run update-api`.** ### Command Pattern diff --git a/src/commands/login.ts b/src/commands/login.ts index a6a9c45..912aa09 100644 --- a/src/commands/login.ts +++ b/src/commands/login.ts @@ -14,6 +14,62 @@ import { promptForToken, } from "../utils/credentials"; +/** Reads the account token from `--token`, or prompts for it interactively. */ +const acquireToken = async (tokenOption?: string): Promise => { + if (tokenOption) { + const token = String(tokenOption).trim(); + if (!token) throw new Error("Token cannot be empty."); + return token; + } + + console.log(ansis.bold("\nCodacy Login\n")); + console.log("You need an Account API Token to authenticate."); + console.log( + `Get one at: ${ansis.cyan("https://app.codacy.com/account/access-management")}`, + ); + console.log(ansis.dim(" My Account > Access Management > API Tokens\n")); + + const token = (await promptForToken("API Token: ")).trim(); + if (!token) throw new Error("Token cannot be empty."); + return token; +}; + +/** + * Confirms the token is a usable account token by reading the account it + * belongs to, translating the API's bare status codes into actionable errors. + * Fails the spinner before throwing so the caller doesn't leave it spinning. + */ +const resolveAndValidateUser = async ( + spinner: ReturnType, +): Promise<{ userName: string; userEmail: string }> => { + try { + const response = await AccountService.getUser(); + return { + userName: response.data.name || "Unknown", + userEmail: response.data.mainEmail, + }; + } catch (apiErr: any) { + spinner.fail("Authentication failed."); + if (apiErr?.status === 401) { + // A repository token lands here too — it is rejected by /user by + // design — so name that case rather than only implying a bad token. + throw new Error( + "Invalid account API token. Check that it is correct and not expired. " + + "If this is a repository (project) token, it can't be used to log in — " + + "pass it per command with --repository-token, or set CODACY_PROJECT_TOKEN.", + ); + } + if (typeof apiErr?.status === "number") { + throw new Error( + `Codacy API returned an error (status ${apiErr.status}). Please try again or check your permissions.`, + ); + } + throw new Error( + "Could not reach the Codacy API. Check your network connection.", + ); + } +}; + export function registerLoginCommand(program: Command) { program .command("login") @@ -44,63 +100,13 @@ pass them per command with --repository-token, or set CODACY_PROJECT_TOKEN.`, "Pass a repository token per command with --repository-token, or set CODACY_PROJECT_TOKEN.", ); - let token: string; - - if (options.token) { - token = String(options.token).trim(); - - if (!token) { - throw new Error("Token cannot be empty."); - } - } else { - console.log(ansis.bold("\nCodacy Login\n")); - console.log("You need an Account API Token to authenticate."); - console.log( - `Get one at: ${ansis.cyan("https://app.codacy.com/account/access-management")}`, - ); - console.log( - ansis.dim(" My Account > Access Management > API Tokens\n"), - ); - - token = await promptForToken("API Token: "); - - if (!token.trim()) { - throw new Error("Token cannot be empty."); - } - - token = token.trim(); - } + const token = await acquireToken(options.token); const spinner = ora("Validating token...").start(); applyAccountToken(token); - let userName: string; - let userEmail: string; - try { - const response = await AccountService.getUser(); - userName = response.data.name || "Unknown"; - userEmail = response.data.mainEmail; - } catch (apiErr: any) { - spinner.fail("Authentication failed."); - if (apiErr?.status === 401) { - // A repository token lands here too — it is rejected by /user by - // design — so name that case rather than only implying a bad token. - throw new Error( - "Invalid account API token. Check that it is correct and not expired. " + - "If this is a repository (project) token, it can't be used to log in — " + - "pass it per command with --repository-token, or set CODACY_PROJECT_TOKEN.", - ); - } - if (typeof apiErr?.status === "number") { - throw new Error( - `Codacy API returned an error (status ${apiErr.status}). Please try again or check your permissions.`, - ); - } - throw new Error( - "Could not reach the Codacy API. Check your network connection.", - ); - } + const { userName, userEmail } = await resolveAndValidateUser(spinner); saveCredentials(token); spinner.succeed(`Logged in as ${ansis.bold(userName)} (${userEmail})`); From 7fbfe511f249a410d456804eac7c2c41ace301ab Mon Sep 17 00:00:00 2001 From: Alejandro Rizzo Date: Tue, 11 Aug 2026 10:59:54 +0100 Subject: [PATCH 3/5] chore: Split compound bullet in AGENTS.md guard exception The escape hatch added for the previous review round packed three actions into one bullet, tripping the compound-instruction rule. One action per line. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 5ee3918..e067caf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -97,7 +97,9 @@ codacy-cloud-cli/ - mixed → `resolveAuth(this)` plus `requireAccountToken(auth, "", "")` per unsupported flag, or `fetchIfAccountToken(...)` to skip an unsupported sub-call - **Guards must run before any request**, and before `resolveRepoArgs()` — that shells out to git and prints an auto-detection line, which is misleading ahead of a refusal. - Exception: a command whose endpoints are all whitelisted needs no guard at all — `resolveAuth(this)` alone is correct (see `tool`, `patterns`, `pattern`). - - If an operation's scope is genuinely unclear, don't guess a guard: confirm the whitelist against the API owners, and record the answer in `SPECS/repository-tokens.md`. + - If an operation's scope is genuinely unclear, don't guess a guard. + - Confirm the whitelist against the API owners instead. + - Record the answer in `SPECS/repository-tokens.md`. - The whitelist is hardcoded in these guards. - **Re-verify the whitelist after every `npm run update-api`.** From 489c1321926bfcf4c1a6287ed381b8f4fb421795 Mon Sep 17 00:00:00 2001 From: Alejandro Rizzo Date: Tue, 11 Aug 2026 11:49:39 +0100 Subject: [PATCH 4/5] fix: Refuse an empty --repository-token instead of widening scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review found a real footgun: `--repository-token "$CODACY_PROJECT_TOKEN"` with the secret unset or misspelled passed an empty string, which resolved as "no flag" and fell through to an ambient CODACY_API_TOKEN or stored login. A CI job asking for a repository-scoped run silently authenticated with a full account token — the exact widening the precedence rule promises can't happen. Verified against a local listener before and after: it sent `api-token=`; it now errors. An explicitly-passed empty flag is now refused; empty *env vars* keep meaning "unset" (the test config depends on that). Flag and env values are trimmed, so whitespace-only tokens get the CLI's own error rather than a server 401. Also from the review: - `unavailable` in `repository --output json` now lists `coverageReports`. Skipping that call forces `expectsCoverage` false, silently suppressing the "missing coverage reports" state, so a repo configured for coverage but missing reports looked identical to a healthy one. README and SPECS claimed coverage was marked when only pull requests were. - Hoist the shared `X-Codacy-Origin` header into `BASE_HEADERS`, so the wholesale `OpenAPI.HEADERS` assignment can't drop anything set at startup. - Strengthen two dashboard tests that asserted only on the token-kind branch and would still have passed with the skip optimization removed; add the untested `CODACY_PROJECT_TOKEN` vs stored-credentials precedence case and the account-token JSON failure path. - Move `CODACY_PROJECT_TOKEN` cleanup into `afterEach` so a failing assertion can't leak the highest-precedence token into later tests. - Document the data-dependent guard exception in AGENTS.md. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 1 + README.md | 4 ++- SPECS/repository-tokens.md | 14 ++++++++++- src/commands/login.test.ts | 9 +++++-- src/commands/logout.test.ts | 9 +++++-- src/commands/repository.test.ts | 42 ++++++++++++++++++++++++++++++-- src/commands/repository.ts | 15 +++++++++++- src/index.ts | 9 +++---- src/utils/auth.test.ts | 39 ++++++++++++++++++++++++++++++ src/utils/auth.ts | 43 +++++++++++++++++++++++++++------ 10 files changed, 163 insertions(+), 22 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e067caf..1f46ba3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -97,6 +97,7 @@ codacy-cloud-cli/ - mixed → `resolveAuth(this)` plus `requireAccountToken(auth, "", "")` per unsupported flag, or `fetchIfAccountToken(...)` to skip an unsupported sub-call - **Guards must run before any request**, and before `resolveRepoArgs()` — that shells out to git and prints an auto-detection line, which is misleading ahead of a refusal. - Exception: a command whose endpoints are all whitelisted needs no guard at all — `resolveAuth(this)` alone is correct (see `tool`, `patterns`, `pattern`). + - Exception: a guard whose condition depends on fetched data must run after that fetch (see `guardForceUnlink` in `tools.ts`, which needs the coding-standard count). Keep those reads whitelisted so nothing doomed is sent, and still refuse before any prompt or mutation. - If an operation's scope is genuinely unclear, don't guess a guard. - Confirm the whitelist against the API owners instead. - Record the answer in `SPECS/repository-tokens.md`. diff --git a/README.md b/README.md index 8e6cddc..ca6db4c 100644 --- a/README.md +++ b/README.md @@ -61,7 +61,7 @@ Codacy accepts repository tokens on a **limited set of repository-scoped operati | `repository`, `repository --reanalyze` | `repository --add`/`--remove`/`--follow`/`--unfollow`/`--link-standard`/`--unlink-standard` | | | `pull-request`, `pull-requests`, `ls`, `directories`, `findings`, `finding` | -`codacy repository` works, but omits the pull request and coverage sections — those endpoints don't accept repository tokens. In `--output json` it marks them as `"unavailable": ["pullRequests"]`. +`codacy repository` works, but omits the pull request and coverage-report sections — those endpoints don't accept repository tokens. In `--output json` it marks them as `"unavailable": ["pullRequests", "coverageReports"]`, so a consumer can tell "none" apart from "couldn't look". Note that skipping coverage reports also suppresses the "waiting for / missing coverage reports" hint on the Analysis row. `codacy login` stores account tokens only; pass repository tokens per command or via `CODACY_PROJECT_TOKEN`. @@ -74,6 +74,8 @@ Codacy accepts repository tokens on a **limited set of repository-scoped operati An explicit `--repository-token` wins outright, so a deliberately scoped run is never silently widened by an environment variable or a stale login. Note that `CODACY_PROJECT_TOKEN` outranks `CODACY_API_TOKEN` (matching the [Codacy Analysis CLI](https://github.com/codacy/analysis-cli)) — unset it if you want your account token used. +Passing `--repository-token` with an **empty** value is an error rather than a fallback. `--repository-token "$CODACY_PROJECT_TOKEN"` with the secret unset is a common CI mistake, and quietly falling back to an account token would run with much wider access than you asked for. An empty *environment variable*, by contrast, simply means "unset". + ## Usage ```bash diff --git a/SPECS/repository-tokens.md b/SPECS/repository-tokens.md index b652633..1527f33 100644 --- a/SPECS/repository-tokens.md +++ b/SPECS/repository-tokens.md @@ -27,6 +27,13 @@ An explicit `--repository-token` wins outright — it never even looks for an account token, so a deliberately-scoped run can't be silently widened by an ambient env var or a stale login. +An explicitly-passed **empty** flag throws (`EMPTY_REPOSITORY_TOKEN_MESSAGE`) +rather than falling through. `--repository-token "$CODACY_PROJECT_TOKEN"` with +the secret unset is a routine CI mistake, and treating it as "no flag" would +hand the run an ambient account token — precisely the widening the precedence +rule exists to prevent. Empty *env vars* keep meaning "unset" (the test config +depends on it). Both flag and env values are trimmed. + > ⚠️ Because `CODACY_PROJECT_TOKEN` outranks `CODACY_API_TOKEN`, and it is the > variable the Codacy coverage reporter reads (so it is routinely exported > job-wide in CI), `vitest.config.mts` blanks it via `test.env` — otherwise token @@ -105,8 +112,13 @@ partial-but-sufficient `repository` dashboard. reads as a bug, and `printPullRequests([])` would claim "No open pull requests", a different and false statement. In JSON, `pullRequests` stays `[]` (so `jq '.pullRequests[]'` and `| length` keep working) and an additive - `unavailable: ["pullRequests"]` distinguishes "none" from "couldn't look". + `unavailable` array distinguishes "none" from "couldn't look". Under an account token the payload is byte-identical to before this change. + `unavailable` lists `coverageReports` too, even though no coverage key is + projected: skipping that call forces `expectsCoverage` false, which silently + suppresses the "waiting for / missing coverage reports" state on the Analysis + row — without the marker, a repo that *is* configured for coverage but has + uploaded none would look identical to a healthy one. - **`login` is account-only.** It validates against `/user`, which a repository token can never reach, and the credentials store holds a single bare token with no record of its kind. Its 401 message names the repository-token case diff --git a/src/commands/login.test.ts b/src/commands/login.test.ts index 5f75c22..3cbd980 100644 --- a/src/commands/login.test.ts +++ b/src/commands/login.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { Command } from "commander"; import { registerLoginCommand } from "./login"; import { AccountService } from "../api/client/services/AccountService"; @@ -114,6 +114,12 @@ describe("login command", () => { }); describe("--repository-token", () => { + // In afterEach, not inline: a failing assertion above must not leak + // this var into the rest of the file. + afterEach(() => { + delete process.env.CODACY_PROJECT_TOKEN; + }); + function warnings(): string { return (console.error as ReturnType).mock.calls .flat() @@ -144,7 +150,6 @@ describe("login command", () => { await program.parseAsync(["node", "test", "login", "--token", "account-token"]); expect(warnings()).not.toContain("--repository-token"); - delete process.env.CODACY_PROJECT_TOKEN; }); it("explains that a repository token cannot be used to log in", async () => { diff --git a/src/commands/logout.test.ts b/src/commands/logout.test.ts index 7b065ba..3dafdaa 100644 --- a/src/commands/logout.test.ts +++ b/src/commands/logout.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { Command } from "commander"; import { registerLogoutCommand } from "./logout"; @@ -45,6 +45,12 @@ describe("logout command", () => { }); describe("--repository-token", () => { + // In afterEach, not inline: a failing assertion above must not leak + // this var into the rest of the file. + afterEach(() => { + delete process.env.CODACY_PROJECT_TOKEN; + }); + it("warns that the flag is ignored, but still logs out", async () => { const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); vi.mocked(deleteCredentials).mockReturnValue(true); @@ -71,7 +77,6 @@ describe("logout command", () => { expect(errorSpy.mock.calls.flat().join("\n")).not.toContain( "--repository-token", ); - delete process.env.CODACY_PROJECT_TOKEN; }); }); }); diff --git a/src/commands/repository.test.ts b/src/commands/repository.test.ts index 2916236..9e226dc 100644 --- a/src/commands/repository.test.ts +++ b/src/commands/repository.test.ts @@ -946,7 +946,11 @@ describe("repository command", () => { }); describe("with a repository token", () => { - /** Mocks the three dashboard calls a repository token *can* make. */ + /** + * Mocks the two dashboard calls whose data these tests assert on. The third + * whitelisted call, `listRepositoryCommits`, is already mocked file-wide by + * `setupDefaultMocks()`. + */ function mockWhitelistedDashboardCalls() { vi.mocked(AnalysisService.getRepositoryWithAnalysis).mockResolvedValue({ data: mockRepoData as any, @@ -993,6 +997,9 @@ describe("repository command", () => { expect(output).toContain("Not shown with a repository token"); // "No open pull requests" would be a different, and false, claim. expect(output).not.toContain("No open pull requests"); + // The note is derived from the token kind, so without this the test would + // still pass if the skip were removed and the doomed call issued anyway. + expect(AnalysisService.listRepositoryPullRequests).not.toHaveBeenCalled(); }); it("emits pullRequests as an empty array plus an unavailable marker in JSON", async () => { @@ -1008,10 +1015,16 @@ describe("repository command", () => { const parsed = JSON.parse(calls[calls.length - 1][0]); // Present and iterable, so `jq '.pullRequests[]'` and `| length` still work. expect(parsed.pullRequests).toEqual([]); - expect(parsed.unavailable).toEqual(["pullRequests"]); + // Coverage is listed too: skipping it silently suppresses the + // "missing coverage reports" state, so consumers need to know. + expect(parsed.unavailable).toEqual(["pullRequests", "coverageReports"]); // The fields the auto-configuration skill reads are unaffected. expect(parsed.repository.fileCount).toBe(83); expect(parsed.repository.repository.standards).toBeDefined(); + // Same reason as above: `unavailable` follows the token kind, so assert + // the call really was skipped rather than merely reported as skipped. + expect(AnalysisService.listRepositoryPullRequests).not.toHaveBeenCalled(); + expect(RepositoryService.listCoverageReports).not.toHaveBeenCalled(); }); it("still supports --reanalyze", async () => { @@ -1101,6 +1114,31 @@ describe("repository command", () => { expect(output).not.toContain("Not shown with a repository token"); }); + it("marks pull requests unavailable in JSON when the call fails", async () => { + vi.mocked(AnalysisService.getRepositoryWithAnalysis).mockResolvedValue({ + data: mockRepoData as any, + }); + vi.mocked(AnalysisService.issuesOverview).mockResolvedValue({ + data: { counts: mockIssuesCounts }, + }); + vi.mocked(AnalysisService.listRepositoryPullRequests).mockRejectedValue( + Object.assign(new Error("Forbidden"), { status: 403 }), + ); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "repository", "gh", "test-org", "test-repo", + "--output", "json", + ]); + + const calls = (console.log as ReturnType).mock.calls; + const parsed = JSON.parse(calls[calls.length - 1][0]); + // The marker tracks "couldn't look", whatever the cause — a failed call + // under an account token, not just a skipped one under a repo token. + expect(parsed.unavailable).toEqual(["pullRequests"]); + expect(parsed.pullRequests).toEqual([]); + }); + it("omits the unavailable marker from JSON", async () => { vi.mocked(AnalysisService.getRepositoryWithAnalysis).mockResolvedValue({ data: mockRepoData as any, diff --git a/src/commands/repository.ts b/src/commands/repository.ts index 3fc5773..b3d22d7 100644 --- a/src/commands/repository.ts +++ b/src/commands/repository.ts @@ -583,6 +583,13 @@ Examples: const expectsCoverage = !!(coverageReportsResponse as any).data?.hasCoverageOverview; const hasCoverageData = data.coverage?.coveragePercentage !== undefined; + const unavailableSections = [ + ...(prsUnavailable ? ["pullRequests"] : []), + // Only skipped, never merely failed — listCoverageReports is guarded + // by fetchIfAccountToken alone. + ...(auth.kind === "account-token" ? [] : ["coverageReports"]), + ]; + if (format === "json") { printJson(pickDeep({ repository: { @@ -593,9 +600,15 @@ Examples: // and `| length` keep working. `unavailable` is what distinguishes // "no open pull requests" from "couldn't look"; pickDeep drops // undefined, so it stays absent whenever the data is real. + // + // Coverage reports are listed too even though no coverage key is + // projected: skipping them forces `expectsCoverage` false, which + // silently suppresses the "missing/waiting for coverage reports" + // state. Without this a repo that *is* configured for coverage but + // has uploaded none is indistinguishable from a healthy one. pullRequests, issuesOverview: issuesCounts, - unavailable: prsUnavailable ? ["pullRequests"] : undefined, + unavailable: unavailableSections.length ? unavailableSections : undefined, }, [ // About "repository.repository.provider", diff --git a/src/index.ts b/src/index.ts index bc19ac6..d6cbd6e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -3,7 +3,7 @@ import { Command } from "commander"; import { OpenAPI } from "./api/client/core/OpenAPI"; import { cliVersion } from "./version"; import { getOutputFormat } from "./utils/output"; -import { repositoryTokenOption } from "./utils/auth"; +import { BASE_HEADERS, repositoryTokenOption } from "./utils/auth"; import { maybeNotifyUpdate } from "./utils/update-check"; import { registerInfoCommand } from "./commands/info"; import { registerRepositoriesCommand } from "./commands/repositories"; @@ -30,10 +30,9 @@ OpenAPI.BASE = (process.env.CODACY_API_BASE_URL || "https://app.codacy.com").rep // known until a command resolves its auth — every API path installs headers // first, via `resolveAuth()` in commands or `applyAccountToken()` in `login`. // Baking in `api-token` would send an empty account header on every -// repository-token run. -OpenAPI.HEADERS = { - "X-Codacy-Origin": "cli-cloud-tool", -}; +// repository-token run. Shared with `applyAuthHeaders`, which replaces +// OpenAPI.HEADERS wholesale and would otherwise drop anything set only here. +OpenAPI.HEADERS = { ...BASE_HEADERS }; program .name("codacy-cloud-cli") diff --git a/src/utils/auth.test.ts b/src/utils/auth.test.ts index 0f376d9..9d6d44b 100644 --- a/src/utils/auth.test.ts +++ b/src/utils/auth.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { Command } from "commander"; import { OpenAPI } from "../api/client/core/OpenAPI"; import { + EMPTY_REPOSITORY_TOKEN_MESSAGE, NO_TOKEN_MESSAGE, applyAccountToken, applyAuthHeaders, @@ -68,6 +69,21 @@ describe("resolveAuthFromToken precedence", () => { expect(loadCredentials).not.toHaveBeenCalled(); }); + it("prefers CODACY_PROJECT_TOKEN over stored credentials", () => { + // The adjacent cases only pin this ordering transitively (project > api, + // api > credentials). Asserted directly so reordering the credentials + // lookup above the project-token check can't slip through. + process.env.CODACY_PROJECT_TOKEN = "env-project"; + vi.mocked(loadCredentials).mockReturnValue("stored"); + + expect(resolveAuthFromToken()).toEqual({ + kind: "repository-token", + token: "env-project", + source: "CODACY_PROJECT_TOKEN", + }); + expect(loadCredentials).not.toHaveBeenCalled(); + }); + it("falls back to stored credentials last", () => { vi.mocked(loadCredentials).mockReturnValue("stored"); @@ -82,6 +98,29 @@ describe("resolveAuthFromToken precedence", () => { expect(() => resolveAuthFromToken()).toThrow(NO_TOKEN_MESSAGE); }); + it("refuses an explicitly empty --repository-token instead of falling back", () => { + // The CI footgun this guards: `--repository-token "$CODACY_PROJECT_TOKEN"` + // with the secret unset. Falling through would run with the ambient account + // token — far wider access than the scoped run that was asked for. + process.env.CODACY_API_TOKEN = "env-account"; + vi.mocked(loadCredentials).mockReturnValue("stored"); + + expect(() => resolveAuthFromToken("")).toThrow(EMPTY_REPOSITORY_TOKEN_MESSAGE); + expect(() => resolveAuthFromToken(" ")).toThrow(EMPTY_REPOSITORY_TOKEN_MESSAGE); + // Crucially, no header is installed at all on the way out — the throw + // precedes applyAuthHeaders, so the ambient account token never reaches + // the client. + expect(OpenAPI.HEADERS).toBeUndefined(); + }); + + it("trims a padded --repository-token", () => { + expect(resolveAuthFromToken(" rt ")).toEqual({ + kind: "repository-token", + token: "rt", + source: "flag", + }); + }); + it("ignores an empty CODACY_PROJECT_TOKEN", () => { process.env.CODACY_PROJECT_TOKEN = ""; process.env.CODACY_API_TOKEN = "env-account"; diff --git a/src/utils/auth.ts b/src/utils/auth.ts index 3adfd31..1182bb6 100644 --- a/src/utils/auth.ts +++ b/src/utils/auth.ts @@ -44,6 +44,13 @@ export const NO_TOKEN_MESSAGE = "No API token found. Provide --repository-token, set CODACY_PROJECT_TOKEN, " + "set CODACY_API_TOKEN, or run 'codacy login'."; +export const EMPTY_REPOSITORY_TOKEN_MESSAGE = + "--repository-token was given an empty value. This is refused rather than " + + "ignored: falling back to an account token would silently run with far wider " + + "access than the scoped run you asked for. Check the variable you passed " + + "(e.g. --repository-token \"$CODACY_PROJECT_TOKEN\" with the secret unset), " + + "or drop the flag to use an account token deliberately."; + const REPOSITORY_TOKEN_FLAGS = "--repository-token "; const REPOSITORY_TOKEN_DESCRIPTION = "repository (project) token, scoped to a single repository (env: CODACY_PROJECT_TOKEN)"; @@ -78,11 +85,21 @@ export function repositoryTokenOption(): Option { */ export function repositoryTokenFlag(command: Command): string | undefined { const own = command.opts().repositoryToken; - if (typeof own === "string" && own !== "") return own; + if (typeof own === "string") return own; const inherited = command.optsWithGlobals().repositoryToken; - return typeof inherited === "string" && inherited !== "" ? inherited : undefined; + return typeof inherited === "string" ? inherited : undefined; } +/** + * Headers sent on every request regardless of token kind. Single source of + * truth: `applyAuthHeaders` replaces `OpenAPI.HEADERS` wholesale, so anything + * only set at startup in `src/index.ts` would be dropped by the first command + * that resolves auth. + */ +export const BASE_HEADERS: Record = { + "X-Codacy-Origin": "cli-cloud-tool", +}; + /** * Point the generated client at a token. Auth is process-global * (`OpenAPI.HEADERS`) because the generated services never accept per-request @@ -94,8 +111,8 @@ export function repositoryTokenFlag(command: Command): string | undefined { export function applyAuthHeaders(auth: RemoteAuth): void { const tokenHeader = auth.kind === "account-token" ? "api-token" : "project-token"; OpenAPI.HEADERS = { + ...BASE_HEADERS, [tokenHeader]: auth.token, - "X-Codacy-Origin": "cli-cloud-tool", }; } @@ -132,18 +149,26 @@ const SOURCE_DESCRIPTIONS: Record = { * An explicit flag wins outright — it never even looks for an account token, so * a deliberately-scoped run can't be silently widened by an ambient env var or * a stale login lying around. + * + * An explicitly-passed but *empty* flag is an error, not a miss. `--repository-token + * "$CODACY_PROJECT_TOKEN"` with the secret unset is a routine CI mistake, and + * treating it as "no flag" would hand the run an ambient account token — the + * widening this function exists to prevent. Empty *env vars* are different: they + * mean "unset" by convention (the test config relies on it), so they fall through. */ function pickAuth(flagToken?: string): RemoteAuth { - if (flagToken) { - return { kind: "repository-token", token: flagToken, source: "flag" }; + if (flagToken !== undefined) { + const token = flagToken.trim(); + if (!token) throw new Error(EMPTY_REPOSITORY_TOKEN_MESSAGE); + return { kind: "repository-token", token, source: "flag" }; } - const projectEnv = process.env.CODACY_PROJECT_TOKEN; + const projectEnv = process.env.CODACY_PROJECT_TOKEN?.trim(); if (projectEnv) { return { kind: "repository-token", token: projectEnv, source: "CODACY_PROJECT_TOKEN" }; } - const accountEnv = process.env.CODACY_API_TOKEN; + const accountEnv = process.env.CODACY_API_TOKEN?.trim(); if (accountEnv) { return { kind: "account-token", token: accountEnv, source: "CODACY_API_TOKEN" }; } @@ -234,6 +259,8 @@ export function repositoryTokenSkipNote(what: string): string { * trains users to ignore the warning that does matter. */ export function warnUnusedRepositoryToken(command: Command, detail: string): void { - if (!repositoryTokenFlag(command)) return; + // Presence, not truthiness: `--repository-token ""` was still typed by the + // user, and is still being ignored here. + if (repositoryTokenFlag(command) === undefined) return; console.error(ansis.yellow(`Warning: --repository-token is ignored by ${detail}`)); } From 3528d45ee1395bed04b05e6679d97e25eee18e05 Mon Sep 17 00:00:00 2001 From: Alejandro Rizzo Date: Tue, 11 Aug 2026 12:09:13 +0100 Subject: [PATCH 5/5] chore: Shorten the data-dependent guard exception in AGENTS.md The bullet added in the previous commit ran to 41 words. Split into four short instructions, one per point. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 1f46ba3..c934732 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -97,7 +97,10 @@ codacy-cloud-cli/ - mixed → `resolveAuth(this)` plus `requireAccountToken(auth, "", "")` per unsupported flag, or `fetchIfAccountToken(...)` to skip an unsupported sub-call - **Guards must run before any request**, and before `resolveRepoArgs()` — that shells out to git and prints an auto-detection line, which is misleading ahead of a refusal. - Exception: a command whose endpoints are all whitelisted needs no guard at all — `resolveAuth(this)` alone is correct (see `tool`, `patterns`, `pattern`). - - Exception: a guard whose condition depends on fetched data must run after that fetch (see `guardForceUnlink` in `tools.ts`, which needs the coding-standard count). Keep those reads whitelisted so nothing doomed is sent, and still refuse before any prompt or mutation. + - Exception: a data-dependent guard runs after the fetch it depends on. + - Example: `guardForceUnlink` in `tools.ts` needs the coding-standard count. + - Keep those reads whitelisted, so nothing doomed is sent. + - Refuse before any prompt or mutation even so. - If an operation's scope is genuinely unclear, don't guess a guard. - Confirm the whitelist against the API owners instead. - Record the answer in `SPECS/repository-tokens.md`.