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

Support retrieving specific repository files before executing script #326

Open
jeremy-boschen opened this issue Jan 19, 2023 · 8 comments
Open
Labels
enhancement New feature or request

Comments

@jeremy-boschen
Copy link

Is your feature request related to a problem? Please describe.

  1. Writing javascript in YAML is less than ideal. Most editors won't fully support syntax highlighting, linting, etc.
  2. Using the checkout action first, a user can require() a local script and run it, but performing a full checkout first for large repositories for workflows that only need to run a single script is less than optimal.

Describe the solution you'd like

  1. Add an additional input such as files which can be retrieved via the github API if not present, before the main script is run.

Describe alternatives you've considered

  1. Checking out the entire repository, then using require() and run.
  2. Using other 3rd party actions to load one or more repository files, then require() and run.
  3. Role our own to do the same thing
  4. Requesting actions/checkout add a files input to reduce retrieved content and use that here.

Additional context
I find writing actions and workflows, even using act, to be a painful experience in general. Any additional features of an action that ease the pain somewhat are greatly appreciated. Using multiple actions to simulate something that an action could do on its own works, but it's less than ideal.

@jeremy-boschen
Copy link
Author

Also, I'd commit to doing the work myself and submitting a PR

@joshmgross joshmgross added the enhancement New feature or request label Jan 21, 2023
@joshmgross
Copy link
Member

👋 Thanks for the feature request @jeremy-boschen, this sounds interesting!

It might be worth thinking about where we'd save these files and how we could make it easy to reference them. It wouldn't be ideal to pollute the current working directory. We could potentially pass some path variable to point to a temporary directory where they were saved.

RUNNER_TEMP might be a good option - https://docs.github.com/en/actions/learn-github-actions/variables#default-environment-variables

Example: https://github.com/actions/toolkit/blob/5804607845972b11605679416b2a1a57e1a3d41e/packages/tool-cache/src/tool-cache.ts#L752-L759

@jeremy-boschen
Copy link
Author

Thanks for the feedback @joshmgross!

It might be worth thinking about where we'd save these files and how we could make it easy to reference them. It wouldn't be ideal to pollute the current working directory. We could potentially pass some path variable to point to a temporary directory where they were saved.

I'm thinking it would depend on whether or not the action can load scripts from another repository.

For my use cases it wouldn't be permitted due to security concerns. If it cannot, then using the working directory would be no different than using checkout, which seems OK to me. I could also see how loading from a temp directory could open some vulnerabilities if that script or ones it depends on do relative loads itself.

If it can, then I agree a temp directory makes sense though my concern of potential exposure within the temp directory applies.

@joshmgross
Copy link
Member

Could you elaborate on how using a temporary directory could lead to vulnerabilities?

@jeremy-boschen
Copy link
Author

Say you have an action prior to github-script doing something like this, maliciously:

// pseudo-code
writeFile('$RUNNER_TEMP/localScript.js');
writeFile('$RUNNER_TEMP/localSettings.json');

Then in a github-script file which was written to $RUNNER_TEMP:

require('./localScript');
callSomeCompromisedMethodFromLocalScript();
// or
const settings = require('./localSettings.json');
doSomethingWithCompromisedSettings(settings);

I don't think tool-cache executes tools from the temp directory, does it? If not, I'd think this shouldn't either.

@joshmgross
Copy link
Member

Couldn't that malicious action just as easily write files to the current directory?

I think if you have a malicious action executing arbitrary code on your machine then there are worse things that could happen even without github-script doing something.

@jeremy-boschen
Copy link
Author

Yes, of course. It was a simplistic example.

@rdhar
Copy link

rdhar commented Nov 13, 2023

On a related note, much like @jeremy-boschen, I would like to strip out snippets of JS code into separate files outside of YAML for ease of legibility. However, I'm struggling to understand how best to achieve something this.

Before/Current
script: |
    const { data: list_comments } = await github.rest.issues.listComments({
    issue_number: context.issue.number,
    owner: context.repo.owner,
    per_page: 100,
    repo: context.repo.repo,
  });
  const get_comment = list_comments
    .sort((a, b) => b.id - a.id)
    .find((comment) => /^keyword/.test(comment.body));
  return {
    body: get_comment.body,
    id: get_comment.id,
  };
After/Proposed
script: |
  require('./comment.js');
// File: comment.js
const { data: list_comments } = await github.rest.issues.listComments({
  issue_number: context.issue.number,
  owner: context.repo.owner,
  per_page: 100,
  repo: context.repo.repo,
});
const get_comment = list_comments
  .sort((a, b) => b.id - a.id)
  .find((comment) => /^keyword/.test(comment.body));
return {
  body: get_comment.body,
  id: get_comment.id,
};

With this, I get: "SyntaxError: await is only valid in async functions and the top level bodies of modules."
If I drop the await, then I get: "ReferenceError: github is not defined."

I'm sure I'm missing something obvious with module.exports = ({ github, context }) => { ... }, but I'm not sure how best to address this particular script which: makes an API call, processes the response, and returns the output. Appreciate any thoughts/inputs, thank you.

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

No branches or pull requests

3 participants