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

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

Open
iamleeg opened this issue Aug 15, 2017 · 1 comment
Open

Comments

@iamleeg
Copy link

iamleeg commented Aug 15, 2017

consumer_contract_params = default_options.merge(consumer_contract_details.merge(interactions: verified_interactions))

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
Copy link
Member

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.

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

No branches or pull requests

2 participants