Skip to content

Commit 5727802

Browse files
authored
CLI: rewrite in rust (#1577)
To date, the [Go CLI](https://github.com/svix) has lived in its own repo and has received infrequent updates, failing to track our APIs as they change. In an effort to improve on this, this diff adds new Rust version of the CLI here in the webhooks repo. The idea is to release the CLI along with the libs as a part of the same process (svix/monorepo-private#9620). Modules in the source tree under `cli_types/` and `cmds/api/` (excluding the `mod.rs`) are largely produced via our new codegen tooling (h/t @svix-jplatte). Other modules under `cmds/` are written by hand, the most involved being `listen` which depends on greenfield code under `relay/`. The relay code was largely inspired by the code seen in the Go CLI, but the overall shape of it has changed to account for GC vs borrow checker. Future work sound be planned to do a design pass now that the pieces are working together since breaking things apart to satisfy rustc left things in an odd shape. ## Differences Comparing the Go and Rust CLIs, there are some differences. The list of features added in the Rust version that are lacking in Go's is long thanks to the new codegen. It'll be more useful to focus on what _isn't here_. ### `--data-*` flags In the Go CLI we exposed flags named `--data-foo` where `foo` would be a field in the expected JSON body sent with the request. These flags have not been carried forward. Instead users will be required to write out the full JSON body as a command-line argument. Serde will give some feedback if the body fails to parse given the expected schema even before the request is made to the server. Future work is planned to include an example body for each place we accept JSON in the long help. ### Import/Export `svix import` and `svix export` have been left behind. For import, users are encouraged to leverage `svix event-type import-openapi` which should fill the same need. For export, I think the best alternative is using `svix event-type list` and handling the responses as needed. ### Server URL no longer mandatory The GO CLI required users to set the URL for the API server they wanted to interact with. In the new Rust CLI, we will default to the issuer of the auth token. Typically this is going to be the safest default since manually specifying a Server URL should only be necessary when doing local development or talking to a non-production-SaaS server. The impacted group for this change would be enterprise customers and Svix employees and the fix is to do what they'd have needed to do all along: set the URL explicitly. ## Verification Reviewers will note that there's a dramatic lack of unit tests 😇 To this, I assure you there's technically more coverage on _this CLI_ than there is on the Go one 🤣. The majority of the verification for this has been manual testing. If you have specific concerns, pitch a test and I can run through the scenario for you (or you can try it yourself!) ## OSS Reviews There are several new-to-us crates included in this diff. I'll call each out in the comments below, looking for 👍🏻 from the security team.
2 parents 2dc19a9 + 603e4ee commit 5727802

36 files changed

+5158
-6
lines changed

.github/workflows/cli-lint.yml

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
name: CLI Lint
2+
3+
on:
4+
push:
5+
branches:
6+
- main
7+
paths:
8+
- 'svix-cli/**'
9+
- 'rust/**'
10+
- '.github/workflows/cli-lint.yml'
11+
pull_request:
12+
paths:
13+
- 'svix-cli/**'
14+
- 'rust/**'
15+
- '.github/workflows/cli-lint.yml'
16+
- 'openapi.json'
17+
18+
# When pushing to a PR, cancel any jobs still running for the previous head commit of the PR
19+
concurrency:
20+
# head_ref is only defined for pull requests, run_id is always unique and defined so if this
21+
# workflow was not triggered by a pull request, nothing gets cancelled.
22+
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
23+
cancel-in-progress: true
24+
25+
env:
26+
CARGO_TERM_COLOR: always
27+
28+
jobs:
29+
check-fmt:
30+
name: Check formatting
31+
runs-on: ubuntu-24.04
32+
steps:
33+
- uses: actions/checkout@v4
34+
35+
- uses: dtolnay/rust-toolchain@nightly
36+
with:
37+
components: rustfmt
38+
39+
- name: rustfmt
40+
run: cargo fmt -- --check
41+
working-directory: svix-cli
42+
43+
test-versions:
44+
name: CLI Lint
45+
runs-on: ubuntu-24.04
46+
strategy:
47+
matrix:
48+
rust: [stable, beta]
49+
steps:
50+
- uses: actions/checkout@v4
51+
52+
- name: Regen openapi libs
53+
run: |
54+
yarn
55+
./regen_openapi.sh
56+
57+
- uses: dtolnay/rust-toolchain@master
58+
with:
59+
toolchain: ${{ matrix.rust }}
60+
components: clippy
61+
62+
- uses: Swatinem/rust-cache@v2
63+
with:
64+
workspaces: "svix-cli -> target"
65+
# only save the cache on the main branch
66+
# cf https://github.com/Swatinem/rust-cache/issues/95
67+
save-if: ${{ github.ref == 'refs/heads/main' }}
68+
# include relevant information in the cache name
69+
prefix-key: "cli-${{ matrix.rust }}"
70+
71+
- uses: taiki-e/install-action@nextest
72+
73+
- name: Clippy
74+
run: cargo clippy --all-targets --all-features -- -D warnings
75+
working-directory: svix-cli
76+
77+
- name: Run tests
78+
run: cargo nextest run
79+
working-directory: svix-cli

.github/workflows/cli-security.yml

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
name: CLI Security
2+
3+
on:
4+
push:
5+
branches:
6+
- main
7+
paths:
8+
- 'svix-cli/**/Cargo.toml'
9+
- 'svix-cli/**/Cargo.lock'
10+
- '.github/workflows/cli-security.yml'
11+
pull_request:
12+
paths:
13+
- 'rust/**/Cargo.toml'
14+
- 'rust/**/Cargo.lock'
15+
- '.github/workflows/cli-security.yml'
16+
17+
jobs:
18+
security_audit:
19+
runs-on: ubuntu-24.04
20+
steps:
21+
- uses: actions/checkout@v4
22+
- uses: EmbarkStudios/cargo-deny-action@v2
23+
with:
24+
manifest-path: svix-cli/Cargo.toml

deny.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ allow = [
3333
confidence-threshold = 0.8
3434
exceptions = [
3535
#{ allow = ["Zlib"], name = "adler32", version = "*" },
36+
{ allow = ["EPL-2.0"], name = "colored_json", version = "*" }
3637
]
3738

3839
[[licenses.clarify]]

rust/src/webhooks.rs

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,23 @@ impl Webhook {
5757
}
5858

5959
pub fn verify<HM: HeaderMap>(&self, payload: &[u8], headers: &HM) -> Result<(), WebhookError> {
60+
self.verify_inner(payload, headers, /* enforce_tolerance */ true)
61+
}
62+
63+
pub fn verify_ignoring_timestamp<HM: HeaderMap>(
64+
&self,
65+
payload: &[u8],
66+
headers: &HM,
67+
) -> Result<(), WebhookError> {
68+
self.verify_inner(payload, headers, /* enforce_tolerance */ false)
69+
}
70+
71+
fn verify_inner<HM: HeaderMap>(
72+
&self,
73+
payload: &[u8],
74+
headers: &HM,
75+
enforce_tolerance: bool,
76+
) -> Result<(), WebhookError> {
6077
let msg_id = Self::get_header(headers, SVIX_MSG_ID_KEY, UNBRANDED_MSG_ID_KEY, "id")?;
6178
let msg_signature = Self::get_header(
6279
headers,
@@ -72,7 +89,9 @@ impl Webhook {
7289
)
7390
.and_then(Self::parse_timestamp)?;
7491

75-
Self::verify_timestamp(msg_ts)?;
92+
if enforce_tolerance {
93+
Self::verify_timestamp(msg_ts)?;
94+
}
7695

7796
let versioned_signature = self.sign(msg_id, msg_ts, payload)?;
7897
let expected_signature = versioned_signature
@@ -317,22 +336,37 @@ mod tests {
317336
let payload = br#"{"email":"[email protected]","username":"test_user"}"#;
318337
let wh = Webhook::new(&secret).unwrap();
319338

320-
let signature = wh
321-
.sign(msg_id, OffsetDateTime::now_utc().unix_timestamp(), payload)
322-
.unwrap();
323-
324-
let mut headers = get_svix_headers(msg_id, &signature);
339+
// Checks that timestamps that are in the future or too old are rejected by
340+
// `verify` but okay for `verify_ignoring_timestamp`.
325341
for ts in [
326342
OffsetDateTime::now_utc().unix_timestamp() - (super::TOLERANCE_IN_SECONDS + 1),
327343
OffsetDateTime::now_utc().unix_timestamp() + (super::TOLERANCE_IN_SECONDS + 1),
328344
] {
345+
let signature = wh.sign(msg_id, ts, payload).unwrap();
346+
let mut headers = get_svix_headers(msg_id, &signature);
329347
headers.insert(
330348
super::SVIX_MSG_TIMESTAMP_KEY,
331349
ts.to_string().parse().unwrap(),
332350
);
333351

334352
assert!(wh.verify(payload, &headers,).is_err());
353+
// Timestamp tolerance is not considered in this case.
354+
assert!(wh.verify_ignoring_timestamp(payload, &headers,).is_ok());
335355
}
356+
357+
let ts = OffsetDateTime::now_utc().unix_timestamp();
358+
let signature = wh.sign(msg_id, ts, payload).unwrap();
359+
let mut headers = get_svix_headers(msg_id, &signature);
360+
headers.insert(
361+
super::SVIX_MSG_TIMESTAMP_KEY,
362+
// Timestamp mismatch!
363+
(ts + 1).to_string().parse().unwrap(),
364+
);
365+
366+
// Both versions should reject the timestamp if it's not the same one used to
367+
// produce the signature.
368+
assert!(wh.verify(payload, &headers,).is_err());
369+
assert!(wh.verify_ignoring_timestamp(payload, &headers,).is_err());
336370
}
337371

338372
#[test]

svix-cli/.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
target/

svix-cli/.rustfmt.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
imports_granularity = "Crate"
2+
group_imports = "StdExternalCrate"

0 commit comments

Comments
 (0)