Skip to content
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

Policy Lang: Enforce that serialize/deserialize can only be used in seal/open #105

Open
chip-so opened this issue Feb 20, 2025 · 7 comments
Labels
policy-compiler Policy language compiler

Comments

@chip-so
Copy link
Contributor

chip-so commented Feb 20, 2025

Because the compiler treats seal/open like pure functions, we can erroneously use serialize/deserialize in pure functions. We should track seal/open as their own kind of StatementContext so we can enforce this.

@chip-so chip-so added the policy-compiler Policy language compiler label Feb 20, 2025
@chip-so
Copy link
Contributor Author

chip-so commented Feb 20, 2025

@ycarmy @mooreoak @jdygert-spok When pondering this I thought maybe this requirement should be relaxed slightly - as spec'd, you can't use serialize/deserialize in a pure function even if it's being called from seal/open. I think this should be dependent on the execution context instead of the statement context. That is, it's allowed in anything called from seal/open, not just from within seal/open itself.

@jdygert-spok
Copy link
Contributor

My initial reaction is that it doesn't make much sense to allow in functions when we don't have generics.

@chip-so
Copy link
Contributor Author

chip-so commented Feb 20, 2025

My initial reaction is that it doesn't make much sense to allow in functions when we don't have generics.

I guess the only reason you'd want to put it in a function is to use it from multiple commands, and in that case you couldn't have a definite return type. That makes sense.

@ycarmy
Copy link
Contributor

ycarmy commented Feb 21, 2025

My initial reaction is that it doesn't make much sense to allow in functions when we don't have generics.

I guess the only reason you'd want to put it in a function is to use it from multiple commands, and in that case you couldn't have a definite return type. That makes sense.

Yeah, I mostly agree with these but if we really want to support arbitrary customizations of these blocks as originally designed, then it might be beneficial to allow calling serialize/deserialize from a function even if we don't have generics yet. If a policy has multiple commands with a common field that's used for serializing/deserializing, then making a single function that handles this portion of the computation within seal/open would be best to ensure consistency where intended.

For more clarity on the potential scenarios I'm thinking about, consider this line in the open block - it is extracting a public key contained in the command so that it can be used for verification. In this policy, that is the only command which does this but someone could write a policy that has a number of different commands that do something similar, for instance, to support letting "guest" users perform those commands and thus requiring the guest user's public key be sent through the command so that the same wire format as all other commands can still be used. This is obviously just a random example based on some of our existing code, but especially with our support for custom envelopes and seal/open blocks, I think it is possible to have other policies that would benefit from having functions that call serialize/deserialize within seal/open.

@chip-so
Copy link
Contributor Author

chip-so commented Feb 21, 2025

If I'm understanding this properly, you're describing a function that deserializes arbitrary commands but always transforms it to the same type. I suppose that works for the types of the function but it does depend on deserialize() having an indeterminate type. To close the holes in the type system, we'll still have to formalize some kind of type for the deserialized value - generics, union types, "any command struct", something like that.

I think we can enforce (via tracing) that the type of deserialize() is the Command whose open ultimately called it, which will work for your use case.

@mooreoak
Copy link
Contributor

Did you just prepose type infringing?

@chip-so
Copy link
Contributor Author

chip-so commented Feb 21, 2025

Did you just prepose type infringing?

(Assuming you meant "inferencing")

Maybe in a limited sense? What I was proposing is you could trace every open block to ensure that any usage of deserialize() is correct, but I'm now realizing that means you'd have run a compilation pass to do the type checking on every one of those cases. Which I guess we could do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
policy-compiler Policy language compiler
Projects
None yet
Development

No branches or pull requests

4 participants