Encrypt the project webhook secret and issue it through the API - #379
Merged
Conversation
projects.webhook_secret held the shared secret in plaintext while
github_app_credentials.webhook_secret_encrypted, the same class of value,
has been Fernet ciphertext since 0019. Migrations 0084-0086 expand, carry
the existing values over, and drop the plaintext column. Existing secrets
are re-encrypted rather than regenerated: the SCM holds the same value and
a new one would refuse every delivery with no symptom but stalled scans.
The column can no longer be written by hand, so activation moves out of the
operator SQL the guide used to hand out and into
POST /v1/projects/{id}/webhook-secret, which returns the plaintext once.
provider is required there, since a secret without one matches nothing in
the gateway lookup. GET /v1/projects/{id}/webhook reports configured state
without the value.
Verification decrypts at delivery time; an unreadable secret is refused the
way a bad signature is and logged distinctly, so a changed encryption key is
tellable from a webhook nobody finished setting up.
`kind=http` is not in VALID_KINDS. The block is an HTTP fence, which the linter counts as executable and so requires an annotation, but a POST with a body and a bearer token is not the GET-with-url shape `kind=api` runs. It is `kind=manual`, matching what the page asks a person to do.
Declaring the two routes in permission-matrix.json is what made test_viewer_matrix consider them: it classifies role-gated routes, and a route with no matrix row was not one. Both are denied to viewer. Reading whether a webhook is configured is part of the same setting as issuing it, and issuing invalidates the current secret.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This product already decided that a webhook shared secret is encrypted at rest.
github_app_credentials.webhook_secret_encryptedhas been Fernet ciphertext since 0019.projects.webhook_secretheld the same class of value in a plaintextVARCHAR(64), and both let whoever holds them forge a delivery this deployment accepts. This finishes applying the existing judgement rather than introducing a new policy.Encryption rather than hashing, because verification needs the value back: GitHub's HMAC is recomputed from it and GitLab's token is compared against it.
Migrations 0084 (add column), 0085 (encrypt what is stored), 0086 (drop the plaintext column). 0085 carries existing values over instead of regenerating them. A new secret would not match what the SCM holds, and the symptom of that mismatch is only that scans stop starting, which is the shape of the ER70 defect this same page produced once already.
The column can no longer be written by hand, so activation had to move out of the operator SQL the guide handed out.
POST /v1/projects/{project_id}/webhook-secretissues the secret and returns it once;provideris required, because a secret with no provider matches nothing in the gateway's lookup.GET /v1/projects/{project_id}/webhookreports whether one is set without returning it. Both areteam_admin, matching the level that edits the project's other settings.At delivery time the ciphertext is decrypted through one helper, so a secret that will not decrypt is refused exactly as a bad signature is. That keeps the status-code oracle closed while the log distinguishes the two causes: a deployment whose key changed needs re-issuing, a project that was never activated needs somebody to finish setting it up.
Verification
Migrations were run against a database seeded at 0083 with one project holding a plaintext secret and one holding none. The backfill log reported one row; the ciphertext was then read out and decrypted, and it matched the original value. Re-running 0085 reported zero rows and left the ciphertext unchanged.
Three mutations were planted and each was confirmed applied before the run. Dropping the provider assignment fails 5 tests; storing the plaintext instead of the ciphertext fails 5; returning the unreadable ciphertext instead of refusing fails 1. The last of those had no coverage until it was written, which is why it is a separate file.
Full backend suite (
tests/unit tests/integration) on a clean database: 9253 passed, 15 skipped. mypy clean across 922 files, ruff clean, ko-style 0 findings, i18n:check in sync.Also changed
The Integrations page told users the shared secret was "set in the project's Settings tab", which does not exist and did not exist before this change either; the guide said operator SQL. Both locales now describe the endpoint.
tests/unit/openapi_endpoints.jsongains the two routes and nothing else.permission-matrix.jsondeclares their gates.