planetscale/ghcommit

Symlinks aren't handled correctly

Opened this issue · 5 comments

tophercullen commented

Unsure if this should be here, or in the ghcommit-action repo. It appears likely there's an assumption being made about the file modes when generating the commit blobs. This causing this utility (and dependent action) to generate defunct commits.

Reproducer: Create a new, or modify an existing, symlink to file in the repo. Run the utility (or Github action).

Expected result: the symlink is created/updated to point to the target file.

Actual result: the symlink points to a non-existent file. The name of this file is the contents of the symlink target.

As I'm able to use another action achieve a similar result successfully (qoomon/actions--create-commit), I presume this isn't a limitation of the GH API, but something specific to its usage here.

joemiller commented

I also suspect it would be in ghcommit-action, however, my initial attempts at recreating are inconclusive at the moment. The ghcommit-action runs git status --porcelain=v1 (with other flags) and a new symlink looks identical to a new file.

tophercullen commented

my initial attempts at recreating are inconclusive at the moment.

The reproducer I outlined above still works. Here it is in essentially bare repo

qoomon (correct): https://github.com/tophercullen/scratch/pull/3/files
Planetscale (incorrect): https://github.com/tophercullen/scratch/pull/4/files

As seen in those PRs, the ghcommit-action is creating a defunct symlink commit.

You can also see this by pulling the branch and checking the file system structure. This is the result:

#ls -lah symlinks/
thelink.txt -> 'blahblahlbahblah'$'\n'

As you can see, and as noted above, the link name is the contents of the symlink target.

joemiller commented

@tophercullen Thanks for those.

I did a quick skim of the qoomon action repo (search) yesterday and it appears it does not use the graphQL createCommitOnBranch mutation that ghcommit uses. Instead it uses the REST commit API and if a github app token is used it is able to create commits signed by github's web-flow.gpg key.

From the https://github.com/qoomon/actions--create-commit README:

Commits getting signed, if a GitHub App token (ghs_***) is used and will be marked as verified in the GitHub web interface.

And this is documented by github here: https://github.com/qoomon/actions--create-commit/blob/main/lib/github.ts

Signature verification for bots will only work if the request is verified and authenticated as the GitHub App or bot and contains no custom author information, custom committer information, and no custom signature information, such as Commits API.

After reading that my next debug step would be to determine if symlinks are supported by the createCommitOnBranch graphql mutation. I suspect it is not. I have not tested it myself yet, but based on what I saw yesterday I assume the answer is no based on how the output from git status --porcelain=v1 does not distinguish between regular file or symlink in its output, which in turn means both types are passed to ghcommit with the --add flag.

The createCommitBranch mutation takes a CreateCommitOnBranchInput which is includes a FileChanges object which has two attributes: additions + deletions. The additions input takes a path and base64 encoded file contents as described here.

This leads me to believe that symlinks are not supported by the graphql API, and some googling led me to this: https://github.com/shogo82148/actions-commit-and-create-pr/

Currently (on 2022-01-03), the createCommitOnBranch mutation doesn't support file types (i.e. regular file, symlink, submodule, ...). All files will be committed as regular files. You can't create executable files, symlinks, submodules, and so on.

I am led to believe symlinks will just not be possible with ghcommit.

The purpose of ghcommit + ghcommit-action is to be the simplest, low friction way of getting verified commits on a branch via CI workflows (github actions or others). Given this limitation I think the landscape for verified commits from CI workflows (or other app/robot contexts) is:

  1. GPG keys. You can give your CI workflow or application a GPG key. The downside here is a static private key in your CI that needs to be rotated regularly to maintain good security practices (ie: people leaving joining a project, company, etc)
  2. REST commit API with Github App token. The downside is the overhead of setting up a github app (or potentially many if you want to isolate by repo) and rotating the key.
  3. The createCommitOnBranch graphql API as implemented by ghcommit and ghcommit-action. Lowest friction method. No static tokens or keys to be rotated, works with the short-lifetime GITHUB_TOKEN's issued to github-actions workflows.

Unless I discover some new information that refutes any of the above, what I think I will do is modify the README to include the following and then close this issue:

  1. Add the https://github.com/qoomon/actions--create-commit + github app token as an alternative
  2. Document the limitations of the createCommitOnBranch graphql mutation
tophercullen commented

A lot to digest, so I apologize ahead of time if I missed and/or misunderstood something.

Given this limitation I think the landscape for verified commits from CI workflows (or other app/robot contexts) is:...

This is indeed the problem I am solving (e.g. requiring signed commits). However, the solutions described don't seem accurate. Something about option 2 or 3 is off, and is why I opened this issue.

If you look those PRs again, you will see that both PR commits are signed and verified with the same key ID. But I have not setup any custom Github Apps. The token in use for both workflows is the short-lifetime GITHUB_TOKEN's issued to github-actions workflows. See the workflow files and this doc.

I do know the custom app setup you refer to. I do need to use such a thing for PRs workflows as the default actions token isn't scoped for team membership, which is required for adding reviewers in certain other actions.

  1. Add the https://github.com/qoomon/actions--create-commit + github app token as an alternative

Just to reiterate, the current implementation of qoomon in that scratch repo, does not use a custom github app token. The workflow as defined creates signed/verified commits with symlinks correctly using the short-lifetime GITHUB_TOKEN's issued to github-actions workflows.

  1. Document the limitations of the createCommitOnBranch graphql mutation

The docs indicate to me a file contents only limitation. In addition to symlinks, other non-content changes will be missed. The only one I could think of off hand, are file permissions. I added a simple reproducer to that same scratch repo that adds execute to a file. Predictably, this change was committed with the qoomon action, and notably absent in ghcommit commit.

Even if documented, these limitations can be exceptionally confusing for users. As-is, the ghcommit actions logs lead the user to believe the change(s) will be committed, and then is incorrect or silently dropped due to the API used. Obviously, my expectation would be for this type of action to support changes typical to a git add; git commit; etc. flow. But If that is untenable for some reason (seems like different API is required), I'd much prefer that the action error if it involves unsupported changes, rather than pushing bad commits or silently dropping changes, and then indicating success.

joemiller commented

@tophercullen If the qoomon action is able to create verified commits with the default GITHUB_TOKEN issued to workflows then that is the superior way to do signed commits from GHA workflows. And that is what I was wondering when I wrote the above. When I have time I will test that out and consider archiving ghcommit and ghcommit-action and moving over to qoomon. ty