Skip to content

Commit

Permalink
CLI: rewrite in rust (#1577)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
svix-onelson authored Dec 23, 2024
2 parents 2dc19a9 + 603e4ee commit 5727802
Show file tree
Hide file tree
Showing 36 changed files with 5,158 additions and 6 deletions.
79 changes: 79 additions & 0 deletions .github/workflows/cli-lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
name: CLI Lint

on:
push:
branches:
- main
paths:
- 'svix-cli/**'
- 'rust/**'
- '.github/workflows/cli-lint.yml'
pull_request:
paths:
- 'svix-cli/**'
- 'rust/**'
- '.github/workflows/cli-lint.yml'
- 'openapi.json'

# When pushing to a PR, cancel any jobs still running for the previous head commit of the PR
concurrency:
# head_ref is only defined for pull requests, run_id is always unique and defined so if this
# workflow was not triggered by a pull request, nothing gets cancelled.
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
cancel-in-progress: true

env:
CARGO_TERM_COLOR: always

jobs:
check-fmt:
name: Check formatting
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@v4

- uses: dtolnay/rust-toolchain@nightly
with:
components: rustfmt

- name: rustfmt
run: cargo fmt -- --check
working-directory: svix-cli

test-versions:
name: CLI Lint
runs-on: ubuntu-24.04
strategy:
matrix:
rust: [stable, beta]
steps:
- uses: actions/checkout@v4

- name: Regen openapi libs
run: |
yarn
./regen_openapi.sh
- uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ matrix.rust }}
components: clippy

- uses: Swatinem/rust-cache@v2
with:
workspaces: "svix-cli -> target"
# only save the cache on the main branch
# cf https://github.com/Swatinem/rust-cache/issues/95
save-if: ${{ github.ref == 'refs/heads/main' }}
# include relevant information in the cache name
prefix-key: "cli-${{ matrix.rust }}"

- uses: taiki-e/install-action@nextest

- name: Clippy
run: cargo clippy --all-targets --all-features -- -D warnings
working-directory: svix-cli

- name: Run tests
run: cargo nextest run
working-directory: svix-cli
24 changes: 24 additions & 0 deletions .github/workflows/cli-security.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
name: CLI Security

on:
push:
branches:
- main
paths:
- 'svix-cli/**/Cargo.toml'
- 'svix-cli/**/Cargo.lock'
- '.github/workflows/cli-security.yml'
pull_request:
paths:
- 'rust/**/Cargo.toml'
- 'rust/**/Cargo.lock'
- '.github/workflows/cli-security.yml'

jobs:
security_audit:
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@v4
- uses: EmbarkStudios/cargo-deny-action@v2
with:
manifest-path: svix-cli/Cargo.toml
1 change: 1 addition & 0 deletions deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ allow = [
confidence-threshold = 0.8
exceptions = [
#{ allow = ["Zlib"], name = "adler32", version = "*" },
{ allow = ["EPL-2.0"], name = "colored_json", version = "*" }
]

[[licenses.clarify]]
Expand Down
46 changes: 40 additions & 6 deletions rust/src/webhooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,23 @@ impl Webhook {
}

pub fn verify<HM: HeaderMap>(&self, payload: &[u8], headers: &HM) -> Result<(), WebhookError> {
self.verify_inner(payload, headers, /* enforce_tolerance */ true)
}

pub fn verify_ignoring_timestamp<HM: HeaderMap>(
&self,
payload: &[u8],
headers: &HM,
) -> Result<(), WebhookError> {
self.verify_inner(payload, headers, /* enforce_tolerance */ false)
}

fn verify_inner<HM: HeaderMap>(
&self,
payload: &[u8],
headers: &HM,
enforce_tolerance: bool,
) -> Result<(), WebhookError> {
let msg_id = Self::get_header(headers, SVIX_MSG_ID_KEY, UNBRANDED_MSG_ID_KEY, "id")?;
let msg_signature = Self::get_header(
headers,
Expand All @@ -72,7 +89,9 @@ impl Webhook {
)
.and_then(Self::parse_timestamp)?;

Self::verify_timestamp(msg_ts)?;
if enforce_tolerance {
Self::verify_timestamp(msg_ts)?;
}

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

let signature = wh
.sign(msg_id, OffsetDateTime::now_utc().unix_timestamp(), payload)
.unwrap();

let mut headers = get_svix_headers(msg_id, &signature);
// Checks that timestamps that are in the future or too old are rejected by
// `verify` but okay for `verify_ignoring_timestamp`.
for ts in [
OffsetDateTime::now_utc().unix_timestamp() - (super::TOLERANCE_IN_SECONDS + 1),
OffsetDateTime::now_utc().unix_timestamp() + (super::TOLERANCE_IN_SECONDS + 1),
] {
let signature = wh.sign(msg_id, ts, payload).unwrap();
let mut headers = get_svix_headers(msg_id, &signature);
headers.insert(
super::SVIX_MSG_TIMESTAMP_KEY,
ts.to_string().parse().unwrap(),
);

assert!(wh.verify(payload, &headers,).is_err());
// Timestamp tolerance is not considered in this case.
assert!(wh.verify_ignoring_timestamp(payload, &headers,).is_ok());
}

let ts = OffsetDateTime::now_utc().unix_timestamp();
let signature = wh.sign(msg_id, ts, payload).unwrap();
let mut headers = get_svix_headers(msg_id, &signature);
headers.insert(
super::SVIX_MSG_TIMESTAMP_KEY,
// Timestamp mismatch!
(ts + 1).to_string().parse().unwrap(),
);

// Both versions should reject the timestamp if it's not the same one used to
// produce the signature.
assert!(wh.verify(payload, &headers,).is_err());
assert!(wh.verify_ignoring_timestamp(payload, &headers,).is_err());
}

#[test]
Expand Down
1 change: 1 addition & 0 deletions svix-cli/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
target/
2 changes: 2 additions & 0 deletions svix-cli/.rustfmt.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
imports_granularity = "Crate"
group_imports = "StdExternalCrate"
Loading

0 comments on commit 5727802

Please sign in to comment.