chore: fix review comments

This commit is contained in:
Simon Chan 2025-09-16 20:40:21 +08:00
parent 538d743c7b
commit c280a447e4
No known key found for this signature in database
GPG key ID: A8B69F750B9BCEDD
5 changed files with 103 additions and 58 deletions

View file

@ -22,7 +22,42 @@ import {
} from "@yume-chan/adb"; } from "@yume-chan/adb";
import type { TangoKey, TangoKeyStorage } from "@yume-chan/adb-credential-web"; import type { TangoKey, TangoKeyStorage } from "@yume-chan/adb-credential-web";
class KeyError extends Error {
path: string;
constructor(message: string, path: string, options?: ErrorOptions) {
super(message, options);
this.path = path;
}
}
/**
* Can't read or parse a private key file.
*
* Check `path` for file path, and `cause` for the error.
*/
class InvalidKeyError extends KeyError {
constructor(path: string, options?: ErrorOptions) {
super(`Can't read private key file at "${path}"`, path, options);
}
}
/**
* Can't read or parse a vendor key.
*
* Check `path` for file path, and `cause` for the error.
*/
class VendorKeyError extends KeyError {
constructor(path: string, options?: ErrorOptions) {
super(`Can't read vendor key file at "${path}"`, path, options);
}
}
export class TangoNodeStorage implements TangoKeyStorage { export class TangoNodeStorage implements TangoKeyStorage {
static readonly KeyError = KeyError;
static readonly InvalidKeyError = InvalidKeyError;
static readonly VendorKeyError = VendorKeyError;
async #getAndroidDirPath() { async #getAndroidDirPath() {
const dir = resolve(homedir(), ".android"); const dir = resolve(homedir(), ".android");
await mkdir(dir, { mode: 0o750, recursive: true }); await mkdir(dir, { mode: 0o750, recursive: true });
@ -74,7 +109,7 @@ export class TangoNodeStorage implements TangoKeyStorage {
.replaceAll(/\x20|\t|\r|\n|\v|\f/g, ""), .replaceAll(/\x20|\t|\r|\n|\v|\f/g, ""),
); );
} catch (e) { } catch (e) {
throw new Error("Invalid private key file: " + path, { cause: e }); throw new InvalidKeyError(path, { cause: e });
} }
} }
@ -106,23 +141,32 @@ export class TangoNodeStorage implements TangoKeyStorage {
async *#readVendorKeys( async *#readVendorKeys(
path: string, path: string,
): AsyncGenerator<MaybeError<TangoKey>, void, void> { ): AsyncGenerator<MaybeError<TangoKey>, void, void> {
const stats = await stat(path); let stats;
try {
stats = await stat(path);
} catch (e) {
return yield new VendorKeyError(path, { cause: e });
}
if (stats.isFile()) { if (stats.isFile()) {
try { try {
yield await this.#readKey(path); return yield await this.#readKey(path);
} catch (e) { } catch (e) {
if (e instanceof Error) { return yield e instanceof KeyError
yield e; ? e
} else { : new VendorKeyError(path, { cause: e });
yield new Error(String(e));
} }
} }
return;
}
if (stats.isDirectory()) { if (stats.isDirectory()) {
for await (const dirent of await opendir(path)) { let dir;
try {
dir = await opendir(path);
} catch (e) {
return yield new VendorKeyError(path, { cause: e });
}
for await (const dirent of dir) {
if (!dirent.isFile()) { if (!dirent.isFile()) {
continue; continue;
} }
@ -131,14 +175,13 @@ export class TangoNodeStorage implements TangoKeyStorage {
continue; continue;
} }
const file = resolve(path, dirent.name);
try { try {
yield await this.#readKey(resolve(path, dirent.name)); yield await this.#readKey(file);
} catch (e) { } catch (e) {
if (e instanceof Error) { yield e instanceof KeyError
yield e; ? e
} else { : new VendorKeyError(path, { cause: e });
yield new Error(String(e));
}
} }
} }
} }
@ -150,11 +193,9 @@ export class TangoNodeStorage implements TangoKeyStorage {
try { try {
yield await this.#readKey(userKeyPath); yield await this.#readKey(userKeyPath);
} catch (e) { } catch (e) {
if (e instanceof Error) { yield e instanceof KeyError
yield e; ? e
} else { : new InvalidKeyError(userKeyPath, { cause: e });
yield new Error(String(e));
}
} }
} }
@ -168,6 +209,12 @@ export class TangoNodeStorage implements TangoKeyStorage {
} }
} }
export namespace TangoNodeStorage {
export type KeyError = typeof KeyError;
export type InvalidKeyError = typeof InvalidKeyError;
export type VendorKeyError = typeof VendorKeyError;
}
// Re-export everything except Web-only storages // Re-export everything except Web-only storages
export { export {
AdbWebCryptoCredentialStore, AdbWebCryptoCredentialStore,

View file

@ -133,28 +133,27 @@ export class TangoPasswordProtectedStorage implements TangoKeyStorage {
bundle.encrypted as Uint8Array<ArrayBuffer>, bundle.encrypted as Uint8Array<ArrayBuffer>,
); );
try {
yield { yield {
privateKey: new Uint8Array(decrypted), privateKey: new Uint8Array(decrypted),
name, name,
}; };
} finally {
// Clear secret memory // Clear secret memory
// * No way to clear `password` and `aesKey` // * No way to clear `password` and `aesKey`
// * all values in `bundle` are not secrets // * all values in `bundle` are not secrets
// * Caller is not allowed to use `decrypted` after `yield` returns // * Caller is not allowed to use `decrypted` after `yield` returns
new Uint8Array(decrypted).fill(0); new Uint8Array(decrypted).fill(0);
}
} catch (e) { } catch (e) {
if (e instanceof DOMException && e.name === "OperationError") { if (e instanceof DOMException && e.name === "OperationError") {
yield new PasswordIncorrectError(); yield new PasswordIncorrectError();
continue; continue;
} }
if (e instanceof Error) { yield e instanceof Error
yield e; ? e
continue; : new Error(String(e), { cause: e });
}
yield new Error(String(e));
} }
} }
} }

View file

@ -187,11 +187,9 @@ export class TangoPrfStorage implements TangoKeyStorage {
new Uint8Array(decrypted).fill(0); new Uint8Array(decrypted).fill(0);
} }
} catch (e) { } catch (e) {
if (e instanceof Error) { yield e instanceof Error
yield e; ? e
} else { : new Error(String(e), { cause: e });
yield new Error(String(e));
}
} }
} }
} }

View file

@ -66,11 +66,17 @@ export class AdbWebCryptoCredentialStore implements AdbCredentialStore {
continue; continue;
} }
try {
// `privateKey` is owned by `#storage` and will be cleared by it // `privateKey` is owned by `#storage` and will be cleared by it
yield { yield {
...rsaParsePrivateKey(result.privateKey), ...rsaParsePrivateKey(result.privateKey),
name: result.name ?? this.#name, name: result.name ?? this.#name,
}; };
} catch (e) {
yield e instanceof Error
? e
: new Error(String(e), { cause: e });
}
} }
} }
} }

View file

@ -77,7 +77,7 @@ export class AdbDefaultAuthenticator implements AdbAuthenticator {
| AsyncIterator<MaybeError<AdbPrivateKey>, void, void> | AsyncIterator<MaybeError<AdbPrivateKey>, void, void>
| undefined; | undefined;
#prevFingerprint: string | undefined; #prevKeyInfo: AdbKeyInfo | undefined;
#firstKey: AdbPrivateKey | undefined; #firstKey: AdbPrivateKey | undefined;
#onKeyLoadError = new EventEmitter<Error>(); #onKeyLoadError = new EventEmitter<Error>();
@ -130,19 +130,14 @@ export class AdbDefaultAuthenticator implements AdbAuthenticator {
this.#firstKey = result; this.#firstKey = result;
} }
if (this.#prevFingerprint) { // A new token implies the previous signature was rejected.
this.#onSignatureRejected.fire({ if (this.#prevKeyInfo) {
fingerprint: this.#prevFingerprint, this.#onSignatureRejected.fire(this.#prevKeyInfo);
name: result.name,
});
} }
const fingerprint = getFingerprint(result); const fingerprint = getFingerprint(result);
this.#prevFingerprint = fingerprint; this.#prevKeyInfo = { fingerprint, name: result.name };
this.#onSignatureAuthentication.fire({ this.#onSignatureAuthentication.fire(this.#prevKeyInfo);
fingerprint,
name: result.name,
});
return { return {
command: AdbCommand.Auth, command: AdbCommand.Auth,