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

DPLT-1044 Add context.fetchFromSocialApi #134

Merged
merged 2 commits into from
Jul 19, 2023

Conversation

morgsmccauley
Copy link
Collaborator

@morgsmccauley morgsmccauley commented Jul 17, 2023

ref: #131

This PR limits the fetch function exposed to the VM to only api.near.social. This limits the potential security vulnerabilities that come with exposing fetch as is.

@morgsmccauley morgsmccauley requested a review from a team as a code owner July 17, 2023 19:58
@gabehamilton
Copy link
Collaborator

I think it'd be worth putting the fetchLatestSocialBlock method on the context so we keep it locked down. It's oddly specific code to have in the runner but it keeps things locked down.

@morgsmccauley
Copy link
Collaborator Author

@gabehamilton what is the motive for keeping things locked down? Is there risk in exposing fetch?

@morgsmccauley
Copy link
Collaborator Author

morgsmccauley commented Jul 18, 2023

@gabehamilton maybe we can have fetchFromSocialApi which is hardcoded to the social API endpoint? That seems limited enough to prevent any damage, but still flexible enough to support a wide variety of use cases?

@gabehamilton
Copy link
Collaborator

fetchFromSocialApi sounds great

@morgsmccauley morgsmccauley changed the title DPLT-1044 Move fetch to context DPLT-1044 Add context.fetchFromSocialApi Jul 18, 2023
@@ -266,6 +264,9 @@ export default class Indexer {
value
);
},
fetchFromSocialApi: async (path, options) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only adding to imperative as it doesn't make sense in a functional context, in most cases you want to use the result of fetch.

@morgsmccauley morgsmccauley force-pushed the DPLT-1044-wrap-fetch-in-context branch from 317061a to ef44c62 Compare July 18, 2023 21:39
@morgsmccauley morgsmccauley merged commit db7cc70 into main Jul 19, 2023
1 check passed
@morgsmccauley morgsmccauley deleted the DPLT-1044-wrap-fetch-in-context branch July 19, 2023 01:45
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