Repository navigation
feat(sources): persist immutable evidence declarations #1717
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ac670f8
cc91c9f
56897ff
d9213c3
68e83c3
0cb6ec1
fad0330
cc290a5
1b455fb
5628e97
67df701
fd2efa1
d071835
965af82
7683e6c
812e6e6
df6e4dc
3eed5f3
ba97791
9f24124
0dcad00
b7f8c2e
bbf8d1d
f87d6fb
02a780c
23d8dcd
df3157e
e39cca0
c58aa7f
9b22c6a
9963fd5
222a637
60d5b39
e872062
35a09d4
bd40d98
3d3527e
57a821f
2cd004f
30942a8
295c85b
5d765eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | |||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -897,7 +897,7 @@ async def submit_source_observation( | ||||||||||
| ) | |||||||||||
| except RepositoryNotFoundError: | |||||||||||
| raise SourceDefinitionNotFoundError(observation.source_type, observation.definition_version) from None | |||||||||||
| _validate_source_observation(observation, manifest) | |||||||||||
| observation = _validate_source_observation(observation, manifest) | |||||||||||
| services = self._services_for(request.scope_id) | |||||||||||
| source_store, source_catalog = services.sources() | |||||||||||
| stored = await source_store.add(observation) | |||||||||||
|
|
@@ -1232,7 +1232,7 @@ async def upload_skill_package( | ||||||||||
| ) | |||||||||||
| async with self.database.transaction() as connection: | |||||||||||
| await self.repositories.skill_packages.add(connection, scope, package) | |||||||||||
| stored = await self.repositories.sources.add(connection, scope, source) | |||||||||||
| stored = await self.repositories.sources.add(connection, scope, source, apply_definition=True) | |||||||||||
| artifacts = () if target is None else (target,) | |||||||||||
| return ( | |||||||||||
| await self | |||||||||||
|
|
@@ -1267,7 +1267,7 @@ async def record_skill_usage( | ||||||||||
| raise ValueError("usage package digest does not match the Skill Revision") # noqa: TRY003 | |||||||||||
| if observation.task_source is not None: | |||||||||||
| await self.repositories.sources.get(connection, scope, observation.task_source) | |||||||||||
| stored = await self.repositories.sources.add(connection, scope, source) | |||||||||||
| stored = await self.repositories.sources.add(connection, scope, source, apply_definition=True) | |||||||||||
| return SourceReceipt(source_ref=stored.ref, sequence=stored.journal_position) | |||||||||||
|
|
|||||||||||
| def external_skills(self, scope_id: str, /) -> ExternalSkillRegistryService: | |||||||||||
|
|
@@ -1337,7 +1337,9 @@ async def import_external_skill( | ||||||||||
| ) | |||||||||||
| async with self.database.transaction() as connection: | |||||||||||
| await self.repositories.skill_packages.add(connection, scope, capture.package) | |||||||||||
| stored, created = await self.repositories.sources.add_with_status(connection, scope, source) | |||||||||||
| stored, created = await self.repositories.sources.add_with_status( | |||||||||||
| connection, scope, source, apply_definition=True | |||||||||||
| ) | |||||||||||
| if created: | |||||||||||
| await self.repositories.processing_pending.raise_source( | |||||||||||
| connection, | |||||||||||
|
|
@@ -1889,29 +1891,44 @@ def _validate_source_definition_manifest( | ||||||||||
| ) | |||||||||||
|
|
|||||||||||
|
|
|||||||||||
| def _validate_source_observation(source: SourceObservation, manifest: SourceDefinitionManifest) -> None: | |||||||||||
| def _validate_source_observation(source: SourceObservation, manifest: SourceDefinitionManifest) -> SourceObservation: | |||||||||||
| """Validate one observation and stamp the Definition-owned evidence declaration. | |||||||||||
|
|
|||||||||||
| The declaration is Definition-owned metadata, so the registered manifest is | |||||||||||
| authoritative for it: an observation cannot manufacture a stronger attestation | |||||||||||
| than the Definition it was produced by. A remote worker does not necessarily | |||||||||||
| read the registered manifest, so a declaration that contradicts it is rejected | |||||||||||
| rather than silently rewritten. An omitted field resolves to the manifest's | |||||||||||
| declaration instead, which is what makes the check impossible to bypass by | |||||||||||
| leaving the field out. | |||||||||||
| """ | |||||||||||
|
|
|||||||||||
| if source.source_type != manifest.name or source.definition_version != manifest.version: | |||||||||||
| raise InvalidSourceObservationError("definition", "does not match the registered manifest identity") | |||||||||||
| if source.definition_fingerprint != manifest.fingerprint: | |||||||||||
| raise InvalidSourceObservationError("fingerprint", "does not match the registered manifest") | |||||||||||
| if len(source.model_dump_json().encode()) > MAX_SOURCE_OBSERVATION_BYTES: | |||||||||||
| if "memory_evidence" in source.__pydantic_fields_set__ and source.memory_evidence != manifest.memory_evidence: | |||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Blocker: the consistency gate is keyed on a serialization side-effect, and the server never stamps the declaration from the registered manifest. This check only fires when the field is present in
So the declaration is never guaranteed to equal the Definition-owned value. Today the impact is contained because nothing in Suggestion — make the server authoritative rather than advisory, for example: source = source.model_copy(update={"memory_evidence": manifest.memory_evidence})(equivalently, drop the field from the transport contract). If explicit client declarations must be preserved, keep the equality check but compare values after defaulting, so omitting the field cannot bypass it. Also: this branch has no test coverage — |
|||||||||||
| raise InvalidSourceObservationError("memory_evidence", "does not match the registered manifest") | |||||||||||
| stamped = source.model_copy(update={"memory_evidence": manifest.memory_evidence}) | |||||||||||
| if len(stamped.model_dump_json().encode()) > MAX_SOURCE_OBSERVATION_BYTES: | |||||||||||
| raise InvalidSourceObservationError("size", "must not exceed 4 MiB") | |||||||||||
| _validate_schema_value(manifest.name, manifest.source_schema, source.payload) | |||||||||||
| _validate_schema_value(manifest.name, manifest.source_schema, stamped.payload) | |||||||||||
|
|
|||||||||||
| declarations = {projection.key: projection for projection in manifest.projections} | |||||||||||
| supplied = {projection.key: projection.value for projection in source.projections} | |||||||||||
| supplied = {projection.key: projection.value for projection in stamped.projections} | |||||||||||
| if declarations.keys() != supplied.keys(): | |||||||||||
| raise InvalidSourceObservationError("projections", "must exactly match the registered manifest") | |||||||||||
| for key, declaration in declarations.items(): | |||||||||||
| value = supplied[key] | |||||||||||
| _validate_schema_value(key.name, declaration.schema_, value) | |||||||||||
| if key == TEXT_EVIDENCE_PROJECTION_KEY: | |||||||||||
| evidence = TextEvidence.model_validate(value) | |||||||||||
| if evidence.source_type != source.source_type or evidence.source_id != source.name: | |||||||||||
| if evidence.source_type != stamped.source_type or evidence.source_id != stamped.name: | |||||||||||
| raise InvalidSourceObservationError( | |||||||||||
| "text-evidence", | |||||||||||
| "source identity does not match the observation envelope", | |||||||||||
| ) | |||||||||||
| return stamped | |||||||||||
|
|
|||||||||||
|
|
|||||||||||
| def _json_schema_validator(name: str, schema: Mapping[str, Any]) -> Validator: | |||||||||||
|
|
|||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Apply the Definition declaration to fresh Subject Sources
runtime.subject_sources.createcreates fresh ContentSources viaSourceRepository.add(builtin/scope/subject_sources.py:131), which now leaves this flag false. With a content Definition declaringsystem_attested / verified / subject-review-v1, creation succeeds but both scoped copies persistuntrusted / unknown / 1; ordinary source capture in the same runtime persists the configured declaration. On SQLite and real OceanBase, reading either Subject Source and replaying that exact object throughcontext.sources.addthen raisesSourceConflictErrorunder the unchanged registry. The previous head stamps both copies correctly and replays them idempotently. Preserve historical declarations during receipt replay while ensuring fresh native writers still apply their registered Definition.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-tested at
df3157e1: Subject creation itself is fixed on SQLite and real OceanBase. Both copies receive the configured declaration, and exact native replay is idempotent.The fresh-writer omission remains P2 in other public paths. With attested
contentand Skill-package-upload Definitions, explicit Atomic Memory creation, Skill package upload, and Profile create/replace all succeed but persist neutral lineage Sources. Reading each Source and passing that exact object tocontext.sources.addunder the unchanged registry raisesSourceConflictError. Ordinary capture in the same runtime remains correctly attested. These outcomes were reproduced on SQLite and real OceanBase; identical SQLite controls atb7f8c2preserve the declarations and replay idempotently.The remaining fresh callers still omit
apply_definition=True:builtin/persistence/records.py:352,394,658and the Skill upload atbuiltin/runtime/relational.py:1222. Please apply the registered declaration to these new captures while keeping historical receipt replay unchanged.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified fixed at
e39cca09on SQLite and real OceanBase. With attested Definitions, explicit Atomic Memory creation/restoration, Skill package upload and usage, Profile create/replace, and both Subject Source copies persist the configured declaration. Their persisted Sources replay idempotently through the native API under the same registry. Restoration also preserves the original Artifact revision.Historical receipt handling still passes the actual Server lifespan with enforced access and a neutral registry; the stored attested Source remains unchanged. External Skill-provider import was inspected in code but was not executed against a provider.