feat(trogon-source-gitlab): scaffold GitLab webhook source#99
Merged
Conversation
PR SummaryMedium Risk Overview Wires the new service into local Docker Compose (env template, service definition, healthcheck, and an ngrok tunnel), and extends Reviewed by Cursor Bugbot for commit 131fc3f. Bugbot is set up for automated code reviews on this repo. Configure here. |
WalkthroughAdds a new GitLab webhook source: a Rust crate with HTTP webhook server, signature verification, JetStream publishing and provisioning, Docker Compose service and Dockerfile, ngrok integration, config/constants, telemetry enum update, and accompanying docs and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant GitLab
participant Ngrok
participant Webhook as Trogon\nWebhook Server
participant Signature as Signature\nVerifier
participant JetStream as NATS\nJetStream
participant DLQ as DLQ Topic
GitLab->>Ngrok: POST /webhook (public URL)
Ngrok->>Webhook: Forward POST /webhook
Webhook->>Signature: verify(x-gitlab-token)
alt token valid
Signature-->>Webhook: ok
Webhook->>Webhook: extract & normalize event
alt event valid
Webhook->>JetStream: Publish to gitlab.{event} (with headers)
JetStream-->>Webhook: ack
Webhook-->>GitLab: 200 OK
else event invalid
Webhook->>DLQ: Publish to gitlab.unroutable (with reject header)
DLQ-->>Webhook: ack
Webhook-->>GitLab: 200 OK
end
else token missing/invalid
Signature-->>Webhook: err
Webhook-->>GitLab: 401 Unauthorized
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
yordis
force-pushed
the
yordis/gitlab-source-scaffold
branch
from
13023c5 to
c94e918
Compare
|
Code Coverage SummaryDetailsDiff against mainResults for commit: 131fc3f Minimum allowed coverage is ♻️ This comment has been updated with latest results |
yordis
force-pushed
the
yordis/gitlab-source-scaffold
branch
3 times, most recently
from
3a2fa6b to
b704ca8
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
yordis
force-pushed
the
yordis/gitlab-source-scaffold
branch
from
b704ca8 to
f15680e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
devops/docker/compose/services/trogon-source-gitlab/README.md (1)
53-55: Consider using a more portable NATS address in the example.The
nats.trogonai.orb.localhostname appears to be OrbStack-specific and won't resolve for users running Docker Compose without OrbStack. Consider usinglocalhost:4222(with appropriate port mapping) or documenting that this address is specific to OrbStack environments.Suggested change
```bash -nats sub -s nats://nats.trogonai.orb.local:4222 "gitlab.>" +nats sub -s nats://localhost:4222 "gitlab.>"Note: This requires exposing port 4222 from the nats container, or you can document the OrbStack-specific hostname usage. </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@devops/docker/compose/services/trogon-source-gitlab/README.mdaround lines
53 - 55, Update the example NATS command string "nats sub -s
nats://nats.trogonai.orb.local:4222 "gitlab.>"" to use a portable address
(e.g., change to "nats://localhost:4222") and either document that this requires
exposing port 4222 from the nats container or explicitly note that the original
hostname is OrbStack-specific; modify the README example and accompanying note
to mention the port-mapping requirement or the OrbStack-only hostname so users
know which option applies.</details> </blockquote></details> <details> <summary>devops/docker/compose/.env.example (1)</summary><blockquote> `19-19`: **Inconsistent timeout unit naming across services.** `GITLAB_NATS_ACK_TIMEOUT_MS` uses milliseconds (matching Linear), while GitHub and Slack use `_SECS` suffix. The default value of `10000` is correct (10 seconds), but the naming inconsistency could cause confusion when configuring multiple services. Consider aligning on one convention across all sources for maintainability. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@devops/docker/compose/.env.exampleat line 19, Summary: The environment
variable name GITLAB_NATS_ACK_TIMEOUT_MS is inconsistent with GitHub/Slack which
use a _SECS suffix; align naming and default units across services. Fix: rename
GITLAB_NATS_ACK_TIMEOUT_MS to GITLAB_NATS_ACK_TIMEOUT_SECS in the .env.example
(and any env files) and change its default from 10000 to 10, or alternatively
convert the other services to _MS—pick the seconds convention to match
GitHub/Slack; then update all code references that read the variable (search for
GITLAB_NATS_ACK_TIMEOUT_MS) to use GITLAB_NATS_ACK_TIMEOUT_SECS and ensure
parsing treats the value as seconds (adjust any parseInt/timeout multipliers
accordingly). Ensure documentation and README mention the chosen unit.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@rsworkspace/crates/trogon-source-gitlab/src/config.rs:
- Around line 13-22: GitlabConfig currently stores webhook_secret as a raw
String and performs multi-field validation at aggregate construction; create a
domain value object (e.g., WebhookSecret) that encapsulates the secret string
and performs its own validation via TryFrom<&str> or a
WebhookSecret::try_from_str / WebhookSecret::new returning Result, then change
GitlabConfig::new (or the struct field) to use WebhookSecret instead of String
and remove webhook-specific validation from the aggregate constructor so only
cross-field invariants remain in GitlabConfig creation; update any uses of
GitlabConfig::webhook_secret to call WebhookSecret.as_str() (or similar) where
raw string is needed.In
@rsworkspace/crates/trogon-source-gitlab/src/server.rs:
- Around line 140-142: Replace the unchecked cast config.max_body_size.as_u64()
as usize with an explicit conversion using
usize::try_from(config.max_body_size.as_u64()) and handle the Result instead of
casting; update the call site that constructs RequestBodyLimitLayer::new(...) to
pass the converted usize (or return/log a clear error or apply a safe fallback)
so overflow is detected and handled – look for RequestBodyLimitLayer::new and
config.max_body_size.as_u64() in server.rs and ensure the TryFrom error is
propagated or handled rather than using a raw as cast.
Nitpick comments:
In@devops/docker/compose/.env.example:
- Line 19: Summary: The environment variable name GITLAB_NATS_ACK_TIMEOUT_MS is
inconsistent with GitHub/Slack which use a _SECS suffix; align naming and
default units across services. Fix: rename GITLAB_NATS_ACK_TIMEOUT_MS to
GITLAB_NATS_ACK_TIMEOUT_SECS in the .env.example (and any env files) and change
its default from 10000 to 10, or alternatively convert the other services to
_MS—pick the seconds convention to match GitHub/Slack; then update all code
references that read the variable (search for GITLAB_NATS_ACK_TIMEOUT_MS) to use
GITLAB_NATS_ACK_TIMEOUT_SECS and ensure parsing treats the value as seconds
(adjust any parseInt/timeout multipliers accordingly). Ensure documentation and
README mention the chosen unit.In
@devops/docker/compose/services/trogon-source-gitlab/README.md:
- Around line 53-55: Update the example NATS command string "nats sub -s
nats://nats.trogonai.orb.local:4222 "gitlab.>"" to use a portable address
(e.g., change to "nats://localhost:4222") and either document that this requires
exposing port 4222 from the nats container or explicitly note that the original
hostname is OrbStack-specific; modify the README example and accompanying note
to mention the port-mapping requirement or the OrbStack-only hostname so users
know which option applies.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `26a3d023-db71-4f09-8363-137206da5e8b` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 1c17914cc34739a36d373de8eb28590e86884358 and f15680eba3ece68d983b35fecaea4a9d676c490d. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `rsworkspace/Cargo.lock` is excluded by `!**/*.lock` </details> <details> <summary>📒 Files selected for processing (12)</summary> * `devops/docker/compose/.env.example` * `devops/docker/compose/compose.yml` * `devops/docker/compose/services/trogon-source-gitlab/Dockerfile` * `devops/docker/compose/services/trogon-source-gitlab/README.md` * `rsworkspace/crates/acp-telemetry/src/service_name.rs` * `rsworkspace/crates/trogon-source-gitlab/Cargo.toml` * `rsworkspace/crates/trogon-source-gitlab/src/config.rs` * `rsworkspace/crates/trogon-source-gitlab/src/constants.rs` * `rsworkspace/crates/trogon-source-gitlab/src/lib.rs` * `rsworkspace/crates/trogon-source-gitlab/src/main.rs` * `rsworkspace/crates/trogon-source-gitlab/src/server.rs` * `rsworkspace/crates/trogon-source-gitlab/src/signature.rs` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Co-authored-by: Zach Bruhnke <zach@ordinary.company> Co-authored-by: Dave <dave@bruhnke.us> Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis
force-pushed
the
yordis/gitlab-source-scaffold
branch
from
f15680e to
131fc3f
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
rsworkspace/crates/trogon-source-gitlab/src/webhook_secret.rs (1)
19-23: Consider rejecting whitespace-only secrets.Current validation accepts
" ", which is usually accidental misconfiguration and hard to diagnose at runtime.♻️ Suggested tweak
pub fn new(s: impl AsRef<str>) -> Result<Self, EmptyWebhookSecret> { let s = s.as_ref(); - if s.is_empty() { + if s.trim().is_empty() { return Err(EmptyWebhookSecret); } Ok(Self(Arc::from(s))) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rsworkspace/crates/trogon-source-gitlab/src/webhook_secret.rs` around lines 19 - 23, The new constructor for WebhookSecret currently only checks s.is_empty() and therefore accepts whitespace-only strings; update WebhookSecret::new to trim the input (e.g., let trimmed = s.as_ref().trim()) and return Err(EmptyWebhookSecret) if trimmed.is_empty(); then construct the WebhookSecret using the trimmed value (or fail if you must preserve original, but still reject whitespace-only by checking trimmed) so that inputs like " " are rejected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@rsworkspace/crates/trogon-source-gitlab/src/webhook_secret.rs`:
- Around line 19-23: The new constructor for WebhookSecret currently only checks
s.is_empty() and therefore accepts whitespace-only strings; update
WebhookSecret::new to trim the input (e.g., let trimmed = s.as_ref().trim()) and
return Err(EmptyWebhookSecret) if trimmed.is_empty(); then construct the
WebhookSecret using the trimmed value (or fail if you must preserve original,
but still reject whitespace-only by checking trimmed) so that inputs like " "
are rejected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 450ab0bd-9b18-45d4-a244-04b61b64c20d
⛔ Files ignored due to path filters (1)
rsworkspace/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
devops/docker/compose/.env.exampledevops/docker/compose/compose.ymldevops/docker/compose/services/trogon-source-gitlab/Dockerfiledevops/docker/compose/services/trogon-source-gitlab/README.mdrsworkspace/crates/acp-telemetry/src/service_name.rsrsworkspace/crates/trogon-source-gitlab/Cargo.tomlrsworkspace/crates/trogon-source-gitlab/src/config.rsrsworkspace/crates/trogon-source-gitlab/src/constants.rsrsworkspace/crates/trogon-source-gitlab/src/lib.rsrsworkspace/crates/trogon-source-gitlab/src/main.rsrsworkspace/crates/trogon-source-gitlab/src/server.rsrsworkspace/crates/trogon-source-gitlab/src/signature.rsrsworkspace/crates/trogon-source-gitlab/src/webhook_secret.rs
✅ Files skipped from review due to trivial changes (5)
- devops/docker/compose/.env.example
- devops/docker/compose/services/trogon-source-gitlab/README.md
- rsworkspace/crates/trogon-source-gitlab/Cargo.toml
- rsworkspace/crates/trogon-source-gitlab/src/constants.rs
- rsworkspace/crates/trogon-source-gitlab/src/server.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- rsworkspace/crates/acp-telemetry/src/service_name.rs
- devops/docker/compose/services/trogon-source-gitlab/Dockerfile
- rsworkspace/crates/trogon-source-gitlab/src/signature.rs
- rsworkspace/crates/trogon-source-gitlab/src/main.rs
- rsworkspace/crates/trogon-source-gitlab/src/lib.rs
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 131fc3f. Configure here.
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.
Summary
X-Gitlab-Token(shared secret, constant-time comparison) and publishes events to NATS JetStreamNatsToken-validated config, DLQ/unroutable withX-GitLab-Reject-Reasonheader, body size limiting, health check endpointCo-authored-by: Zach Bruhnke zach@ordinary.company
Co-authored-by: Dave dave@bruhnke.us