diff --git a/.github/workflows/xtest.yml b/.github/workflows/xtest.yml index c14af20c..9a303046 100644 --- a/.github/workflows/xtest.yml +++ b/.github/workflows/xtest.yml @@ -48,11 +48,6 @@ on: type: string default: "" description: "Comma-separated feature names to treat as supported regardless of what each SDK's `cli.sh supports` reports. Use when evaluating a fix that has not been released yet: the version gates live in this repo and answer 'no' for exactly those unreleased builds, so the cells would otherwise skip. An unknown name fails the run rather than being ignored." - force-platform-supports: - required: false - type: string - default: "" - description: "Comma-separated platform features to treat as supported. Does not override SDK gates or enable service configuration. Unknown names fail the run." workflow_call: inputs: platform-ref: @@ -91,11 +86,6 @@ on: required: false type: string default: "" - force-platform-supports: - required: false - type: string - default: "" - description: "Comma-separated platform features to treat as supported. Does not override SDK gates or enable service configuration. Unknown names fail the run." schedule: - cron: "30 6 * * *" # 0630 UTC - cron: "0 5 * * 1,3" # 500 UTC (Monday, Wednesday) @@ -111,7 +101,6 @@ concurrency: # nothing" -- so the PR gate and the nightlies are unaffected. env: XT_FORCE_SUPPORTS: ${{ inputs.force-supports }} - XT_FORCE_PLATFORM_SUPPORTS: ${{ inputs.force-platform-supports }} jobs: resolve-versions: diff --git a/AGENTS.md b/AGENTS.md index a128ce89..ffa86444 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -61,10 +61,6 @@ See `xtest/AGENTS.md` for the full table of `--sdks`, `--containers`, - `XT_FORCE_SUPPORTS` — comma-separated feature names to treat as supported regardless of what each SDK's `cli.sh supports` reports. See below. -- `XT_FORCE_PLATFORM_SUPPORTS` — comma-separated platform features to treat as - supported, independent of SDK overrides. It does not enable service settings. - In CI, use `force-platform-supports`. See `xtest/README.md` for usage. - ### Evaluating an unreleased fix: `XT_FORCE_SUPPORTS` `SDK.supports(feature)` answers from the `supports` case statements in @@ -86,10 +82,6 @@ It applies to every SDK in the run — to force one side only, narrow with than being ignored, since a silently-ignored typo is indistinguishable from a clean run. In CI, pass `force-supports` to the `X-Test` workflow dispatch. -A platform-only feature — one in `PLATFORM_ONLY_FEATURES` in `xtest/tdfs.py`, -such as `kas_uri_from_kao` — raises here too, and directs you to -`XT_FORCE_PLATFORM_SUPPORTS`. - Note `versions resolve` (which backs the workflow's `*-ref` inputs) does **not** accept the `pr:N` shorthand — pass a branch name there instead. diff --git a/xtest/AGENTS.md b/xtest/AGENTS.md index 29bf2ea7..5997f809 100644 --- a/xtest/AGENTS.md +++ b/xtest/AGENTS.md @@ -36,8 +36,7 @@ Beyond the repo-wide ones in `../AGENTS.md`: | Variable | Purpose | |----------|---------| | `XT_TMP_DIR` | Root for generated fixtures and ciphertexts (default `tmp/`). Point at a large volume for `medium`/`large` runs. | -| `XT_FORCE_SUPPORTS` | Comma-separated features to treat as supported, bypassing the `cli.sh supports` gate. For evaluating a fix before it releases — see `../AGENTS.md`. Unknown names raise, as do platform-only ones (`PLATFORM_ONLY_FEATURES` in `tdfs.py`). | -| `XT_FORCE_PLATFORM_SUPPORTS` | Comma-separated platform features to treat as supported. Independent of SDK overrides; does not enable service configuration. Unknown names raise. | +| `XT_FORCE_SUPPORTS` | Comma-separated features to treat as supported, bypassing the `cli.sh supports` gate. For evaluating a fix before it releases — see `../AGENTS.md`. Unknown names raise. | ## Authoring a New Test diff --git a/xtest/README.md b/xtest/README.md index e3272355..6bdfcc40 100644 --- a/xtest/README.md +++ b/xtest/README.md @@ -116,18 +116,6 @@ uv sync --extra dev pytest ``` -#### Testing unreleased platform features - -Set `XT_FORCE_PLATFORM_SUPPORTS` to a comma-separated list of platform features -to bypass their test gates. In CI, use the `force-platform-supports` input. -SDK overrides use `XT_FORCE_SUPPORTS` separately. These overrides do not enable -service configuration. - -Both reject unknown feature names. A few features are platform-only -(`PLATFORM_ONLY_FEATURES` in `tdfs.py`); naming one in `XT_FORCE_SUPPORTS` is -rejected too, because it would force the feature on for every SDK while leaving -the platform gate the tests read untouched — a green run that tested nothing. - #### Run TDF Tests ```shell diff --git a/xtest/tdfs.py b/xtest/tdfs.py index 60bf26e9..e98961c4 100644 --- a/xtest/tdfs.py +++ b/xtest/tdfs.py @@ -10,7 +10,7 @@ import zipfile from collections.abc import Callable from pathlib import Path -from typing import Any, Literal, TypeIs, cast, get_args +from typing import Any, Literal, TypeIs, get_args import jsonschema import pytest @@ -169,10 +169,8 @@ def is_sdk_type(val: str) -> TypeIs[sdk_type]: "hexless", "hexaflexible", "kasallowlist", - # Platform: resolve managed keys using the KAS URI from the KAO. Force-only - # until the first supported release is known -- via XT_FORCE_PLATFORM_SUPPORTS, - # since it is listed in PLATFORM_ONLY_FEATURES below. KAS also needs the - # setting on. + # Platform: resolve managed keys using the KAS URI from the KAO. + # KAS also needs the setting on. "kas_uri_from_kao", # Allow and respect assigning specific keys (kas url + key id) to attributes, # including splitting with multiple keys on the same kas (sdk feature), @@ -212,60 +210,23 @@ def is_sdk_type(val: str) -> TypeIs[sdk_type]: ] -ALL_FEATURES: frozenset[feature_type] = frozenset(get_args(feature_type)) - -#: Features only the platform can report, so only ``XT_FORCE_PLATFORM_SUPPORTS`` -#: can force them. -#: -#: Both overrides validate against the same ``feature_type`` Literal, so without -#: this set ``XT_FORCE_SUPPORTS=kas_uri_from_kao`` parses cleanly, forces the -#: feature on for every *SDK*, and leaves the platform gate the test actually -#: consults untouched. That is the silent no-op the validation exists to prevent, -#: just reached by naming the wrong variable instead of misspelling the feature. -#: -#: Deliberately narrow. Most features are legitimately both-sided -- -#: ``autoconfigure`` and ``ecwrap`` need the SDK to emit them *and* the platform -#: to honour them, so forcing either side alone is a reasonable thing to ask for. -#: Only add a name here when no ``sdk/*/cli.sh`` has a ``supports`` case for it. -PLATFORM_ONLY_FEATURES: frozenset[feature_type] = frozenset({"kas_uri_from_kao"}) - -SDK_FORCEABLE_FEATURES = ALL_FEATURES - PLATFORM_ONLY_FEATURES - - -def _parse_forced_supports( - raw: str, - *, - source: str = "XT_FORCE_SUPPORTS", - allowed: frozenset[feature_type] = SDK_FORCEABLE_FEATURES, -) -> frozenset[feature_type]: - """Parse an SDK or platform override into validated feature names. +def _parse_forced_supports(raw: str) -> frozenset[str]: + """Parse ``XT_FORCE_SUPPORTS`` into a set of feature names. An unrecognised name is a hard error rather than a no-op. The override exists to turn a skip into a real result, so a typo that quietly left the skip in place would be indistinguishable from a clean run -- which is the exact failure mode the override is meant to escape. - - ``allowed`` narrows that to the features this particular override can act - on; a real feature aimed at the wrong variable is the same silent no-op as a - typo, so it is rejected the same way. The default is the SDK-side set, - because failing closed is what keeps a new platform-only feature from - quietly becoming forceable from both. """ names = {n.strip() for n in raw.split(",") if n.strip()} - unknown = names - ALL_FEATURES + known = set(get_args(feature_type)) + unknown = names - known if unknown: raise ValueError( - f"{source} names unknown feature(s) {sorted(unknown)}; " - f"valid features are {sorted(allowed)}" + f"XT_FORCE_SUPPORTS names unknown feature(s) {sorted(unknown)}; " + f"valid features are {sorted(known)}" ) - misdirected = names - allowed - if misdirected: - raise ValueError( - f"{source} cannot force {sorted(misdirected)}: platform-only " - "feature(s) that no SDK shim reports on. Use " - "XT_FORCE_PLATFORM_SUPPORTS instead." - ) - return cast(frozenset[feature_type], frozenset(names)) + return frozenset(names) #: Features to treat as supported no matter what the SDK reports. @@ -278,8 +239,7 @@ def _parse_forced_supports( #: #: Applies to every SDK in the run. To force a feature for one side only, narrow #: the run with ``--sdks-encrypt`` / ``--sdks-decrypt`` rather than adding -#: per-SDK syntax here. Names in :data:`PLATFORM_ONLY_FEATURES` are rejected -- -#: see :data:`FORCED_PLATFORM_SUPPORTS`. +#: per-SDK syntax here. FORCED_SUPPORTS = _parse_forced_supports(os.environ.get("XT_FORCE_SUPPORTS", "")) if FORCED_SUPPORTS: @@ -291,23 +251,6 @@ def _parse_forced_supports( ) -# Platform overrides are independent of SDK overrides and service configuration. -FORCED_PLATFORM_SUPPORTS = _parse_forced_supports( - os.environ.get("XT_FORCE_PLATFORM_SUPPORTS", ""), - source="XT_FORCE_PLATFORM_SUPPORTS", - # The platform side can force anything: a both-sided feature such as - # ``ecwrap`` is as legitimate here as ``kas_uri_from_kao``. - allowed=ALL_FEATURES, -) - -if FORCED_PLATFORM_SUPPORTS: - logger.warning( - "XT_FORCE_PLATFORM_SUPPORTS is set: treating %s as supported by the " - "platform. SDK gates and service configuration are unchanged.", - ", ".join(sorted(FORCED_PLATFORM_SUPPORTS)), - ) - - container_version = Literal["4.2.2", "4.3.0"] policy_type = Literal["plaintext", "encrypted"] @@ -326,7 +269,6 @@ class PlatformFeatureSet(BaseModel): def __init__(self, **kwargs: dict[str, Any]): super().__init__(**kwargs) - self.features.update(FORCED_PLATFORM_SUPPORTS) v = os.getenv("PLATFORM_VERSION") if not v: print("PLATFORM_VERSION unset or empty; defaulting to 0.9.0") diff --git a/xtest/test_tdfs_units.py b/xtest/test_tdfs_units.py index 2a9c17b4..e6d86ed7 100644 --- a/xtest/test_tdfs_units.py +++ b/xtest/test_tdfs_units.py @@ -40,101 +40,6 @@ def test_unknown_name_raises(self): with pytest.raises(ValueError, match="unknown feature"): tdfs._parse_forced_supports("hexles") - def test_platform_only_name_is_rejected_by_the_sdk_override(self): - """Naming the wrong variable is the same no-op as a typo, so it fails the same. - - XT_FORCE_SUPPORTS=kas_uri_from_kao would otherwise parse, force the - feature on for every SDK, and leave the platform gate the tests actually - consult untouched -- a green run that tested nothing. - """ - with pytest.raises(ValueError, match="XT_FORCE_PLATFORM_SUPPORTS instead"): - tdfs._parse_forced_supports("kas_uri_from_kao") - - def test_platform_only_name_is_accepted_by_the_platform_override(self): - assert tdfs._parse_forced_supports( - "kas_uri_from_kao", - source="XT_FORCE_PLATFORM_SUPPORTS", - allowed=tdfs.ALL_FEATURES, - ) == frozenset({"kas_uri_from_kao"}) - - def test_both_sided_features_stay_forceable_from_either_side(self): - """The exclusion is narrow on purpose; ecwrap needs both sides to cooperate.""" - assert "ecwrap" in tdfs.SDK_FORCEABLE_FEATURES - assert tdfs._parse_forced_supports("ecwrap") == frozenset({"ecwrap"}) - assert tdfs._parse_forced_supports( - "ecwrap", - source="XT_FORCE_PLATFORM_SUPPORTS", - allowed=tdfs.ALL_FEATURES, - ) == frozenset({"ecwrap"}) - - def test_a_typo_is_reported_as_unknown_not_as_misdirected(self): - """The two failures have different fixes, so they must not share a message.""" - with pytest.raises(ValueError, match="unknown feature") as excinfo: - tdfs._parse_forced_supports("kas_uri_from_ka0") - assert "XT_FORCE_PLATFORM_SUPPORTS instead" not in str(excinfo.value) - - def test_the_sdk_valid_list_does_not_advertise_platform_only_names(self): - """A rejection that then lists the rejected name as valid is a dead end.""" - with pytest.raises(ValueError, match="unknown feature") as excinfo: - tdfs._parse_forced_supports("hexles") - assert "kas_uri_from_kao" not in str(excinfo.value) - - -# --- Platform forced support ------------------------------------------------ - - -@pytest.mark.parametrize("platform_version", ["0.12.0", "main", ""]) -@pytest.mark.parametrize("forced", [False, True]) -def test_platform_forced_supports( - monkeypatch: pytest.MonkeyPatch, platform_version: str, forced: bool -): - monkeypatch.setenv("PLATFORM_VERSION", platform_version) - # A source tag alone must not imply feature support. - monkeypatch.setenv("PLATFORM_TAG", "main") - monkeypatch.setattr(tdfs, "FORCED_SUPPORTS", tdfs._parse_forced_supports("dpop")) - monkeypatch.setattr( - tdfs, - "FORCED_PLATFORM_SUPPORTS", - tdfs._parse_forced_supports("dpop_nonce_challenge" if forced else ""), - ) - monkeypatch.setattr(tdfs, "_fetch_well_known", lambda: None) - - features = tdfs.PlatformFeatureSet() - assert ("dpop_nonce_challenge" in features.features) is forced - # An SDK override must not force the corresponding platform feature. - assert "dpop" not in features.features - - -def test_platform_forced_supports_do_not_leak(monkeypatch: pytest.MonkeyPatch): - monkeypatch.setenv("PLATFORM_VERSION", "main") - monkeypatch.setattr( - tdfs, - "FORCED_PLATFORM_SUPPORTS", - tdfs._parse_forced_supports("dpop_nonce_challenge"), - ) - forced = tdfs.PlatformFeatureSet() - monkeypatch.setattr(tdfs, "FORCED_PLATFORM_SUPPORTS", frozenset()) - unforced = tdfs.PlatformFeatureSet() - assert "dpop_nonce_challenge" in forced.features - assert "dpop_nonce_challenge" not in unforced.features - - -def test_platform_override_does_not_override_sdk(monkeypatch: pytest.MonkeyPatch): - monkeypatch.setattr(tdfs, "FORCED_SUPPORTS", frozenset()) - monkeypatch.setattr( - tdfs, - "FORCED_PLATFORM_SUPPORTS", - tdfs._parse_forced_supports("dpop_nonce_challenge"), - ) - sdk = object.__new__(tdfs.SDK) - sdk._supports = {"dpop_nonce_challenge": False} - assert not sdk.supports("dpop_nonce_challenge") - - -def test_unknown_platform_override_names_source(): - with pytest.raises(ValueError, match="XT_FORCE_PLATFORM_SUPPORTS names unknown"): - tdfs._parse_forced_supports("dpop_typo", source="XT_FORCE_PLATFORM_SUPPORTS") - # --- tdfs.zip64_reader_is_broken ----------------------------------------------