v0.2.0: インターフェース簡素化と aws login 対応#1
Conversation
- config.yml を廃止 - Secret ID を直接引数として受け取るシンプルな設計 - configure/add/list コマンドを廃止 - デフォルト値: profile=default, region=ap-northeast-1
BREAKING CHANGES: - Secret ID を直接引数として指定する方式に変更 - config.yml を廃止(事前設定が不要に) - configure/add/list コマンドを削除 Changes: - kagi import <secret-id> の新しいインターフェース - kagi download <secret-id> の新しいインターフェース - デフォルト値: --profile=default, --region=ap-northeast-1 - Config モジュールを削除 - README.md を全面的に書き換え - CHANGELOG.md を追加 - v0.1.x からの移行ガイドを追加
- AWS_ACCESS_KEY_ID と AWS_SECRET_ACCESS_KEY が設定されている場合は環境変数を優先 - AWS_SESSION_TOKEN にも対応 - 一時的な認証情報(Session Token付き)が使えるようになった
- import/download コマンドに --debug オプションを追加 - デバッグログの出力を制御可能に - デフォルトではデバッグログは表示されない
- AWS_PROFILE 環境変数を使用して AWS SDK のデフォルト認証情報チェーンに任せる - Aws::SharedCredentials を使わず credentials: nil を渡す - aws login, aws sso login, ~/.aws/credentials の全てに対応
- ENV['AWS_PROFILE'] ではなく Client.new(profile: profile) を使用 - これで aws login の認証情報が正しく読み込まれる
- Aws::SharedCredentials が aws login に非対応だった問題 - profile オプションを使った解決策 - AWS SDK の認証情報チェーンの仕組み - 対応状況の比較表
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if has_env_creds | ||
| $stderr.puts "DEBUG: 環境変数から認証情報を使用します" if debug | ||
| # 環境変数から明示的に認証情報を作成(Session Token にも対応) | ||
| credentials = Aws::Credentials.new( | ||
| ENV['AWS_ACCESS_KEY_ID'], |
There was a problem hiding this comment.
Respect --profile when env creds are set
The new client creation path always branches on has_env_creds and builds credentials from AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY, never reaching the profile-based branch when those variables are present. In practice, if a shell or CI exports these env vars and the user runs kagi import ... --profile other, the explicit profile is silently ignored and the command still uses the env-backed account, which regresses the previous behavior where providing --profile forced that profile’s credentials. Users now cannot switch accounts without manually unsetting env vars.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR represents a major version bump (v0.1.0 → v0.2.0) that significantly simplifies the Kagi CLI interface by removing the config.yml dependency and adding support for modern AWS authentication methods, particularly the aws login command introduced in 2024.
Key Changes:
- Removed config.yml-based configuration in favor of direct Secret ID specification
- Added support for multiple AWS authentication methods including
aws login,aws sso login, environment variables, and IAM roles - Introduced
--debugoption for troubleshooting authentication issues
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/kagi/version.rb | Version bumped from 0.1.0 to 0.2.0 |
| lib/kagi.rb | Removed ConfigError class as config.yml is no longer used |
| lib/kagi/config.rb | Deleted entire config module (breaking change) |
| lib/kagi/secrets.rb | Enhanced authentication to support aws login via AWS SDK's default credential chain; added debug parameter |
| lib/kagi/cli.rb | Simplified command interface to accept Secret ID as direct argument; removed configure/add/list commands |
| spec/kagi/config_spec.rb | Deleted config tests as config module was removed |
| README.md | Complete rewrite of usage documentation to reflect new simplified interface |
| CHANGELOG.md | New file documenting all changes between v0.1.0 and v0.2.0 |
| docs/simplify-interface-requirements.md | New requirements document explaining the interface simplification |
| docs/aws-login-support.md | New technical documentation explaining aws login implementation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - `lib/kagi/config.rb` モジュールを削除 | ||
| - `ConfigError` クラスを削除 | ||
|
|
||
| ### Added |
There was a problem hiding this comment.
The CHANGELOG is missing key features mentioned in the PR description. The following additions should be documented in the "### Added" section:
aws loginsupport (2024年の新しいブラウザベース認証に対応)- Environment variable authentication priority (環境変数の認証情報を優先的に使用)
--debugoption (デバッグログを表示するオプションを追加)- Support for multiple authentication methods (aws login, aws sso login, IAM Role, etc.)
These are significant new features that users would want to know about when reading the changelog.
| ### Added | |
| ### Added | |
| - `aws login` サポート(2024年の新しいブラウザベース認証に対応) | |
| - 環境変数の認証情報を優先的に使用 | |
| - `--debug` オプション(デバッグログを表示するオプションを追加) | |
| - 複数の認証方式に対応(aws login, aws sso login, IAM Role など) |
| rescue JSON::ParserError => e | ||
| # デバッグ用に実際のシークレット内容を表示(先頭100文字のみ) | ||
| preview = resp.secret_string[0..100] | ||
| raise SecretsError, "シークレットの JSON パースに失敗しました。\n" \ | ||
| "シークレットは JSON 形式である必要があります。\n" \ | ||
| "実際の内容(先頭100文字): #{preview.inspect}\n" \ | ||
| "パースエラー: #{e.message}" |
There was a problem hiding this comment.
The resp variable is not accessible in the JSON::ParserError rescue block. If JSON parsing fails, resp is defined in the main body but may not be in scope when the exception is caught. This will cause an undefined local variable or method 'resp' error.
To fix this, you should define resp outside the begin block or move the error handling:
def fetch(secret_id, profile:, region:, debug: false)
client = create_client(profile: profile, region: region, debug: debug)
resp = nil
begin
resp = client.get_secret_value(secret_id: secret_id)
JSON.parse(resp.secret_string)
rescue JSON::ParserError => e
preview = resp&.secret_string&.[](0..100) || "N/A"
raise SecretsError, "シークレットの JSON パースに失敗しました。\n" \
"シークレットは JSON 形式である必要があります。\n" \
"実際の内容(先頭100文字): #{preview.inspect}\n" \
"パースエラー: #{e.message}"
# ... other rescue blocks
end
end| def fetch(secret_id, profile:, region:, debug: false) | ||
| client = create_client(profile: profile, region: region, debug: debug) |
There was a problem hiding this comment.
The new debug parameter added to the fetch and create_client methods lacks test coverage. Given that this project uses comprehensive RSpec testing (as seen in spec/kagi/env_formatter_spec.rb), tests should be added to verify:
- Debug output is written to stderr when
debug: true - No debug output when
debug: false(default) - Correct debug messages for environment variable authentication
- Correct debug messages for profile-based authentication
Consider adding a test file like spec/kagi/secrets_spec.rb to cover this new functionality.
| @@ -8,8 +8,8 @@ module Secrets | |||
| module_function | |||
|
|
|||
| # AWS Secrets Manager からシークレットを取得する | |||
There was a problem hiding this comment.
The comment for the fetch method should be updated to document the new debug parameter. Currently, it only says "AWS Secrets Manager からシークレットを取得する" but doesn't mention the debug parameter that was added. Consider updating to:
# AWS Secrets Manager からシークレットを取得する
#
# @param secret_id [String] Secret ID
# @param profile [String] AWS Profile name
# @param region [String] AWS Region
# @param debug [Boolean] デバッグログを出力するかどうか (デフォルト: false)
# @return [Hash] パースされたシークレットの内容| # AWS Secrets Manager からシークレットを取得する | |
| # AWS Secrets Manager からシークレットを取得する | |
| # | |
| # @param secret_id [String] Secret ID | |
| # @param profile [String] AWS Profile name | |
| # @param region [String] AWS Region | |
| # @param debug [Boolean] デバッグログを出力するかどうか (デフォルト: false) | |
| # @return [Hash] パースされたシークレットの内容 |
| "パースエラー: #{e.message}" | ||
| end | ||
|
|
||
| # AWS クライアントを作成する |
There was a problem hiding this comment.
The comment for the create_client method should be updated to document the new debug parameter. Consider adding:
# AWS クライアントを作成する
#
# @param profile [String] AWS Profile name
# @param region [String] AWS Region
# @param debug [Boolean] デバッグログを出力するかどうか (デフォルト: false)
# @return [Aws::SecretsManager::Client] AWS Secrets Manager クライアント| # AWS クライアントを作成する | |
| # AWS クライアントを作成する | |
| # | |
| # @param profile [String] AWS Profile name | |
| # @param region [String] AWS Region | |
| # @param debug [Boolean] デバッグログを出力するかどうか (デフォルト: false) | |
| # @return [Aws::SecretsManager::Client] AWS Secrets Manager クライアント |
| def create_client(profile:, region:, debug: false) | ||
| # デバッグ: 環境変数の状態を確認 | ||
| has_env_creds = ENV['AWS_ACCESS_KEY_ID'] && ENV['AWS_SECRET_ACCESS_KEY'] | ||
|
|
||
| # 認証方法を判定 | ||
| if has_env_creds | ||
| $stderr.puts "DEBUG: 環境変数から認証情報を使用します" if debug | ||
| # 環境変数から明示的に認証情報を作成(Session Token にも対応) | ||
| credentials = Aws::Credentials.new( | ||
| ENV['AWS_ACCESS_KEY_ID'], | ||
| ENV['AWS_SECRET_ACCESS_KEY'], | ||
| ENV['AWS_SESSION_TOKEN'] # Session Token がない場合は nil になる | ||
| ) | ||
|
|
||
| Aws::SecretsManager::Client.new( | ||
| region: region, | ||
| credentials: credentials | ||
| ) |
There was a problem hiding this comment.
[nitpick] The authentication precedence could be confusing. When environment variables are set, the profile parameter is ignored, but this behavior isn't clearly documented or communicated to users.
Consider one of these approaches:
- Add a warning message when
profileis specified but environment variables take precedence:$stderr.puts "WARNING: --profile オプションが指定されていますが、環境変数 (AWS_ACCESS_KEY_ID) が優先されます" if debug && profile != "default" - Add documentation to the README and method docstrings explaining this precedence
- Consider making the behavior more explicit by checking if both are present and warning the user
The current implementation follows AWS SDK conventions, but explicit communication about this behavior would improve the user experience.
| - 🔐 **AWS Secrets Manager と統合** - 秘匿情報の一元管理 | ||
| - 🚀 **シンプルなコマンド** - `kagi download` で `.env` を即座に生成 | ||
| - 🎯 **柔軟な設定** - プロジェクト/環境ごとに AWS Profile/Region を管理 | ||
| - 🚀 **シンプルなコマンド** - Secret ID を直接指定するだけ |
There was a problem hiding this comment.
The README's "特徴" (Features) section should mention the aws login support, which is one of the main features highlighted in the PR description. The PR description states this version adds support for the new browser-based authentication method introduced in 2024, but this isn't mentioned in the README.
Consider adding a feature bullet point like:
- 🔐 **AWS Secrets Manager と統合** - 秘匿情報の一元管理
- 🚀 **シンプルなコマンド** - Secret ID を直接指定するだけ
- 🌐 **複数の認証方式に対応** - aws login, aws sso login, 環境変数, IAM Role など
- 💎 **Ruby 製** - AWS SDK を直接使用 (AWS CLI 不要)Or add a dedicated section explaining the authentication methods supported.
| - 🚀 **シンプルなコマンド** - Secret ID を直接指定するだけ | |
| - 🚀 **シンプルなコマンド** - Secret ID を直接指定するだけ | |
| - 🌐 **複数の認証方式に対応** - aws login, aws sso login, 環境変数, IAM Role など |
| ### `kagi import <secret-id>` | ||
|
|
||
| ### `kagi add <project> <env>` | ||
| 環境変数を export する形式で出力します。 | ||
|
|
||
| プロジェクト/環境を設定に追加します。対話式またはオプション指定で追加できます。 | ||
| **引数:** | ||
| - `<secret-id>` - AWS Secrets Manager の Secret ID(必須) | ||
|
|
||
| **オプション:** | ||
| - `--secret-id SECRET_ID` - Secret ID を指定 | ||
| - `--profile PROFILE` - AWS Profile を指定 | ||
| - `--region REGION` - AWS Region を指定 | ||
| - `--profile PROFILE` - AWS Profile を指定(デフォルト: `default`) | ||
| - `--region REGION` - AWS Region を指定(デフォルト: `ap-northeast-1`) | ||
|
|
There was a problem hiding this comment.
The --debug option is mentioned in the PR description as a new feature but is not documented in the README command reference sections. Users won't know about this debugging capability unless they use --help.
Consider adding the --debug option to the command reference for both import and download commands:
**オプション:**
- `--profile PROFILE` - AWS Profile を指定(デフォルト: `default`)
- `--region REGION` - AWS Region を指定(デフォルト: `ap-northeast-1`)
- `--debug` - デバッグログを表示|
|
||
| ### Fixed | ||
| - JSON パースエラー時のエラーメッセージを改善 | ||
| - シークレットの実際の内容(先頭100文字)を表示 |
There was a problem hiding this comment.
JSON パースエラー時にシークレットの実際の内容(先頭100文字)をエラーメッセージとして表示する変更は、シークレット値そのものをログや標準エラー出力に漏らす危険があります。Secrets Manager のシークレットには API キーやパスワード等が含まれるため、ログを閲覧できるユーザーや侵害者がこれらの値を直接取得できてしまいます。GetSecretValue 失敗時には secret_id やエラー内容のみを出力し、シークレット文字列本体は一切出力しないように実装を変更してください(どうしてもプレビューが必要な場合も、本番では無効化される明示的なデバッグフラグの下で、値をマスクした形でのみ表示すべきです)。
| - シークレットの実際の内容(先頭100文字)を表示 | |
| - シークレット値は表示せず、secret_id とエラー内容のみを出力 |
| "secretsmanager:GetSecretValue" | ||
| ], | ||
| "Resource": "arn:aws:secretsmanager:*:*:secret:kagi/*" | ||
| "Resource": "arn:aws:secretsmanager:*:*:secret:*" |
There was a problem hiding this comment.
IAM ポリシーの Resource を arn:aws:secretsmanager:*:*:secret:* にしていると、このロールを付与したエンティティからアカウント内のあらゆる Secrets Manager シークレットを読み取れるようになり、最小権限の原則に反します。Kagi からアクセスさせたいシークレットの名前空間にプレフィックスを付けるなどしてリソースを絞り込み、例としても arn:aws:secretsmanager:*:*:secret:your-prefix/* のように特定アプリ用シークレットのみにアクセスを許可する形に変更してください。
| "Resource": "arn:aws:secretsmanager:*:*:secret:*" | |
| "Resource": "arn:aws:secretsmanager:*:*:secret:your-prefix/*" // ← 必要なシークレットのプレフィックスに変更してください |
概要
config.yml を廃止し、Secret ID を直接引数として受け取るシンプルな設計に変更しました。
また、2024年に追加された
aws loginコマンドの認証情報にも対応しました。主な変更
破壊的変更 (BREAKING CHANGES)
kagi import <secret-id>/kagi download <secret-id>configure,add,list新機能
AWS_ACCESS_KEY_IDなどの環境変数を優先的に使用対応する認証方式
AWS_ACCESS_KEY_IDなど)aws login(新しいブラウザベース認証)aws sso login(SSO 認証)~/.aws/credentials(従来のプロファイル)使用例
Before (v0.1.x)
# 事前設定が必要 kagi add compal dev --secret-id compal/dev kagi import compal devAfter (v0.2.0)
ドキュメント
テスト
全 8 テストが成功しています。
バージョン
0.1.0 → 0.2.0