Replace Antminer plugin with ASIC-RS#479
Draft
ankitgoswami wants to merge 2 commits into
Draft
Conversation
github-actions
Bot
added
documentation
🔐 Codex Security Review
Review SummaryOverall Risk: MEDIUM Findings[MEDIUM] Stock Antminer Firmware Upload Uses the Wrong Multipart Field
[MEDIUM] Firmware Upload Buffers Large Images and Error Bodies in Memory
[MEDIUM] Log Download Caps gRPC Pages but Not Total Log Memory
NotesNo hardcoded wallet, payout address, or pool hijack logic was found in the reviewed diff. No new SQL injection or authentication bypass findings stood out in the changed hunks. Generated by Codex Security Review | |
ankitgoswami
changed the title
b-rowan
reviewed
Jun 22, 2026
b-rowan
left a comment
b-rowan
left a comment
Contributor
There was a problem hiding this comment.
Gradually, then suddenly.
🥳
| @@ -484,4 +548,32 @@ mod tests { | |||
| assert_eq!(make_to_family("LuxOS"), Some(FAMILY_ANTMINER)); | |||
| assert_eq!(make_to_family("Marathon"), Some(FAMILY_ANTMINER)); | |||
| } | |||
|
|
|||
| #[test] | |||
Contributor
There was a problem hiding this comment.
Might be worth adding a test which creates a "real" asic-rs type, then checking against that. That will ensure that the tests inform a user if an asic-rs update breaks this expectation.
Comment on lines
+966
to
+1049
| async fn update_stock_antminer_firmware(&self, image: FirmwareImage) -> anyhow::Result<bool> { | ||
| let auth = self | ||
| .auth | ||
| .lock() | ||
| .await | ||
| .clone() | ||
| .ok_or_else(|| anyhow::anyhow!("stock Antminer firmware fallback requires auth"))?; | ||
|
|
||
| let host = &self.info.host; | ||
| let client = reqwest::Client::builder() | ||
| .timeout(FIRMWARE_UPDATE_TIMEOUT) | ||
| .build() | ||
| .map_err(|e| anyhow::anyhow!("failed to create firmware upload client: {e}"))?; | ||
|
|
||
| let challenge_url = format!("http://{host}:80/cgi-bin/get_system_info.cgi"); | ||
| let challenge = client | ||
| .get(&challenge_url) | ||
| .timeout(OP_TIMEOUT) | ||
| .send() | ||
| .await | ||
| .map_err(|e| anyhow::anyhow!("firmware auth challenge failed: {e}"))?; | ||
| let www_authenticate = challenge | ||
| .headers() | ||
| .get("www-authenticate") | ||
| .ok_or_else(|| anyhow::anyhow!("firmware auth challenge missing WWW-Authenticate"))? | ||
| .to_str() | ||
| .map_err(|e| anyhow::anyhow!("invalid firmware auth challenge header: {e}"))?; | ||
|
|
||
| let upload_path = "/cgi-bin/upgrade.cgi"; | ||
| let mut prompt = digest_auth::parse(www_authenticate) | ||
| .map_err(|e| anyhow::anyhow!("invalid firmware auth challenge: {e}"))?; | ||
| let context = AuthContext::new_with_method( | ||
| &auth.username, | ||
| auth.password.expose_secret(), | ||
| upload_path, | ||
| None::<&[u8]>, | ||
| HttpMethod::POST, | ||
| ); | ||
| let authorization = prompt | ||
| .respond(&context) | ||
| .map_err(|e| anyhow::anyhow!("failed to build firmware auth header: {e}"))?; | ||
|
|
||
| let FirmwareImage { filename, bytes } = image; | ||
| let part = reqwest::multipart::Part::bytes(bytes) | ||
| .file_name(filename) | ||
| .mime_str("application/octet-stream") | ||
| .map_err(|e| anyhow::anyhow!("failed to build firmware upload part: {e}"))?; | ||
| let form = reqwest::multipart::Form::new().part("firmware", part); | ||
|
|
||
| let upload_url = format!("http://{host}:80{upload_path}"); | ||
| let response = client | ||
| .post(upload_url) | ||
| .multipart(form) | ||
| .header(AUTHORIZATION, authorization.to_header_string()) | ||
| .timeout(FIRMWARE_UPDATE_TIMEOUT) | ||
| .send() | ||
| .await | ||
| .map_err(|e| anyhow::anyhow!("firmware upload HTTP request failed: {e}"))?; | ||
|
|
||
| let status = response.status(); | ||
| let body = response | ||
| .text() | ||
| .await | ||
| .map_err(|e| anyhow::anyhow!("failed to read firmware upload response body: {e}"))?; | ||
| if !status.is_success() { | ||
| return Err(anyhow::anyhow!( | ||
| "firmware upload failed with status code {status}: {body}" | ||
| )); | ||
| } | ||
|
|
||
| let parsed: serde_json::Value = serde_json::from_str(&body) | ||
| .map_err(|e| anyhow::anyhow!("invalid firmware upload response: {e}"))?; | ||
| let ok = parsed.get("code").and_then(|v| v.as_str()) == Some("U000") | ||
| && parsed.get("stats").and_then(|v| v.as_str()) == Some("success"); | ||
| if !ok { | ||
| let message = parsed | ||
| .get("msg") | ||
| .and_then(|v| v.as_str()) | ||
| .unwrap_or("unknown error"); | ||
| return Err(anyhow::anyhow!("firmware upload rejected: {message}")); | ||
| } | ||
|
|
||
| Ok(true) | ||
| } |
Contributor
There was a problem hiding this comment.
Is this function still required? Should have been added in asic-rs a while back, any big differences between them that need to be addressed?
Comment on lines
+19
to
+20
| asic-rs = { git = "https://github.com/256foundation/asic-rs.git", rev = "43d706ef31aa154f8bea0bd7ef8e81caa9cbe491" } | ||
| asic-rs-core = { git = "https://github.com/256foundation/asic-rs.git", rev = "43d706ef31aa154f8bea0bd7ef8e81caa9cbe491" } |
Contributor
There was a problem hiding this comment.
0.7.0 just released, should work for these purposes?
| // NewServer starts a mock Antminer server with RPC on 127.0.0.1:4028 and HTTP on a random port. | ||
| // Not safe for parallel tests (shares port 4028 with WhatsMiner mock). | ||
| // NewServer starts a mock Antminer server with RPC on 127.0.0.1:4028 and HTTP on port 80. | ||
| // Port 80 is required because ASIC-RS hardcodes stock Antminer HTTP connections to port 80. |
Contributor
There was a problem hiding this comment.
Is this an issue? I web.port exists on the asic-rs types, but I don't think we expose a way to change it, is it worth changing this on the asic-rs side?
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
2 participants
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
plugin/asicrsthe stock Antminer driver.mastercommit43d706ef31aa154f8bea0bd7ef8e81caa9cbe491and enable stock Antminer support.000087to move activedriver_name = 'antminer'discovered devices toasicrsand normalize stock Antminer discovery port from4028to80.Test Plan
cargo checkinplugin/asicrscargo testinplugin/asicrsjust rebuild-plugin asicrsjust test-contractDB_PASSWORD=fleet go test ./internal/domain/stores/sqlstores -run TestMigration000087_MigratesPairedAntminerDiscoveredDevicesToAsicrs -count=1go test ./internal/domain/plugins -count=1go test ./cmd/fleetnode ./internal/domain/plugins ./internal/domain/telemetrygo test ./internal/domain/miner -run 'TestEffectiveCapabilitiesForDevice'go test ./internal/domain/fleetmanagement -run 'TestListMinerStateSnapshots_PopulatesSiteIDAndLabel'go test ./internal/domain/pairing -run 'TestHandleAuthenticationRequiredPairing|TestCanonicalCIDR|TestMergeAutoDiscoveryTargets|TestResolveNmapTargets|TestValidateNmapTargets|TestShouldSkipNetworkOrGatewayAddress'go test ./internal/domain/command -run 'TestUpdateMinerPassword|TestCapability|TestHasAnyCapability'go work syncgit diff --check