Conversation
PR SummaryMedium Risk Overview Updates local dev plumbing to run it: new Reviewed by Cursor Bugbot for commit 8f01a0e. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 8 minutes and 40 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
WalkthroughAdds a new Linear webhook receiver crate and Docker Compose service ( Changes
Sequence DiagramsequenceDiagram
actor Client as Linear API
participant Server as HTTP Server<br/>(trogon-source-linear)
participant Sig as Signature Module
participant NATS as JetStream
Client->>Server: POST /webhook (body + linear-signature)
Server->>Server: Read header, parse body, check timestamp
Server->>Sig: verify(secret, body, signature)
Sig-->>Server: valid / invalid
alt valid
Server->>Server: validate type & action tokens
Server->>NATS: Publish to {prefix}.{type}.{action} with headers
NATS-->>Server: Ack / Timeout
alt Ack success
Server-->>Client: 200 OK
else Ack fail
Server->>NATS: attempt stream recreation (stream_op_timeout)
Server->>NATS: retry publish (ack_timeout)
alt retry success
Server-->>Client: 200 OK
else
Server-->>Client: 500
end
end
else invalid
Server-->>Client: 400/401
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
889ab51 to
174a2cc
Compare
Code Coverage SummaryDetailsDiff against mainResults for commit: 8f01a0e Minimum allowed coverage is ♻️ This comment has been updated with latest results |
dddb064 to
d94cf9b
Compare
a01f323 to
739bbdc
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@devops/docker/compose/compose.yml`:
- Line 51: The compose.yml currently defaults LINEAR_WEBHOOK_SECRET to an empty
string which allows predictable empty-key signature checks; remove the fallback
so the service uses the raw environment value (use LINEAR_WEBHOOK_SECRET without
":-" default) and/or add startup validation in the application entrypoint (check
process.env.LINEAR_WEBHOOK_SECRET) to fail fast if LINEAR_WEBHOOK_SECRET is
unset, ensuring webhook signature verification always uses a real secret.
In `@rsworkspace/crates/trogon-source-linear/src/config.rs`:
- Around line 24-25: LinearConfig currently allows webhook_secret:
Option<String>, which lets the receiver run open or accept empty keys; instead
enforce a non-empty secret at startup by requiring and validating
LINEAR_WEBHOOK_SECRET when constructing LinearConfig (change the field to a
plain String or keep Option but fail if None or empty). Update the config loader
where LinearConfig is built (refer to LinearConfig and webhook_secret) to
error/exit if the env var is missing or equals "" and adjust handle_webhook to
assume a validated non-empty secret (or keep current check but it will never see
None/empty after validation).
- Around line 27-28: The config currently accepts empty strings for
subject_prefix and stream_name (pub subject_prefix: String, pub stream_name:
String), which lets invalid env values slip through; update the Config
construction/factory (the constructor or load_from_env function that produces
this struct) to validate these fields: if the env value is empty or whitespace
either replace it with the intended default or return an error so invalid
configs are rejected at load time, and prefer wrapping these fields in small
domain types (e.g., AcpPrefix or LinearStreamName) whose constructors enforce
non-empty/format rules so invalid instances are unrepresentable; apply the same
validation for the other fields noted around lines 56–61.
In `@rsworkspace/crates/trogon-source-linear/src/server.rs`:
- Around line 35-37: The initial ensure_stream(&js, &config.stream_name,
&config.subject_prefix, config.stream_max_age).await call can block
indefinitely; wrap that call in the same timeout used on recovery
(LINEAR_NATS_STREAM_OP_TIMEOUT_MS) via
tokio::time::timeout(Duration::from_millis(LINEAR_NATS_STREAM_OP_TIMEOUT_MS),
...) and propagate a suitable error if the timeout elapses so startup fails
fast; apply this change where jetstream::new(nats) gives js and ensure_stream is
invoked so startup uses the identical stream-op timeout as the recovery path.
- Around line 258-260: The replay-protection currently skips verification when
webhookTimestamp is missing because the combined condition lets payloads
through; change the logic so that when state.timestamp_tolerance is
Some(tolerance) you require a parseable timestamp from
parsed.get("webhookTimestamp").and_then(|v| v.as_u64()) and reject the request
if that timestamp is absent or malformed. Concretely, in the handler around
state.timestamp_tolerance and parsed.get("webhookTimestamp") (look for the if
let involving tolerance and ts_ms), first match/if let on
state.timestamp_tolerance => Some(tolerance) and then attempt to extract ts_ms
separately; if ts_ms is None return an error (or reject the request) instead of
skipping the replay check, otherwise perform the existing tolerance check using
tolerance and ts_ms. Ensure you reference state.timestamp_tolerance,
parsed.get("webhookTimestamp"), ts_ms and tolerance in the updated flow.
- Line 5: AppState currently stores raw async_nats::jetstream::Context and
handlers call .publish_with_headers() directly, bypassing trogon_nats
abstractions; change AppState to hold a dyn trogon_nats::PublishClient (or
generic over P: PublishClient), update the webhook publisher function signature
to accept that PublishClient (or generic P) so tests can inject mocks and
standard retry/reconnect logic is used (update usages where connect() is called
to extract the PublishClient); wrap the initial ensure_stream() call inside the
configured config.stream_op_timeout similarly to the recovery path so startup
uses the same timeout; and tighten the replay protection by requiring
webhookTimestamp whenever config.timestamp_tolerance is Some—modify the guard in
the webhook handler (the code around webhookTimestamp/timestamp_tolerance
checks) to reject requests missing webhookTimestamp when tolerance is
configured.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f541e5a1-dfb3-4a3c-b7c4-e1e2c68c9484
⛔ Files ignored due to path filters (1)
rsworkspace/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
devops/docker/compose/.env.exampledevops/docker/compose/compose.ymldevops/docker/compose/services/trogon-source-linear/Dockerfiledevops/docker/compose/services/trogon-source-linear/README.mdrsworkspace/crates/trogon-source-linear/Cargo.tomlrsworkspace/crates/trogon-source-linear/src/config.rsrsworkspace/crates/trogon-source-linear/src/constants.rsrsworkspace/crates/trogon-source-linear/src/lib.rsrsworkspace/crates/trogon-source-linear/src/main.rsrsworkspace/crates/trogon-source-linear/src/server.rsrsworkspace/crates/trogon-source-linear/src/signature.rs
87d1d03 to
5fc0301
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
rsworkspace/crates/trogon-source-linear/src/config.rs (2)
60-65:⚠️ Potential issue | 🟠 MajorReject or normalize blank
LINEAR_SUBJECT_PREFIX/LINEAR_STREAM_NAMEat load time.Line 60 and Line 63 accept empty env values as-is, which defers failure into runtime stream/subject operations. Normalize blanks to defaults (or hard-fail) in
from_env.🧹 Proposed fix
subject_prefix: env .var("LINEAR_SUBJECT_PREFIX") - .unwrap_or_else(|_| DEFAULT_SUBJECT_PREFIX.to_string()), + .ok() + .map(|v| v.trim().to_owned()) + .filter(|v| !v.is_empty()) + .unwrap_or_else(|| DEFAULT_SUBJECT_PREFIX.to_string()), stream_name: env .var("LINEAR_STREAM_NAME") - .unwrap_or_else(|_| DEFAULT_STREAM_NAME.to_string()), + .ok() + .map(|v| v.trim().to_owned()) + .filter(|v| !v.is_empty()) + .unwrap_or_else(|| DEFAULT_STREAM_NAME.to_string()),As per coding guidelines, "Prefer domain-specific value objects over primitives (e.g.,
AcpPrefixnotString), with each type's factory guaranteeing correctness at construction—invalid instances should be unrepresentable".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rsworkspace/crates/trogon-source-linear/src/config.rs` around lines 60 - 65, The code currently accepts empty env vars for LINEAR_SUBJECT_PREFIX and LINEAR_STREAM_NAME in the from_env constructor when initializing subject_prefix and stream_name; update from_env so it trims the env value and either substitutes DEFAULT_SUBJECT_PREFIX / DEFAULT_STREAM_NAME when the trimmed value is empty or return an Err to fail fast; implement this normalization/validation in the subject_prefix and stream_name creation (or delegate to domain-specific constructors like AcpPrefix/AcpStream) so invalid blank strings are rejected at load time rather than later in stream/subject operations.
50-54:⚠️ Potential issue | 🟠 MajorTrim
LINEAR_WEBHOOK_SECRETbefore emptiness validation.Line 53 only rejects
"";" "still passes and boots with an unusable secret. Fail fast on whitespace-only values too.🔒 Proposed fix
webhook_secret: env .var("LINEAR_WEBHOOK_SECRET") .ok() - .filter(|s| !s.is_empty()) + .map(|s| s.trim().to_owned()) + .filter(|s| !s.is_empty()) .expect("LINEAR_WEBHOOK_SECRET is required"),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rsworkspace/crates/trogon-source-linear/src/config.rs` around lines 50 - 54, The webhook_secret construction currently only rejects empty strings but accepts whitespace-only values; update the retrieval of LINEAR_WEBHOOK_SECRET (the env.var("LINEAR_WEBHOOK_SECRET") chain that builds webhook_secret) to trim the string before checking emptiness—i.e., map or filter the option to .trim() (or call .map(str::trim).filter(|s| !s.is_empty())) so whitespace-only values are rejected and expect(...) still triggers for missing/blank secrets.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@rsworkspace/crates/trogon-source-linear/src/config.rs`:
- Around line 60-65: The code currently accepts empty env vars for
LINEAR_SUBJECT_PREFIX and LINEAR_STREAM_NAME in the from_env constructor when
initializing subject_prefix and stream_name; update from_env so it trims the env
value and either substitutes DEFAULT_SUBJECT_PREFIX / DEFAULT_STREAM_NAME when
the trimmed value is empty or return an Err to fail fast; implement this
normalization/validation in the subject_prefix and stream_name creation (or
delegate to domain-specific constructors like AcpPrefix/AcpStream) so invalid
blank strings are rejected at load time rather than later in stream/subject
operations.
- Around line 50-54: The webhook_secret construction currently only rejects
empty strings but accepts whitespace-only values; update the retrieval of
LINEAR_WEBHOOK_SECRET (the env.var("LINEAR_WEBHOOK_SECRET") chain that builds
webhook_secret) to trim the string before checking emptiness—i.e., map or filter
the option to .trim() (or call .map(str::trim).filter(|s| !s.is_empty())) so
whitespace-only values are rejected and expect(...) still triggers for
missing/blank secrets.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6055106b-b16a-46a9-bca3-bf29fa9d7ba8
⛔ Files ignored due to path filters (1)
rsworkspace/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
devops/docker/compose/.env.exampledevops/docker/compose/compose.ymldevops/docker/compose/services/trogon-source-linear/Dockerfiledevops/docker/compose/services/trogon-source-linear/README.mdrsworkspace/crates/trogon-source-linear/Cargo.tomlrsworkspace/crates/trogon-source-linear/src/config.rsrsworkspace/crates/trogon-source-linear/src/constants.rsrsworkspace/crates/trogon-source-linear/src/lib.rsrsworkspace/crates/trogon-source-linear/src/main.rsrsworkspace/crates/trogon-source-linear/src/server.rsrsworkspace/crates/trogon-source-linear/src/signature.rs
✅ Files skipped from review due to trivial changes (7)
- devops/docker/compose/.env.example
- rsworkspace/crates/trogon-source-linear/src/main.rs
- devops/docker/compose/services/trogon-source-linear/README.md
- rsworkspace/crates/trogon-source-linear/src/constants.rs
- rsworkspace/crates/trogon-source-linear/src/lib.rs
- rsworkspace/crates/trogon-source-linear/Cargo.toml
- devops/docker/compose/services/trogon-source-linear/Dockerfile
🚧 Files skipped from review as they are similar to previous changes (3)
- rsworkspace/crates/trogon-source-linear/src/signature.rs
- devops/docker/compose/compose.yml
- rsworkspace/crates/trogon-source-linear/src/server.rs
11d378e to
f6ed725
Compare
f1dc828 to
e5bd6c6
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ 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 e5bd6c6. Configure here.
99c874c to
c69b0f7
Compare
…ents to NATS JetStream Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>

trogon-source-linearto match the existingtrogon-source-*naming convention