Regression due to #31237: Local paths not valid return values any more

[dr-yd]

Terraform Version

Working: v1.2.2
Broken:

Terraform v1.2.3
on darwin_arm64
+ provider registry.terraform.io/hashicorp/archive v2.2.0
+ provider registry.terraform.io/hashicorp/aws v4.21.0
+ provider registry.terraform.io/hashicorp/random v3.3.2

Your version of Terraform is out of date! The latest version
is 1.2.4. You can update by downloading from https://www.terraform.io/downloads.html

Same behavior in v1.2.4.

Terraform Configuration Files

n/a

Debug Output

Debug output available upon request directly to maintainers but should be easy to reproduce.

Expected Behavior

Returning a local path from an HTTP module source should be possible.

Actual Behavior

Behavior as expected in v1.2.2. In v1.2.3, sample output:

│ Could not download module "website_cloudfront_redirection" (cloudfront.website.tf:44) source code from
│ "https://example.com/cloudfront": error downloading 'https://example.com/cloudfront': invalid source
│ string: /Users/me/src/infra/terraform/aws/modules

Steps to Reproduce

Return a local path as a module location from an HTTP module source.

Additional Context

We're using a service that pins Terraform module versions depending on the repository and branch. When run locally for testing, local module paths are returned as the location. (To pipelines, git locations are returned, this works.) Pull #31237 makes this impossible. If there's a syntax to reference local paths currently, we're unaware of it.

Furthermore, the documentation is now incorrect as it states "In either case, the result is interpreted as another module source address using one of the forms documented elsewhere on this page." The local path location is documented on the same page.

https://www.terraform.io/language/modules/sources

References

#31237

[jbardin]

Hi @dr-yd,

Thanks for filing the issue. I'm not sure what exactly has changed from that commit, mostly because I surprised that the use case you've shown previously worked. The registry protocol was defined such that a path returned in X-Terraform-Get from an http URL is considered a relative path to the first request.

The value of X-Terraform-Get may instead be a relative URL, indicated by beginning with /, ./ or ../, in which case it is resolved relative to the full URL of the download endpoint to produce an HTTP URL module source.

Arbitrary protocol switching is specifically something that was not to be allowed, and the referenced patch was part of the security fixes from upstream. As this was being handled incorrectly before, and the fixes are just dis-allowing the incorrect protocol switch to evaluate local files, I have a feeling that this portion of the protocol was actually never implemented in go-getter since it's a feature the Terraform registry has ever used.

As to the issue of a regression, that will be something we have to review. Since it's also not working correctly now per the documentation (only the protocol switch was fixed, no changes were made to the redirection mechanism), and the old behavior was contrary to the security guidelines, the correct fix may be to implement the protocol as specified.

Thanks!

[dr-yd]

Hi @jbardin ,

thanks for commenting. Interesting to see that we were using an unintended functionality - I just followed the sentence from the documentation that I quoted and it worked. You answer is a bit alarming. Does it mean that switching to git (== SSH) protocol is against your intentions as well? That would be a huge problem for us.

Background: We have a central modules repo and a number of project repos that use it. It's not rare that we have to introduce breaking changes to the modules but former customers won't pay to upgrade the project repo to the new versions. So we pin them to old releases of the modules repo. (Using the NETRC env var and a .netrc.dev etc, modules are then resolved via project repo == username and branch == "password".) If that functionality gets removed, I don't think we can continue using Terraform.

If switching to git / SSH is allowable, what about adding a file:// protocol like browsers do? What is your threat model for allowing arbitrary URIs as module sources?

[jbardin]

Oh yes, while arbitrary protocol switching is something that is not desired for security reasons (having a remote system force the loading of any local file could be combined with other attack vectors), we did try to maintain the documented protocol combinations for compatibility. Switching to git should therefor work, which appears to be the case from my local testing.

[dr-yd]

Ah, that's great to hear - so at least the pipelines won't break in the future. For local development, there are workarounds (e.g. running a webserver locally and just serving local files from there) but if you come to the conclusion that there should be some official way to reference local files, that would of course be preferable from my POV. Since it's not a pressing issue, maybe you can find the time to discuss this and update the thread for one of the next releases. Thanks!

[Andres-Lu]

Hi @jbardin !

We've recently bumped into some issues when attempting to make a protocol switch from http to git and were curious around how you got that local testing done to allow switching to git.

For context, we're hosting vanity redirects through a static website towards GitHub sources that hold our Terraform modules. The url referenced in our X-Terraform-Get headers is a Git/SSH format (git@github.com:<org>/<repo>.git) and we're getting the same invalid source string error back from Terraform. Is this currently supported on Terraform 1.2.3+ or rather go-getter 1.6.2+? If not, are there any plans to support this in the future?

Upgrading modules...
Downloading http://our-url.com/test-module for test...
╷
│ Error: Failed to download module
│
│ Could not download module "test" (test.tf:5) source code from "http://our-url.com/test-module": error downloading 'http://our-url.com/test-module': invalid source string: git@github.com:<org>/<repo>.git

[dr-yd]

@Andres-Lu

For git, the response must be something like

x-terraform-get: git::ssh://git@git.example.com/terraform/modules.git//vpc?ref=v1.2.3

---

Re: Topic, I only recently noticed that the file:// protocol actually is included and still usable in TF 1.3.2. Only returning a naked filename is a problem. I didn't even check in the light of @jbardin 's response and I'm not sure if that's slated for removal so I'm leaving the issue open. For now, I'm just really happy to have 1.3.