Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 24 additions & 12 deletions .mise/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -878,6 +878,7 @@ depends = [
"actionlint-check",
"agent-lint-check",
"ast-grep-check",
"ast-grep-check-rule-tests",
"branch-up-to-date-check",
"cargo-check",
"cargo-clippy-check",
Expand Down Expand Up @@ -905,6 +906,7 @@ depends = [
"repo-check",
"ryl-check",
"semgrep-check",
"semgrep-check-rule-tests",
"shellcheck-check",
"shellcheck-mise-check",
"shfmt-mise-check",
Expand Down Expand Up @@ -1021,32 +1023,42 @@ echo "macos-sdk-check: SDK $sdk (minimum $min)"
shell = "{{ vars.task_shell }}"

[tasks.ast-grep-check]
depends = ["python-safety-rules-test"]
depends = ["ast-grep-check-rule-tests"]
# --no-ignore hidden so the gha-* YAML rules reach .github/workflows.
# ast-grep skips dot-dirs by default; gitignored paths like target/ stay skipped.
description = "Run ast-grep structural-search rules"
run = "ast-grep scan --error --no-ignore hidden -c config/ast-grep/sgconfig.yaml"

[tasks.python-safety-rules-test]
description = "Test Python exception safety rules against unsafe and intentional recovery handlers"
run = """
ast-grep test -c config/ast-grep/sgconfig.yaml --skip-snapshot-tests
work=target/scratch/python-safety-rules
coreutils mkdir -p "$work"
yq -r '.code' config/semgrep-tests/python-exception-safety.yaml >"$work/python-exception-safety.py"
semgrep --test --metrics=off --config config/semgrep/python-exception-safety.yaml "$work/python-exception-safety.py"
"""
shell = "{{ vars.task_shell }}"
# The valid/invalid cases in config/ast-grep/tests, one file per rule id.
[tasks.ast-grep-check-rule-tests]
description = "Run the ast-grep rule tests against their valid and invalid cases (ast-grep test)"
run = "ast-grep test -c config/ast-grep/sgconfig.yaml --skip-snapshot-tests"

[tasks.ast-grep-fix]
description = "Apply autofix for ast-grep rules that define a `fix:` field"
run = "ast-grep scan --update-all --no-ignore hidden -c config/ast-grep/sgconfig.yaml"

[tasks.semgrep-check]
depends = ["python-safety-rules-test"]
depends = ["semgrep-check-rule-tests"]
description = "Run Semgrep code and configuration rules"
run = "semgrep scan --config config/semgrep --error --metrics=off ."

# Each case file in config/semgrep-tests is `<rule file stem>.<ext>.yaml`, its annotated target under `code:`.
# The target is wrapped in YAML so the repo carries no loose source file in that language; it is unwrapped into
# target/ under its real name, where `semgrep --test` pairs it with the rule file of the same stem.
[tasks.semgrep-check-rule-tests]
description = "Run the Semgrep rule tests against their annotated cases (semgrep --test)"
run = """
work=target/scratch/semgrep-rule-tests
coreutils rm -rf "$work"
coreutils mkdir -p "$work"
for case in config/semgrep-tests/*.yaml; do
yq -r '.code' "$case" >"$work/$(coreutils basename "$case" .yaml)"
done
semgrep --test --metrics=off --config config/semgrep "$work"
"""
shell = "{{ vars.task_shell }}"

[tasks.semgrep-fix]
description = "Apply autofix for Semgrep rules that define a `fix:` field"
run = "semgrep scan --config config/semgrep --autofix --metrics=off ."
Expand Down
11 changes: 11 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -1249,6 +1249,17 @@ When the rewrite can't be expressed as a single template (multiple match shapes,
structural restructuring), keep the rule check-only and write a brief note in the rule body explaining why the
autofix wasn't viable.

### When you write or change an ast-grep or semgrep rule, give it tests

A new ast-grep or semgrep rule lands with its test cases in the same change, and a modified rule lands with cases
covering the shapes the modification added or stopped matching -- just as a conftest policy carries its
`_test.rego` beside it. ast-grep cases go in `config/ast-grep/tests/<rule-id>.yaml` as `valid:` / `invalid:`
snippets; semgrep cases go in `config/semgrep-tests/` as annotated targets. `ast-grep-check-rule-tests` and
`semgrep-check-rule-tests` run them. Cover both directions: at least one snippet the rule must flag, and the
nearest legitimate code it must leave alone, since a pattern that over-matches fails the repo far more often
than one that under-matches. Confirm a new case can fail -- run the tool against a broken copy under
`target/scratch/` and watch it go red -- because a case the rule never pairs with passes silently.

### NEVER delete a lint rule without explicit user permission

**Do not delete a rule from `config/ast-grep/rules/`, `config/semgrep/`, `config/conftest/policy/`, `config/taplo/`,
Expand Down
11 changes: 9 additions & 2 deletions CHECKS.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# Checks

- **Checks:** 72
- **Checks:** 74
- **Tools:** 47
- **Shell-script checks:** 9
- **Checks with a repair pass:** 21
- **Rules:** 339 custom local rules, or non-default strict linter settings
- **Rules:** 340 custom local rules, or non-default strict linter settings

Every check is listed across every `MISE_ENV`, filed under the tool it runs. Each entry names the env whose config
declares it, the aggregate that runs it as part of `mise run check`, the `fix` or `fmt` pass that repairs what it
Expand All @@ -31,6 +31,8 @@ gen:checks` from the task tables in `.mise/config*.toml`, the rule files under `

- `ast-grep-check` (`default` env; run by `check:rust`) -- Run ast-grep structural-search rules. Repaired by
`ast-grep-fix`. Config: `config/ast-grep/sgconfig.yaml`.
- `ast-grep-check-rule-tests` (`default` env; run by `check:rust`) -- Run the ast-grep rule tests against their valid
and invalid cases (ast-grep test). Config: `config/ast-grep/sgconfig.yaml`.

Rules, from `config/ast-grep/rules`:

Expand Down Expand Up @@ -230,6 +232,9 @@ Rules, from `config/ast-grep/rules`:
- `no-string-new` -- `String::new()` is banned (`DeepSource RS-W1079`): construct empty strings with `String::default()`
instead, so the zero value routes through the `Default` impl like every other default-constructed value in the
codebase.
- `no-uds-listener` -- HTTP servers here listen on TCP only. A Unix-domain-socket listener gives requests no peer
address, and the handlers take the client address from the socket peer rather than from client-supplied `Forwarded` /
`X-Forwarded-For` headers -- so `ws_handler` refuses every request arriving on one. Bind a TCP address instead.
- `org-identity-lives-in-et-org` -- This constant spells out the organisation name or the crate prefix. Both belong to
`et-org`, which is the one place either literal is written: take the value from `et_org::ORG`, `et_org::CRATE_PREFIX`,
`et_org::NPM_SCOPE`, `et_org::REPOSITORY_URL` or `et_org::IMAGE_REGISTRY`, or build a longer string from them, rather
Expand Down Expand Up @@ -765,6 +770,8 @@ Rules, from `config/conftest/policy`:

- `semgrep-check` (`default` env; run by `check:rust`) -- Run Semgrep code and configuration rules. Repaired by
`semgrep-fix`. Config: `.semgrepignore`, `config/semgrep`.
- `semgrep-check-rule-tests` (`default` env; run by `check:rust`) -- Run the Semgrep rule tests against their annotated
cases (semgrep --test). Config: `.semgrepignore`, `config/semgrep`, `config/semgrep-tests`.

Rules, from `config/semgrep`:

Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
[coverage]: https://codecov.io/gh/edge-toolkit/core
[crate-badge]: https://img.shields.io/crates/v/et-ws-server
[crate]: https://crates.io/crates/et-ws-server
[checks-badge]: https://img.shields.io/badge/checks-%3E%3D320-blue
[checks-badge]: https://img.shields.io/badge/checks-%3E%3D340-blue
[checks]: CHECKS.md
[license-badge]: https://img.shields.io/badge/license-MIT%2BApache--2.0-blue
[license]: #licence
Expand Down
12 changes: 12 additions & 0 deletions config/ast-grep/rules/no-uds-listener.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
id: no-uds-listener
language: Rust
severity: error
message: |
HTTP servers here listen on TCP only. A Unix-domain-socket listener gives requests no peer address, and the
handlers take the client address from the socket peer rather than from client-supplied `Forwarded` /
`X-Forwarded-For` headers -- so `ws_handler` refuses every request arriving on one. Bind a TCP address instead.
rule:
pattern: $SERVER.$METHOD($$$ARGS)

Check warning on line 9 in config/ast-grep/rules/no-uds-listener.yaml

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

config/ast-grep/rules/no-uds-listener.yaml#L9

Using a single metavariable as a pattern drastically slows down the rule performance because it will match every expression in a file.

Check warning on line 9 in config/ast-grep/rules/no-uds-listener.yaml

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

config/ast-grep/rules/no-uds-listener.yaml#L9

Using patterns like `$SERVER.$METHOD($$$ARGS).$Y` may be too general and may slow down the rule performance.
constraints:
METHOD:
regex: ^(bind_uds|listen_uds)$
19 changes: 19 additions & 0 deletions config/ast-grep/tests/no-uds-listener.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
id: no-uds-listener
valid:
- |
fn serve() {
let _server = HttpServer::new(app).bind(("127.0.0.1", 8080));
}
- |
fn serve(listener: std::net::TcpListener) {
let _server = HttpServer::new(app).listen(listener);
}
invalid:
- |
fn serve() {
let _server = HttpServer::new(app).bind_uds("/run/et.sock");
}
- |
fn serve(listener: std::os::unix::net::UnixListener) {
let _server = HttpServer::new(app).listen_uds(listener);
}
2 changes: 1 addition & 1 deletion config/jscpd-baseline.json
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,6 @@
"f3eba887cab8ffc1": 1,
"f45581aa7a8806a0": 1,
"f5104a6be7ba929d": 1,
"f538dacb09de8e14": 1,
"f773b3122d6cb21a": 1,
"f8dacf11c2b785d4": 1,
"f964c720cdcd7ac0": 1,
Expand All @@ -321,6 +320,7 @@
"fb78fa42d28ddc83": 1,
"fb8747afe01f9463": 1,
"fb8c4a24a3cbfb47": 1,
"fcad2abbc735eba6": 1,
"ff0bc219f7f029b9": 1
}
}
27 changes: 10 additions & 17 deletions generated/dart-ws/lib/ws_messages.dart
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
// ignore_for_file: unnecessary_cast
final class AgentSummary {
final String agentId;
final String? lastKnownIp;
final String lastKnownIp;
final AgentConnectionState state;

const AgentSummary({
required this.agentId,
this.lastKnownIp,
required this.lastKnownIp,
required this.state,
});

static AgentSummaryBuilder builder({
required String agentId,
String? lastKnownIp,
required String lastKnownIp,
required AgentConnectionState state,
}) => AgentSummaryBuilder(
agentId: agentId,
lastKnownIp: lastKnownIp == null ? null : (lastKnownIp as String),
lastKnownIp: lastKnownIp,
state: state,
);
AgentSummaryBuilder toBuilder() => AgentSummaryBuilder(
agentId: agentId,
lastKnownIp: lastKnownIp == null ? null : (lastKnownIp as String),
lastKnownIp: lastKnownIp,
state: state,
);

Expand All @@ -32,11 +32,7 @@ final class AgentSummary {
};
factory AgentSummary.fromJson(Map<String, dynamic> json) => AgentSummary(
agentId: json["agent_id"] as String,
lastKnownIp: json["last_known_ip"] == null
? null
: json["last_known_ip"] == null
? null
: json["last_known_ip"] as String,
lastKnownIp: json["last_known_ip"] as String,
state: AgentConnectionState.fromJson(json["state"]),
);

Expand Down Expand Up @@ -69,13 +65,13 @@ final class AgentSummary {

@override
int get hashCode =>
Object.hashAll([agentId.hashCode, lastKnownIp?.hashCode, state.hashCode]);
Object.hashAll([agentId.hashCode, lastKnownIp.hashCode, state.hashCode]);
}

/// Builder class for [AgentSummary]
final class AgentSummaryBuilder {
String agentId;
String? lastKnownIp;
String lastKnownIp;
AgentConnectionState state;

AgentSummaryBuilder({
Expand All @@ -84,11 +80,8 @@ final class AgentSummaryBuilder {
required this.state,
});

AgentSummary build() => AgentSummary(
agentId: agentId,
lastKnownIp: lastKnownIp == null ? null : (lastKnownIp as String),
state: state,
);
AgentSummary build() =>
AgentSummary(agentId: agentId, lastKnownIp: lastKnownIp, state: state);
}

final class CloudEvent {
Expand Down
7 changes: 6 additions & 1 deletion generated/specs/wit/deps/et-ws-messages/messages.wit
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,14 @@ interface messages {
broadcast,
}
type spec-version = string;
/// An IPv4 or IPv6 address, as a fixed-size tuple of its octets or hextets.
variant ip-address {
ipv4(tuple<u8, u8, u8, u8>),
ipv6(tuple<u16, u16, u16, u16, u16, u16, u16, u16>),
}
record agent-summary {
agent-id: string,
last-known-ip: option<string>,
last-known-ip: ip-address,
state: agent-connection-state,
}
record cloud-event {
Expand Down
4 changes: 1 addition & 3 deletions generated/specs/ws.kdl
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@ enum "SpecVersion" {
}
class "AgentSummary" {
field "agentId" type="String"
field "lastKnownIp" type="String?" {
defaults-to null
}
field "lastKnownIp" type="String"
field "state" type="AgentConnectionState"
}
class "CloudEvent" {
Expand Down
6 changes: 3 additions & 3 deletions generated/specs/ws.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -290,14 +290,14 @@ components:
agent_id:
type: string
last_known_ip:
type:
- string
- "null"
format: ip
type: string
state:
$ref: "#/components/schemas/AgentConnectionState"
required:
- agent_id
- state
- last_known_ip
type: object
CloudEvent:
description: |-
Expand Down
6 changes: 4 additions & 2 deletions libs/edge-toolkit/src/ws.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::net::IpAddr;

use chrono::{DateTime, Utc};
use serde::{Deserialize, Serialize};

Expand Down Expand Up @@ -95,12 +97,12 @@ pub enum AgentConnectionState {
pub struct AgentSummary {
pub agent_id: String,
pub state: AgentConnectionState,
pub last_known_ip: Option<String>,
pub last_known_ip: IpAddr,
}

impl AgentSummary {
#[must_use]
pub const fn new(agent_id: String, state: AgentConnectionState, last_known_ip: Option<String>) -> Self {
pub const fn new(agent_id: String, state: AgentConnectionState, last_known_ip: IpAddr) -> Self {
Self {
agent_id,
state,
Expand Down
13 changes: 7 additions & 6 deletions libs/edge-toolkit/src/ws_server.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::collections::BTreeMap;
use std::net::IpAddr;
use std::sync::{Arc, Mutex, MutexGuard, PoisonError};

use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -90,7 +91,7 @@ pub struct PendingDirectMessage {
#[non_exhaustive]
pub struct AgentRecord<S> {
pub state: AgentConnectionState,
pub last_known_ip: Option<String>,
pub last_known_ip: IpAddr,
Comment thread
jayvdb marked this conversation as resolved.
#[serde(skip)]
pub session: Option<S>,
#[serde(default)]
Expand All @@ -100,7 +101,7 @@ pub struct AgentRecord<S> {
impl<S> AgentRecord<S> {
/// Construct a record with no pending direct messages.
#[must_use]
pub const fn new(state: AgentConnectionState, last_known_ip: Option<String>, session: Option<S>) -> Self {
pub const fn new(state: AgentConnectionState, last_known_ip: IpAddr, session: Option<S>) -> Self {
Self {
state,
last_known_ip,
Expand Down Expand Up @@ -165,7 +166,7 @@ impl<S: Clone + Send + 'static> AgentRegistry<S> {
&self,
requested_id: Option<String>,
new_id: String,
client_ip: &str,
client_ip: IpAddr,
session: S,
) -> (String, ConnectStatus) {
let mut agents = lock_agents(&self.agents);
Expand All @@ -174,7 +175,7 @@ impl<S: Clone + Send + 'static> AgentRegistry<S> {
&& let Some(record) = agents.get_mut(requested)
{
record.state = AgentConnectionState::Connected;
record.last_known_ip = Some(client_ip.to_string());
record.last_known_ip = client_ip;
record.session = Some(session);
return (requested.to_string(), ConnectStatus::Reconnected);
}
Expand All @@ -187,7 +188,7 @@ impl<S: Clone + Send + 'static> AgentRegistry<S> {
assigned_id.clone(),
AgentRecord {
state: AgentConnectionState::Connected,
last_known_ip: Some(client_ip.to_string()),
last_known_ip: client_ip,
session: Some(session),
pending_direct_messages: BTreeMap::new(),
},
Expand Down Expand Up @@ -231,7 +232,7 @@ impl<S: Clone + Send + 'static> AgentRegistry<S> {
.map(|(agent_id, record)| AgentSummary {
agent_id: agent_id.clone(),
state: record.state.clone(),
last_known_ip: record.last_known_ip.clone(),
last_known_ip: record.last_known_ip,
})
.collect::<Vec<_>>();
drop(agents);
Expand Down
Loading
Loading