feat: add settings structs to the public API #1447

ok-nick · 2025-09-24T20:35:23Z

This PR exposes our settings structs to the public API and adds json schemas to them for better compatibility with our bindings.

For reviewers:

What fields do we want exposed and what fields do we not want exposed?
Do we want to move around any fields?

codspeed-hq · 2025-09-25T18:32:24Z

CodSpeed Performance Report

Merging #1447 will not alter performance

_{Comparing ok-nick/pub-settings (9d80172) with main (2ffde3d)}

Summary

✅ 16 untouched
⏩ 2 skipped¹

2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

sdk/src/settings/builder.rs

sdk/src/settings/mod.rs

sdk/src/settings/builder.rs

…er for version

ok-nick · 2025-09-29T19:04:20Z

sdk/src/settings/mod.rs

+    /// <div class="warning">
+    /// Verifying trust is REQUIRED by the C2PA spec. This option should only be used for development or testing.
+    /// </div>
+    pub verify_trust: bool,


Maybe we should have verify_c2pa_trust and verify_cawg_trust to be clear they are separate and also to clarify it doesn't encompass timestamp trust. I'll add to the docs that verify_timestamp_trust should be used in that case.

It seems to me that if you configure the Trust settings, you probably want to use them. So why do we need a separate set of flags to enable them?

We currently do have a separate set of flags to enable them, see Trust::verify_trust_list (CAWG) and Verify::verify_trust (C2PA).

Ok, but why? Trust lists are required by the spec, I guess disabling could be useful for debugging?

I'd like to hear @scouten-adobe input on this.

We have a separate Trust structure (see https://github.com/contentauth/c2pa-rs/pull/1447/files/aee4a005f7ebabead90b13bb5f9afe92a177f40f#diff-40b334ea0cacc187a239b5763c0403155527f660c81e8e90462bc01a3c36bf0dR50) which already occurs twice, once for CAWG and once for C2PA.

I'd like to see us move as much of this configuration into that tree and then the question of which one you are enabling/disabling will be much more obvious (IMHO).

sdk/src/settings/builder.rs

sdk/src/settings/mod.rs

tmathern · 2025-09-30T20:56:05Z

sdk/src/settings/mod.rs

-// TODO: all of these settings aren't implemented
-// Settings for core C2PA-RS functionality
+/// Settings to configure core features.
+#[cfg_attr(feature = "json_schema", derive(schemars::JsonSchema))]


Shouldn't we keep core features config SDK-internal?

I think the core settings in this PR are reasonable to expose to the user.

Let's make sure we get Maurice's opinion on these

sdk/src/settings/mod.rs

sdk/examples/c2pa.toml

sdk/src/settings/builder.rs

gpeacock · 2025-10-06T16:19:31Z

sdk/src/settings/mod.rs

-// TODO: all of these settings aren't implemented
-// Settings for core C2PA-RS functionality
+/// Settings to configure core features.
+#[cfg_attr(feature = "json_schema", derive(schemars::JsonSchema))]


Let's make sure we get Maurice's opinion on these

sdk/src/settings/mod.rs

scouten-adobe · 2025-10-10T20:50:39Z

sdk/src/settings/mod.rs

+    /// <div class="warning">
+    /// Verifying trust is REQUIRED by the C2PA spec. This option should only be used for development or testing.
+    /// </div>
+    pub verify_trust: bool,


We have a separate Trust structure (see https://github.com/contentauth/c2pa-rs/pull/1447/files/aee4a005f7ebabead90b13bb5f9afe92a177f40f#diff-40b334ea0cacc187a239b5763c0403155527f660c81e8e90462bc01a3c36bf0dR50) which already occurs twice, once for CAWG and once for C2PA.

I'd like to see us move as much of this configuration into that tree and then the question of which one you are enabling/disabling will be much more obvious (IMHO).

mauricefisher64 · 2025-10-13T14:11:11Z

sdk/src/settings/builder.rs

    /// An image in WEBP format.
    WebP,
    /// An image in TIFF format.
    Tiff,


Why do we support TIFF thumbnails>

It's naturally supported by the image crate although I have no objection to removing it.

mauricefisher64 · 2025-10-13T14:18:16Z

cli/tests/fixtures/trust/cawg_sign_settings.toml

 #operating_system.name = "macOS"
-# Or if the name isn't specified, it can be inferred automatically.
-operating_system.infer = true
+# Or specify "auto" to infer the operating system automatically.


Why are we fetching OCSP responses for every certificate as a default!

certificate_status_fetch = "all"
certificate_status_should_override = true

Hm, that's interesting. @scouten-adobe any input on this?

mauricefisher64 · 2025-10-13T14:28:09Z

sdk/src/settings/builder.rs

+    /// Fetch OCSP for all manifests.
    All,
+    /// Fetch OCSP for the active manifest only.
    Active,


How is this meant to be used in the settings file? Without an "off" choice then the presence of OCSPFetchScope will mean we are fetching responses. I do not thing fetching should ever default to on.

certificate_status_fetch is defined as Option<OcspFetchScope> that defaults to None.

mauricefisher64 · 2025-10-13T14:38:28Z

sdk/src/settings/mod.rs

+    /// Whether to fetch the certificates OCSP status during validation.
+    ///
+    /// The default value is false.
+    pub ocsp_fetch: bool,


Has nothing to do with Reader, the value determines whether we fetch OCSP during validation.

Yep, that's what I was trying to convey. The docs seem to match your description, which part seems unclear?

sdk/tests/fixtures/test_settings.toml

…b-settings

ok-nick added 6 commits September 24, 2025 15:43

feat: add settings structs to public API

550319b

feat: add json schemas for settings structs

2d710cb

docs: document all settings structs and fields

10aed0e

docs: document settings modules

992ec27

docs: finishing documenting settings structs and fix doc lints

17927f5

docs: document main settings struct fields

be43767

ok-nick marked this pull request as ready for review September 25, 2025 18:13

ok-nick added the safe to test label Sep 25, 2025

fix: naming and old field tests

f752fb5

fix: add back trust.verify_trust_list

40591e7

gpeacock requested changes Sep 26, 2025

View reviewed changes

ok-nick added 4 commits September 29, 2025 14:29

fix: doc verify setting warnings, private icon resource, single integ…

a89476f

…er for version

fix: specify claim generator info operating system as enum

b3e6cb4

fix: example settings files

aee4a00

docs: clarify verify_trust doesn't include verify_timestamp_trust

2ae744f

ok-nick commented Sep 29, 2025

View reviewed changes

sdk/src/settings/mod.rs Show resolved Hide resolved

docs: improve certificate status fetch/override docs

3cfa107

ok-nick force-pushed the ok-nick/pub-settings branch from 9faaa21 to 3cfa107 Compare September 29, 2025 20:23

ok-nick commented Sep 29, 2025

View reviewed changes

sdk/src/settings/mod.rs Show resolved Hide resolved

sdk/src/settings/mod.rs Show resolved Hide resolved

ok-nick requested a review from gpeacock September 29, 2025 20:30

tmathern reviewed Sep 30, 2025

View reviewed changes

scouten-adobe mentioned this pull request Oct 1, 2025

feat: Make settings not thread-local #1444

Merged

This was referenced Oct 1, 2025

feat: impl ReaderBuilder #1465

Open

Tracking issue for settings #1471

Open

Generate cross-language documentation for settings #1473

Open

gpeacock requested changes Oct 6, 2025

View reviewed changes

ok-nick commented Oct 6, 2025

View reviewed changes

sdk/src/settings/mod.rs Show resolved Hide resolved

docs: clarify ocsp_fetch and certificate status settings

3b2cb72

ok-nick added 4 commits October 8, 2025 11:23

fix: private OcspFetchScope

9fb4585

docs: clarify remote_manifest_fetch docs

4efca84

docs: clarify thumbnail settings

62f34dd

fix: use LLVM triples as claim generator OS convention

7d93444

scouten-adobe requested changes Oct 10, 2025

View reviewed changes

gpeacock and others added 2 commits October 11, 2025 16:40

Merge branch 'main' into ok-nick/pub-settings

2791e68

docs: remove hidden attribute for pub(crate) fields and clean up

585c9bc

ok-nick requested a review from mauricefisher64 October 13, 2025 13:47

mauricefisher64 reviewed Oct 13, 2025

View reviewed changes

sdk/tests/fixtures/test_settings.toml Show resolved Hide resolved

ok-nick added 5 commits October 13, 2025 12:16

fix: add c2pa-kp-claimSigning OID to trusted EKUs

f3d9ff4

Merge branch 'main' of github.com:contentauth/c2pa-rs into ok-nick/pu…

c2dd7f8

…b-settings

Merge branch 'main' of github.com:contentauth/c2pa-rs into ok-nick/pu…

d5816e6

…b-settings

fix: don't serde skip builder.actions, only schemars skip

2e221f4

fix: feature flag schemars(skip)

9d80172

feat: add settings structs to the public API #1447

Are you sure you want to change the base?

feat: add settings structs to the public API #1447

Conversation

ok-nick commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codspeed-hq bot commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging #1447 will not alter performance

Summary

Footnotes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

ok-nick commented Sep 24, 2025 •

edited

Loading

codspeed-hq bot commented Sep 25, 2025 •

edited

Loading