fix(local-agent): review follow-up 小批(TLS Mi-3/4 + WP-0 S-3/4)
- Mi-3: insecure Transport 改用 DefaultTransport.Clone() 只覆寫 TLSConfig (保留 proxy/timeout,消除「開 skip 順便改掉 proxy 行為」副作用) - Mi-4: exchange 200 分支檢查 Success 欄位(避免 200+success:false 落到 誤導性的 missing session_token;既有回歸測試改用直接斷言防護不減反增) - S-3: collectLocalDevices 全空 serial 濾掉不送 devices 陣列(payload 對稱) - S-4: 假序號比對統一用 EqualFold(防未來 bridge 輸出 casing 變化) Reviewer 通過(0C/0M/1Mi/3Sug)。兩 module build/vet/test + -race 綠、 gitleaks 0、TLS 行為級測試全 PASS。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
59c57fa481
commit
26b433eb10
@ -3,6 +3,7 @@ package device
|
||||
import (
|
||||
"fmt"
|
||||
"log"
|
||||
"strings"
|
||||
"sync"
|
||||
|
||||
"visiona-agent/server/internal/driver"
|
||||
@ -102,11 +103,21 @@ func (m *Manager) Start() {
|
||||
m.rebuildSerialIndexLocked()
|
||||
}
|
||||
|
||||
// isFakeSerial reports whether a serial is the pyusb-fallback placeholder.
|
||||
// Uses EqualFold to match the backend's comparison (visionA-backend
|
||||
// pairing_exchange.go), so both layers treat the fake serial identically even
|
||||
// if the Python bridge ever emits a different hex casing (e.g. "0X00000000").
|
||||
// The current value is all-zero (no casing difference), so this is defensive
|
||||
// alignment for future bridge output changes (WP-0 review S-4).
|
||||
func isFakeSerial(serial string) bool {
|
||||
return strings.EqualFold(serial, fakeSerialNumber)
|
||||
}
|
||||
|
||||
// serialIdentity normalizes a serial for identity comparison: empty and the
|
||||
// fake pyusb-fallback serial both mean "no identity" (ADR-018 R2), so they
|
||||
// compare equal to each other and never equal to a real kn_number.
|
||||
func serialIdentity(serial string) string {
|
||||
if serial == fakeSerialNumber {
|
||||
if isFakeSerial(serial) {
|
||||
return ""
|
||||
}
|
||||
return serial
|
||||
@ -203,7 +214,7 @@ func (m *Manager) rebuildSerialIndexLocked() {
|
||||
idx := make(map[string]string, len(m.sessions))
|
||||
for id, s := range m.sessions {
|
||||
serial := s.Driver.Info().SerialNumber
|
||||
if serial == "" || serial == fakeSerialNumber {
|
||||
if serial == "" || isFakeSerial(serial) {
|
||||
continue
|
||||
}
|
||||
if prev, dup := idx[serial]; dup {
|
||||
|
||||
@ -199,6 +199,31 @@ func TestManager_SerialIndex_SkipsEmptyAndFakeSerial(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestManager_SerialIndex_FakeSerialCaseInsensitive verifies S-4: the fake
|
||||
// serial is matched case-insensitively (EqualFold), aligned with the backend.
|
||||
// A hex-casing variant like "0X00000000" must still be treated as "no serial"
|
||||
// and never routable, guarding against future bridge output casing changes.
|
||||
func TestManager_SerialIndex_FakeSerialCaseInsensitive(t *testing.T) {
|
||||
const fakeUpper = "0X00000000" // same value, different hex casing
|
||||
mgr := NewManager(NewRegistry(), "")
|
||||
mgr.mu.Lock()
|
||||
mgr.sessions["kl520-0"] = NewSession(&testDriver{info: driver.DeviceInfo{ID: "kl520-0", SerialNumber: fakeUpper}})
|
||||
mgr.rebuildSerialIndexLocked()
|
||||
mgr.mu.Unlock()
|
||||
|
||||
if _, err := mgr.GetDevice(fakeUpper); err == nil {
|
||||
t.Errorf("GetDevice(%s) expected error (fake serial variant must not be routable)", fakeUpper)
|
||||
}
|
||||
// serialIdentity should also treat the variant as no-identity.
|
||||
if got := serialIdentity(fakeUpper); got != "" {
|
||||
t.Errorf("serialIdentity(%s) = %q, want \"\" (fake serial variant = no identity)", fakeUpper, got)
|
||||
}
|
||||
// The device remains reachable by its local id.
|
||||
if _, err := mgr.GetDevice("kl520-0"); err != nil {
|
||||
t.Errorf("GetDevice(kl520-0) error = %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// ==========================================================================
|
||||
// Rescan identity check (WP-0 review Minor #1): positional synthetic IDs vs
|
||||
// serial identity — the serial index must reflect current detection.
|
||||
|
||||
@ -293,10 +293,15 @@ func (e *HTTPPairingExchanger) exchangeReal(pairingToken string) (ExchangeResult
|
||||
// 注意:只在 client.Transport == nil(含 NewHTTPPairingExchanger 的預設 client)
|
||||
// 時覆寫,避免踩掉呼叫者注入的自訂 Transport;且不直接改 e.Client(用區域複本),
|
||||
// 不影響傳入的共享 client。
|
||||
//
|
||||
// Clone DefaultTransport 而非新建 zero-value &http.Transport{}:只覆寫
|
||||
// TLSClientConfig(跳過憑證驗證),保留預設的 Proxy: http.ProxyFromEnvironment
|
||||
// 與 dial / TLS handshake timeout。避免「開 skip 順便改掉 proxy 行為」的隱性
|
||||
// 副作用(例如經 corp proxy 的 dev 環境會被迫改走直連)。
|
||||
tr := http.DefaultTransport.(*http.Transport).Clone()
|
||||
tr.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} //nolint:gosec // dev-only, gated by VISIONA_INSECURE_SKIP_TLS_VERIFY
|
||||
c := *client
|
||||
c.Transport = &http.Transport{
|
||||
TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec // dev-only, gated by VISIONA_INSECURE_SKIP_TLS_VERIFY
|
||||
}
|
||||
c.Transport = tr
|
||||
client = &c
|
||||
}
|
||||
|
||||
@ -329,6 +334,11 @@ func (e *HTTPPairingExchanger) exchangeReal(pairingToken string) (ExchangeResult
|
||||
if err := json.Unmarshal(respBody, &ok); err != nil {
|
||||
return ExchangeResult{}, fmt.Errorf("decode exchange response: %w", err)
|
||||
}
|
||||
// HTTP 200 + success:false 是 envelope 契約允許表達、但正常不該發生的異常組合。
|
||||
// 先明確攔截,避免落到下方「missing session_token」這個誤導性訊息。
|
||||
if !ok.Success {
|
||||
return ExchangeResult{}, fmt.Errorf("exchange returned success=false: %s", truncate(string(respBody), 256))
|
||||
}
|
||||
// session_token 在 envelope 的 data. 底下(見 exchangeResponse 註解)。
|
||||
if ok.Data.SessionToken == "" {
|
||||
return ExchangeResult{}, errors.New("exchange response missing session_token")
|
||||
@ -370,12 +380,24 @@ func (e *HTTPPairingExchanger) collectLocalDevices() []exchangeDevice {
|
||||
}
|
||||
out := make([]exchangeDevice, 0, len(devs))
|
||||
for _, d := range devs {
|
||||
// serial 是 devices payload 的唯一用途(雲端據此填 serial_number 做 serial
|
||||
// 路由)。空 serial 的裝置對雲端無意義、雲端本來就會濾掉——agent 端先濾,
|
||||
// 減少 payload 面積、也讓語意對稱(WP-0 review S-3)。
|
||||
serial := strings.TrimSpace(d.SerialNumber)
|
||||
if serial == "" {
|
||||
continue
|
||||
}
|
||||
out = append(out, exchangeDevice{
|
||||
SerialNumber: strings.TrimSpace(d.SerialNumber),
|
||||
SerialNumber: serial,
|
||||
DeviceType: d.DeviceType,
|
||||
Firmware: d.Firmware,
|
||||
})
|
||||
}
|
||||
// 全部 serial 皆空 → 回 nil,讓 payload 省略 devices 欄位(omitempty),
|
||||
// 與「無 DeviceLister」的舊行為一致。
|
||||
if len(out) == 0 {
|
||||
return nil
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
|
||||
@ -158,7 +158,12 @@ func TestExchangeRealRawEnvelope(t *testing.T) {
|
||||
|
||||
// TestExchangeRealTopLevelSessionTokenRejected 是回歸防護:確認 agent 不會誤接
|
||||
// 「session_token 在頂層」的舊格式(contract drift 前的假設)。雲端不再回這種格式,
|
||||
// 若 agent 又退回解頂層,這個測試會抓到(會解不到 data.session_token → 報 missing)。
|
||||
// 若 agent 又退回解頂層,這個測試會抓到。
|
||||
//
|
||||
// legacyBody 既無 `success:true` 也無 `data.session_token`,故現行實作在 200 分支
|
||||
// 先被 Mi-4 的 success 檢查攔下(success=false);即使未來調整攔截順序、也必然落到
|
||||
// missing session_token。兩種訊息都代表「舊頂層格式被拒」,核心防護意圖不變——只要
|
||||
// exchange 失敗且非退回解頂層即可。
|
||||
func TestExchangeRealTopLevelSessionTokenRejected(t *testing.T) {
|
||||
const legacyBody = `{"session_token":"vAs_toplevel","account":"old@x"}`
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
@ -168,12 +173,16 @@ func TestExchangeRealTopLevelSessionTokenRejected(t *testing.T) {
|
||||
defer srv.Close()
|
||||
|
||||
ex := NewHTTPPairingExchanger(srv.URL)
|
||||
_, err := ex.Exchange("vAc_0123456789abcdef0123456789abcdef")
|
||||
res, err := ex.Exchange("vAc_0123456789abcdef0123456789abcdef")
|
||||
if err == nil {
|
||||
t.Fatal("expected error when session_token is only at top level (legacy format no longer accepted)")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "missing session_token") {
|
||||
t.Errorf("err = %v, want 'missing session_token'", err)
|
||||
// 關鍵回歸斷言:絕不能誤接頂層的 "vAs_toplevel"。
|
||||
if res.SessionToken != "" {
|
||||
t.Errorf("SessionToken = %q, want empty (top-level session_token must not be accepted)", res.SessionToken)
|
||||
}
|
||||
if !strings.Contains(err.Error(), "missing session_token") && !strings.Contains(err.Error(), "success=false") {
|
||||
t.Errorf("err = %v, want 'missing session_token' or 'success=false'", err)
|
||||
}
|
||||
}
|
||||
|
||||
@ -487,6 +496,102 @@ func TestExchangeReal_NoDeviceLister_NoDevicesField(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestExchangeReal_AllEmptySerials_NoDevicesField 驗證 S-3:DeviceLister 回的
|
||||
// 裝置全部 serial 皆空時,payload 省略 devices 欄位(與無 DeviceLister 舊行為一致),
|
||||
// 不送出對雲端無意義的空 serial 陣列。
|
||||
func TestExchangeReal_AllEmptySerials_NoDevicesField(t *testing.T) {
|
||||
var rawBody []byte
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
buf := new(strings.Builder)
|
||||
_, _ = io.Copy(buf, r.Body)
|
||||
rawBody = []byte(buf.String())
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(exchangeResponse{
|
||||
Success: true,
|
||||
Data: exchangeResponseData{SessionToken: "vAs_" + strings.Repeat("d", 64)},
|
||||
})
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
ex := NewHTTPPairingExchanger(srv.URL)
|
||||
ex.DeviceLister = func() ([]LocalDevice, error) {
|
||||
// 兩顆都無序號(no-SDK demo / 撈到但序號空白)。
|
||||
return []LocalDevice{
|
||||
{SerialNumber: "", DeviceType: "kneron_kl520"},
|
||||
{SerialNumber: " ", DeviceType: "kneron_kl720"},
|
||||
}, nil
|
||||
}
|
||||
|
||||
if _, err := ex.Exchange("vAc_0123456789abcdef0123456789abcdef"); err != nil {
|
||||
t.Fatalf("Exchange: %v", err)
|
||||
}
|
||||
if strings.Contains(string(rawBody), "devices") {
|
||||
t.Errorf("全部 serial 皆空時 payload 不應含 devices 欄位:%s", rawBody)
|
||||
}
|
||||
}
|
||||
|
||||
// TestExchangeReal_PartialEmptySerials_OnlyKeepsNonEmpty 驗證 S-3:混合清單中
|
||||
// 空 serial 被濾掉、只留有序號的裝置。
|
||||
func TestExchangeReal_PartialEmptySerials_OnlyKeepsNonEmpty(t *testing.T) {
|
||||
var gotBody map[string]interface{}
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
_ = json.NewDecoder(r.Body).Decode(&gotBody)
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(exchangeResponse{
|
||||
Success: true,
|
||||
Data: exchangeResponseData{SessionToken: "vAs_" + strings.Repeat("e", 64)},
|
||||
})
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
ex := NewHTTPPairingExchanger(srv.URL)
|
||||
ex.DeviceLister = func() ([]LocalDevice, error) {
|
||||
return []LocalDevice{
|
||||
{SerialNumber: "", DeviceType: "kneron_kl520"},
|
||||
{SerialNumber: "0x1A2B3C4D", DeviceType: "kneron_kl720"},
|
||||
}, nil
|
||||
}
|
||||
|
||||
if _, err := ex.Exchange("vAc_0123456789abcdef0123456789abcdef"); err != nil {
|
||||
t.Fatalf("Exchange: %v", err)
|
||||
}
|
||||
devices, ok := gotBody["devices"].([]interface{})
|
||||
if !ok {
|
||||
t.Fatalf("request body 缺 devices 欄位:%v", gotBody)
|
||||
}
|
||||
if len(devices) != 1 {
|
||||
t.Fatalf("devices 長度 = %d, want 1(空 serial 應被濾除)", len(devices))
|
||||
}
|
||||
d0 := devices[0].(map[string]interface{})
|
||||
if d0["serial_number"] != "0x1A2B3C4D" {
|
||||
t.Errorf("devices[0].serial_number = %v, want 0x1A2B3C4D", d0["serial_number"])
|
||||
}
|
||||
}
|
||||
|
||||
// TestExchangeReal_SuccessFalse_ReturnsClearError 驗證 Mi-4:HTTP 200 但
|
||||
// envelope success=false 時回明確錯誤,而非誤導性的 "missing session_token"。
|
||||
func TestExchangeReal_SuccessFalse_ReturnsClearError(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(http.StatusOK)
|
||||
// 200 + success:false(異常組合,契約允許表達)。
|
||||
_ = json.NewEncoder(w).Encode(exchangeResponse{Success: false})
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
ex := NewHTTPPairingExchanger(srv.URL)
|
||||
_, err := ex.Exchange("vAc_0123456789abcdef0123456789abcdef")
|
||||
if err == nil {
|
||||
t.Fatal("expected error on 200 + success=false")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "success=false") {
|
||||
t.Errorf("err = %v, want to mention success=false(非誤導性 missing session_token)", err)
|
||||
}
|
||||
if strings.Contains(err.Error(), "missing session_token") {
|
||||
t.Errorf("err = %v — 不應落到誤導性的 missing session_token 訊息", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestNewLocalDeviceLister_ParsesEnvelope 驗證 NewLocalDeviceLister 能解析
|
||||
// local server GET /api/devices 的 envelope(success + data.devices)。
|
||||
func TestNewLocalDeviceLister_ParsesEnvelope(t *testing.T) {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user