Skip to content

fix(main): パッケージ ID を展開先パスに使う前に検証する - #2465

Merged
hal-shu-sato merged 1 commit into
mainfrom
fix/package-id-path-escape
Aug 29, 2026
Merged

fix(main): パッケージ ID を展開先パスに使う前に検証する#2465
hal-shu-sato merged 1 commit into
mainfrom
fix/package-id-path-escape

Conversation

@hal-shu-sato

Copy link
Copy Markdown
Member

内容

installPackageArchivepackageState.id をそのまま展開先フォルダ名に使っています。

return await unzip(archivePath, packageState.id);
// 非アーカイブ拡張子の場合
const newFolder = path.join(path.dirname(archivePath), packageState.id);

unzipgetTargetPathpath.resolve で組み立てるだけで、resolveInside を通していません

同じ packageId を扱う openPackageFolder には既にガードがあります。

// packageId はリモート由来のため、データフォルダ外は拒否する
if (!isParent(dataDir, folderPath)) {  }

展開先だけがこの関門を通っていませんでした。

変更

既存の防御線に載せます。AGENTS.md が「境界で守るのはパス・コマンド・URL に到達するフィールドだけで、形の細部はサービス層(resolveInside / safeRemove / execFileSync)が二重に守る」と書いている設計に沿った形です。

場所 変更
unzip 基点を明示して resolveInside を通す
packageInstall 非アーカイブ経路も同じ関門へ
packages(tRPC 境界) idisSafeRelativePath で検証(files[].filename 等と同じ扱い)

unzip をファイル単位に作り替えず resolveInside を挟むだけにしたのは、Data 配下という基点が zipPath から決まるためです。基点の取り方(Data 配下なら 1 階層上、それ以外はその場)は変えていません。

テスト

unzip.test.ts に 2 件追加しました。

  • 展開先の外へ出る folderName を拒否して展開しない — 修正前は落ちます
  • Data フォルダ配下では 1 階層上を基点にしつつ、その外へは出さない — 実運用の配置(userData/Data/package/archive/x.zip)を模し、基点の取り方が変わっていないことも同時に確認

どちらも脱出先を後片付けの対象内に置いてあるので、テストが外部にファイルを残しません。

あわせて

これがマージされると、unzip の展開先を展開前に掃除する変更(前回のインストールの残骸が新しいインストールに混入する件)も安全に足せるようになります。いまは targetPath が関門を通っていないため、remove() を足すと「書ける」より危険な「消せる」を無検証のパスに対して行うことになるので保留していました。


調査と文面の作成に Claude Code を使用しています。

installPackageArchive は packageState.id をそのまま展開先フォルダ名に
使っている。unzip の getTargetPath は path.resolve で組み立てるだけで
resolveInside を通しておらず、非アーカイブ経路の path.join も同じ。

同じ packageId を扱う openPackageFolder には
「packageId はリモート由来のため、データフォルダ外は拒否する」という
isParent ガードが既に入っている。展開先だけがこの関門を通っていなかった。

既存の防御線に載せる。

  unzip           基点を明示して resolveInside を通す
  packageInstall  非アーカイブ経路も同じ関門へ
  packages(api)   tRPC 境界で id を isSafeRelativePath で検証する

境界での検証は files[].filename 等と同じ扱いで、AGENTS.md が
「形の細部はサービス層が二重に守る」と書いている設計に沿う。
@hal-shu-sato
hal-shu-sato merged commit 6bec306 into main Aug 29, 2026
9 checks passed
@hal-shu-sato
hal-shu-sato deleted the fix/package-id-path-escape branch August 29, 2026 18:46
pull Bot pushed a commit to ch2pw/apm that referenced this pull request Aug 29, 2026
unzip は targetPath を消さずに overwrite: 'a' で展開する。overwrite は
アーカイブに在るものを上書きするだけなので、旧バージョンにしか無かった
ファイルは残り続ける。

targetPath は同じパッケージ(Data/package/{id})・同じアーカイブ名で
再利用されるため、残骸は次のインストールに持ち込まれる。install() が
ファイル単位でコピーする通常経路は列挙されたものしか拾わないが、
isDirectory のエントリと isProgram(展開結果を丸ごとコピー)は
subtree ごと持っていくので、削除されたはずのファイルが AviUtl 側へ
再配置される。

展開前に消す。targetPath は resolveInside を通しているので、消す対象は
必ず基点の内側にある(この PR を team-apm#2465 の上に積んでいるのはそのため。
関門を通っていない状態で remove を足すと、書き込みより危険な操作を
無検証のパスに対して行うことになる)。

代償として、展開に失敗すると前回の展開物も失われる。アーカイブは
Data/{core,package}/archive に残っているのでやり直せる。
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant