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

Review toError and fromError sensitivity filter, agent service sensitive information removal, PK specific error passthrough, and unknown errors #564

Open
tegefaulkes opened this issue Oct 10, 2023 · 7 comments
Labels
development Standard development

Comments

@tegefaulkes
Copy link
Contributor

Specification

The current implementation of toError and fromError is basic and minimally functional. It needs to be reviewed and expanded and tested to handle all kinds of errors.

We also need the sensitive functionality for the client service. The error stack needs to be stripped from serialised errors.

Additional context

Tasks

  1. Review and expand toError and fromError
@CMCDragonkai
Copy link
Member

Is this being done with your recent changes to js-rpc? Is there a PR for this?

@tegefaulkes
Copy link
Contributor Author

Yeah, I need to fix it now. There isn't a PR for this yet.

@CMCDragonkai
Copy link
Member

What still needs to be done here? I see alot of change for this RPC error thing but no spec as to what has been changed, or what the target is. How is RPC errors still receiving so much changes?

@tegefaulkes
Copy link
Contributor Author

The toError and fromError was expanded during the final push on friday. But I'm still not sure it's fully polished. We can consider this done, but the functions could still be improved.

@CMCDragonkai
Copy link
Member

CMCDragonkai commented Nov 5, 2023

Need to prove all of these cases with tests.

  1. There needs to be tests that prove that the sensitive works and completely removes all sensitive information from errors passed back to the client of JS-RPC.
  2. There needs to be tests in PK that proves that sensitive information is removed from the errors in the agent service. (BUT NOT THE client service).
  3. Prove that all errors can be serialised and unserialised fromError to toError. This includes basic JS errors, and also Polykey-specific errors.
  4. Prove that unknown errors is appropriately captured on the client side of the RPC.

@CMCDragonkai CMCDragonkai changed the title Review toError and fromError impementation Review toError and fromError sensitivity filter, agent service sensitive information removal, PK specific error passthrough, and unknown errors Nov 5, 2023
@amydevs
Copy link
Member

amydevs commented Nov 10, 2023

It was found that the RPCServer was never using the replacer passed in from the constructor parameter. So no error stacks were ever being redacted. Furthermore, I think the filterSensitive utility function that returned a replacer, is not sufficient for our usecase, as any value with key of stack would be redacted. So stack would have to become a reserved keyword if the replacer returned from filterSensitive were used. I'm in the process of changing these

@CMCDragonkai
Copy link
Member

Well in terms of filtering, we have to decide whether it is more explicit, in the sense of filtering a specific path in a structure, or filtering based on patterns. A hybrid form is a JSON-path like structure, where you can filter specific or pattern match.

An easier way is just a regex applied to the path somehow.

The most simplest for now is a specific key path, since we already know what kind of sensitive data we want to filter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development
Development

No branches or pull requests

3 participants