Repository navigation
Add expression quote-escaping test & implement server job timeouts an… - #6
Merged
Merged
Conversation
…d runner lease reaper
There was a problem hiding this comment.
4 issues found across 7 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/aksh-runner-server/src/github.rs">
<violation number="1" location="crates/aksh-runner-server/src/github.rs:467">
P1: Reject empty webhook secrets, not just missing ones. An empty `AKSH_WEBHOOK_SECRET` is treated as configured and allows signatures generated with a public empty key.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Comment on lines
+467
to
+470
| let secret = shared.state.webhook_secret.as_ref().ok_or_else(|| { | ||
| warn!("Webhook secret not configured on server, rejecting request"); | ||
| StatusCode::UNAUTHORIZED | ||
| })?; |
There was a problem hiding this comment.
P1: Reject empty webhook secrets, not just missing ones. An empty AKSH_WEBHOOK_SECRET is treated as configured and allows signatures generated with a public empty key.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/aksh-runner-server/src/github.rs, line 467:
<comment>Reject empty webhook secrets, not just missing ones. An empty `AKSH_WEBHOOK_SECRET` is treated as configured and allows signatures generated with a public empty key.</comment>
<file context>
@@ -452,15 +464,18 @@ pub(crate) async fn handle_github_webhook(
- if !verify_signature(secret, &body, sig_header) {
- return Err(StatusCode::UNAUTHORIZED);
- }
+ let secret = shared.state.webhook_secret.as_ref().ok_or_else(|| {
+ warn!("Webhook secret not configured on server, rejecting request");
+ StatusCode::UNAUTHORIZED
</file context>
Suggested change
| let secret = shared.state.webhook_secret.as_ref().ok_or_else(|| { | |
| warn!("Webhook secret not configured on server, rejecting request"); | |
| StatusCode::UNAUTHORIZED | |
| })?; | |
| let secret = shared | |
| .state | |
| .webhook_secret | |
| .as_deref() | |
| .filter(|secret| !secret.is_empty()) | |
| .ok_or_else(|| { | |
| warn!("Webhook secret not configured on server, rejecting request"); | |
| StatusCode::UNAUTHORIZED | |
| })?; |
Bnjoroge1
added a commit
that referenced
this pull request
Aug 7, 2026
Add expression quote-escaping test & implement server job timeouts an…
Bnjoroge1
added a commit
that referenced
this pull request
Aug 7, 2026
Add expression quote-escaping test & implement server job timeouts an…
Bnjoroge1
added a commit
that referenced
this pull request
Aug 7, 2026
Add expression quote-escaping test & implement server job timeouts an…
Bnjoroge1
added a commit
that referenced
this pull request
Aug 7, 2026
Add expression quote-escaping test & implement server job timeouts an…
Bnjoroge1
added a commit
that referenced
this pull request
Aug 7, 2026
Add expression quote-escaping test & implement server job timeouts an…
Bnjoroge1
added a commit
that referenced
this pull request
Oct 9, 2026
- #9 cancel: 202 only when the locked cancel transitioned a live run; already-terminal and archived runs answer GitHub's 409 'Cannot cancel a workflow run that is completed.' Backend CancelOutcome gains run_cancelled so the REST shim reads the real transition result. - #6 suite rerequest: rerun every terminal run whose checks reported at the suite's head_sha via check_run_report_coords (status_check_sha / effective sha), not just the newest run by checkout sha; skip suites of unregistered Apps when the payload names one. - #8: malformed JSON / wrong JSON types → GitHub-shaped 422 regardless of Content-Type; empty and null bodies keep defaults. - REST rerun shims pass the authenticated caller as triggering_actor. - fix missing triggering_actor arg on dispatch.rs rerun calls. - clippy -D warnings cleanups in test targets (toolchain 1.97).
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
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.
…d runner lease reaper
Summary by cubic
Adds a GitHub App webhook that triggers workflows on push/PR and reports per‑job status to GitHub Checks. Adds server‑side job timeouts and a lease reaper to auto‑cancel/fail stuck jobs, plus a one‑click App registration flow, tests, and docs.
New Features
/api/v1/github/webhookswith HMAC signature verification (requiresAKSH_WEBHOOK_SECRET); loads workflows fromAKSH_LOCAL_WORKSPACEor GitHub and triggers matching runs.AKSH_GITHUB_TOKENis unset./api/v1/github/registerand/api/v1/github/callback; docs atdocs/github-app-webhook.md; tests for webhook flows, manifest conversion, reaper behavior, and escaped single quotes in expressions.Bug Fixes
last_renewed_atin the legacy non‑renewingnext_messagepath to avoid false lease renewals.Written for commit 745884b. Summary will update on new commits.