From b18fafdd670410fbddb8b3fb009bad8c5b85c664 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Tue, 25 Aug 2026 13:43:30 -0400 Subject: [PATCH 1/2] docs(agents): patch tags come off a release branch, so --contains is empty by design MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The retracted finding on this PR surfaced the actual reason my badge check and the reviewer's disagreed, and it is worth recording because it will recur. Harper cuts patch releases from the v5.2 release branch, not from main. v5.2.5's commit (6a1699f1) diverges from main at 374408e3 and is not an ancestor of it. So git tag --contains on a main merge commit returns nothing for any v5.2.x tag no matter how recent that tag is — the feature can only appear there via a back-port. That means an empty --contains says "the next minor", not "nobody has released since", and the existence of a newer patch tag is not evidence against a v5.3.0 badge. Mis-reading that is what produced four Critical findings here. Also notes to compare ^{commit} rather than tag objects: an annotated tag's own SHA differs from the commit it points at, which is why the reviewer and I cited v5.2.5 as two different hashes and were both right. --- AGENTS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 143bcf67..d7f5d7c9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -56,6 +56,8 @@ Prefer plain ASCII characters in Markdown unless a typographic character is genu - Derive the version from the core release the change ships in, stripping prerelease suffixes (`5.1.0-beta.1` → `v5.1.0`). - **Determine that release from the core repo's git tags, not from the feature branch's `package.json`.** A branch reading `5.2.0-beta.3` says which release was open when the branch started, not which one the change lands in — if a release is cut before the feature merges, the badge is silently wrong. Check `git tag --sort=-creatordate` for the newest release, and `git tag --contains ` for whether the change is in one; a merged-but-untagged feature ships in the _next_ version, which may be a minor bump. Re-check on every refresh pass of a long-lived docs PR, because a release cut between passes invalidates a badge that was correct when written. - **Fetch tags and confirm the tag object before trusting the answer.** `git tag --contains` is only as good as the local tag it compares against: a stale or divergent tag of the same name gives a confidently wrong answer in either direction. Run `git fetch --tags`, then check `git rev-parse ` against `git ls-remote --tags origin `. When a badge is disputed, the decisive check is not `--contains` at all but whether the feature's files exist at the tag — `git ls-tree -r --name-only | grep `, or `git show :` — since that cannot be confounded by ancestry or by which tag your clone happens to hold. +- **Patch tags are cut from a release branch, not from `main`, so `--contains` returns nothing for them by design.** `v5.2.5`'s commit diverges from `main` and is not an ancestor of it — the `v5.2` line is its own branch with selective cherry-picks. A feature merged to `main` therefore appears in **no** `v5.2.x` tag however recent that tag is, and shows up only when a release is cut from `main` (the next minor) or someone back-ports the commit. So an empty `--contains` on a main merge means "the next minor", not "nobody has released since" — and a newer patch tag existing is not evidence against it. Check the release branch directly (`git branch -r --contains `) if you need to know whether a back-port happened. +- **Compare commits, not tag objects.** An annotated tag's own SHA differs from the commit it points at, so two people can cite `v5.2.5` as different hashes and both be right. Use `^{commit}`. - Each minor release gets a file under `release-notes//` (e.g. `release-notes/v5-lincoln/5.1.md`); the sidebar picks it up automatically. - Absolute links from `release-notes/` (or `learn/`) into current reference docs use the versioned path `/reference/v5/...` — the reference plugin maps the current version to the `v5` URL path. - When documenting a change from a core/pro PR, cross-link the feature PR and the docs PR in both descriptions. From 69e1e30873ef5b78e878878e29e3e32e0f76eaeb Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Tue, 25 Aug 2026 13:56:51 -0400 Subject: [PATCH 2/2] docs(security): fix a policy example that would be rejected, and three gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four findings from @kriszyp, all verified in source first. **The `operations`-scope example could not have been accepted.** It pinned the repository and gated the ref via `environment` but pinned no workflow, so `assertPolicyIsSpecific` would refuse it — in a block presented as copyable. Added `workflow_ref`. Worth noting the failure mode: the page states the three structural requirements correctly a few paragraphs above, and I still wrote an example violating one of them, which is an argument for checking examples against the rules the same page states rather than against intent. **Canonical audience form applies to every issuer, not just GitHub Actions.** `assertAudienceIsCanonical` requires an http(s) audience to carry an explicit port and a trailing slash — `https://my-instance.example.com:9925/`, not `https://my-instance.example.com` — because the audience is matched byte-for-byte and the CLI requests its token for the normalized target. A plausible-looking generic-issuer value is rejected at write time. Documented ahead of the specificity discussion, and the "generic audiences are not checked" line now says what it does *not* cover. **`list_oidc_trust` can return `invalid_reason`**, set when a policy names a user that was later deleted or deactivated. This matters more than a missing field usually would, because the docs send readers to `list_oidc_trust` to diagnose an exchange rejection that deliberately says nothing — and a policy carrying `invalid_reason` is the fastest answer available. Documented with that framing. **The one-style table still sent CI to a refresh token.** It now leads with workload identity where the runtime supports it, refresh token as fallback. I had made this edit earlier and lost it: when the OIDC content was extracted to this branch I re-applied the precedence entry and the section but not the table row, so the page recommended a rotatable secret three paragraphs above telling readers they need no stored credential. --- reference/cli/authentication.md | 10 +++++----- reference/operations-api/operations.md | 12 ++++++++++-- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/reference/cli/authentication.md b/reference/cli/authentication.md index 78c5d524..8dd4275e 100644 --- a/reference/cli/authentication.md +++ b/reference/cli/authentication.md @@ -50,11 +50,11 @@ For remote Operations API commands, the CLI uses the first complete authenticati :::tip **Configure one credential style per context, not two.** Precedence exists to resolve a conflict, but it resolves it silently, and the ways this page describes for authentication to go wrong all need two styles live at once: a payload `username=`/`password=` pair takes over when a token stops resolving, a blank token variable hands the run to whatever saved login the machine has. Pick one and leave the others unset: -| Context | Use | -| ----------------------- | ----------------------------------- | -| CI/CD pipeline | `HARPER_CLI_REFRESH_TOKEN` | -| A one-off admin command | `auth_username=` / `auth_password=` | -| Local development | `harper login` | +| Context | Use | +| ----------------------- | ---------------------------------------------------------------------------------------------------------------- | +| CI/CD pipeline | [Workload identity](#workload-identity-oidc) where the runtime supports it; otherwise `HARPER_CLI_REFRESH_TOKEN` | +| A one-off admin command | `auth_username=` / `auth_password=` | +| Local development | `harper login` | Setting a token _and_ leaving `username=`/`password=` on the command is the combination that turns a token failure into an identity change rather than an error. One caveat: this rule bounds _which_ credential is used, not what happens when none resolves. A loopback node authorizes a request that arrives with no credential at all as superuser, so the style matters less there than whether a credential is attached — see the refresh-behavior note below. ::: diff --git a/reference/operations-api/operations.md b/reference/operations-api/operations.md index ba3981e8..ea650ad3 100644 --- a/reference/operations-api/operations.md +++ b/reference/operations-api/operations.md @@ -709,7 +709,11 @@ The user the policy names is the privilege boundary: a matching run gets that us "audience": "https://my-instance.harperdb.io:9925/", "user": "ci-deploy", "operations": ["deploy_component", "get_deployment", "restart_service"], - "claims": { "repository_id": "67890", "environment": "production" } + "claims": { + "repository_id": "67890", + "workflow_ref": "HarperFast/my-app/.github/workflows/deploy.yml@refs/heads/main", + "environment": "production" + } } ``` @@ -735,9 +739,11 @@ A scoped token also cannot trade itself for a browser session: `create_authentic A constrained claim that is **absent** from the token fails rather than passes, so a policy cannot be weakened by an issuer that stops emitting a claim. +**An http(s) audience must be the exact string the CI client asks for**, which means an explicit port and a trailing slash — `https://my-instance.example.com:9925/`, not `https://my-instance.example.com`. This is checked for **every** issuer, not only GitHub Actions, and the shorter form is rejected at write time. The audience is matched byte-for-byte at verification, and the CLI requests its token for the normalized target, which supplies `:9925` and the trailing slash — so a policy written without them could never authenticate. An audience that is not an http(s) URL (an `api://` identifier, a bare GUID) is not constrained. + **The audience should identify this instance.** For GitHub Actions, Harper rejects the provider's shared default — anything shaped like `https://github.com/` — because that value is shared by every repository under the owner, so accepting it would make a token minted by any of them valid here. -That check is a guard against the one known-dangerous value, not a proof of correctness: Harper does not compare the audience against its own identity, so an arbitrary or mistyped value is accepted at write time and instead fails to match at exchange time, when the CLI derives the audience from its target URL. Use the instance URL your CI targets. For an issuer with no registered profile the audience is not checked for specificity at all, and the required `sub` pin is what binds the policy to one principal. +That check is a guard against the one known-dangerous value, not a proof of correctness: Harper does not compare the audience against its own identity, so an arbitrary or mistyped value is accepted at write time and instead fails to match at exchange time, when the CLI derives the audience from its target URL. Use the instance URL your CI targets. For an issuer with no registered profile the audience is not checked for _specificity_ — the canonical-form rule above still applies — and the required `sub` pin is what binds the policy to one principal. ##### Policy specificity for GitHub Actions @@ -810,6 +816,8 @@ Lists every policy, **including disabled ones**, sorted by `id`. **super_user on Returns `{ "policies": [ ... ] }`. Each entry carries `id`, `issuer`, `audience`, `claims`, `user`, `operations` (`null` when unscoped), `enabled`, `description`, `updated_by`, and timestamps. +An entry may also carry **`invalid_reason`** — set when the policy cannot currently authenticate anyone, because it is malformed or because the user it names was deleted or deactivated after the policy was written. Check for it first when diagnosing a rejected exchange: the exchange will not say what failed, so a policy you expected to match that carries an `invalid_reason` is the fastest explanation available. + #### `drop_oidc_trust` Stops every workflow that matched the policy from exchanging again. **super_user only.** Fails with `404` if no policy has that `id`.