feat: allow overriding the latex-environment source org/repo (#32)#33
Conversation
aldc hardcoded smkwlab/latex-environment as the devcontainer source, so other orgs had to edit the script to distribute their own fork. Make the source configurable while preserving the smkwlab default. - REPOSITORY_OWNER / REPOSITORY_NAME now read ALDC_REPOSITORY_OWNER / ALDC_REPOSITORY_NAME (defaults smkwlab / latex-environment). - Validate both against the GitHub login / repository character set, since the repo name is also used as the extracted directory in cd and rm -rf. - Document the variables in --help and the Development guide. The release branch name stays fixed by convention.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| ;; | ||
| esac | ||
| case "$REPOSITORY_NAME" in | ||
| ''|*[!A-Za-z0-9._-]*) |
There was a problem hiding this comment.
REPOSITORY_NAME のバリデーションでドット（.）を許可していますが、.. のようなパス traversal に相当する名前も通過してしまいます。GitHubのリポジトリ名はドットを含むことができますが、.. はリポジトリ名として無効です。
以下のように追加チェックを入れることを推奨します：
case "$REPOSITORY_NAME" in
''|*[!A-Za-z0-9._-]*|..|.)
echo "Error: invalid ALDC_REPOSITORY_NAME: ${REPOSITORY_NAME}" >&2
exit 1
;;
esacこれにより rm -rf での事故リスクをより確実に防げます。
There was a problem hiding this comment.
acf0495 で対応しました。REPOSITORY_NAME の検証に .・*..*（.. を含む任意の名前）の拒否を追加しました。ご提案の ..|. より広く a..b 等の連続ドットも弾きます（GitHub のリポジトリ名としても無効なため）。..・.・a..b・..evil・foo/../bar の拒否をテスト済みです。なお現状も NAME は常に ${NAME}-release と suffix され / も拒否のため traversal には至りませんが、将来のリファクタに備えた堅牢化として採用しました。
| # extracted directory in `cd` and `rm -rf` below, so validate the character set | ||
| # (GitHub login / repository name) to avoid accidents from a mistyped value. | ||
| case "$REPOSITORY_OWNER" in | ||
| ''|*[!A-Za-z0-9-]*) |
There was a problem hiding this comment.
ℹ️ [LOW] GitHubのオーナー名（ユーザー名・組織名）は、実際には先頭や末尾にハイフンを置けず、連続ハイフン（--）も不可という制約があります。現在の正規表現 *[!A-Za-z0-9-]* はこれらを許容してしまいます。
セキュリティ上の問題（path traversal防止）という観点では現状でも十分ですが、誤設定防止という観点では不完全です。少なくともコメントに「GitHub の全制約は検証していない」旨を記載するか、より厳密なチェックを追加することを検討してください。
例えば先頭・末尾のハイフンチェック：
case "$REPOSITORY_OWNER" in
''|*[!A-Za-z0-9-]*|-*|*-)
echo "Error: invalid ALDC_REPOSITORY_OWNER: ${REPOSITORY_OWNER}" >&2
exit 1
;;
esacThere was a problem hiding this comment.
owner 名は URL（引用済み curl）にのみ使われ、展開ディレクトリ（cd / rm -rf）には REPOSITORY_NAME しか使いません。したがって owner の先頭/連続ハイフン等は URL が 404 になるだけでローカル事故には至らないため、文字種検証で十分と判断します（GitHub の命名細則は clone/download 時のエラーに委ねます）。現状維持とします。
| echo "Environment Variables:" | ||
| echo " ALDC_QUIET=1 Enable quiet mode" | ||
| echo " ALDC_QUIET=1 Enable quiet mode" | ||
| echo " ALDC_REPOSITORY_OWNER Source org/user (default: smkwlab)" |
There was a problem hiding this comment.
ℹ️ [LOW] ヘルプメッセージの列幅が揃っていません。ALDC_REPOSITORY_OWNER と ALDC_REPOSITORY_NAME の説明列の開始位置が ALDC_QUIET=1 と異なります（ALDC_REPOSITORY_OWNER は末尾スペースが1つ少ない）。
ALDC_QUIET=1 Enable quiet mode
ALDC_REPOSITORY_OWNER Source org/user (default: smkwlab)
ALDC_REPOSITORY_NAME Source repo (default: latex-environment)
ALDC_REPOSITORY_OWNER の後のスペースが4つ、ALDC_REPOSITORY_NAME の後のスペースが5つになっており、揃っていません。統一することを推奨します。
AI review feedback on PR #33: - REPOSITORY_NAME validation allowed dots, so "." and ".." (and any ".." sequence) passed. Reject them explicitly to harden the cd / rm -rf paths. - Align the Environment Variables help columns.
There was a problem hiding this comment.
Pull request overview
This PR removes the last functional hardcoding of smkwlab/latex-environment by allowing the source latex-environment repository (org/user and repo name) to be overridden via environment variables, while keeping the default behavior unchanged for existing users.
Changes:
- Added
ALDC_REPOSITORY_OWNER/ALDC_REPOSITORY_NAMEenvironment-variable overrides for the source repo inaldc. - Added validation for the override values before using them to form URLs / derived paths.
- Updated developer documentation and
--helpoutput to document the new environment variables.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
aldc |
Adds env-var overrides and validation; updates --help to document new variables. |
docs/CLAUDE-DEVELOPMENT.md |
Documents the new override variables and their validation behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # owner/repo are embedded in the download URL and, more importantly, used as the | ||
| # extracted directory in `cd` and `rm -rf` below, so validate the character set | ||
| # (GitHub login / repository name) to avoid accidents from a mistyped value. |
There was a problem hiding this comment.
5ae356a で修正しました。ご指摘のとおり展開ディレクトリ（cd / rm -rf）に使われるのは REPOSITORY_NAME のみで、REPOSITORY_OWNER は URL のみです。コメントと dev-doc をその旨に正確化しました。
| case "$REPOSITORY_NAME" in | ||
| ''|*[!A-Za-z0-9._-]*) | ||
| echo "Error: invalid ALDC_REPOSITORY_NAME: ${REPOSITORY_NAME}" >&2 | ||
| exit 1 | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
5ae356a で対応しました。REPOSITORY_OWNER/REPOSITORY_NAME の検証に先頭 - の拒否（-*）を追加しました。ご指摘のとおり cd/rm は引用しても先頭 - をオプションとして解釈するため（実際に rm -rf "-rf-release" は rm: invalid option で失敗）、実バグとして採用しました。
| Both are validated against the GitHub login / repository character set before | ||
| being used in the download URL and the extracted directory (`cd` / `rm -rf`). | ||
| The release branch name is fixed to `release` by convention. |
There was a problem hiding this comment.
5ae356a で修正しました。ご指摘のとおり展開ディレクトリ（cd / rm -rf）に使われるのは REPOSITORY_NAME のみで、REPOSITORY_OWNER は URL のみです。コメントと dev-doc をその旨に正確化しました。
| ;; | ||
| esac | ||
| case "$REPOSITORY_NAME" in | ||
| ''|.|*..*|*[!A-Za-z0-9._-]*) |
There was a problem hiding this comment.
🚨 [HIGH] [HIGH] REPOSITORY_NAME のバリデーションで . 単体を弾いていない
現在のパターン ''|.|*..*|*[!A-Za-z0-9._-]* は . 単体を弾こうとしていますが、case のパターンマッチにおいて |. の . は「任意の1文字」ではなくリテラルのドットとして扱われます。そのため . 単体は正しく拒否されます。
しかし .. 単体（2文字）は *..* にマッチしません（*..* は「.. を含む3文字以上」にしかマッチしない）。cd や rm -rf に .. が渡されると path traversal が発生します。
# 修正案
case "$REPOSITORY_NAME" in
''|'.'|'..'|*[!A-Za-z0-9._-]*|*..*)
echo "Error: invalid ALDC_REPOSITORY_NAME: ${REPOSITORY_NAME}" >&2
exit 1
;;
esacまたは、より明示的に:
case "$REPOSITORY_NAME" in
''|'.'|'..')
echo "Error: invalid ALDC_REPOSITORY_NAME: ${REPOSITORY_NAME}" >&2; exit 1 ;;
*[!A-Za-z0-9._-]*|*..*)
echo "Error: invalid ALDC_REPOSITORY_NAME: ${REPOSITORY_NAME}" >&2; exit 1 ;;
esac.. を明示的にパターンに追加することで、path traversal の防止を確実にしてください。
There was a problem hiding this comment.
こちらは誤検知と考えます。shell の case/glob では * は空文字列にもマッチするため、*..* は ..（2文字）にもマッチします。実測でも現行パターン ''|.|*..*|*[!A-Za-z0-9._-]* は .. を REJECT します（.・a..b・..evil も同様）。したがって .. は既に正しく拒否されており、path traversal は発生しません。加えて 5ae356a で先頭 - も拒否したため、この検証はより堅牢になっています。コード変更は不要と判断しました。
| # owner/repo are embedded in the download URL and, more importantly, used as the | ||
| # extracted directory in `cd` and `rm -rf` below, so validate the character set | ||
| # (GitHub login / repository name) to avoid accidents from a mistyped value. |
There was a problem hiding this comment.
5ae356a で修正しました。ご指摘のとおり展開ディレクトリ（cd / rm -rf）に使われるのは REPOSITORY_NAME のみで、REPOSITORY_OWNER は URL のみです。コメントと dev-doc をその旨に正確化しました。
| case "$REPOSITORY_NAME" in | ||
| ''|.|*..*|*[!A-Za-z0-9._-]*) | ||
| echo "Error: invalid ALDC_REPOSITORY_NAME: ${REPOSITORY_NAME}" >&2 | ||
| exit 1 | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
5ae356a で対応しました。REPOSITORY_OWNER/REPOSITORY_NAME の検証に先頭 - の拒否（-*）を追加しました。ご指摘のとおり cd/rm は引用しても先頭 - をオプションとして解釈するため（実際に rm -rf "-rf-release" は rm: invalid option で失敗）、実バグとして採用しました。
| Both are validated against the GitHub login / repository character set before | ||
| being used in the download URL and the extracted directory (`cd` / `rm -rf`). |
There was a problem hiding this comment.
5ae356a で修正しました。ご指摘のとおり展開ディレクトリ（cd / rm -rf）に使われるのは REPOSITORY_NAME のみで、REPOSITORY_OWNER は URL のみです。コメントと dev-doc をその旨に正確化しました。
…32) Second round of AI review feedback on PR #33: - A leading dash in REPOSITORY_OWNER/REPOSITORY_NAME survived validation and, via cd/rm on "${REPOSITORY_NAME}-${BRANCH}", is parsed as an option even when quoted (rm errors with "invalid option"). Reject leading "-" in both. - Correct the code comment and dev-doc: only REPOSITORY_NAME is used as the extracted directory (cd / rm -rf); REPOSITORY_OWNER appears only in the URL.
| ;; | ||
| esac | ||
| case "$REPOSITORY_NAME" in | ||
| ''|.|-*|*..*|*[!A-Za-z0-9._-]*) |
There was a problem hiding this comment.
REPOSITORY_NAME のドット単体（.）が許可されている
case パターン ''|.|-*|*..*|*[!A-Za-z0-9._-]*) は .. を含む文字列を拒否しますが、単一ドット . は拒否していません。
''→ 空文字列を拒否 ✓.→ 許可されてしまう ✗..→*..*にマッチするため拒否 ✓
cd "${REPOSITORY_NAME}-${BRANCH}" や rm -rf "${REPOSITORY_NAME}-${BRANCH}" で使われるため、ALDC_REPOSITORY_NAME=. を設定すると cd .-.release のような無害なパスになりますが、意図しない動作を招く可能性があります。また将来的な利用方法の変化を考えると、単一ドットも拒否しておくほうが安全です。
case "$REPOSITORY_NAME" in
''|.|..-*|*..*|-*|*[!A-Za-z0-9._-]*)あるいはシンプルに
''|.*|-*|*..*|*[!A-Za-z0-9._-]*)とすることで、ドット始まりの名前（.hidden 等）も含めて拒否できます（GitHub のリポジトリ名はドット始まりを許可していますが、aldc の用途では不要と思われます）。
| # would be parsed as an option by cd/rm even when quoted), and — for the name — | ||
| # no dot-only / `..` traversal value. | ||
| case "$REPOSITORY_OWNER" in | ||
| ''|-*|*[!A-Za-z0-9-]*) |
There was a problem hiding this comment.
ℹ️ [LOW] [LOW] REPOSITORY_OWNER のパターンに *- （末尾ダッシュ）の拒否が含まれていない
GitHub のユーザー名・Organization 名は末尾にハイフンを置けない仕様です（[A-Za-z0-9](-?[A-Za-z0-9])* 相当）。現在のパターン ''|-*|*[!A-Za-z0-9-]*) では foo- や foo--bar も通過します。
セキュリティ上の直接的なリスクは低いですが、誤設定を早期に検出するという観点では、末尾ダッシュや連続ダッシュも拒否するとより堅牢になります。
case "$REPOSITORY_OWNER" in
''|-*|*-|*--*|*[!A-Za-z0-9-]*)| The source is overridable for other-org deployments (defaults preserve | ||
| smkwlab behavior): | ||
|
|
||
| | Variable | Default | Purpose | | ||
| |----------|---------|---------| | ||
| | `ALDC_REPOSITORY_OWNER` | `smkwlab` | Org/user hosting the latex-environment fork. | | ||
| | `ALDC_REPOSITORY_NAME` | `latex-environment` | Repository name to download. | | ||
|
|
||
| `ALDC_REPOSITORY_OWNER` is used only in the download URL; `ALDC_REPOSITORY_NAME` | ||
| is additionally used as the extracted directory in `cd` / `rm -rf`. Both are | ||
| validated (GitHub character set, no leading dash, and — for the name — no `.` | ||
| or `..` traversal) before use. The release branch name is fixed to `release` | ||
| by convention. |
There was a problem hiding this comment.
06751a1 で対応しました。Security Considerations の「Downloads from official smkwlab repository only」を、既定は smkwlab/latex-environment だが ALDC_REPOSITORY_OWNER/NAME で override 可能（文字検証あり）である旨に更新しました。実装との不整合を解消しています。
Third round of AI review feedback on PR #33: - Reject only the exact traversal names "." and ".." rather than any value containing ".." (*..*). Since "/" is already rejected, an embedded ".." such as "foo..bar" cannot form a path component, so blocking it was over-strict. - Update the Security Considerations note: the source is no longer smkwlab-only but overridable via ALDC_REPOSITORY_OWNER/NAME (character-validated).
| ;; | ||
| esac | ||
| case "$REPOSITORY_NAME" in | ||
| ''|.|..|-*|*[!A-Za-z0-9._-]*) |
There was a problem hiding this comment.
*[!A-Za-z0-9._-]* は、ドットを含む名前（例: my.repo）を許可しますが、.. のみを個別に弾いているだけで、... や .hidden のような名前は通過します。
より重要な問題として、cd や rm -rf で使われるのは ${REPOSITORY_NAME}-${BRANCH} という形式（ZIP展開後のディレクトリ名）であり、REPOSITORY_NAME 単体ではありません。GitHub の ZIP アーカイブは {repo}-{branch}/ という形式で展開されるため、実際に使われるパスは ${REPOSITORY_NAME}-${BRANCH} です。スクリプト内でこのディレクトリ名を変数として定義し、それを検証・使用する方が安全で明確です。
EXTRACTED_DIR="${REPOSITORY_NAME}-${BRANCH}"
# cd "${EXTRACTED_DIR}" / rm -rf "${EXTRACTED_DIR}" として使用There was a problem hiding this comment.
実害はないと考えます。展開ディレクトリは常に ${REPOSITORY_NAME}-${BRANCH}（= ...-release 等）と suffix されるため、cd/rm に渡る値が exact ./.. になることはなく path traversal は発生しません。また ...・.hidden のようなドット始まり/ドットのみ相当の名前は GitHub のリポジトリ名として無効で、ダウンロード URL が 404 になります。現行の検証（文字種＋先頭ハイフン＋ exact ./.. 拒否）は proportionate と判断し、現状維持とします。
| # would be parsed as an option by cd/rm even when quoted), and — for the name — | ||
| # no dot-only / `..` traversal value. | ||
| case "$REPOSITORY_OWNER" in | ||
| ''|-*|*[!A-Za-z0-9-]*) |
There was a problem hiding this comment.
REPOSITORY_OWNER の検証パターン *[!A-Za-z0-9-]* はアンダースコア _ を許可していません。GitHub の Organization 名や User 名にはアンダースコアが使用可能です（例: my_org）。意図的な制限であればコメントに明記すべきですが、もし GitHub の実際の命名規則に合わせるなら *[!A-Za-z0-9_-]* とすべきです。
There was a problem hiding this comment.
こちらは誤検知と考えます。GitHub のユーザー名・組織名は「英数字と単一ハイフンのみ」で、アンダースコア _ は使用できません（例 my_org は無効。ハイフンも先頭・末尾・連続は不可）。したがって owner を [A-Za-z0-9-] に制限するのは GitHub の実際の命名規則に沿った正しい制限です。コード変更は不要と判断します。
AI レビュー対応まとめ（収束）4 巡のレビューに対応し、実体的指摘はすべて解消しました。CI（review）は全 commit で green です。 採用・修正した指摘:
据え置いた指摘（根拠付きで返信済み）:
検証: 既定値は smkwlab/latex-environment のままで挙動不変です。レビュー対応は収束とし、merge 可能な状態です。 |
概要
aldc 唯一の機能的な org 依存点（
smkwlab/latex-environment固定）を解消し、他 org が自前の latex-environment fork を配布できるようにする。既定は smkwlab/latex-environment のままで挙動不変。Resolves #32
変更内容
REPOSITORY_OWNER="${ALDC_REPOSITORY_OWNER:-smkwlab}"/REPOSITORY_NAME="${ALDC_REPOSITORY_NAME:-latex-environment}"で env 上書き可能にcd（101行）とrm -rf（134行）にも使われるため、GitHub 命名文字種で検証を追加（誤設定・path traversal 事故の防止）--helpの Environment Variables とdocs/CLAUDE-DEVELOPMENT.mdに新変数を追記release固定のまま検証
bash -n: OKaldc --help: 新変数が表示されるsmkwlab/latex-environment、owner のみ / owner+name 上書きが正しく URL に反映../../etc・a/b・空・x;rm等の owner/repo を拒否（rm -rf事故防止を確認）関連: latex-ecosystem #105