Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion .github/workflows/build-and-publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,13 @@ jobs:
shell: bash

- name: Build wheel with bundled CLI
run: python scripts/build_wheel.py --version "${{ inputs.version }}" --skip-sdist --clean
# Passed through the environment rather than interpolated into the
# command text: a "${{ }}" expansion is spliced in before bash ever
# sees the line, so a crafted `version` input would become shell.
# "$VERSION" is expanded by bash, as one word, and cannot be.
env:
VERSION: ${{ inputs.version }}
run: python scripts/build_wheel.py --version "$VERSION" --skip-sdist --clean
shell: bash

- uses: actions/upload-artifact@v4
Expand Down
168 changes: 142 additions & 26 deletions scripts/update_version.py
Original file line number Diff line number Diff line change
@@ -1,49 +1,165 @@
#!/usr/bin/env python3
"""Update version in pyproject.toml and __init__.py files."""
r"""Update the SDK version in pyproject.toml and src/claude_agent_sdk/_version.py.

This script is the release flow's writer: the version it is handed (ultimately
from a workflow_dispatch input, see .github/workflows/build-and-publish.yml)
is written verbatim into two source files as a *string literal* -- a TOML one
in pyproject.toml and a Python one in _version.py. That makes the value code,
not data, so it is validated against an allowlist before it is used anywhere,
and substituted with a callable replacement so it can never be reinterpreted.

Two distinct hazards, both real, both closed here:

1. **Quote breakout.** An unvalidated value written as ``version = "{v}"``
escapes its literal the moment it contains a double quote: a ``v`` of
``0.0.0"\nimport os; os.system("...")\n#`` turns _version.py into a
source file that executes on import. VERSION_PATTERN admits no quote, no
backslash, and no newline; json.dumps() then emits the literal, so the
quoting is done by a serializer rather than by an f-string.

2. **Backslash expansion in the replacement.** ``re.sub(pat, repl, s)`` with
a *string* ``repl`` does not insert it literally -- it runs it through
escape processing first, so a ``\1`` or ``\g<0>`` in the version is
expanded against the match, and a lone ``\`` raises. The replacements
below are therefore **callables**: re.sub() inserts a callable's return
value literally, with no escape pass. This is belt-and-braces given the
pattern already excludes backslashes, and it is the property the tests
pin.

The allowlist is deliberately narrow. Every version this project has ever
released -- all 121 git tags, and all 121 releases on PyPI -- is a plain
``MAJOR.MINOR.PATCH`` with no suffix. The pattern additionally admits the
PEP 440 pre-release / post-release / dev-release suffixes that PyPI would
accept for a *future* ``0.3.0rc1``-style release, so release day is not the
day someone discovers this script rejects a legitimate version. Every
character it can admit is drawn from ``[0-9a-z.]``: there is no metacharacter,
quote, backslash, or space in the admissible set, which is what makes both
hazards above unreachable rather than merely unlikely.

Note: scripts/_cli_version_validation.py (added by #1117) validates the
bundled *CLI* version and enforces the same two properties for the same
reasons. The two allowlists differ -- a CLI version has a ``-dev.…`` suffix
and dist-tags, an SDK version has PEP 440 suffixes and must satisfy PyPI --
but the validate-then-substitute-with-a-callable shape is common to both, and
they should be unified behind one shared validator once #1117 lands.
"""

import json
import re
import sys
from pathlib import Path

# MAJOR.MINOR.PATCH, plus the optional PEP 440 suffixes PyPI would accept.
# Admissible characters are exactly [0-9a-z.] -- see the module docstring for
# why that, and not the numeric arity, is the security boundary. Never widen
# this to admit a quote, a backslash, whitespace, or a shell metacharacter.
#
# Deliberately unanchored, and matched with fullmatch() rather than match():
# with "^...$" a later swap to match() would silently accept a trailing
# newline ("0.2.119\n"); unanchored, the same swap accepts an obvious prefix
# like "0.2.119; id" and fails loudly in the tests.
VERSION_PATTERN = re.compile(
r"[0-9]+\.[0-9]+\.[0-9]+" # 0.2.119 -- every version ever released
r"(?:(?:a|b|rc)[0-9]+)?" # 0.3.0rc1 (PEP 440 pre-release)
r"(?:\.post[0-9]+)?" # 0.3.0.post1 (PEP 440 post-release)
r"(?:\.dev[0-9]+)?" # 0.3.0.dev1 (PEP 440 dev release)
)

def update_version(new_version: str) -> None:
"""Update version in project files."""
# Update pyproject.toml
pyproject_path = Path("pyproject.toml")
content = pyproject_path.read_text()

# Only update the version field in [project] section
content = re.sub(
r'^version = "[^"]*"',
f'version = "{new_version}"',
def validate_version(version: str, *, source: str = "version") -> str:
"""Return ``version`` if it is a version this project could publish, else raise.

Surrounding whitespace is stripped first: a trailing "\\n" from a file read
or a stray space from YAML is unambiguous in intent. Interior whitespace is
not stripped and is rejected by the pattern.

Args:
version: The candidate version string.
source: Where the value came from, used in the error message.

Returns:
The stripped, validated version.

Raises:
ValueError: If ``version`` is not a fullmatch of VERSION_PATTERN.
"""
candidate = version.strip()

if VERSION_PATTERN.fullmatch(candidate):
return candidate

Check warning on line 90 in scripts/update_version.py

View check run for this annotation

Claude / Claude Code Review

validate_version() strips whitespace but the workflow keeps the raw padded $VERSION

validate_version() strips surrounding whitespace and accepts the padded input, but the stripped value never leaves this script — the workflow's job-level `VERSION: ${{ inputs.version }}` feeds the raw string to every downstream step, so a version with a trailing newline (possible via an API-dispatched `workflow_dispatch`) now passes the gate, publishes to PyPI, pushes the bump commit, and only then fails at `git tag -a "v$VERSION"` — a partial release needing manual repair. Consider requiring `v
Comment on lines +84 to +90

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 validate_version() strips surrounding whitespace and accepts the padded input, but the stripped value never leaves this script — the workflow's job-level VERSION: ${{ inputs.version }} feeds the raw string to every downstream step, so a version with a trailing newline (possible via an API-dispatched workflow_dispatch) now passes the gate, publishes to PyPI, pushes the bump commit, and only then fails at git tag -a "v$VERSION" — a partial release needing manual repair. Consider requiring version == version.strip() so padded input is rejected at the gate, or having the workflow consume the validated value instead of the raw input.

Extended reasoning...

What happens

validate_version() deliberately strips surrounding whitespace before matching (candidate = version.strip(), pinned by test_surrounding_whitespace_is_stripped). That's fine for the two files this script rewrites — they get the stripped value. But the stripped value never propagates back to the workflow: .github/workflows/build-and-publish.yml sets a job-level env: VERSION: ${{ inputs.version }} and every subsequent step consumes the raw input — the quota pre-flight's data["releases"][version] dict lookup, git commit -m "chore: release v$VERSION", git tag -a "v$VERSION", git push origin "v$VERSION", gh release create, and the changelog awk -v ver="$VERSION" '$2 == ver'.

Concrete walkthrough (trailing newline via API dispatch)

The GitHub UI's single-line input field can't produce a newline, but an API-dispatched workflow_dispatch (or gh workflow run -f version="$(printf '0.2.120\n')") can, since JSON string inputs admit \n. Trace it through:

  1. build-wheels: build_wheel.py --version "$VERSION"update_version.py strips → validation passes, wheels build as 0.2.120. (A quoted newline is a legal single argv word in bash — not a syntax error.)
  2. publish / Update version files: passes; both files get the clean "0.2.120".
  3. Quota pre-flight: keys data["releases"]["0.2.120\n"] — a guaranteed miss, so the already-uploaded-file skip silently never fires on a re-run after a partial upload.
  4. twine upload: the release is published to PyPI (irreversible).
  5. Commit + Push to main: the version-bump commit is pushed.
  6. Create tag: git tag -a "v0.2.120\n"fatal: not a valid tag name (git's check-ref-format rejects any whitespace in a refname; verified, exit 128). The job dies here.

Net result: package on PyPI and bump commit on main, but no tag and no GitHub release — manual repair required. Even if the tag step were reached, the release-notes awk ($2 == ver) would never match the CHANGELOG heading.

Addressing the refutation

One verifier argued this is entirely pre-existing. That's half right, and worth being precise about:

  • Trailing space (the realistic paste case): pre-existing, not a regression. packaging.version accepts and normalizes surrounding whitespace (its pattern carries anchored \s*), so pre-PR the same padded space also built, published, and died at the same git tag step. The PR changes nothing here.
  • Trailing newline: a genuine (if exotic) regression. The refutation is correct that the original claim of a "pre-PR bash syntax error" is wrong — a newline inside double quotes is legal bash. But the corrected mechanism, verified independently by two verifiers, still fails early pre-PR: the old unvalidated update_version.py wrote the literal newline into pyproject.toml's TOML string, which tomllib rejects (Illegal character '\n'), so python -m build failed in the build-wheels job, before the publish job ran — nothing built, nothing published. Post-PR, the strip launders that input through to the post-publish tag failure at step 6.

So merging as-is doesn't break any realistic dispatch path — hence nit, not blocking — but the strip does widen the accepted input set beyond what the untouched downstream workflow can handle, and it sits oddly against the PR's own stated rationale: "publishing a different string than the caller asked for is worse than failing." The strip is exactly that — the script proceeds on a different string than the caller passed, while the rest of the pipeline keeps the original.

Fix

Either of:

  • (a) Drop the strip, or additionally require version == version.strip(), so padded input is rejected at the gate like every other malformed input — a one-liner that matches the PR's fail-loud philosophy (adjust test_surrounding_whitespace_is_stripped accordingly); or
  • (b) Have a validation step echo the canonical version to $GITHUB_OUTPUT and make later workflow steps consume that instead of the raw input.

Option (a) is simpler and closes the space-padded pre-existing hazard too.

# "v0.2.119" is the single most likely typo -- the tags carry a leading
# "v" but the version does not. Say so, rather than printing a regex and
# leaving the reader to spot it. Not normalized away: publishing a
# different string than the caller asked for is worse than failing.
if candidate[:1] in ("v", "V") and VERSION_PATTERN.fullmatch(candidate[1:]):
raise ValueError(
f"Invalid {source}: {version!r}. "
f"Did you mean {candidate[1:]!r}? (no leading 'v')"
)

raise ValueError(
f"Invalid {source}: {version!r}. "
f"Expected a version matching {VERSION_PATTERN.pattern}"
)


def _substitute(path: Path, pattern: str, assignment: str, version: str) -> str:
"""Return ``path``'s text with ``assignment``'s version literal set to ``version``.

The replacement is a **callable**, so re.sub() inserts its return value
literally instead of running it through backslash-escape processing -- a
"\\1" or "\\g<0>" in ``version`` stays those four characters. json.dumps()
emits the quoted literal, which is valid in both TOML and Python.

Raises:
ValueError: If ``pattern`` does not match, which would otherwise write
the file back unchanged and report success.
"""
content = path.read_text()
literal = f"{assignment} = {json.dumps(version)}"
new_content, count = re.subn(
pattern,
lambda _match: literal,
content,
count=1,
flags=re.MULTILINE,
)
if count != 1:
raise ValueError(f"Could not find a {assignment!r} assignment in {path}")
return new_content


pyproject_path.write_text(content)
print(f"Updated pyproject.toml to version {new_version}")
def update_version(new_version: str) -> None:
"""Validate ``new_version`` and write it into pyproject.toml and _version.py."""
version = validate_version(new_version)

# Update _version.py
pyproject_path = Path("pyproject.toml")
version_path = Path("src/claude_agent_sdk/_version.py")
content = version_path.read_text()

# Only update __version__ assignment
content = re.sub(
r'^__version__ = "[^"]*"',
f'__version__ = "{new_version}"',
content,
count=1,
flags=re.MULTILINE,
# Compute both substitutions before writing either, so a missing anchor in
# the second file cannot leave the first one already rewritten.
pyproject_content = _substitute(
pyproject_path, r'^version = "[^"]*"', "version", version
)
version_content = _substitute(
version_path, r'^__version__ = "[^"]*"', "__version__", version
)

version_path.write_text(content)
print(f"Updated _version.py to version {new_version}")
pyproject_path.write_text(pyproject_content)
print(f"Updated pyproject.toml to version {version}")

version_path.write_text(version_content)
print(f"Updated _version.py to version {version}")


if __name__ == "__main__":
if len(sys.argv) != 2:
print("Usage: python scripts/update_version.py <version>")
sys.exit(1)
sys.exit("Usage: python scripts/update_version.py <version>")

update_version(sys.argv[1])
try:
update_version(sys.argv[1])
except ValueError as exc:
# A clean one-line message, not a traceback: the caller is a release
# workflow and the reader is whoever typed the bad version.
sys.exit(str(exc))
Loading
Loading