From e21dcbfe7d337b3f922905c025504de599bc41d2 Mon Sep 17 00:00:00 2001 From: FZ2000 Date: Thu, 30 Jul 2026 21:54:51 -0700 Subject: [PATCH 1/2] Close the PyPI-name exposure, drop the unnecessary OAuth secret, add release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from a post-publication review, in descending order of how badly they can bite. THE PYPI NAME IS UNCLAIMED AND THE README TELLS PEOPLE TO INSTALL IT pypi.org/pypi/da-cli/json -> 404 (free, anyone can register it) pypi.org/pypi/dacli/json -> 200 (an unrelated project) README.md:112 -> `pipx install da-cli` The line was captioned "not yet available", but the command sat right there to copy, and the name is registrable by anyone today. This package installs a `da` console script onto PATH and handles DeviantArt OAuth tokens in the macOS Keychain, so an impostor under that name is not a harmless mistake — it is a credential-handling binary arriving by the route the project's own README recommended. The section now states plainly that the name is unregistered, that anything currently under it is not this project, and why that matters here specifically. It also notes the near-miss `dacli`, since a mistyped install already lands on someone else's package. The real fix is to claim the name, which is what release.yml is for. CLIENT TYPE: RECOMMEND PUBLIC, NOT CONFIDENTIAL The setup guide's recommended value was Confidential, and §3c then walks the user through storing a client_secret. That is the wrong default for what this tool is. DeviantArt's own registration form — visible in the screenshot the guide tells you to match — says: "Choose Public if your code is visible to users or runs on their device. Examples: desktop apps ... Your app will authenticate with client_id and PKCE — no secret required." da-cli is a desktop CLI on the user's machine, and the code already agrees: `client_secret` is guarded at both call sites (auth.py:122, 830) while PKCE with S256 does the actual work (auth.py:781, 828). Verified both before changing the recommendation. So the happy path was creating a long-lived secret that PKCE exists to make unnecessary — expanding the credential surface of a tool whose entire threat model is about local credential storage. Recommending Public removes one secret at rest from every new install. Confidential is still supported and still documented, as the aside it should be. RELEASE WORKFLOW Tag-triggered, publishing to PyPI via Trusted Publishing (OIDC) rather than a stored API token — a PyPI token is valid indefinitely and lives in repo secrets, while an OIDC token is minted per run and expires in 15 minutes. PEP 740 attestations come free: they default to on for trusted publishing, so there is no second signing step to maintain. `id-token: write` is scoped to the publish job alone, which does nothing but download an artifact and upload it. Nothing else in the workflow can reach a publishing credential. Two guards worth calling out: - The tag must equal `dacli.__version__`. The version is `dynamic`, so nothing otherwise couples the two, and without the check you can ship v0.5.0 containing a package that reports 0.4.0. Tested both ways: it passes on v0.3.0 and fires on v0.4.0. - The wheel is installed into a clean venv and every submodule imported via pkgutil, plus a py.typed assertion. Every other job runs `da` out of the checkout, where `import dacli` always resolves — which masks packaging faults completely. Release notes come from CHANGELOG.md rather than --generate-notes: generated notes are a list of merged PRs, and this project has three. That extraction had a bug I caught by testing it rather than assuming — `##+` matched the `### Added` subsection directly beneath the heading, collapsing the body to zero characters. Anchored to exactly two hashes it returns 1551 characters for 0.3.0, and falls back cleanly for a tag with no section. The file must not be renamed: the workflow filename is part of the identity PyPI validates the OIDC token against. Header says so. Setup that cannot live in the repo — a PyPI pending publisher, the `pypi` environment with a v* tag rule, and tag protection — is documented in the workflow header, including why the environment is worth having on a solo project (PyPI refuses a token minted outside it) and why required reviewers are not (you would approve your own deploy, or deadlock). --- .github/workflows/release.yml | 164 ++++++++++++++++++++++++++++++++++ README.md | 14 +-- docs/getting-started.md | 2 +- 3 files changed, 173 insertions(+), 7 deletions(-) create mode 100644 .github/workflows/release.yml diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 0000000..4265cf7 --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,164 @@ +name: Release + +# --------------------------------------------------------------------------- +# Tag-triggered release: build, verify, publish to PyPI, cut a GitHub Release. +# +# DO NOT RENAME THIS FILE. The workflow filename is part of the identity PyPI +# checks when it validates the OIDC token. Renaming it silently breaks +# publishing, and the resulting error does not point here. +# +# ONE-TIME SETUP, before the first tag (none of it lives in this repo): +# +# 1. PyPI -> your account sidebar -> Publishing -> add a *pending* publisher. +# It is under the account, not a project, because `da-cli` does not exist +# on PyPI yet; a pending publisher converts to a real one on first +# publish and does not reserve the name before then. +# owner: FZ2000 repo: da-cli workflow: release.yml environment: pypi +# +# 2. This repo -> Settings -> Environments -> new environment named `pypi`, +# with a deployment branch/tag rule allowing `v*` only. +# +# Do NOT add required reviewers on a solo project: "prevent self-review" +# is off by default so you would merely approve your own deploy, and +# turning it on would deadlock you. The environment earns its place for a +# different reason — PyPI refuses an OIDC token minted outside the +# registered environment, so the constraint is enforced by a third party +# rather than by your own discipline. +# +# 3. Settings -> Tags -> protect `v*` so only you can create one. +# +# WHY OIDC AND NOT AN API TOKEN: a PyPI API token is valid indefinitely and +# lives in your repo secrets. An OIDC token is minted per-run and expires in +# 15 minutes, and there is nothing to leak in between. +# --------------------------------------------------------------------------- + +on: + push: + tags: ["v*"] + +# Least privilege at the top. The publish job raises `id-token` for itself +# only — GitHub's own docs discourage granting it workflow-wide. +permissions: + contents: read + +jobs: + build: + name: Build and verify + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - uses: actions/checkout@v7 + with: + # This job needs no credentials; a checkout that leaves one behind + # is a needless one. + persist-credentials: false + + - uses: actions/setup-python@v7 + with: + python-version: "3.13" + cache: pip + cache-dependency-path: pyproject.toml + + - name: Tag must match dacli.__version__ + # The version is `dynamic` from dacli.__version__, so nothing + # otherwise couples it to the tag. Without this check you can ship + # v0.5.0 containing a package that reports 0.4.0, and PyPI will + # cheerfully accept it. + run: | + v=$(python -c 'import dacli; print(dacli.__version__)') + if [ "v$v" != "$GITHUB_REF_NAME" ]; then + echo "::error::tag $GITHUB_REF_NAME does not match dacli.__version__ ($v)" + exit 1 + fi + echo "::notice::tag $GITHUB_REF_NAME matches dacli.__version__" + + - name: Build sdist and wheel + run: | + python -m pip install --upgrade pip build + rm -rf build dist ./*.egg-info + python -m build + + - name: The wheel must work without the source tree + # Every other job runs `da` out of the checkout, where `import dacli` + # resolves to the source and therefore always works. That masks + # packaging faults completely — a wheel missing a subpackage passes + # all of them. + run: | + python -m venv /tmp/relcheck + /tmp/relcheck/bin/pip install --quiet dist/*.whl + /tmp/relcheck/bin/da --version + /tmp/relcheck/bin/da --help > /dev/null + /tmp/relcheck/bin/python - <<'PY' + import pathlib, pkgutil, importlib, dacli + assert (pathlib.Path(dacli.__file__).parent / "py.typed").exists(), "py.typed missing from the wheel" + for m in pkgutil.walk_packages(dacli.__path__, "dacli."): + importlib.import_module(m.name) + print("wheel imports cleanly, py.typed present") + PY + + - uses: actions/upload-artifact@v7 + with: + name: dist + path: dist/ + if-no-files-found: error + + publish: + name: Publish to PyPI + needs: build + runs-on: ubuntu-latest + timeout-minutes: 15 + environment: pypi + permissions: + # Job-scoped, deliberately. This is the only job that mints an OIDC + # token, and it does nothing but download an artifact and upload it — + # keep it that way, so a compromised action elsewhere in CI has no path + # to a publishing credential. + id-token: write + steps: + - uses: actions/download-artifact@v7 + with: + name: dist + path: dist/ + + - name: Publish + # No username, no password, no token. Attestations (PEP 740, backed by + # Sigstore) are on by default for Trusted Publishing, so there is + # nothing extra to configure and no second signing step to maintain. + uses: pypa/gh-action-pypi-publish@release/v1 + + github-release: + name: GitHub Release + needs: publish + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: write # to create the release + steps: + - uses: actions/checkout@v7 + - uses: actions/download-artifact@v7 + with: + name: dist + path: dist/ + - name: Create the release + env: + GH_TOKEN: ${{ github.token }} + # --verify-tag refuses to invent a tag that does not already exist. + # Notes come from CHANGELOG.md rather than --generate-notes: generated + # notes are a list of merged pull requests, and a hand-written + # changelog says more than a list of PR titles. + run: | + notes=$(python3 - <<'PY' + import pathlib, re, os + tag = os.environ["GITHUB_REF_NAME"].lstrip("v") + text = pathlib.Path("CHANGELOG.md").read_text() + # `\n## ` is exactly two hashes on purpose: `##+` also matches the + # `### Added` subsection directly below the heading, which collapses + # the captured body to nothing. + m = re.search(rf"^## \[?{re.escape(tag)}\]?[^\n]*\n(.*?)(?=\n## |\Z)", text, re.S | re.M) + print(m.group(1).strip() if m else f"See CHANGELOG.md for {tag}.") + PY + ) + gh release create "$GITHUB_REF_NAME" dist/* \ + --verify-tag \ + --title "$GITHUB_REF_NAME" \ + --notes "$notes" diff --git a/README.md b/README.md index bedbb36..974b6a1 100644 --- a/README.md +++ b/README.md @@ -105,13 +105,15 @@ cd ~/Documents/da-cli ./install.sh # copies da + the dacli package to ~/.local/share/da-cli/ and symlinks ~/.local/bin/da → that copy ``` -**Option B — pip install** — *not yet available; da-cli is not published to -PyPI. Use Option A.* When it is published, this will work: +**Option B — pip install.** Not available: **da-cli is not published to +PyPI**, and the name is currently unregistered. Use Option A. -```bash -pipx install da-cli # or: pip install da-cli -da --version -``` +Do not `pip install da-cli` on the strength of this README. Until the name +appears here as published, anything under it on PyPI is not this project — +and since this package installs a `da` command onto your `PATH` and handles +DeviantArt OAuth tokens, installing an impostor is not a harmless mistake. + +Note also that `dacli` (no hyphen) on PyPI is an unrelated project. Python 3.10+ required (uses `argparse.BooleanOptionalAction` and `X | None` syntax). No third-party runtime dependencies. diff --git a/docs/getting-started.md b/docs/getting-started.md index 1485cb5..d592325 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -128,7 +128,7 @@ Here's what each field means: | **Title** | `da-cli` | Any name you like; this is just a label. | | **Description** | *(leave blank)* | Optional; not used by da-cli. | | **OAuth2 Redirect URI Whitelist** | `https://localhost:8765/` | This is the address da-cli listens on for the login callback. **Must match exactly** — including the trailing slash. | -| **Client type** | **Confidential** | Issues a `client_secret`, which da-cli stores in the macOS Keychain and sends on the token exchange. It is **optional** — every code path guards on it, and without one da-cli authenticates as a public client using PKCE, which is what DeviantArt's own form recommends for desktop apps. Choose Confidential if you want the secret; Public works too. | +| **Client type** | **Public** | da-cli is a desktop tool running on your machine, which is exactly the case DeviantArt's own form describes: *"Your app will authenticate with client_id and PKCE — no secret required."* Public means there is no `client_secret` to store, leak, or rotate. Choose **Confidential** only if you specifically want one; da-cli supports it (every code path guards on `client_secret` being present) but does not need it, and the screenshot below predates this recommendation. | | **Download URL** | *(leave blank)* | Not used by da-cli. | | **Original URLs Whitelist** | *(leave blank)* | Not used by da-cli. | From 58aed608b00b63ba358a023f93aeb6bea31acb4e Mon Sep 17 00:00:00 2001 From: FZ2000 Date: Thu, 30 Jul 2026 22:00:25 -0700 Subject: [PATCH 2/2] release: SHA-pin every action in the publishing workflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL's `actions` queries flagged release.yml on the PR that introduced it — "Unpinned tag for a non-immutable Action" — and blocked the merge. The finding is correct and worth acting on rather than dismissing. release.yml is the only workflow that mints a publishing credential (`id-token: write`). A tag is mutable: whoever controls an action's repo can repoint `@v7` at new code, and that code would then run in the job that publishes to PyPI. This is not hypothetical — it is the shape of both the tj-actions and reviewdog compromises. All seven references are now pinned to a full commit SHA, each with a `# vX.Y.Z` comment. The comment is load-bearing, not decoration: Renovate disables updates for a bare SHA it cannot attribute to a version, so an uncommented pin goes stale silently and forever. With the comment, Renovate keeps it current and rewrites both together. Worth stating what this does NOT fix, since SHA-pinning is easy to over-trust: it locks the outer reference only. A composite action that internally does `uses: some/action@v1` still resolves a mutable tag the caller cannot reach. That is the reason the publish job is kept to two steps — download an artifact, upload it — and does nothing else. CI's own workflows are deliberately left on tags for now: those jobs run with `contents: read` and hold no secrets, and Renovate is configured with `helpers:pinGitHubActionDigests` to pin them once the app is installed. Pinning by hand what a bot is configured to pin correctly would just create a second source of truth. --- .github/workflows/release.yml | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 4265cf7..2ac3b76 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -27,6 +27,16 @@ name: Release # # 3. Settings -> Tags -> protect `v*` so only you can create one. # +# WHY EVERY ACTION HERE IS SHA-PINNED: this is the only workflow that mints a +# publishing credential, so a moved tag on a third-party action is a direct +# path to a compromised release. Each pin carries a `# vX.Y.Z` comment because +# Renovate disables updates for a bare SHA it cannot attribute to a version — +# the comment is what keeps a pin maintained rather than merely frozen. +# +# This locks the OUTER reference only. A composite action that internally uses +# a mutable tag is still reachable, which is why the publish job does nothing +# but download an artifact and upload it. +# # WHY OIDC AND NOT AN API TOKEN: a PyPI API token is valid indefinitely and # lives in your repo secrets. An OIDC token is minted per-run and expires in # 15 minutes, and there is nothing to leak in between. @@ -47,13 +57,13 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 15 steps: - - uses: actions/checkout@v7 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: # This job needs no credentials; a checkout that leaves one behind # is a needless one. persist-credentials: false - - uses: actions/setup-python@v7 + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: "3.13" cache: pip @@ -96,7 +106,7 @@ jobs: print("wheel imports cleanly, py.typed present") PY - - uses: actions/upload-artifact@v7 + - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: dist path: dist/ @@ -115,7 +125,7 @@ jobs: # to a publishing credential. id-token: write steps: - - uses: actions/download-artifact@v7 + - uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 with: name: dist path: dist/ @@ -124,7 +134,7 @@ jobs: # No username, no password, no token. Attestations (PEP 740, backed by # Sigstore) are on by default for Trusted Publishing, so there is # nothing extra to configure and no second signing step to maintain. - uses: pypa/gh-action-pypi-publish@release/v1 + uses: pypa/gh-action-pypi-publish@dc37677b2e1c63e2034f94d8a5b11f265b73ba33 # v1.14.2 github-release: name: GitHub Release @@ -134,8 +144,8 @@ jobs: permissions: contents: write # to create the release steps: - - uses: actions/checkout@v7 - - uses: actions/download-artifact@v7 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 with: name: dist path: dist/