search

pact-foundation / pact-mock_service

Provides a mock service for use with Pact
https://pact.io
MIT License
73 stars 69 forks source link

Should the client be able to control where the server writes pact files? #74

Open iamleeg opened 7 years ago

iamleeg commented 7 years ago

https://github.com/pact-foundation/pact-mock_service/blob/c8dcbc2609d78f433b7f1cd4a27b47628a2b7c4d/lib/pact/mock_service/request_handlers/pact_post.rb#L29

The configuration options and the post body are merged on that line to configure the ConsumerContractWriter. As I found when investigating a problem integrating pact-js with pack-mock_service, this means that the fields sent in the request depend on the launch configuration: a client may not know how the server was launched and therefore whether to supply pact_dir as part of the request body.

Further, the use of Hash#merge here means that the client can always override the configured pact_dir, controlling the output folder. That means the client can force the server to e.g. fill up a different volume than the intended pact_dir location, or write pacts to a folder not expected when the service was configured.

It seems that it should be an error not to provide a pact_dir on launch, or a reasonable default should be chosen, and that the mock service should not read it from the post request on writing a pact.

bethesque commented 7 years ago

That's a fair point about the security. I had not considered that.

The reason it's like it is, is that the mock service was originally only designed to support ruby specs, and was called by some hooks in the rspec code. Once we decided to reuse it for other projects, we moved this option into a startup option, but also maintained the original method of setting it.

You've made a good argument for removing this original method.