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

Allow spin.alt host for self-requests #3003

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

itowlson
Copy link
Contributor

This is a proposal to address fermyon/spin-js-sdk#298.

The issue there is that JS guests use the inbuilt fetch function to make HTTP requests. If fetch sees a URL without a host, it helpfully prepends the host it thinks it's running on before letting Spin see it. So for a self-request such as fetch("/back"), Spin sees http://localhost:3000/back or whatever. Which is not on the allow list so gets the banhammer.

This PR reluctantly compromises with fetch by allowing self-requests via the pseudo-host self.alt. So if you do fetch("http://self.alt/back"), Spin will treat it as a self-request: it will validate permission using the self-request permission and route it as a self request.

The developer experience is, admittedly, not very lovely. But it hopefully gives JS folks an escape hatch.

Notes for reviewers:

  • Self-request permission is still given via allowed_outbound_hosts = ["http://self"], not http://self.alt. Should we allow self.alt as another way of expressing the same permission?
  • I implemented this only in wasi-http because that's what JS goes through. Should I implement it in spin-http too? It looked faffier there because of the way the types line up, but certainly do-able.

@karthik2804
Copy link
Contributor

@itowlson this overall looks good, an alternate suggestion that came up during the original PR adding this feature also seems to indicate that this will benefit go.
#1710 (comment)

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

Successfully merging this pull request may close these issues.

2 participants