From 1841c89ab26ff34351d860638853a15e29d68654 Mon Sep 17 00:00:00 2001 From: Eric Scouten Date: Thu, 17 Sep 2026 13:13:27 -0700 Subject: [PATCH] Add reusable c2pa-rs RC-preflight workflow; fix latent test bugs Adds .github/workflows/test-c2pa-rs-source-build.yml: a workflow that builds the native FFI library from an arbitrary c2pa-rs git ref (needed when testing a release candidate that has no crates.io publish and no prebuilt GitHub release binaries) instead of downloading a release asset. It's a no-op on ordinary PRs -- it only activates when a PR commits a target ref to c2pa-rs-preflight-ref.txt, at which point it reruns automatically on every push to that PR, same as any other check. Also fixes three latent test bugs found while probing c2pa-rs 0.91.0-rc.1 with this tool, all confirmed to still pass against the currently-pinned release: - Four ingredient tests declared testPath2 (an actual JPEG fixture) as "image/png" when adding it as an ingredient. Harmless while add_ingredient() didn't validate ingredient bytes against the declared MIME type, but still a bug in the test. - Several ingredient-archive tests built manifests with an empty or c2pa.placed-only actions list. Added an explicit leading c2pa.created/c2pa.opened action (or reordered where one already existed) to match what a real C2PA manifest should look like. - test_swapped_builder_is_freed_exactly_once asserted a swapped builder handle's address always differs from the original. That's an implementation detail of the native allocator, not a guarantee; relaxed it to check the invariant that actually matters (freed exactly once). Co-Authored-By: Claude Sonnet 5 --- .../workflows/test-c2pa-rs-source-build.yml | 204 ++++++++++++++++++ tests/test_unit_tests.py | 130 +++++++++-- 2 files changed, 315 insertions(+), 19 deletions(-) create mode 100644 .github/workflows/test-c2pa-rs-source-build.yml diff --git a/.github/workflows/test-c2pa-rs-source-build.yml b/.github/workflows/test-c2pa-rs-source-build.yml new file mode 100644 index 00000000..4fde0afe --- /dev/null +++ b/.github/workflows/test-c2pa-rs-source-build.yml @@ -0,0 +1,204 @@ +name: Test against c2pa-rs built from source + +# Validates c2pa-python against a c2pa-rs git ref that has no published +# release artifacts (e.g. a release candidate tag), by building the native +# library from source instead of downloading a prebuilt one. +# +# This is the reusable tool for RC preflights: commit the target ref to +# c2pa-rs-preflight-ref.txt on a PR branch. Its mere presence is what opts +# the PR in, so this reruns automatically on every push to that PR -- same +# as any other check -- instead of you having to remember to re-dispatch by +# hand while iterating on fixes. Delete the file again once the PR is done +# with the RC (or once c2pa-rs ships a real release and you bump +# c2pa-native-version.txt through the normal process instead). +# +# Deliberately no workflow_dispatch trigger here: dispatching this workflow +# against the default branch would run in a context with write access to +# the default branch's Actions cache scope, while checking out and +# executing an arbitrary, unvalidated c2pa-rs ref -- exactly the cache +# poisoning pattern CodeQL's actions/cache-poisoning/poisonable-step query +# looks for. A same-repo pull_request only ever gets write access to its +# own branch's cache scope, so that path doesn't have the same exposure. + +on: + pull_request: + types: + - opened + - reopened + - synchronize + - labeled + +permissions: + contents: read + +jobs: + resolve-ref: + name: Resolve c2pa-rs ref to test + runs-on: ubuntu-latest + outputs: + ref: ${{ steps.resolve.outputs.ref }} + steps: + - uses: actions/checkout@v4 + - name: Resolve ref + id: resolve + run: | + if [ -f c2pa-rs-preflight-ref.txt ]; then + ref="$(tr -d '\r\n' < c2pa-rs-preflight-ref.txt)" + else + ref="" + fi + echo "ref=$ref" >> "$GITHUB_OUTPUT" + if [ -z "$ref" ]; then + echo "No c2pa-rs-preflight-ref.txt in this tree -- nothing to test, downstream jobs will skip." + else + echo "Testing against c2pa-rs ref: $ref" + fi + + tests-unix: + name: Unit tests (Unix, ${{ matrix.os }}) + needs: resolve-ref + if: | + needs.resolve-ref.outputs.ref != '' && ( + github.event_name != 'pull_request' || + github.event.pull_request.author_association == 'COLLABORATOR' || + github.event.pull_request.author_association == 'MEMBER' || + github.event.pull_request.user.login == 'dependabot[bot]' || + contains(github.event.pull_request.labels.*.name, 'safe to test') + ) + + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + os: [ macos-latest, ubuntu-latest, ubuntu-24.04-arm ] + + steps: + - name: Checkout c2pa-python + uses: actions/checkout@v4 + with: + path: c2pa-python + + - name: Checkout c2pa-rs (${{ needs.resolve-ref.outputs.ref }}) + uses: actions/checkout@v4 + with: + repository: contentauth/c2pa-rs + ref: ${{ needs.resolve-ref.outputs.ref }} + path: c2pa-rs + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.10" + # No pip cache here: this job checks out and builds an arbitrary, + # not-necessarily-reviewed c2pa-rs ref, and CodeQL flags caching in + # that context as a cache-poisoning vector into the default branch. + + - name: Install project dependencies + working-directory: c2pa-python + run: | + python -m pip install -r requirements.txt + python -m pip install -r requirements-dev.txt + + - name: Build native library from c2pa-rs source + working-directory: c2pa-python + env: + C2PA_RS_PATH: ${{ github.workspace }}/c2pa-rs + # Build for the runner's own arch rather than the universal2 macOS + # default: it's what a local `pip install -e .` picks up anyway, + # and skips the slow cross-compiled second-arch OpenSSL build. + C2PA_LIBS_PLATFORM: ${{ matrix.os == 'macos-latest' && 'aarch64-apple-darwin' || (matrix.os == 'ubuntu-24.04-arm' && 'aarch64-unknown-linux-gnu' || 'x86_64-unknown-linux-gnu') }} + run: python scripts/build_local_artifacts.py --clean + + - name: Install package in development mode + working-directory: c2pa-python + run: | + pip uninstall -y c2pa + pip install -e . + + - name: Verify installation + working-directory: c2pa-python + run: python -c "from c2pa import C2paError; print('C2paError imported successfully')" + + - name: Run tests + working-directory: c2pa-python + run: python ./tests/test_unit_tests.py + + tests-windows: + name: Unit tests (Windows, ${{ matrix.runs-on }}) + needs: resolve-ref + if: | + needs.resolve-ref.outputs.ref != '' && ( + github.event_name != 'pull_request' || + github.event.pull_request.author_association == 'COLLABORATOR' || + github.event.pull_request.author_association == 'MEMBER' || + github.event.pull_request.user.login == 'dependabot[bot]' || + contains(github.event.pull_request.labels.*.name, 'safe to test') + ) + + runs-on: ${{ matrix.runs-on }} + strategy: + fail-fast: false + matrix: + include: + - runs-on: windows-latest + python-version: "3.10" + - runs-on: windows-11-arm + python-version: "3.11" # win-arm runner needs 3.11 at least + + steps: + - name: Checkout c2pa-python + uses: actions/checkout@v4 + with: + path: c2pa-python + + - name: Checkout c2pa-rs (${{ needs.resolve-ref.outputs.ref }}) + uses: actions/checkout@v4 + with: + repository: contentauth/c2pa-rs + ref: ${{ needs.resolve-ref.outputs.ref }} + path: c2pa-rs + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + # No pip cache here: this job checks out and builds an arbitrary, + # not-necessarily-reviewed c2pa-rs ref, and CodeQL flags caching in + # that context as a cache-poisoning vector into the default branch. + + - name: Install ARM64 OpenSSL via vcpkg (Windows ARM64) + if: matrix.runs-on == 'windows-11-arm' + shell: pwsh + run: | + # Pre-installed OpenSSL on runner fails build. + # Static OpenSSL to avoid runtime DLL load complexities. + & "$env:VCPKG_INSTALLATION_ROOT\vcpkg.exe" install openssl:arm64-windows-static-md + $vcpkgRoot = "$env:VCPKG_INSTALLATION_ROOT\installed\arm64-windows-static-md" + echo "OPENSSL_DIR=$vcpkgRoot" >> $env:GITHUB_ENV + echo "OPENSSL_STATIC=1" >> $env:GITHUB_ENV + + - name: Install project dependencies + working-directory: c2pa-python + run: | + python -m pip install -r requirements.txt + python -m pip install -r requirements-dev.txt + + - name: Build native library from c2pa-rs source + working-directory: c2pa-python + env: + C2PA_RS_PATH: ${{ github.workspace }}\c2pa-rs + run: python scripts\build_local_artifacts.py --clean + + - name: Install package in development mode + working-directory: c2pa-python + run: | + pip uninstall -y c2pa + pip install -e . + + - name: Verify installation + working-directory: c2pa-python + run: python -c "from c2pa import C2paError; print('C2paError imported successfully')" + + - name: Run tests + working-directory: c2pa-python + run: python .\tests\test_unit_tests.py diff --git a/tests/test_unit_tests.py b/tests/test_unit_tests.py index 4bff6dbb..d8eca739 100644 --- a/tests/test_unit_tests.py +++ b/tests/test_unit_tests.py @@ -2629,7 +2629,17 @@ def test_write_ingredient_archive_produces_readable_archive(self): def test_add_ingredient_from_archive_roundtrip(self): manifest = { "claim_generator_info": [{"name": "c2pa-test", "version": "0.1.0"}], - "assertions": [], + "assertions": [ + { + "label": "c2pa.actions", + "data": { + "actions": [{ + "action": "c2pa.created", + "digitalSourceType": "http://c2pa.org/digitalsourcetype/empty", + }] + } + } + ], } builder = Builder.from_json(manifest) ingredient_json = { @@ -2662,7 +2672,17 @@ def test_add_ingredient_from_archive_roundtrip(self): def test_add_ingredient_from_archive_preserves_instance_id(self): manifest = { "claim_generator_info": [{"name": "c2pa-test", "version": "0.1.0"}], - "assertions": [], + "assertions": [ + { + "label": "c2pa.actions", + "data": { + "actions": [{ + "action": "c2pa.created", + "digitalSourceType": "http://c2pa.org/digitalsourcetype/empty", + }] + } + } + ], } archive_builder = Builder.from_json(manifest) ingredient_json = { @@ -2695,7 +2715,17 @@ def test_add_ingredient_from_archive_preserves_instance_id(self): def test_add_ingredient_from_archive_preserves_instance_id_component_of(self): manifest = { "claim_generator_info": [{"name": "c2pa-test", "version": "1.0"}], - "assertions": [], + "assertions": [ + { + "label": "c2pa.actions", + "data": { + "actions": [{ + "action": "c2pa.created", + "digitalSourceType": "http://c2pa.org/digitalsourcetype/empty", + }] + } + } + ], } archive_builder = Builder.from_json(manifest) ingredient_json = { @@ -2729,7 +2759,17 @@ def test_add_ingredient_from_archive_preserves_instance_id_component_of(self): def test_add_ingredient_from_archive_preserves_instance_id_input_to(self): manifest = { "claim_generator_info": [{"name": "c2pa-test", "version": "1.0"}], - "assertions": [], + "assertions": [ + { + "label": "c2pa.actions", + "data": { + "actions": [{ + "action": "c2pa.created", + "digitalSourceType": "http://c2pa.org/digitalsourcetype/empty", + }] + } + } + ], } archive_builder = Builder.from_json(manifest) ingredient_json = { @@ -2763,7 +2803,17 @@ def test_add_ingredient_from_archive_preserves_instance_id_input_to(self): def test_add_ingredient_from_archive_roundtrip_parent_of(self): manifest = { "claim_generator_info": [{"name": "c2pa-test", "version": "1.0"}], - "assertions": [], + "assertions": [ + { + "label": "c2pa.actions", + "data": { + "actions": [{ + "action": "c2pa.created", + "digitalSourceType": "http://c2pa.org/digitalsourcetype/empty", + }] + } + } + ], } builder = Builder.from_json(manifest) ingredient_json = { @@ -2797,7 +2847,17 @@ def test_add_ingredient_from_archive_roundtrip_parent_of(self): def test_add_ingredient_from_archive_roundtrip_input_to(self): manifest = { "claim_generator_info": [{"name": "c2pa-test", "version": "1.0"}], - "assertions": [], + "assertions": [ + { + "label": "c2pa.actions", + "data": { + "actions": [{ + "action": "c2pa.created", + "digitalSourceType": "http://c2pa.org/digitalsourcetype/empty", + }] + } + } + ], } builder = Builder.from_json(manifest) ingredient_json = { @@ -2963,7 +3023,17 @@ def test_ingredient_from_archive_linked_to_edited_action(self): def test_add_two_ingredient_archives_to_one_builder(self): manifest = { "claim_generator_info": [{"name": "c2pa-test", "version": "1.0"}], - "assertions": [], + "assertions": [ + { + "label": "c2pa.actions", + "data": { + "actions": [{ + "action": "c2pa.created", + "digitalSourceType": "http://c2pa.org/digitalsourcetype/empty", + }] + } + } + ], } archives = [] for title, instance_id in [("A.jpg", "ingredient-A"), ("B.jpg", "ingredient-B")]: @@ -3001,7 +3071,17 @@ def test_add_two_ingredient_archives_to_one_builder(self): def test_write_ingredient_archive_only_contains_requested_ingredient(self): manifest = { "claim_generator_info": [{"name": "c2pa-test", "version": "1.0"}], - "assertions": [], + "assertions": [ + { + "label": "c2pa.actions", + "data": { + "actions": [{ + "action": "c2pa.created", + "digitalSourceType": "http://c2pa.org/digitalsourcetype/empty", + }] + } + } + ], } archive_builder = Builder.from_json(manifest) for title, instance_id in [("A.jpg", "ingredient-A"), ("B.jpg", "ingredient-B")]: @@ -3444,7 +3524,7 @@ def test_builder_add_multiple_ingredients(self): # Test adding another ingredient ingredient_json = '{"test": "ingredient2"}' with open(self.testPath2, 'rb') as f: - builder.add_ingredient(ingredient_json, "image/png", f) + builder.add_ingredient(ingredient_json, "image/jpeg", f) builder.close() @@ -3464,7 +3544,7 @@ def test_builder_add_multiple_ingredients_2(self): # Test adding another ingredient with a JSON string ingredient_json = '{"test": "ingredient2"}' with open(self.testPath2, 'rb') as f: - builder.add_ingredient(ingredient_json, "image/png", f) + builder.add_ingredient(ingredient_json, "image/jpeg", f) builder.close() @@ -3493,7 +3573,7 @@ def test_builder_add_multiple_ingredients_and_resources(self): ingredient_json = '{"test": "ingredient2"}' with open(self.testPath2, 'rb') as f: - builder.add_ingredient(ingredient_json, "image/png", f) + builder.add_ingredient(ingredient_json, "image/jpeg", f) builder.close() @@ -3551,7 +3631,7 @@ def test_builder_add_multiple_ingredients_and_resources_interleaved(self): ingredient_json = '{"test": "ingredient2"}' with open(self.testPath2, 'rb') as f: - builder.add_ingredient(ingredient_json, "image/png", f) + builder.add_ingredient(ingredient_json, "image/jpeg", f) builder.close() @@ -5546,6 +5626,10 @@ def test_link_archive_label_on_signing_builder_placed(self): "label": "c2pa.actions.v2", "data": { "actions": [ + { + "action": "c2pa.created", + "digitalSourceType": "http://c2pa.org/digitalsourcetype/empty", + }, { "action": "c2pa.placed", "parameters": { @@ -5689,16 +5773,16 @@ def test_link_archive_two_ingredients_labels(self): "data": { "actions": [ { - "action": "c2pa.placed", + "action": "c2pa.opened", + "digitalSourceType": "http://cv.iptc.org/newscodes/digitalsourcetype/digitalCreation", "parameters": { - "ingredientIds": ["ingredient-for-placed"] + "ingredientIds": ["ingredient-for-opened"] }, }, { - "action": "c2pa.opened", - "digitalSourceType": "http://cv.iptc.org/newscodes/digitalsourcetype/digitalCreation", + "action": "c2pa.placed", "parameters": { - "ingredientIds": ["ingredient-for-opened"] + "ingredientIds": ["ingredient-for-placed"] }, }, ] @@ -5781,6 +5865,10 @@ def test_link_archive_multiple_ingredients_in_one_placed_action(self): "label": "c2pa.actions.v2", "data": { "actions": [ + { + "action": "c2pa.created", + "digitalSourceType": "http://c2pa.org/digitalsourcetype/empty", + }, { "action": "c2pa.placed", "parameters": { @@ -8539,9 +8627,13 @@ def test_swapped_builder_is_freed_exactly_once(self): builder.close() builder.close() - # Only the replacement is must be freed here. + # The replacement must be freed exactly once; a second close() is a + # no-op. We don't separately assert original_handle's count here: the + # native allocator may legally reuse the just-freed original address + # for the replacement Box (same-size free-then-alloc within one FFI + # call), so swapped_handle and original_handle can be the same + # pointer value. The check above already covers that case correctly. self.assertEqual(self._free_count(freed, swapped_handle), 1) - self.assertEqual(self._free_count(freed, original_handle), 0) def test_repeated_swaps_on_one_builder(self): # Each with_archive consumes the handle the previous one returned, so