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
21 changes: 19 additions & 2 deletions commands/apps.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,13 @@ type InstallAppRequest struct {
SigningIdentity string `json:"signingIdentity"`
}

// InstallAppResult is returned on a successful install, including the app
// metadata parsed from the installed file when available.
type InstallAppResult struct {
Message string `json:"message"`
App *utils.AppMetadata `json:"app,omitempty"`
}

func InstallAppCommand(req InstallAppRequest) *CommandResponse {
if req.Path == "" {
return NewErrorResponse(fmt.Errorf("path is required"))
Expand Down Expand Up @@ -151,9 +158,19 @@ func InstallAppCommand(req InstallAppRequest) *CommandResponse {
return NewErrorResponse(fmt.Errorf("failed to install app on device %s: %w", targetDevice.ID(), err))
}

return NewSuccessResponse(MessageResult{
result := InstallAppResult{
Message: fmt.Sprintf("Installed app from '%s' on device %s", req.Path, targetDevice.ID()),
})
}

// metadata extraction is best-effort: a parse failure must not turn a
// successful install into an error.
if meta, err := utils.ParseAppMetadata(req.Path); err != nil {
Comment thread
gmegidish marked this conversation as resolved.
utils.Verbose("failed to parse app metadata from %s: %v", req.Path, err)
} else {
result.App = meta
}

return NewSuccessResponse(result)
}

type AppPathRequest struct {
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ require (
github.com/gorilla/websocket v1.5.3
github.com/hashicorp/golang-lru/v2 v2.0.7
github.com/sevlyar/go-daemon v0.1.6
github.com/shogo82148/androidbinary v1.0.5
github.com/sirupsen/logrus v1.9.3
github.com/spf13/cobra v1.9.1
github.com/stretchr/testify v1.10.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/f
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/sevlyar/go-daemon v0.1.6 h1:EUh1MDjEM4BI109Jign0EaknA2izkOyi0LV3ro3QQGs=
github.com/sevlyar/go-daemon v0.1.6/go.mod h1:6dJpPatBT9eUwM5VCw9Bt6CdX9Tk6UWvhW3MebLDRKE=
github.com/shogo82148/androidbinary v1.0.5 h1:7afvcNw+vT84R0ugrL/u/DIrGYylC66yNvt0Y0j7rrM=
github.com/shogo82148/androidbinary v1.0.5/go.mod h1:FzpR5bLAXR3VsAUG4BRCFaUm0WV6YD4Ldu+m05tr9Vk=
github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ=
github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ=
github.com/songgao/water v0.0.0-20200317203138-2b4b6d7c09d8 h1:TG/diQgUe0pntT/2D9tmUCz4VNwm9MfrtPr0SU2qSX8=
Expand Down
135 changes: 135 additions & 0 deletions utils/appmeta.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
package utils

import (
"archive/zip"
"fmt"
"io"
"os"
"path/filepath"
"strconv"
"strings"

"github.com/shogo82148/androidbinary/apk"
"howett.net/plist"
)

// AppMetadata holds identity and version information parsed from an app file
// (.apk, .ipa, .zip, or .app). Field names are unified across platforms:
// PackageName is the Android package / iOS CFBundleIdentifier, Version is the
// Android versionName / iOS CFBundleShortVersionString (marketing version), and
// VersionCode is the Android versionCode / iOS CFBundleVersion (build number).
type AppMetadata struct {
PackageName string `json:"packageName"`
Version string `json:"version,omitempty"`
VersionCode string `json:"versionCode,omitempty"`
}

// ParseAppMetadata reads identity and version metadata from an app file,
// dispatching by extension. It parses the file locally and does not touch any
// device.
func ParseAppMetadata(path string) (*AppMetadata, error) {
lower := strings.ToLower(path)
switch {
case strings.HasSuffix(lower, ".apk"):
return parseApkMetadata(path)
case strings.HasSuffix(lower, ".ipa"), strings.HasSuffix(lower, ".zip"):
return parseIpaMetadata(path)
case strings.HasSuffix(lower, ".app"):
return parseAppDirMetadata(path)
default:
return nil, fmt.Errorf("unsupported app file type: %s", path)
}
}

func parseApkMetadata(path string) (*AppMetadata, error) {
pkg, err := apk.OpenFile(path)
if err != nil {
return nil, fmt.Errorf("failed to open apk: %w", err)
}
defer func() { _ = pkg.Close() }()

meta := &AppMetadata{
PackageName: pkg.PackageName(),
}

manifest := pkg.Manifest()
if versionName, err := manifest.VersionName.String(); err == nil {
meta.Version = versionName
}
if versionCode, err := manifest.VersionCode.Int32(); err == nil {
meta.VersionCode = strconv.FormatInt(int64(versionCode), 10)
}

return meta, nil
}
Comment on lines +44 to +64

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify androidbinary API compatibility

# Check the API exported by the library at v1.0.5
echo "=== Fetching androidbinary v1.0.5 API ==="
rg -A 3 'func.*PackageName|func.*Manifest|func.*VersionName|func.*VersionCode' \
  --iglob '**/androidbinary/**/*.go' 2>/dev/null || \
  echo "Note: Library not in current workspace; manual verification needed"

# Search Go package documentation
echo -e "\n=== Manual verification needed ==="
echo "Please verify the following at https://pkg.go.dev/github.com/shogo82148/androidbinary@v1.0.5/apk:"
echo "  1. apk.OpenFile(path) returns *Pkg with PackageName() method"
echo "  2. Pkg.Manifest() returns manifest with VersionName and VersionCode fields"  
echo "  3. VersionName has String() method, VersionCode has Int32() method"

Repository: mobile-next/mobilecli

Length of output: 517


Validate non-empty APK package name

parseApkMetadata always returns success and may set AppMetadata.PackageName to "" when pkg.PackageName() is empty; treat that as an error so callers don’t receive degenerate metadata.

🛡️ Suggested validation
 	meta := &AppMetadata{
 		PackageName: pkg.PackageName(),
 	}
+	if meta.PackageName == "" {
+		return nil, fmt.Errorf("APK manifest missing package name")
+	}
 
 	manifest := pkg.Manifest()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func parseApkMetadata(path string) (*AppMetadata, error) {
pkg, err := apk.OpenFile(path)
if err != nil {
return nil, fmt.Errorf("failed to open apk: %w", err)
}
defer func() { _ = pkg.Close() }()
meta := &AppMetadata{
PackageName: pkg.PackageName(),
}
manifest := pkg.Manifest()
if versionName, err := manifest.VersionName.String(); err == nil {
meta.Version = versionName
}
if versionCode, err := manifest.VersionCode.Int32(); err == nil {
meta.VersionCode = strconv.FormatInt(int64(versionCode), 10)
}
return meta, nil
}
func parseApkMetadata(path string) (*AppMetadata, error) {
pkg, err := apk.OpenFile(path)
if err != nil {
return nil, fmt.Errorf("failed to open apk: %w", err)
}
defer func() { _ = pkg.Close() }()
meta := &AppMetadata{
PackageName: pkg.PackageName(),
}
if meta.PackageName == "" {
return nil, fmt.Errorf("APK manifest missing package name")
}
manifest := pkg.Manifest()
if versionName, err := manifest.VersionName.String(); err == nil {
meta.Version = versionName
}
if versionCode, err := manifest.VersionCode.Int32(); err == nil {
meta.VersionCode = strconv.FormatInt(int64(versionCode), 10)
}
return meta, nil
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@utils/appmeta.go` around lines 44 - 64, parseApkMetadata currently returns
success even when pkg.PackageName() is empty, producing degenerate AppMetadata;
update parseApkMetadata to validate that pkg.PackageName() is non-empty after
opening the APK and return a descriptive error (e.g., fmt.Errorf("apk has empty
package name")) if it is empty (ensure pkg is closed via the existing defer), so
AppMetadata.PackageName is never "" on success; keep the existing population of
Version/VersionCode logic unchanged.


// parseIpaMetadata reads the top-level app's Info.plist from an .ipa or .zip
// archive. It works for both .ipa (Payload/Foo.app/Info.plist) and simulator
// .zip (Foo.app/Info.plist) layouts.
func parseIpaMetadata(path string) (*AppMetadata, error) {
reader, err := zip.OpenReader(path)
if err != nil {
return nil, fmt.Errorf("failed to open archive: %w", err)
}
defer func() { _ = reader.Close() }()

for _, file := range reader.File {
if !isAppInfoPlist(file.Name) {
continue
}

rc, err := file.Open()
if err != nil {
return nil, fmt.Errorf("failed to open Info.plist: %w", err)
}

data, err := io.ReadAll(rc)
_ = rc.Close()
if err != nil {
return nil, fmt.Errorf("failed to read Info.plist: %w", err)
}

return decodeInfoPlist(data)
}

return nil, fmt.Errorf("no app Info.plist found in %s", path)
}

func parseAppDirMetadata(path string) (*AppMetadata, error) {
data, err := os.ReadFile(filepath.Join(path, "Info.plist"))
if err != nil {
return nil, fmt.Errorf("failed to read Info.plist: %w", err)
}

return decodeInfoPlist(data)
}

// isAppInfoPlist reports whether a zip entry name is the top-level app bundle's
// Info.plist, e.g. "Payload/Foo.app/Info.plist" or "Foo.app/Info.plist". A
// nested bundle's plist (frameworks, plugins) has more path segments and is
// excluded.
func isAppInfoPlist(name string) bool {
name = strings.TrimPrefix(name, "Payload/")
parts := strings.Split(name, "/")
return len(parts) == 2 && strings.HasSuffix(parts[0], ".app") && parts[1] == "Info.plist"
}

func decodeInfoPlist(data []byte) (*AppMetadata, error) {
// infoPlist mirrors the keys we extract from an iOS Info.plist.
type infoPlist struct {
CFBundleIdentifier string `plist:"CFBundleIdentifier"`
CFBundleShortVersionString string `plist:"CFBundleShortVersionString"`
CFBundleVersion string `plist:"CFBundleVersion"`
}

var info infoPlist
if _, err := plist.Unmarshal(data, &info); err != nil {
return nil, fmt.Errorf("failed to parse Info.plist: %w", err)
}

return &AppMetadata{
PackageName: info.CFBundleIdentifier,
Version: info.CFBundleShortVersionString,
VersionCode: info.CFBundleVersion,
}, nil
}
117 changes: 117 additions & 0 deletions utils/appmeta_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
package utils

import (
"archive/zip"
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// sampleInfoPlist is a minimal XML Info.plist; howett.net/plist parses both XML
// and binary plists, so XML keeps the test fixtures readable.
const sampleInfoPlist = `<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>CFBundleIdentifier</key>
<string>com.mobilenext.playground</string>
<key>CFBundleShortVersionString</key>
<string>1.4.0</string>
<key>CFBundleVersion</key>
<string>42</string>
</dict>
</plist>`

// writeZip writes a zip file with the given entries to a temp file and returns its path.
func writeZip(t *testing.T, entries map[string]string) string {
t.Helper()
path := filepath.Join(t.TempDir(), "app.zip")

f, err := os.Create(path)
require.NoError(t, err)

w := zip.NewWriter(f)
for name, content := range entries {
entry, err := w.Create(name)
require.NoError(t, err)
_, err = entry.Write([]byte(content))
require.NoError(t, err)
}
require.NoError(t, w.Close())
require.NoError(t, f.Close())

return path
}

func TestParseAppMetadataFromIpaLayout(t *testing.T) {
ipa := writeZip(t, map[string]string{
"Payload/Playground.app/Info.plist": sampleInfoPlist,
})
// rename to .ipa so extension dispatch picks the iOS path
ipaPath := ipa + ".ipa"
require.NoError(t, os.Rename(ipa, ipaPath))

meta, err := ParseAppMetadata(ipaPath)
require.NoError(t, err)
assert.Equal(t, "com.mobilenext.playground", meta.PackageName)
assert.Equal(t, "1.4.0", meta.Version)
assert.Equal(t, "42", meta.VersionCode)
}

func TestParseAppMetadataFromSimulatorZipLayout(t *testing.T) {
// simulator .zip has the .app at the archive root, not under Payload/
zipPath := writeZip(t, map[string]string{
"Playground.app/Info.plist": sampleInfoPlist,
})

meta, err := ParseAppMetadata(zipPath)
require.NoError(t, err)
assert.Equal(t, "com.mobilenext.playground", meta.PackageName)
assert.Equal(t, "1.4.0", meta.Version)
assert.Equal(t, "42", meta.VersionCode)
}

func TestParseAppMetadataIgnoresNestedBundlePlists(t *testing.T) {
// a framework's Info.plist must not shadow the top-level app's
ipa := writeZip(t, map[string]string{
"Payload/Playground.app/Frameworks/Other.framework/Info.plist": `<plist><dict><key>CFBundleIdentifier</key><string>com.other.framework</string></dict></plist>`,
"Payload/Playground.app/Info.plist": sampleInfoPlist,
})
ipaPath := ipa + ".ipa"
require.NoError(t, os.Rename(ipa, ipaPath))

meta, err := ParseAppMetadata(ipaPath)
require.NoError(t, err)
assert.Equal(t, "com.mobilenext.playground", meta.PackageName)
}

func TestParseAppMetadataFromAppDirectory(t *testing.T) {
appDir := filepath.Join(t.TempDir(), "Playground.app")
require.NoError(t, os.Mkdir(appDir, 0o750))
require.NoError(t, os.WriteFile(filepath.Join(appDir, "Info.plist"), []byte(sampleInfoPlist), 0o600))

meta, err := ParseAppMetadata(appDir)
require.NoError(t, err)
assert.Equal(t, "com.mobilenext.playground", meta.PackageName)
assert.Equal(t, "1.4.0", meta.Version)
assert.Equal(t, "42", meta.VersionCode)
}

func TestParseAppMetadataFromApk(t *testing.T) {
// sample.apk is a stripped-down fixture: just the binary AndroidManifest.xml
// plus an empty resources.arsc (the parser requires the latter to exist).
// package/version are literal manifest attributes, so no resource table is needed.
meta, err := ParseAppMetadata("testdata/sample.apk")
require.NoError(t, err)
assert.Equal(t, "com.example.helloworld", meta.PackageName)
assert.Equal(t, "1.0", meta.Version)
assert.Equal(t, "1", meta.VersionCode)
}

func TestParseAppMetadataRejectsUnknownExtension(t *testing.T) {
_, err := ParseAppMetadata("/tmp/whatever.txt")
assert.Error(t, err)
}
Comment on lines +114 to +117

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use a cross-platform path for the test.

The hardcoded /tmp/whatever.txt path is Unix-specific and will fail on Windows where /tmp/ doesn't exist in the same form. Since ParseAppMetadata dispatches by extension before attempting to open the file, use a simple relative path instead.

🔧 Proposed fix
 func TestParseAppMetadataRejectsUnknownExtension(t *testing.T) {
-	_, err := ParseAppMetadata("/tmp/whatever.txt")
+	_, err := ParseAppMetadata("whatever.txt")
 	assert.Error(t, err)
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func TestParseAppMetadataRejectsUnknownExtension(t *testing.T) {
_, err := ParseAppMetadata("/tmp/whatever.txt")
assert.Error(t, err)
}
func TestParseAppMetadataRejectsUnknownExtension(t *testing.T) {
_, err := ParseAppMetadata("whatever.txt")
assert.Error(t, err)
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@utils/appmeta_test.go` around lines 111 - 114, The test
TestParseAppMetadataRejectsUnknownExtension uses a Unix-only path
"/tmp/whatever.txt"; change the argument to ParseAppMetadata to a simple
relative filename (e.g., "whatever.txt" or "whatever.unknown") so the test
dispatches by extension without relying on an OS-specific temp directory; update
the call in TestParseAppMetadataRejectsUnknownExtension accordingly to use the
relative path.

Binary file added utils/testdata/sample.apk
Binary file not shown.
Loading