From a2ca05260716c56cfee1943333fe64986081ff70 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 6 Nov 2026 00:18:03 +0000 Subject: [PATCH 1/6] Discover module package icons by convention --- .github/scripts/build_payload.py | 9 ++++++--- .github/scripts/test_build_payload.py | 25 ++++++++++++++++++++++--- Branding/manifest.json | 1 - Cookieconsent/manifest.json | 1 - README.md | 4 +++- manifest.schema.json | 1 - tools/validate.php | 13 ++++++++----- 7 files changed, 39 insertions(+), 15 deletions(-) diff --git a/.github/scripts/build_payload.py b/.github/scripts/build_payload.py index 82c2064..5ea6d45 100644 --- a/.github/scripts/build_payload.py +++ b/.github/scripts/build_payload.py @@ -163,6 +163,12 @@ def main() -> None: manifest_path = os.path.join(ext_dir, "manifest.json") meta = json.loads(Path(manifest_path).read_text()) + if "icon_url" in meta: + fail("Remove icon_url from manifest.json; use icon.svg, icon.png, icon.webp or icon.avif at the package root") + icons = [Path(ext_dir) / name for name in ("icon.svg", "icon.png", "icon.webp", "icon.avif") if (Path(ext_dir) / name).exists()] + if len(icons) > 1 or any(icon.is_symlink() or not icon.is_file() for icon in icons): + fail("Include at most one regular package icon") + for field in ( "manifest_version", "type", @@ -263,9 +269,6 @@ def main() -> None: "version": version, "download_url": download_url, } - icon_url = meta.get("icon_url", "") - if isinstance(icon_url, str) and icon_url: - content["icon_url"] = icon_url if "homepage_url" in meta: content["homepage_url"] = meta["homepage_url"] diff --git a/.github/scripts/test_build_payload.py b/.github/scripts/test_build_payload.py index edad533..65e2632 100644 --- a/.github/scripts/test_build_payload.py +++ b/.github/scripts/test_build_payload.py @@ -30,7 +30,7 @@ def setUp(self): "manifest_version": 1, "id": "example_module", "type": "module", "name": "Example", "description": "Example module", "version": "1.2.3", "author": {"name": "Author", "url": "https://example.com"}, - "license": "MIT OR Apache-2.0", "icon_url": "assets/icon.svg", + "license": "MIT OR Apache-2.0", "requires": {"fossbilling": "^0.8.9 || ^1.0", "php": ">=8.3 <9", "modules": {"branding": "^1.2"}}, } (self.package / "README.md").write_text("# Example\n") @@ -57,13 +57,32 @@ def test_preserves_manifest_metadata_and_compound_requirements(self): self.assertEqual(create["readme"], "# Example\n") self.assertEqual(create, {**update, "id": "example_module"}) - def test_optional_homepage_and_icon(self): + def test_optional_homepage_without_icon_metadata(self): self.meta["homepage_url"] = "https://example.com" - del self.meta["icon_url"] create, _ = self.build() self.assertEqual(create["homepage_url"], "https://example.com") self.assertNotIn("icon_url", create) + def test_icons_are_discovered_without_manifest_metadata(self): + for format in ("svg", "png", "webp", "avif"): + icon = self.package / f"icon.{format}" + icon.write_text("image bytes") + try: + create, _ = self.build() + self.assertNotIn("icon_url", create) + finally: + icon.unlink() + + def test_rejects_duplicate_icons_and_manifest_icon_field(self): + (self.package / "icon.svg").write_text("svg") + (self.package / "icon.png").write_text("png") + with self.assertRaises(SystemExit): + self.build() + (self.package / "icon.png").unlink() + self.meta["icon_url"] = "icon.svg" + with self.assertRaises(SystemExit): + self.build() + def test_reruns_replace_the_same_version_and_preserve_other_requirements(self): old = {"tag": "1.0.0", "date": "2026-01-01", "download_url": "https://example.com/old.zip", "requires": {"fossbilling": "~0.7", "php": "^8.2", "modules": {"old": "^1"}}} create, _ = self.build([{"tag": "example_module-v1.2.3"}, old]) diff --git a/Branding/manifest.json b/Branding/manifest.json index a2e1a26..5de2399 100644 --- a/Branding/manifest.json +++ b/Branding/manifest.json @@ -8,6 +8,5 @@ "author": {"name": "FOSSBilling", "url": "https://fossbilling.org"}, "license": "Apache-2.0", "homepage_url": "https://fossbilling.org", - "icon_url": "icon.svg", "requires": {"fossbilling": ">=0.8.9", "php": ">=8.3", "modules": {}} } diff --git a/Cookieconsent/manifest.json b/Cookieconsent/manifest.json index 8bf8011..b7a0f41 100644 --- a/Cookieconsent/manifest.json +++ b/Cookieconsent/manifest.json @@ -8,6 +8,5 @@ "author": {"name": "FOSSBilling", "url": "https://fossbilling.org"}, "license": "Apache-2.0", "homepage_url": "https://fossbilling.org", - "icon_url": "icon.svg", "requires": {"fossbilling": ">=0.8.9", "php": ">=8.3", "modules": {}} } \ No newline at end of file diff --git a/README.md b/README.md index f8e4076..27f9a14 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,7 @@ Tag the release `-v` (e.g. `branding-v0.8.9`, version matching the package's `manifest.json`) and attach the distributable `.zip` as a release asset. The zip must contain the package directory contents at the archive root, with no wrapper folder, so it installs as-is. -Publishing a release runs `.github/workflows/publish.yml`, which sends the manifest v1 fields directly to the extensions API and approves the new revision. `type: "module"`, string licenses, optional `homepage_url`, package-relative `icon_url`, author metadata, and the complete `requires` object are preserved. A manual run is available via `workflow_dispatch` with the same tag. Both triggers read package files from that tag, while using publisher tooling from the reviewed workflow ref. +Publishing a release runs `.github/workflows/publish.yml`, which sends the manifest v1 fields directly to the extensions API and approves the new revision. `type: "module"`, string licenses, optional `homepage_url`, author metadata, and the complete `requires` object are preserved. A manual run is available via `workflow_dispatch` with the same tag. Both triggers read package files from that tag, while using publisher tooling from the reviewed workflow ref. FOSSBilling, PHP, and module constraints are retained verbatim, including compound Composer ranges. Each release stores its own requirements. Deploy the API manifest migration and matching extensions site before running this publisher; it refuses history without `requires` instead of discarding compatibility information. @@ -58,3 +58,5 @@ The client authenticates with `client_credentials` and is bound to the extensions v2 README, "Machine publishing"). If a publish fails with 409, a revision is already pending review — a moderator resolves it in the review queue, then re-run the workflow. + +Extension icons are discovered at the package root: include one of `icon.svg`, `icon.png`, `icon.webp`, or `icon.avif`. Omit the icon for the generic fallback. Icon filenames are not declared in `manifest.json`. diff --git a/manifest.schema.json b/manifest.schema.json index bff03b6..52b2ba7 100644 --- a/manifest.schema.json +++ b/manifest.schema.json @@ -21,7 +21,6 @@ }, "license": {"type": "string", "minLength": 1, "pattern": "^[A-Za-z0-9.\\-+_:() |]+$"}, "homepage_url": {"type": "string", "pattern": "^https?://"}, - "icon_url": {"type": "string", "pattern": "^(?!/)(?![a-zA-Z][a-zA-Z0-9+.-]*:)(?!.*\\.\\.)[^\\\\]+$"}, "requires": { "type": "object", "required": ["fossbilling", "php"], "additionalProperties": false, "properties": { diff --git a/tools/validate.php b/tools/validate.php index 5384b56..eb31fdc 100644 --- a/tools/validate.php +++ b/tools/validate.php @@ -47,11 +47,14 @@ foreach (['author.url' => $m['author']['url'] ?? null, 'homepage_url' => $m['homepage_url'] ?? null] as $field => $url) { if ($url !== null && (!filter_var($url, FILTER_VALIDATE_URL) || !in_array(strtolower((string) parse_url($url, PHP_URL_SCHEME)), ['http', 'https'], true))) { $errors[] = $field . ' must be an http(s) URL'; } } - if (isset($m['icon_url'])) { - $icon = $m['icon_url']; - $resolved = realpath($dir . '/' . $icon) ?: $dir . '/' . $icon; - $base = realpath($dir); - if (!is_file($dir . '/' . $icon) || $base === false || !str_starts_with($resolved, $base . DIRECTORY_SEPARATOR)) { $errors[] = 'icon_url target missing or escapes package dir'; } + $icons = array_filter(['icon.svg', 'icon.png', 'icon.webp', 'icon.avif'], static fn ($icon) => is_file($dir . '/' . $icon)); + if (count($icons) > 1) { + $errors[] = 'include only one package icon: icon.svg, icon.png, icon.webp or icon.avif'; + } + foreach ($icons as $icon) { + if (is_link($dir . '/' . $icon)) { + $errors[] = 'package icons must not be symbolic links'; + } } try { $parser->normalize($m['version'] ?? ''); if (str_starts_with((string) ($m['version'] ?? ''), 'v')) { $errors[] = 'version must not have a "v" prefix'; } } catch (\UnexpectedValueException) { $errors[] = 'version is not valid semver'; } From c79583664fed4bbfc8ea69a72535bd2e5bd66b79 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 6 Nov 2026 00:34:18 +0000 Subject: [PATCH 2/6] Publish directory-built module packages --- .github/scripts/build_payload.py | 106 +++++++++++++++++++---- .github/scripts/test_build_payload.py | 60 ++++++++++++- .github/scripts/test_publish_contract.py | 5 +- .github/workflows/publish.yml | 1 + README.md | 34 ++++++-- 5 files changed, 178 insertions(+), 28 deletions(-) diff --git a/.github/scripts/build_payload.py b/.github/scripts/build_payload.py index 5ea6d45..e6eb81b 100644 --- a/.github/scripts/build_payload.py +++ b/.github/scripts/build_payload.py @@ -17,12 +17,15 @@ API keeps its changelog history; without a token only the new entry is submitted. -The manifest metadata is sent unchanged using the Extensions v2 contract. +The source ZIP and manifest are built by the package API before submission; +release entries reference the returned immutable artifact and SHA-256. Usage: build_payload.py """ import json +import re +import uuid import os from pathlib import Path import subprocess @@ -103,7 +106,7 @@ def fetch_existing_releases(ext_id: str) -> list | None: for entry in releases: if not isinstance(entry, dict): fail(f"Extensions API response for '{ext_id}' has a malformed release entry") - if not isinstance(entry.get("requires"), dict) or not all(isinstance(entry["requires"].get(key), str) and entry["requires"][key] for key in ("fossbilling", "php")): + if not isinstance(entry.get("requires"), dict) or not all(isinstance(entry["requires"].get(key), str) and entry["requires"][key] for key in ("fossbilling",)): fail(f"Extensions API response for '{ext_id}' has a release without manifest requirements; deploy the API migration before publishing") known.append( { @@ -114,6 +117,9 @@ def fetch_existing_releases(ext_id: str) -> list | None: "download_url", "changelog_url", "requires", + "artifact_id", + "sha256", + "is_latest", ) if key in entry } @@ -148,9 +154,66 @@ def merge_releases(new_release: dict, ext_id: str, history: list) -> list: for entry in history if canonical_tag(entry.get("tag"), ext_id) != new_release["tag"] ) + if new_release.get("is_latest"): + merged = [new_release, *({key: value for key, value in entry.items() if key != "is_latest"} for entry in merged[1:])] return merged[:MAX_RELEASES] +def api_json(path: str, data: bytes | None = None, content_type: str | None = None) -> dict: + token = os.environ.get("API_TOKEN", "") + base = os.environ.get("API_BASE_URL", "").rstrip("/") + if not token or not base: + fail("API_TOKEN and API_BASE_URL are required to build directory packages") + headers = {"Authorization": f"Bearer {token}"} + if content_type: + headers["Content-Type"] = content_type + request = urllib.request.Request(base + path, data=data, headers=headers) + try: + with urllib.request.urlopen(request, timeout=60) as response: + body = response.read(MAX_CONTENT_BYTES + 1) + if len(body) > MAX_CONTENT_BYTES: + fail("Package API response exceeds its size limit") + document = json.loads(body) + result = document.get("result") if isinstance(document, dict) else None + except (urllib.error.URLError, ValueError) as exc: + fail(f"Package API request failed: {exc}") + if not isinstance(result, dict): + fail("Package API returned no artifact metadata") + return result + + +def build_package(manifest: dict, source_url: str) -> dict: + if not os.environ.get("API_TOKEN") or not os.environ.get("API_BASE_URL"): + fail("API_TOKEN and API_BASE_URL are required to build directory packages") + expected = f"https://github.com/{os.environ['GITHUB_REPOSITORY']}/releases/download/" + if not source_url.startswith(expected): + fail("Source ZIP must be a release asset in this GitHub repository") + try: + with urllib.request.urlopen(source_url, timeout=60) as response: + source = response.read(5 * 1024 * 1024 + 1) + except (urllib.error.URLError, TimeoutError) as exc: + fail(f"Source ZIP download failed: {exc}") + if len(source) > 5 * 1024 * 1024: + fail("Source ZIP exceeds the 5 MiB package limit") + boundary = "package-" + uuid.uuid4().hex + data = ( + f'--{boundary}\r\nContent-Disposition: form-data; name="manifest"\r\nContent-Type: application/json\r\n\r\n'.encode() + + json.dumps(manifest, ensure_ascii=False).encode() + + f'\r\n--{boundary}\r\nContent-Disposition: form-data; name="source"; filename="source.zip"\r\nContent-Type: application/zip\r\n\r\n'.encode() + + source + f'\r\n--{boundary}--\r\n'.encode() + ) + artifact = api_json("/packages", data, f"multipart/form-data; boundary={boundary}") + try: + artifact_id = str(uuid.UUID(artifact["id"])) + except (KeyError, ValueError, TypeError, AttributeError): + fail("Package API returned an invalid artifact ID") + if artifact.get("manifest") != manifest or not isinstance(artifact.get("sha256"), str) or not re.fullmatch(r"[a-f0-9]{64}", artifact["sha256"]): + fail("Package API artifact does not match the submitted manifest or checksum format") + if artifact.get("download_url") != os.environ["API_BASE_URL"].rstrip("/") + f"/packages/{artifact_id}/download": + fail("Package API returned an unexpected download URL") + return artifact + + def main() -> None: ext_id, version, tag = sys.argv[1], sys.argv[2], sys.argv[3] try: @@ -236,7 +299,11 @@ def main() -> None: "(archive root must be the package directory contents, with no " "wrapper folder) before publishing." ) - download_url = zips[0] + if len(zips) != 1: + fail("Release must have exactly one source ZIP asset") + history = fetch_existing_releases(ext_id) + artifact = build_package(meta, zips[0]) + download_url = artifact["download_url"] # The API keys releases by plain semver version, not by git tag: # sorting, latest-release resolution, and the directory's edit form @@ -247,6 +314,8 @@ def main() -> None: "date": published_at, "download_url": download_url, "requires": requires, + "artifact_id": artifact["id"], + "sha256": artifact["sha256"], } if html_url: new_release["changelog_url"] = html_url @@ -254,24 +323,25 @@ def main() -> None: license_name = meta["license"] if not isinstance(license_name, str) or not license_name.strip(): fail(f"{manifest_path} license must be a non-empty license expression") - history = fetch_existing_releases(ext_id) - content: dict = { - "manifest_version": 1, - "type": meta["type"], - "author": meta["author"], - "requires": requires, - "name": meta["name"], - "description": meta["description"], + managed = [entry for entry in history or [] if entry.get("artifact_id") and canonical_tag(entry.get("tag"), ext_id) != version] + # Stable publishes become latest. Prereleases retain an existing managed selection. + previous = next((entry for entry in managed if entry.get("is_latest")), None) + previous = previous or next((entry for entry in managed if "-" not in entry["tag"].split("+", 1)[0]), None) or next(iter(managed), None) + selected_manifest = meta + if "-" not in version.split("+", 1)[0] or previous is None: + new_release["is_latest"] = True + else: + selected_manifest = api_json(f"/packages/{previous['artifact_id']}")["manifest"] + previous["is_latest"] = True + content = {key: value for key, value in selected_manifest.items() if key != "id"} + content.update({ "releases": merge_releases(new_release, ext_id, history or []), - "license": license_name, "readme": readme, "source": {"type": "github", "repo": os.environ.get("GITHUB_REPOSITORY", "FOSSBilling/modules")}, - "version": version, - "download_url": download_url, - } - - if "homepage_url" in meta: - content["homepage_url"] = meta["homepage_url"] + }) + selected_release = next(entry for entry in content["releases"] if entry.get("is_latest")) + content["version"] = selected_release["tag"] + content["download_url"] = selected_release["download_url"] create_content = dict(content) create_content["id"] = ext_id diff --git a/.github/scripts/test_build_payload.py b/.github/scripts/test_build_payload.py index 65e2632..1c937d4 100644 --- a/.github/scripts/test_build_payload.py +++ b/.github/scripts/test_build_payload.py @@ -41,7 +41,7 @@ def setUp(self): def build(self, history=None): (self.package / "manifest.json").write_text(json.dumps(self.meta)) - with patch.dict(os.environ, {"GITHUB_REPOSITORY": "FOSSBilling/modules"}), patch.object(publisher.sys, "argv", ["build_payload.py", "example_module", "1.2.3", "example_module-v1.2.3"]), patch.object(publisher.subprocess, "run", return_value=SimpleNamespace(stdout=json.dumps(self.release))), patch.object(publisher, "fetch_existing_releases", return_value=history): + with patch.dict(os.environ, {"GITHUB_REPOSITORY": "FOSSBilling/modules"}), patch.object(publisher.sys, "argv", ["build_payload.py", "example_module", self.meta["version"], "example_module-v" + self.meta["version"]]), patch.object(publisher.subprocess, "run", return_value=SimpleNamespace(stdout=json.dumps(self.release))), patch.object(publisher, "fetch_existing_releases", return_value=history), patch.object(publisher, "build_package", return_value={"id": "134cf413-b5ab-4f57-b2ad-520020af880d", "sha256": "a" * 64, "manifest": dict(self.meta), "download_url": "https://api.example.test/extensions/v2/packages/134cf413-b5ab-4f57-b2ad-520020af880d/download"}): publisher.main() return json.loads(Path("payload-create.json").read_text()), json.loads(Path("payload-update.json").read_text()) @@ -57,6 +57,28 @@ def test_preserves_manifest_metadata_and_compound_requirements(self): self.assertEqual(create["readme"], "# Example\n") self.assertEqual(create, {**update, "id": "example_module"}) + def test_release_references_the_directory_built_package(self): + create, _ = self.build([{ "tag": "1.0.0", "is_latest": True, "artifact_id": "old", "sha256": "b" * 64, "requires": self.meta["requires"] }]) + release = create["releases"][0] + self.assertEqual(release["artifact_id"], "134cf413-b5ab-4f57-b2ad-520020af880d") + self.assertEqual(release["sha256"], "a" * 64) + self.assertTrue(release["is_latest"]) + self.assertEqual(create["download_url"], release["download_url"]) + self.assertNotIn("is_latest", create["releases"][1]) + self.assertEqual(create["releases"][1]["artifact_id"], "old") + + def test_prerelease_preserves_the_selected_managed_manifest_and_download(self): + old = {**self.meta, "version": "1.0.0", "description": "Stable listing"} + self.meta["version"] = "1.3.0-beta.1" + history = [{"tag": "1.0.0", "date": "2026-01-01", "download_url": "https://api.example.test/stable.zip", "artifact_id": "old-artifact", "sha256": "b" * 64, "requires": old["requires"], "is_latest": True}] + with patch.object(publisher, "api_json", return_value={"manifest": old}): + create, _ = self.build(history) + self.assertEqual(create["version"], "1.0.0") + self.assertEqual(create["description"], "Stable listing") + self.assertEqual(create["download_url"], history[0]["download_url"]) + self.assertNotIn("is_latest", create["releases"][0]) + self.assertTrue(create["releases"][1]["is_latest"]) + def test_optional_homepage_without_icon_metadata(self): self.meta["homepage_url"] = "https://example.com" create, _ = self.build() @@ -143,5 +165,41 @@ def test_fetch_retains_requires_and_rejects_pre_migration_history(self): publisher.fetch_existing_releases("example") + + +class PackageApiTests(unittest.TestCase): + def setUp(self): + self.enterContext(redirect_stdout(io.StringIO())) + self.enterContext(patch.dict(os.environ, {"API_TOKEN": "test-token", "API_BASE_URL": "https://api.example.test/extensions/v2", "GITHUB_REPOSITORY": "FOSSBilling/modules"})) + self.manifest = {"manifest_version": 1, "id": "example", "version": "1.0.0"} + self.artifact = {"id": "134cf413-b5ab-4f57-b2ad-520020af880d", "sha256": "a" * 64, "manifest": self.manifest, "download_url": "https://api.example.test/extensions/v2/packages/134cf413-b5ab-4f57-b2ad-520020af880d/download"} + self.source = "https://github.com/FOSSBilling/modules/releases/download/example-v1.0.0/source.zip" + + def test_uploads_the_exact_source_and_manifest_with_auth_only_on_the_api_call(self): + calls = [] + def open_url(request, **kwargs): + calls.append(request) + return io.BytesIO(b"source ZIP" if isinstance(request, str) else json.dumps({"result": self.artifact}).encode()) + with patch.object(publisher.urllib.request, "urlopen", side_effect=open_url): + self.assertEqual(publisher.build_package(self.manifest, self.source), self.artifact) + self.assertEqual(calls[0], self.source) + self.assertEqual(calls[1].full_url, "https://api.example.test/extensions/v2/packages") + self.assertEqual(calls[1].get_header("Authorization"), "Bearer test-token") + self.assertIn(b'name="manifest"', calls[1].data) + self.assertIn(json.dumps(self.manifest).encode(), calls[1].data) + self.assertIn(b"source ZIP", calls[1].data) + + def test_rejects_oversized_sources_before_uploading(self): + with patch.object(publisher.urllib.request, "urlopen", return_value=io.BytesIO(b"x" * (5 * 1024 * 1024 + 1))) as request: + with self.assertRaises(SystemExit): + publisher.build_package(self.manifest, self.source) + self.assertEqual(request.call_count, 1) + + def test_rejects_mismatched_artifact_metadata_and_foreign_download_urls(self): + for replacement in [{"manifest": {}}, {"sha256": "invalid"}, {"download_url": "https://other.test/package.zip"}]: + artifact = {**self.artifact, **replacement} + with self.subTest(replacement=replacement), patch.object(publisher.urllib.request, "urlopen", return_value=io.BytesIO(b"ZIP")), patch.object(publisher, "api_json", return_value=artifact), self.assertRaises(SystemExit): + publisher.build_package(self.manifest, self.source) + if __name__ == "__main__": unittest.main() diff --git a/.github/scripts/test_publish_contract.py b/.github/scripts/test_publish_contract.py index 1050688..a69cec0 100644 --- a/.github/scripts/test_publish_contract.py +++ b/.github/scripts/test_publish_contract.py @@ -110,8 +110,9 @@ def test_build_uses_trusted_sibling_imports_despite_pythonpath(self): env = {**os.environ, 'PATH': f"{bin_dir}:{os.environ['PATH']}", 'PYTHONPATH': str(release), 'API_TOKEN': 'offline', 'API_BASE_URL': '', 'GITHUB_REPOSITORY': 'FOSSBilling/modules', 'EXT_ID': 'branding', 'VERSION': version, 'RELEASE_TAG': f'branding-v{version}'} result = subprocess.run(['bash', '-e', '-c', workflow_run('Build extension payload')], cwd=release, env=env, capture_output=True, text=True) self.assertFalse((release / 'hostile-imported').exists(), 'Tagged module imported via PYTHONPATH') - self.assertEqual(result.returncode, 0, result.stderr) - self.assertEqual(json.loads((release / 'payload-create.json').read_text())['id'], 'branding') + self.assertNotEqual(result.returncode, 0) + self.assertIn("API_TOKEN and API_BASE_URL are required", result.stdout) + self.assertFalse((release / 'payload-create.json').exists()) def test_create_and_update_accept_the_merged_api_statuses(self): for exists, method, code in [(False, "POST", 201), (True, "PUT", 202)]: diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 02bbbbc..cc206ec 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -55,6 +55,7 @@ jobs: echo "::error::Token request failed: $RESP" exit 1 fi + echo "::add-mask::$ACCESS" echo "access_token=$ACCESS" >> "$GITHUB_OUTPUT" env: PUBLISHER_CLIENT_ID: ${{ vars.MODULES_PUBLISHER_CLIENT_ID }} diff --git a/README.md b/README.md index 27f9a14..0b30700 100644 --- a/README.md +++ b/README.md @@ -26,15 +26,35 @@ Tag the release `-v` (e.g. `branding-v0.8.9`, version matching the package's `manifest.json`) and attach the distributable `.zip` as a release asset. The zip must contain the package directory contents at the archive root, with no wrapper folder, so it installs as-is. -Publishing a release runs `.github/workflows/publish.yml`, which sends the manifest v1 fields directly to the extensions API and approves the new revision. `type: "module"`, string licenses, optional `homepage_url`, author metadata, and the complete `requires` object are preserved. A manual run is available via `workflow_dispatch` with the same tag. Both triggers read package files from that tag, while using publisher tooling from the reviewed workflow ref. +Publishing runs the trusted tooling from `.github/workflows/publish.yml` against +package metadata checked out from the release tag. It uploads the single GitHub +ZIP asset and manifest to `POST /packages`; the directory builds the distributable +and returns an immutable artifact ID, SHA-256 and download URL. The submitted +release references those values, preserving earlier managed artifacts and +external release history. The workflow then approves the revision using its +existing `extensions:publish` permission. -FOSSBilling, PHP, and module constraints are retained verbatim, including compound Composer ranges. Each release stores its own requirements. Deploy the API manifest migration and matching extensions site before running this publisher; it refuses history without `requires` instead of discarding compatibility information. +This publisher requires the package-enabled Extensions v2 API. Keep these changes +local until that API is ready to deploy. The machine client must have +`extensions:push`, an active developer mapping and permission to publish for the +same developer. Package ZIPs remain private until the revision is approved. -The release becomes a new extensions directory release entry keyed by the version: -date, changelog link, and the distributable asset come from the GitHub -release itself, and the entry is prepended to the extension's release -history. A manual re-run via `workflow_dispatch` with the same tag -replaces the entry in place. A public 404 can also mean a delisted or unpublished extension: if creation then conflicts, publishing stops for manual correction instead of updating without its release history. Once the extension is public, retry to fetch its history first. +Stable releases become the selected latest release. Prereleases retain the +previous managed selection when one exists; the listing's manifest metadata +continues to describe that selected package. A rerun of a version must upload +identical source bytes. The API reuses an identical artifact and rejects changed +source bytes for that version. Publish changed files under a new version. + +The publisher refuses source ZIPs over 5 MiB, ambiguous releases with multiple ZIP +assets, unexpected artifact URLs and mismatched manifests. FOSSBilling, PHP and +module constraints remain unchanged. Existing release history may omit PHP; +new module manifests must still declare it. The readme, source and changelog links +remain directory metadata outside the generated manifest. + +Publishing reads existing public release history before submitting. A public +404 may mean a delisted or unpublished extension: a subsequent create conflict +stops publishing rather than switching to an update without that history. +Resolve the pending or hidden listing before retrying. ## Secrets From f0f94638a9ba898e7314453d39a11ff81b35ac00 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 6 Nov 2026 00:38:36 +0000 Subject: [PATCH 3/6] Preserve latest release selection on publisher retries --- .github/scripts/build_payload.py | 28 ++++++++++++++++----------- .github/scripts/test_build_payload.py | 20 +++++++++++++++++++ 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/.github/scripts/build_payload.py b/.github/scripts/build_payload.py index e6eb81b..66dc34c 100644 --- a/.github/scripts/build_payload.py +++ b/.github/scripts/build_payload.py @@ -13,9 +13,8 @@ git tag (``"branding-v0.8.9"``): the API sorts and compares releases by semver tag on every read, and the extensions site derives the editable version from it. The entry is prepended to the extension's already -published releases (fetched from the API when ``API_TOKEN`` is set) so the -API keeps its changelog history; without a token only the new entry -is submitted. +published releases so the API keeps its changelog history. Publishing +requires ``API_TOKEN`` and ``API_BASE_URL``. The source ZIP and manifest are built by the package API before submission; release entries reference the returned immutable artifact and SHA-256. @@ -24,14 +23,14 @@ """ import json -import re -import uuid import os +import re from pathlib import Path import subprocess import sys import urllib.error import urllib.request +import uuid from resolve_release import parse_release_tag @@ -106,7 +105,8 @@ def fetch_existing_releases(ext_id: str) -> list | None: for entry in releases: if not isinstance(entry, dict): fail(f"Extensions API response for '{ext_id}' has a malformed release entry") - if not isinstance(entry.get("requires"), dict) or not all(isinstance(entry["requires"].get(key), str) and entry["requires"][key] for key in ("fossbilling",)): + requirements = entry.get("requires") + if not isinstance(requirements, dict) or not isinstance(requirements.get("fossbilling"), str) or not requirements["fossbilling"]: fail(f"Extensions API response for '{ext_id}' has a release without manifest requirements; deploy the API migration before publishing") known.append( { @@ -156,7 +156,11 @@ def merge_releases(new_release: dict, ext_id: str, history: list) -> list: ) if new_release.get("is_latest"): merged = [new_release, *({key: value for key, value in entry.items() if key != "is_latest"} for entry in merged[1:])] - return merged[:MAX_RELEASES] + retained = merged[:MAX_RELEASES] + selected = next((entry for entry in merged if entry.get("is_latest")), None) + if selected is not None and selected not in retained: + retained[-1] = selected + return retained def api_json(path: str, data: bytes | None = None, content_type: str | None = None) -> dict: @@ -263,6 +267,10 @@ def main() -> None: if not isinstance(requires.get("php"), str) or not requires["php"].strip(): fail(f"{manifest_path} requires.php must be a non-empty version constraint string") + license_name = meta["license"] + if not isinstance(license_name, str) or not license_name.strip(): + fail(f"{manifest_path} license must be a non-empty license expression") + readme_path = os.path.join(ext_dir, "README.md") if os.path.isfile(readme_path): readme = Path(readme_path).read_text() @@ -320,15 +328,13 @@ def main() -> None: if html_url: new_release["changelog_url"] = html_url - license_name = meta["license"] - if not isinstance(license_name, str) or not license_name.strip(): - fail(f"{manifest_path} license must be a non-empty license expression") managed = [entry for entry in history or [] if entry.get("artifact_id") and canonical_tag(entry.get("tag"), ext_id) != version] # Stable publishes become latest. Prereleases retain an existing managed selection. previous = next((entry for entry in managed if entry.get("is_latest")), None) previous = previous or next((entry for entry in managed if "-" not in entry["tag"].split("+", 1)[0]), None) or next(iter(managed), None) selected_manifest = meta - if "-" not in version.split("+", 1)[0] or previous is None: + current = next((entry for entry in history or [] if canonical_tag(entry.get("tag"), ext_id) == version), None) + if (current and current.get("is_latest")) or "-" not in version.split("+", 1)[0] or previous is None: new_release["is_latest"] = True else: selected_manifest = api_json(f"/packages/{previous['artifact_id']}")["manifest"] diff --git a/.github/scripts/test_build_payload.py b/.github/scripts/test_build_payload.py index 1c937d4..f41052d 100644 --- a/.github/scripts/test_build_payload.py +++ b/.github/scripts/test_build_payload.py @@ -79,6 +79,26 @@ def test_prerelease_preserves_the_selected_managed_manifest_and_download(self): self.assertNotIn("is_latest", create["releases"][0]) self.assertTrue(create["releases"][1]["is_latest"]) + def test_retry_keeps_a_selected_prerelease_selected(self): + self.meta["version"] = "1.3.0-beta.1" + history = [ + {"tag": "1.3.0-beta.1", "artifact_id": "current", "is_latest": True}, + {"tag": "1.0.0", "artifact_id": "stable", "download_url": "https://example.test/stable.zip", "requires": self.meta["requires"]}, + ] + create, _ = self.build(history) + self.assertEqual(create["version"], "1.3.0-beta.1") + self.assertTrue(create["releases"][0]["is_latest"]) + self.assertNotIn("is_latest", create["releases"][1]) + + def test_release_limit_retains_the_selected_release(self): + new = {"tag": "2.0.0-beta.1"} + history = [{"tag": f"1.0.{i}"} for i in range(100)] + history[-1]["is_latest"] = True + merged = publisher.merge_releases(new, "example_module", history) + self.assertEqual(len(merged), 100) + self.assertEqual(merged[0], new) + self.assertIn(history[-1], merged) + def test_optional_homepage_without_icon_metadata(self): self.meta["homepage_url"] = "https://example.com" create, _ = self.build() From b160fa62a337ab550298c0383bcd3267ea15924a Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 6 Nov 2026 00:39:02 +0000 Subject: [PATCH 4/6] Document publisher deployment dependency --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 0b30700..279b2a6 100644 --- a/README.md +++ b/README.md @@ -34,8 +34,8 @@ release references those values, preserving earlier managed artifacts and external release history. The workflow then approves the revision using its existing `extensions:publish` permission. -This publisher requires the package-enabled Extensions v2 API. Keep these changes -local until that API is ready to deploy. The machine client must have +Deploy the package-enabled Extensions v2 API before using this publisher in +production. The machine client must have `extensions:push`, an active developer mapping and permission to publish for the same developer. Package ZIPs remain private until the revision is approved. From e9e5a3b1c9fedb1d4f0b1400fba66148cf95b9dc Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sat, 7 Nov 2026 14:52:48 +0000 Subject: [PATCH 5/6] Wait for queued package preparation before publishing --- .github/scripts/build_payload.py | 114 +++++++++++--------------- .github/scripts/test_build_payload.py | 43 +++++++--- README.md | 9 +- 3 files changed, 85 insertions(+), 81 deletions(-) diff --git a/.github/scripts/build_payload.py b/.github/scripts/build_payload.py index 66dc34c..add18ec 100644 --- a/.github/scripts/build_payload.py +++ b/.github/scripts/build_payload.py @@ -28,6 +28,7 @@ from pathlib import Path import subprocess import sys +import time import urllib.error import urllib.request import uuid @@ -71,18 +72,11 @@ def find_package_dir(ext_id: str) -> str: def fetch_existing_releases(ext_id: str) -> list | None: - """Fetch the extension's currently published releases for merge history. - - Returns None for a new extension (API 404) or when no API token is - configured (offline use). Any other failure is fatal: silently - submitting without history would wipe the extension's changelog. - Entries are reduced to the release fields the API schema - accepts, so a newer API response shape can never fail validation. - """ + """Read published releases; only a 404 means the extension is new.""" api_token = os.environ.get("API_TOKEN", "") api_base = os.environ.get("API_BASE_URL", "") if not api_token or not api_base: - return None + fail("API_TOKEN and API_BASE_URL are required to publish releases") http_request = urllib.request.Request( f"{api_base.rstrip('/')}/extensions/{ext_id}", @@ -101,58 +95,18 @@ def fetch_existing_releases(ext_id: str) -> list | None: releases = result.get("releases") if isinstance(result, dict) else None if not isinstance(releases, list): fail(f"Extensions API response for '{ext_id}' has no releases list") - known = [] - for entry in releases: - if not isinstance(entry, dict): - fail(f"Extensions API response for '{ext_id}' has a malformed release entry") - requirements = entry.get("requires") - if not isinstance(requirements, dict) or not isinstance(requirements.get("fossbilling"), str) or not requirements["fossbilling"]: - fail(f"Extensions API response for '{ext_id}' has a release without manifest requirements; deploy the API migration before publishing") - known.append( - { - key: entry[key] - for key in ( - "tag", - "date", - "download_url", - "changelog_url", - "requires", - "artifact_id", - "sha256", - "is_latest", - ) - if key in entry - } - ) - return known - - -def canonical_tag(tag: object, ext_id: str) -> object: - """Collapse the pre-fix `-v` tag shape to its version. - - Comparison-only: stored entries are never rewritten here. - """ - if isinstance(tag, str) and tag.startswith(f"{ext_id}-v"): - return tag[len(ext_id) + 2 :] - return tag + if not all(isinstance(entry, dict) for entry in releases): + fail(f"Extensions API response for '{ext_id}' has a malformed release entry") + return releases -def merge_releases(new_release: dict, ext_id: str, history: list) -> list: - """Prepend the new release to the published history. - - Same-tag re-runs replace in place so a retried publish is idempotent; - the API caps the list at its 100-release maximum, oldest dropped. - A pre-fix entry keyed by git tag (`-v`) counts as the same - release as its version, so re-publishing that version replaces the twin - instead of doubling it; other legacy entries are preserved as-is. - Ordering beyond newest-first does not matter here: the API re-sorts by - semver tag on every read. - """ +def merge_releases(new_release: dict, history: list) -> list: + """Replace a same-version release and retain at most MAX_RELEASES entries.""" merged = [new_release] merged.extend( entry for entry in history - if canonical_tag(entry.get("tag"), ext_id) != new_release["tag"] + if entry.get("tag") != new_release["tag"] ) if new_release.get("is_latest"): merged = [new_release, *({key: value for key, value in entry.items() if key != "is_latest"} for entry in merged[1:])] @@ -163,7 +117,7 @@ def merge_releases(new_release: dict, ext_id: str, history: list) -> list: return retained -def api_json(path: str, data: bytes | None = None, content_type: str | None = None) -> dict: +def api_json(path: str, data: bytes | None = None, content_type: str | None = None, *, timeout: float = 60) -> dict: token = os.environ.get("API_TOKEN", "") base = os.environ.get("API_BASE_URL", "").rstrip("/") if not token or not base: @@ -173,13 +127,13 @@ def api_json(path: str, data: bytes | None = None, content_type: str | None = No headers["Content-Type"] = content_type request = urllib.request.Request(base + path, data=data, headers=headers) try: - with urllib.request.urlopen(request, timeout=60) as response: + with urllib.request.urlopen(request, timeout=timeout) as response: body = response.read(MAX_CONTENT_BYTES + 1) if len(body) > MAX_CONTENT_BYTES: fail("Package API response exceeds its size limit") document = json.loads(body) result = document.get("result") if isinstance(document, dict) else None - except (urllib.error.URLError, ValueError) as exc: + except (urllib.error.URLError, TimeoutError, ValueError) as exc: fail(f"Package API request failed: {exc}") if not isinstance(result, dict): fail("Package API returned no artifact metadata") @@ -206,7 +160,35 @@ def build_package(manifest: dict, source_url: str) -> dict: + f'\r\n--{boundary}\r\nContent-Disposition: form-data; name="source"; filename="source.zip"\r\nContent-Type: application/zip\r\n\r\n'.encode() + source + f'\r\n--{boundary}--\r\n'.encode() ) - artifact = api_json("/packages", data, f"multipart/form-data; boundary={boundary}") + job = api_json("/packages", data, f"multipart/form-data; boundary={boundary}") + try: + job_id = str(uuid.UUID(job["id"])) + except (KeyError, ValueError, TypeError, AttributeError): + fail("Package API returned an invalid job ID") + deadline = time.monotonic() + 180 + artifact = None + while True: + if job.get("id") != job_id or job.get("extension_id") != manifest["id"]: + fail("Package processing job does not match the submitted extension") + if job.get("status") == "ready": + artifact = job.get("artifact") + break + if job.get("status") == "failed": + error = job.get("error") + message = error.get("message") if isinstance(error, dict) else None + fail(f"Package preparation failed: {message or 'unknown error'}") + if job.get("status") not in ("queued", "processing"): + fail("Package API returned an invalid processing state") + remaining = deadline - time.monotonic() + if remaining <= 0: + break + time.sleep(min(1, remaining)) + remaining = deadline - time.monotonic() + if remaining <= 0: + break + job = api_json(f"/package-jobs/{job_id}", timeout=min(60, remaining)) + if not isinstance(artifact, dict): + fail(f"Package job {job_id} is still processing; retry publishing to reuse it") try: artifact_id = str(uuid.UUID(artifact["id"])) except (KeyError, ValueError, TypeError, AttributeError): @@ -319,7 +301,7 @@ def main() -> None: # the changelog and download URLs. new_release: dict = { "tag": version, - "date": published_at, + "date": published_at[:10] if published_at else "", "download_url": download_url, "requires": requires, "artifact_id": artifact["id"], @@ -328,12 +310,12 @@ def main() -> None: if html_url: new_release["changelog_url"] = html_url - managed = [entry for entry in history or [] if entry.get("artifact_id") and canonical_tag(entry.get("tag"), ext_id) != version] - # Stable publishes become latest. Prereleases retain an existing managed selection. - previous = next((entry for entry in managed if entry.get("is_latest")), None) - previous = previous or next((entry for entry in managed if "-" not in entry["tag"].split("+", 1)[0]), None) or next(iter(managed), None) + other_releases = [entry for entry in history or [] if entry.get("tag") != version] + # Stable publishes become latest. Prereleases retain an existing selection. + previous = next((entry for entry in other_releases if entry.get("is_latest")), None) + previous = previous or next((entry for entry in other_releases if "-" not in entry["tag"].split("+", 1)[0]), None) or next(iter(other_releases), None) selected_manifest = meta - current = next((entry for entry in history or [] if canonical_tag(entry.get("tag"), ext_id) == version), None) + current = next((entry for entry in history or [] if entry.get("tag") == version), None) if (current and current.get("is_latest")) or "-" not in version.split("+", 1)[0] or previous is None: new_release["is_latest"] = True else: @@ -341,7 +323,7 @@ def main() -> None: previous["is_latest"] = True content = {key: value for key, value in selected_manifest.items() if key != "id"} content.update({ - "releases": merge_releases(new_release, ext_id, history or []), + "releases": merge_releases(new_release, history or []), "readme": readme, "source": {"type": "github", "repo": os.environ.get("GITHUB_REPOSITORY", "FOSSBilling/modules")}, }) diff --git a/.github/scripts/test_build_payload.py b/.github/scripts/test_build_payload.py index f41052d..65673c6 100644 --- a/.github/scripts/test_build_payload.py +++ b/.github/scripts/test_build_payload.py @@ -62,6 +62,7 @@ def test_release_references_the_directory_built_package(self): release = create["releases"][0] self.assertEqual(release["artifact_id"], "134cf413-b5ab-4f57-b2ad-520020af880d") self.assertEqual(release["sha256"], "a" * 64) + self.assertEqual(release["date"], self.release["published_at"][:10]) self.assertTrue(release["is_latest"]) self.assertEqual(create["download_url"], release["download_url"]) self.assertNotIn("is_latest", create["releases"][1]) @@ -94,7 +95,7 @@ def test_release_limit_retains_the_selected_release(self): new = {"tag": "2.0.0-beta.1"} history = [{"tag": f"1.0.{i}"} for i in range(100)] history[-1]["is_latest"] = True - merged = publisher.merge_releases(new, "example_module", history) + merged = publisher.merge_releases(new, history) self.assertEqual(len(merged), 100) self.assertEqual(merged[0], new) self.assertIn(history[-1], merged) @@ -127,7 +128,7 @@ def test_rejects_duplicate_icons_and_manifest_icon_field(self): def test_reruns_replace_the_same_version_and_preserve_other_requirements(self): old = {"tag": "1.0.0", "date": "2026-01-01", "download_url": "https://example.com/old.zip", "requires": {"fossbilling": "~0.7", "php": "^8.2", "modules": {"old": "^1"}}} - create, _ = self.build([{"tag": "example_module-v1.2.3"}, old]) + create, _ = self.build([{"tag": "1.2.3"}, old]) self.assertEqual([entry["tag"] for entry in create["releases"]], ["1.2.3", "1.0.0"]) self.assertEqual(create["releases"][1], old) @@ -175,16 +176,14 @@ def test_fetch_only_treats_404_as_a_new_extension(self): with self.assertRaises(SystemExit): publisher.fetch_existing_releases("example") - def test_fetch_retains_requires_and_rejects_pre_migration_history(self): - requirements = {"fossbilling": "^0.8 || ^1", "php": "^8", "modules": {"branding": "*"}} - entry = {"tag": "1.0.0", "requires": requirements, "unknown": "ignored"} + def test_fetch_returns_current_release_fields_unchanged(self): + entry = {"tag": "1.0.0", "requires": {"fossbilling": "^0.8", "php": "^8"}, "artifact_id": "package", "sha256": "a" * 64} with patch.dict(os.environ, {"API_TOKEN": "test", "API_BASE_URL": "https://api.example.test"}), patch.object(publisher.urllib.request, "urlopen", return_value=io.StringIO(json.dumps({"result": {"releases": [entry]}}))): - self.assertEqual(publisher.fetch_existing_releases("example"), [{"tag": "1.0.0", "requires": requirements}]) - with patch.dict(os.environ, {"API_TOKEN": "test", "API_BASE_URL": "https://api.example.test"}), patch.object(publisher.urllib.request, "urlopen", return_value=io.StringIO(json.dumps({"result": {"releases": [{"tag": "old", "min_fossbilling_version": "0.6"}]}}))): - with self.assertRaises(SystemExit): - publisher.fetch_existing_releases("example") - + self.assertEqual(publisher.fetch_existing_releases("example"), [entry]) + def test_missing_credentials_stop_history_reads(self): + with patch.dict(os.environ, {"API_TOKEN": "", "API_BASE_URL": ""}), self.assertRaises(SystemExit): + publisher.fetch_existing_releases("example") class PackageApiTests(unittest.TestCase): @@ -199,7 +198,7 @@ def test_uploads_the_exact_source_and_manifest_with_auth_only_on_the_api_call(se calls = [] def open_url(request, **kwargs): calls.append(request) - return io.BytesIO(b"source ZIP" if isinstance(request, str) else json.dumps({"result": self.artifact}).encode()) + return io.BytesIO(b"source ZIP" if isinstance(request, str) else json.dumps({"result": {"id":"44444444-4444-4444-8444-444444444444","extension_id":"example","status":"ready","artifact":self.artifact}}).encode()) with patch.object(publisher.urllib.request, "urlopen", side_effect=open_url): self.assertEqual(publisher.build_package(self.manifest, self.source), self.artifact) self.assertEqual(calls[0], self.source) @@ -209,6 +208,26 @@ def open_url(request, **kwargs): self.assertIn(json.dumps(self.manifest).encode(), calls[1].data) self.assertIn(b"source ZIP", calls[1].data) + def test_polls_a_queued_package_before_returning_the_artifact(self): + job = {"id": "44444444-4444-4444-8444-444444444444", "extension_id": "example", "status": "queued", "artifact": None} + ready = {**job, "status": "ready", "artifact": self.artifact} + with patch.object(publisher.urllib.request, "urlopen", return_value=io.BytesIO(b"ZIP")), patch.object(publisher, "api_json", side_effect=[job, {**job, "status": "processing"}, ready]) as request, patch.object(publisher.time, "sleep"): + self.assertEqual(publisher.build_package(self.manifest, self.source), self.artifact) + self.assertEqual(request.call_args_list[-1].args, ("/package-jobs/" + job["id"],)) + + def test_stops_polling_at_the_deadline(self): + job = {"id": "44444444-4444-4444-8444-444444444444", "extension_id": "example", "status": "queued"} + with patch.object(publisher.urllib.request, "urlopen", return_value=io.BytesIO(b"ZIP")), patch.object(publisher, "api_json", return_value=job) as request, patch.object(publisher.time, "monotonic", side_effect=[0, 179, 179.5, 180]), patch.object(publisher.time, "sleep"): + with self.assertRaises(SystemExit): + publisher.build_package(self.manifest, self.source) + self.assertEqual(request.call_count, 2) + self.assertEqual(request.call_args.kwargs, {"timeout": 0.5}) + + def test_failed_job_stops_publishing(self): + job = {"id": "44444444-4444-4444-8444-444444444444", "extension_id": "example", "status": "failed", "error": {"message": "Invalid archive"}} + with patch.object(publisher.urllib.request, "urlopen", return_value=io.BytesIO(b"ZIP")), patch.object(publisher, "api_json", return_value=job), self.assertRaises(SystemExit): + publisher.build_package(self.manifest, self.source) + def test_rejects_oversized_sources_before_uploading(self): with patch.object(publisher.urllib.request, "urlopen", return_value=io.BytesIO(b"x" * (5 * 1024 * 1024 + 1))) as request: with self.assertRaises(SystemExit): @@ -218,7 +237,7 @@ def test_rejects_oversized_sources_before_uploading(self): def test_rejects_mismatched_artifact_metadata_and_foreign_download_urls(self): for replacement in [{"manifest": {}}, {"sha256": "invalid"}, {"download_url": "https://other.test/package.zip"}]: artifact = {**self.artifact, **replacement} - with self.subTest(replacement=replacement), patch.object(publisher.urllib.request, "urlopen", return_value=io.BytesIO(b"ZIP")), patch.object(publisher, "api_json", return_value=artifact), self.assertRaises(SystemExit): + with self.subTest(replacement=replacement), patch.object(publisher.urllib.request, "urlopen", return_value=io.BytesIO(b"ZIP")), patch.object(publisher, "api_json", return_value={"id":"44444444-4444-4444-8444-444444444444","extension_id":"example","status":"ready","artifact":artifact}), self.assertRaises(SystemExit): publisher.build_package(self.manifest, self.source) if __name__ == "__main__": diff --git a/README.md b/README.md index 279b2a6..c6b4789 100644 --- a/README.md +++ b/README.md @@ -28,14 +28,17 @@ matching the package's `manifest.json`) and attach the distributable contents at the archive root, with no wrapper folder, so it installs as-is. Publishing runs the trusted tooling from `.github/workflows/publish.yml` against package metadata checked out from the release tag. It uploads the single GitHub -ZIP asset and manifest to `POST /packages`; the directory builds the distributable -and returns an immutable artifact ID, SHA-256 and download URL. The submitted +ZIP asset and manifest to `POST /packages`; the directory queues validation and package preparation, returning a private job ID. +The publisher polls that job until it has an immutable artifact ID, SHA-256 and +download URL. A failed or timed-out job stops publication; a retry reuses queued +or completed work. The submitted release references those values, preserving earlier managed artifacts and external release history. The workflow then approves the revision using its existing `extensions:publish` permission. Deploy the package-enabled Extensions v2 API before using this publisher in -production. The machine client must have +production. Its queue, private storage and package mode must be configured +and smoke-tested first. The machine client must have `extensions:push`, an active developer mapping and permission to publish for the same developer. Package ZIPs remain private until the revision is approved. From edb3af27b12ec95fa09f4ca2420b0b0df171ff85 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sat, 7 Nov 2026 19:10:08 +0000 Subject: [PATCH 6/6] Preserve local publisher identity integration --- .github/scripts/build_payload.py | 8 ++++---- .github/scripts/test_build_payload.py | 18 +++++++++++------- README.md | 6 ++++-- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/.github/scripts/build_payload.py b/.github/scripts/build_payload.py index add18ec..88bf648 100644 --- a/.github/scripts/build_payload.py +++ b/.github/scripts/build_payload.py @@ -4,10 +4,10 @@ plus the package README, resolves the download URL from the GitHub release's .zip asset, and writes two API content files: -* ``payload-create.json`` for POST /extensions (includes the ``id`` the +* ``payload-create.json`` for POST /extensions (includes the ``identifier`` the API requires on create), * ``payload-update.json`` for PUT /extensions/{id} (identical except the - ``id``, which that endpoint rejects as an unknown field). + ``identifier``, which that endpoint rejects as an unknown field). The new release entry is keyed by the plain version (``"0.8.9"``), not the git tag (``"branding-v0.8.9"``): the API sorts and compares releases @@ -168,7 +168,7 @@ def build_package(manifest: dict, source_url: str) -> dict: deadline = time.monotonic() + 180 artifact = None while True: - if job.get("id") != job_id or job.get("extension_id") != manifest["id"]: + if job.get("id") != job_id or job.get("extension_identifier") != manifest["id"]: fail("Package processing job does not match the submitted extension") if job.get("status") == "ready": artifact = job.get("artifact") @@ -332,7 +332,7 @@ def main() -> None: content["download_url"] = selected_release["download_url"] create_content = dict(content) - create_content["id"] = ext_id + create_content["identifier"] = ext_id size = len(json.dumps(create_content, ensure_ascii=False, separators=(",", ":")).encode("utf-8")) if size > MAX_CONTENT_BYTES: diff --git a/.github/scripts/test_build_payload.py b/.github/scripts/test_build_payload.py index 65673c6..3aa3ed8 100644 --- a/.github/scripts/test_build_payload.py +++ b/.github/scripts/test_build_payload.py @@ -47,15 +47,19 @@ def build(self, history=None): def test_preserves_manifest_metadata_and_compound_requirements(self): create, update = self.build() + self.assertEqual(create["identifier"], self.meta["id"]) + self.assertNotIn("id", create) + self.assertNotIn("identifier", update) for field in self.meta: - self.assertEqual(create[field], self.meta[field]) + if field != "id": + self.assertEqual(create[field], self.meta[field]) self.assertNotIn("website", create) self.assertNotIn("homepage_url", create) self.assertNotIn("id", update) self.assertEqual(create["releases"][0]["requires"], self.meta["requires"]) self.assertNotIn("min_fossbilling_version", create["releases"][0]) self.assertEqual(create["readme"], "# Example\n") - self.assertEqual(create, {**update, "id": "example_module"}) + self.assertEqual(create, {**update, "identifier": "example_module"}) def test_release_references_the_directory_built_package(self): create, _ = self.build([{ "tag": "1.0.0", "is_latest": True, "artifact_id": "old", "sha256": "b" * 64, "requires": self.meta["requires"] }]) @@ -198,7 +202,7 @@ def test_uploads_the_exact_source_and_manifest_with_auth_only_on_the_api_call(se calls = [] def open_url(request, **kwargs): calls.append(request) - return io.BytesIO(b"source ZIP" if isinstance(request, str) else json.dumps({"result": {"id":"44444444-4444-4444-8444-444444444444","extension_id":"example","status":"ready","artifact":self.artifact}}).encode()) + return io.BytesIO(b"source ZIP" if isinstance(request, str) else json.dumps({"result": {"id":"44444444-4444-4444-8444-444444444444","extension_identifier":"example","status":"ready","artifact":self.artifact}}).encode()) with patch.object(publisher.urllib.request, "urlopen", side_effect=open_url): self.assertEqual(publisher.build_package(self.manifest, self.source), self.artifact) self.assertEqual(calls[0], self.source) @@ -209,14 +213,14 @@ def open_url(request, **kwargs): self.assertIn(b"source ZIP", calls[1].data) def test_polls_a_queued_package_before_returning_the_artifact(self): - job = {"id": "44444444-4444-4444-8444-444444444444", "extension_id": "example", "status": "queued", "artifact": None} + job = {"id": "44444444-4444-4444-8444-444444444444", "extension_identifier": "example", "status": "queued", "artifact": None} ready = {**job, "status": "ready", "artifact": self.artifact} with patch.object(publisher.urllib.request, "urlopen", return_value=io.BytesIO(b"ZIP")), patch.object(publisher, "api_json", side_effect=[job, {**job, "status": "processing"}, ready]) as request, patch.object(publisher.time, "sleep"): self.assertEqual(publisher.build_package(self.manifest, self.source), self.artifact) self.assertEqual(request.call_args_list[-1].args, ("/package-jobs/" + job["id"],)) def test_stops_polling_at_the_deadline(self): - job = {"id": "44444444-4444-4444-8444-444444444444", "extension_id": "example", "status": "queued"} + job = {"id": "44444444-4444-4444-8444-444444444444", "extension_identifier": "example", "status": "queued"} with patch.object(publisher.urllib.request, "urlopen", return_value=io.BytesIO(b"ZIP")), patch.object(publisher, "api_json", return_value=job) as request, patch.object(publisher.time, "monotonic", side_effect=[0, 179, 179.5, 180]), patch.object(publisher.time, "sleep"): with self.assertRaises(SystemExit): publisher.build_package(self.manifest, self.source) @@ -224,7 +228,7 @@ def test_stops_polling_at_the_deadline(self): self.assertEqual(request.call_args.kwargs, {"timeout": 0.5}) def test_failed_job_stops_publishing(self): - job = {"id": "44444444-4444-4444-8444-444444444444", "extension_id": "example", "status": "failed", "error": {"message": "Invalid archive"}} + job = {"id": "44444444-4444-4444-8444-444444444444", "extension_identifier": "example", "status": "failed", "error": {"message": "Invalid archive"}} with patch.object(publisher.urllib.request, "urlopen", return_value=io.BytesIO(b"ZIP")), patch.object(publisher, "api_json", return_value=job), self.assertRaises(SystemExit): publisher.build_package(self.manifest, self.source) @@ -237,7 +241,7 @@ def test_rejects_oversized_sources_before_uploading(self): def test_rejects_mismatched_artifact_metadata_and_foreign_download_urls(self): for replacement in [{"manifest": {}}, {"sha256": "invalid"}, {"download_url": "https://other.test/package.zip"}]: artifact = {**self.artifact, **replacement} - with self.subTest(replacement=replacement), patch.object(publisher.urllib.request, "urlopen", return_value=io.BytesIO(b"ZIP")), patch.object(publisher, "api_json", return_value={"id":"44444444-4444-4444-8444-444444444444","extension_id":"example","status":"ready","artifact":artifact}), self.assertRaises(SystemExit): + with self.subTest(replacement=replacement), patch.object(publisher.urllib.request, "urlopen", return_value=io.BytesIO(b"ZIP")), patch.object(publisher, "api_json", return_value={"id":"44444444-4444-4444-8444-444444444444","extension_identifier":"example","status":"ready","artifact":artifact}), self.assertRaises(SystemExit): publisher.build_package(self.manifest, self.source) if __name__ == "__main__": diff --git a/README.md b/README.md index c6b4789..29b2394 100644 --- a/README.md +++ b/README.md @@ -78,8 +78,10 @@ Approve a run only after checking which tag and ref it is publishing. The client authenticates with `client_credentials` and is bound to the `fossbilling` developer profile with publish rights (see the API repo's -extensions v2 README, "Machine publishing"). If a publish fails with 409, -a revision is already pending review — a moderator resolves it in the +extensions v2 README, "Machine publishing"). Registration uses the profile's +UUID `id`; its public `identifier` remains `fossbilling`. Package manifests keep +their public extension `id`, while directory creation sends it as `identifier`. +If a publish fails with 409, a revision is already pending review — a moderator resolves it in the review queue, then re-run the workflow. Extension icons are discovered at the package root: include one of `icon.svg`, `icon.png`, `icon.webp`, or `icon.avif`. Omit the icon for the generic fallback. Icon filenames are not declared in `manifest.json`.