mobile: support attested labels on the Android node - #381
Conversation
The mobile FFI had no way to declare labels, so a phone always enrolled unlabeled and any mesh call with required_labels against it was refused fail-closed. Add a labels field to the FFI start config and an EnrollNode parameter (labels are attested only at enrollment), plumbed from a new field on the app's enrollment screen using the CLI --labels syntax. The comma-separated parser moves from cmd/sam-node to api.ParseLabels so the CLI and the FFI share one implementation.
A closure created inside a State method shares its context with the sibling setState closures, so Isolate.run received the widget state and its DynamicLibrary and failed with "Illegal argument in isolate message". Every enroll path in the app was affected.
Dex rejects the app's device flow and loopback redirects unless the client lists them. Debug builds also need a network security config: Flutter blocks plain http to anything but loopback, and the kind Dex is plain http.
The external MCP url, name and description fields were prefilled with the local android-remote server's values. Turn them into hints so the fields start empty and the values read as examples. Also exclude build/ and android/ from the analyzer so `flutter analyze` only reports on our own sources.
The Start button stayed enabled until _running was set, so a second tap re-entered _start() about two seconds later. Its HttpServer.bind hit the port the first call had already bound, the SocketException was swallowed by a debugPrint, and the failing call's error path then stopped the server the first call had successfully started. The node stayed up with a dead backend on 9090, so it withheld phone-sensors from the DHT and mesh discovery returned "initialize: EOF" or nothing at all, flapping with the health probe. Guard _start() against re-entry, let the bind error propagate so a failure aborts the start instead of declaring a service that points at a dead port, and only stop the embedded server when this call is the one that started it. The button now reports "Starting…" while it runs.
The app only shows the labels field on its enrollment screen, so after a relaunch StartNode ran with no labels: discovery gossiped none and the next Biscuit renewal re-enrolled without them. EnrollNode now writes the labels to the app's data directory and StartNode reads them back when the start config carries none.
show_cluster_logs used gateway_ip, which polls for two minutes and exits non-zero, so under set -e `run.sh -l` opened nothing when a gateway had no address yet.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors label parsing by moving the parsing logic to a shared api.ParseLabels function and integrates label support into the mobile application's enrollment and startup flows. It also improves the mobile app's start button state handling and error reporting. The review feedback suggests enhancing the label parsing robustness by trimming whitespace from the input string, adding corresponding test coverage, and ensuring that parsed labels are semantically validated using api.ValidateLabels during both node enrollment and startup in the mobile FFI layer.
| labels, err := api.ParseLabels(config.Labels) | ||
| if err != nil { | ||
| return fmt.Errorf("invalid labels: %w", err) | ||
| } | ||
| // The app only shows the labels field on its enrollment screen, so an | ||
| // empty start config reuses the labels the node enrolled with. | ||
| if len(labels) == 0 { | ||
| if labels, err = loadEnrolledLabels(config.DataDir); err != nil { | ||
| return err | ||
| } | ||
| } |
There was a problem hiding this comment.
The parsed labels should be validated using api.ValidateLabels to ensure that they conform to the required key/value specifications (e.g., length limits, allowed characters) before starting the node. Currently, only syntactic parsing is performed, which could allow malformed labels to be used at runtime.
labels, err := api.ParseLabels(config.Labels)
if err != nil {
return fmt.Errorf("invalid labels: %w", err)
}
// The app only shows the labels field on its enrollment screen, so an
// empty start config reuses the labels the node enrolled with.
if len(labels) == 0 {
if labels, err = loadEnrolledLabels(config.DataDir); err != nil {
return err
}
}
if err := api.ValidateLabels(labels); err != nil {
return fmt.Errorf("invalid labels: %w", err)
}| parsedLabels, err := api.ParseLabels(labels) | ||
| if err != nil { | ||
| return fmt.Errorf("invalid labels: %w", err) | ||
| } |
There was a problem hiding this comment.
Validate the parsed labels using api.ValidateLabels before saving them to the local labels file and proceeding with enrollment. This prevents enrolling with or persisting malformed labels that would fail validation later.
parsedLabels, err := api.ParseLabels(labels)
if err != nil {
return fmt.Errorf("invalid labels: %w", err)
}
if err := api.ValidateLabels(parsedLabels); err != nil {
return fmt.Errorf("invalid labels: %w", err)
}| func ParseLabels(s string) (map[string]string, error) { | ||
| if s == "" { | ||
| return nil, nil | ||
| } |
There was a problem hiding this comment.
Trimming leading and trailing whitespace from the input string s at the beginning of ParseLabels ensures that whitespace-only strings (e.g., " ") are consistently treated as empty and return nil, nil, rather than returning an empty map.
| func ParseLabels(s string) (map[string]string, error) { | |
| if s == "" { | |
| return nil, nil | |
| } | |
| func ParseLabels(s string) (map[string]string, error) { | |
| s = strings.TrimSpace(s) | |
| if s == "" { | |
| return nil, nil | |
| } |
| if labels, err := ParseLabels(""); err != nil || labels != nil { | ||
| t.Fatalf("empty string should yield no labels, got %v, %v", labels, err) | ||
| } |
There was a problem hiding this comment.
Add a test case to verify that whitespace-only strings are correctly parsed as empty/nil labels, matching the updated ParseLabels behavior.
| if labels, err := ParseLabels(""); err != nil || labels != nil { | |
| t.Fatalf("empty string should yield no labels, got %v, %v", labels, err) | |
| } | |
| if labels, err := ParseLabels(""); err != nil || labels != nil { | |
| t.Fatalf("empty string should yield no labels, got %v, %v", labels, err) | |
| } | |
| if labels, err := ParseLabels(" "); err != nil || labels != nil { | |
| t.Fatalf("whitespace-only string should yield no labels, got %v, %v", labels, err) | |
| } |
|
This needs to be reworked as we decided to move the labels to the config from the CLI flags |
1 participant
The mobile FFI had no way to declare labels, so a phone always enrolled unlabeled and any call against it with
required_labelswas refused by the caller's labels gate. Getting the label flow to work end to end on the emulator also surfaced several bugs in the app and gaps in the kind dev setup, fixed here.Labels
api.ParseLabels: thekey=valueparser moves fromcmd/sam-nodeintoapi/labels.goso the CLI and the FFI share it; the CLI calls it directly.EnrollNodeandMobileConfiggainlabels. Enrollment mints them into the Biscuit; the FFI keeps a copy in the app's data directory and reuses it on start and renewal, since the app only shows the labels field on its enrollment screen.App fixes
Isolate.runclosures inside State methods capturedthisand itsDynamicLibrary. They now run from top-level functions.start()is guarded and errors propagate.Kind dev setup
bootstrap.nodeLabels={*}so dev nodes can attest labels; debug builds allow cleartext http to the kind cluster.Docs: the mobile README and site page no longer claim the
mobile-ffi-*targets copy intojniLibs, give the right x86_64 paths, and describe how the app carries labels across starts.Tests:
api.ParseLabelsunit test; the FFI lifecycle test asserts the enroll request carries the labels, that they are persisted, and that a label-lessStartNodefalls back to them; a negative test checksStartNoderejects malformed labels.