fix(vdr): reject VDR operations containing unknown protobuf fields - #237
fix(vdr): reject VDR operations containing unknown protobuf fields#237yshyn-iohk wants to merge 1 commit into
Conversation
…lds (#60) VDR (storage) operations that carry unknown protobuf fields are now considered invalid and silently skipped during DID resolution. PRISM (SSI) operations are intentionally left lenient to preserve forward-compatibility with future protocol extensions. Implementation: - Add ProcessError::VdrOperationContainsUnknownFields error variant - Add has_unknown_fields() helper that checks SpecialFields via UnknownFields::iter() - Guard create_storage / update_storage / deactivate_storage in V1Processor against unknown fields on both the outer PrismOperation wrapper and the inner storage-entry message Tests: - create/update/deactivate storage with unknown fields in inner message are all rejected (storage entry absent / unchanged / still active) - create storage with unknown fields on the outer PrismOperation wrapper is also rejected - create_did with unknown fields on the outer PrismOperation wrapper is still valid (SSI tolerance confirmed) Signed-off-by: Yurii Shynbuiev <yurii.shynbuiev@iohk.io>
993cf47 to
399e406
Compare
|
yshyn-iohk
changed the title
| operation: ProtoCreateStorageEntry, | ||
| prism_operation_special_fields: SpecialFields, | ||
| ) -> Result<DidStateRc, ProcessError> { | ||
| ensure_no_vdr_unknown_fields(&prism_operation_special_fields, &operation.special_fields)?; |
There was a problem hiding this comment.
I think the desired behavior is slightly different. In the code, it will reject the operation and not update the storage state. What we want is for it to update the storage, but have a flag to mark it as invalid. Once the storage entry sees an operation with unknown fields, it becomes invalid and cannot be queried, which is not the case here.
I need to recheck the wording in the spec again, but you can already use this input to steer your agent.
|
@yshyn-iohk @patextreme Should we just close de PR and is that anything to fix here? |
|
The storage state needs some work before merging. Let's keep this PR open and revisit when we have bandwidth to sort it out. |
Pull request was converted to draft
3 participants
Summary
Fixes #60
VDR (storage) operations that carry unknown protobuf fields are now considered invalid and silently skipped during DID resolution. PRISM (SSI) operations are intentionally left lenient to preserve forward-compatibility with future protocol extensions.
The check covers both the outer
PrismOperationwrapper and the inner storage-entry message (ProtoCreateStorageEntry/ProtoUpdateStorageEntry/ProtoDeactivateStorageEntry).Changes
lib/did-prism/src/protocol/error.rs— newProcessError::VdrOperationContainsUnknownFieldsvariantlib/did-prism/src/protocol/v1.rs—has_unknown_fields()helper + guard at the top ofcreate_storage,update_storage, anddeactivate_storagelib/did-prism/tests/storage_operation.rs— 5 new testsTest plan
create_storage_with_unknown_fields_is_invalid— inner message unknown fields, entry is not recordedupdate_storage_with_unknown_fields_is_invalid— inner message unknown fields, original entry unchangeddeactivate_storage_with_unknown_fields_is_invalid— inner message unknown fields, entry remains activecreate_storage_with_unknown_fields_in_prism_operation_wrapper_is_invalid— outer wrapper unknown fields, entry is not recordedcreate_did_with_unknown_fields_in_prism_operation_wrapper_is_still_valid— SSI operations tolerate unknown fields (forward-compat)cargo fmt— cleancargo clippy --all-features -- -D warnings— cleancargo test --all-features— all tests pass (storage_operation suite: 25 → 30)