Skip to content

feat(vcs): accept a base64 or escaped-newline GitHub App private key - #261

Merged
HarshMN2345 merged 1 commit into
mainfrom
feat/vcs-github-private-key-formats
Sep 12, 2026
Merged

feat(vcs): accept a base64 or escaped-newline GitHub App private key#261
HarshMN2345 merged 1 commit into
mainfrom
feat/vcs-github-private-key-formats

Conversation

@HarshMN2345

@HarshMN2345 HarshMN2345 commented Sep 11, 2026

Copy link
Copy Markdown
Member

Two open questions:

  1. Is one parameter with format detection okay, instead of the separate _APP_VCS_GITHUB_PRIVATE_KEY_BASE64 proposed in 🚀 Feature: Allow base64 for GitHub private key appwrite/appwrite#7732?
  2. Should the escaped \n handling stay in, or should this be base64 only?

What

GitHub::generateAccessToken() accepts the private key as:

  • a PEM (unchanged)
  • the PEM base64-encoded, on one line or wrapped
  • the PEM on one line with escaped \n newlines

Why one parameter

Detection can't misfire. A strict base64_decode() rejects the - in every PEM, so a key that works today is never decoded. A decoded value replaces the input only if it is itself a PEM. The Origin adapter already accepts a PEM or base64 through the same parameter.

The \n replacement only runs on values containing -----BEGIN, so file:// paths are left alone.

Testing

  • A new unit test signs a JWT with each encoding and checks it with openssl_verify. Without the change, all three cases throw Failed to read the GitHub App private key.
  • bin/monorepo check vcs and the vcs unit tier pass.

Follow-up in Appwrite

  • Bump utopia-php/vcs; the lock is on 5.2.5, so this also brings in 5.3.0.
  • Update the _APP_VCS_GITHUB_PRIVATE_KEY description and the self-hosting docs.
  • Test with a real GitHub App, because CI skips the VCSGitHub suite.

generateAccessToken() now also takes the PEM base64-encoded, on one
line or wrapped, or on one line with newlines escaped as \n. A raw PEM
never passes strict base64 decoding, so keys that already work are
used unchanged.
@greptile-apps

greptile-apps Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge with no actionable correctness, security, or repository-rule violations identified.

Summary

  • Accepts strict base64-encoded PEM values, including base64 containing whitespace.
  • Converts escaped newlines in PEM-formatted values before OpenSSL parsing.
  • Documents the accepted formats and verifies that each representation produces a valid signed JWT.

Reviews (1) · Last reviewed commit: "feat(vcs): accept a base64 or escaped-ne..."

@HarshMN2345
HarshMN2345 merged commit d974cdf into main Sep 12, 2026
5 of 6 checks passed
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