[] Add getAppMeta/getAppLicenses to lib/api.ts, create app/about/page.tsx and app/licenses/page.tsx (with tests)#298
Conversation
…page.tsx and app/licenses/page.tsx (with tests)
![orange-codens[bot]](https://avatars.githubusercontent.com/in/4028936?s=60&v=4){kind=small}
There was a problem hiding this comment.
🍊 Orange Codens レビュー
全体的な実装方針は PRD の要件を満たしており、about/page.tsx での cleanup フラグや getAppLicenses のフォールバック設計など良い配慮が見られる。ただし getAppMeta / getAppLicenses 両関数で HTTP 4xx/5xx エラーが握り潰される（response.ok チェック欠落）点と、licenses/page.tsx でアンマウント後の state 更新を防ぐ cleanup がない点が high 相当の問題として存在する。また UI テキストの日本語/英語混在（medium/low）やエラーケースのテスト不足も改善が必要。
🔒 セキュリティ: 本 PR で追加された getAppMeta / getAppLicenses は baseURL を検証なく fetch に渡しており、サーバーサイドで利用された際に SSRF となりうる設計上のリスクがあります。また、ライセンスページでは API レスポンス由来の entry.url を無検証で href にセットしており、バックエンド侵害時に javascript: URI 等を通じた XSS / フィッシングが成立します。加えて HTTP エラーレスポンスを ok チェックなしに JSON パースするため、エラー詳細が意図せずクライアントへ露出する可能性があります。
🔴 critical: 1 / 🟠 high: 4 / 🟡 medium: 4 / 🔵 low: 2
その他の指摘 (6 件)
- 🟡 [medium]
lib/api.ts:260— HTTP エラーレスポンスを検証せずに JSON パースしており、エラー内容がクライアントに漏洩する恐れがある (confidence: 0.90,sec.sast.validation.missing_http_error_check) - 🟡 [medium]
app/about/page.tsx:34— ロード失敗時にも version が 'dev' を表示し、エラーが区別できない (confidence: 0.85,code.logic.loading_state_incorrect) - 🟡 [medium]
__tests__/app/about.test.tsx:1— about.test.tsx に API エラーケースのテストがない (confidence: 0.85,maintainability.test.error_case_missing) - 🟡 [medium]
app/licenses/page.tsx:39— loading=false 後の licenses?.third_party に不要な optional chaining がある (confidence: 0.78,code.logic.optional_chaining_unnecessary) - 🔵 [low]
app/licenses/page.tsx:40— UI テキストの言語が混在している (confidence: 0.95,maintainability.i18n.mixed_language) - 🔵 [low]
app/licenses/page.tsx:53— table row の key に name+version を使うと重複時に一意性が保証されない (confidence: 0.70,maintainability.naming.inconsistent_key_strategy)
head: e02290d | Orange Codens (P1)
| return fetchWithToken<ApiUserSummary>(token, "/api/v3/user"); | ||
| } | ||
|
|
||
| export async function getAppMeta(baseURL: string): Promise<AppMeta> { |
There was a problem hiding this comment.
🔴 [critical] SSRF: 呼び出し元が任意の baseURL を渡せる getAppMeta / getAppLicenses
CWE-918: Server-Side Request Forgery (SSRF)
該当箇所:
export async function getAppMeta(baseURL: string): Promise<AppMeta> {
const response = await fetch(`${baseURL}/api/meta`);export async function getAppLicenses(baseURL: string): Promise<AppLicenses> {
try {
const response = await fetch(`${baseURL}/api/licenses`);baseURL は呼び出し元から完全に自由な文字列として渡されます。現状の呼び出しは process.env.NEXT_PUBLIC_API_BASE_URL ?? "" (クライアントサイド) ですが、この関数がサーバーサイドで利用される場合（例: Server Actions・Route Handler・SSR）、攻撃者がリクエストを細工して http://169.254.169.254/ 等の内部エンドポイントや任意の外部ホストへリクエストを誘導できます。また、file:// / gopher:// 等のスキームも許容されます。
推奨対応:
baseURLをホワイトリスト検証する、またはサーバー側では環境変数から直接取得して引数経由で外部から注入できない構造にする。- 少なくともスキームを
https:/http:に限定し、ホスト部を許可リストと照合する。
```ts
const ALLOWED_ORIGINS = new Set([
process.env.NEXT_PUBLIC_API_BASE_URL ?? "",
].filter(Boolean));
function assertAllowedBaseURL(baseURL: string): void {
const { origin } = new URL(baseURL);
if (!ALLOWED_ORIGINS.has(origin)) {
throw new Error(`Disallowed baseURL: ${baseURL}`);
}
}
export async function getAppMeta(baseURL: string): Promise<AppMeta> {
assertAllowedBaseURL(baseURL);
const response = await fetch(`${baseURL}/api/meta`);
return response.json() as Promise<AppMeta>;
}
<sub>`sec.sast.ssrf.user_url` | confidence: 0.75</sub>
| } | ||
|
|
||
| export async function getAppMeta(baseURL: string): Promise<AppMeta> { | ||
| const response = await fetch(`${baseURL}/api/meta`); |
There was a problem hiding this comment.
🟠 [high] getAppMeta が HTTP エラーレスポンスを無視する
該当箇所:
export async function getAppMeta(baseURL: string): Promise<AppMeta> {
const response = await fetch(`${baseURL}/api/meta`);
return response.json() as Promise<AppMeta>;
}fetch はネットワーク到達可能ならば 4xx/5xx でも例外を投げない。response.ok チェックがないため、サーバーが 500 を返したときに JSON パースがエラー JSON を AppMeta として返すか、非 JSON ボディで SyntaxError が発生し、呼び出し元 (about/page.tsx) でハンドリングされないままレンダリングが壊れる。getAppLicenses は try/catch でフォールバックを返しているのに対し、getAppMeta には同様の保護がない。
最低限 if (!response.ok) throw new Error(...) を追加するか、about/page.tsx 側で catch してエラー表示すべき。
@@ -259,4 +259,6 @@
export async function getAppMeta(baseURL: string): Promise<AppMeta> {
const response = await fetch(`${baseURL}/api/meta`);
+ if (!response.ok) {
+ throw new Error(`Failed to fetch app meta: ${response.status}`);
+ }
return response.json() as Promise<AppMeta>;
}code.error_handling.http_error_ignored | confidence: 0.92
| return (await response.json()) as AppLicenses; | ||
| } catch { | ||
| return { app_license: "", third_party: [] }; | ||
| } |
There was a problem hiding this comment.
🟠 [high] getAppLicenses が HTTP エラーレスポンスを握り潰す
該当箇所:
export async function getAppLicenses(baseURL: string): Promise<AppLicenses> {
try {
const response = await fetch(`${baseURL}/api/licenses`);
return (await response.json()) as AppLicenses;
} catch {
return { app_license: "", third_party: [] };
}
}fetch 自体は HTTP 4xx/5xx で例外を投げない。サーバーが 500 を返した場合、response.json() は（JSON ボディがあれば）成功してしまい、フォールバックに入らない。response.ok チェックを追加することで、HTTP エラー時にも意図したフォールバックが返るようにすべき。
@@ -265,5 +265,8 @@
export async function getAppLicenses(baseURL: string): Promise<AppLicenses> {
try {
const response = await fetch(`${baseURL}/api/licenses`);
+ if (!response.ok) {
+ return { app_license: "", third_party: [] };
+ }
return (await response.json()) as AppLicenses;
} catch {
return { app_license: "", third_party: [] };
}
}code.error_handling.http_error_ignored | confidence: 0.92
| getAppLicenses(process.env.NEXT_PUBLIC_API_BASE_URL ?? "") | ||
| .then(setLicenses) | ||
| .finally(() => setLoading(false)); | ||
| }, []); |
There was a problem hiding this comment.
🟠 [high] licenses/page.tsx がアンマウント後に state を更新する可能性がある
該当箇所:
useEffect(() => {
getAppLicenses(process.env.NEXT_PUBLIC_API_BASE_URL ?? "")
.then(setLicenses)
.finally(() => setLoading(false));
}, []);about/page.tsx では cancelled フラグで cleanup しているのに、licenses/page.tsx には cleanup 関数が存在しない。ページから素早く離脱した場合、Promise が解決されたタイミングでアンマウント済みのコンポーネントに setLicenses / setLoading が呼ばれ、React の警告 (Can't perform a React state update on an unmounted component) が発生する（React 18 strict mode では開発時に表示される）。about/page.tsx と同様に cancelled フラグを導入すべき。
@@ -12,7 +12,14 @@
useEffect(() => {
- getAppLicenses(process.env.NEXT_PUBLIC_API_BASE_URL ?? "")
- .then(setLicenses)
- .finally(() => setLoading(false));
- }, []);
+ let cancelled = false;
+ getAppLicenses(process.env.NEXT_PUBLIC_API_BASE_URL ?? "")
+ .then((data) => { if (!cancelled) setLicenses(data); })
+ .finally(() => { if (!cancelled) setLoading(false); });
+ return () => { cancelled = true; };
+ }, []);code.null_safety.unmounted_state_update | confidence: 0.88
| className="text-primary underline" | ||
| > | ||
| {entry.url} | ||
| </a> |
There was a problem hiding this comment.
🟠 [high] Open Redirect / 任意 URL インジェクション: APIレスポンスの entry.url を href に直接使用
CWE-601: URL Redirection to Untrusted Site ('Open Redirect') / CWE-20: Improper Input Validation
該当箇所:
<a
href={entry.url}
target="_blank"
rel="noopener noreferrer"
className="text-primary underline"
>
{entry.url}
</a>entry.url はバックエンドの /api/licenses から返されるサードパーティライセンス情報のフィールドです。このレスポンスが攻撃者によって改ざんされた場合（例: 中間者攻撃、バックエンド侵害）、javascript:alert(1) や data:text/html,... など任意のスキームを含む URL がそのまま href にセットされ、XSS やフィッシングに利用されます。rel="noopener noreferrer" はタブナビゲーションの制御には有効ですが、javascript: URI の実行は防げません。
推奨対応:
function isSafeUrl(url: string): boolean {
try {
const { protocol } = new URL(url);
return protocol === "https:" || protocol === "http:";
} catch {
return false;
}
}レンダリング前に isSafeUrl(entry.url) を検証し、不正な URL はリンクとしてレンダリングしない。
```tsx
{isSafeUrl(entry.url) ? (
<a href={entry.url} target="_blank" rel="noopener noreferrer" className="text-primary underline">
{entry.url}
</a>
) : (
<span>{entry.url}</span>
)}
<sub>`sec.sast.injection.open_redirect` | confidence: 0.85</sub>
…o /api/meta, keep main's workflow-job-logs test
There was a problem hiding this comment.
🍊 Orange Codens レビュー
PRD 要件の骨格（API 型・ヘルパー・About/Licenses ページ・基本テスト）は diff 上そろっていますが、getAppMeta/getAppLicenses の HTTP エラー処理が不十分で、About ページでは reject 時の unhandled rejection、Licenses ページでは third_party 欠落時の TypeError とアンマウント後 setState のリスクがあります。テストは About の重複と API ヘルパー層の未カバーが目立ちます。
🔒 セキュリティ: 本 PR は公開向け about/licenses ページと API ヘルパーの追加であり、認可漏れ・SSRF・インジェクション系の直接 exploitable な問題は diff 上は見当たりません。一方で API 応答の URL を検証せず href にバインドしており、バックエンド改ざん時に DOM ベース XSS へ発展しうるデータフローが新設されています。http/https スキーム限定の検証を href 設定前および API レイヤー双方に追加することを推奨します。
🟡 medium: 8 / 🔵 low: 3
その他の指摘 (11 件)
- 🟡 [medium]
lib/api.ts:658— getAppMeta が HTTP エラーレスポンスを検証していない (confidence: 0.92,code.error_handling.missing_response_ok) - 🟡 [medium]
app/licenses/page.tsx:35— third_party 欠落時に TypeError でクラッシュする (confidence: 0.90,code.null_safety.optional_chaining) - 🟡 [medium]
app/about/page.tsx:17— getAppMeta 失敗時に unhandled rejection が発生する (confidence: 0.88,code.error_handling.unhandled_rejection) - 🟡 [medium]
app/licenses/page.tsx:12— Licenses ページにアンマウント時の setState ガードがない (confidence: 0.86,code.react.unmount_setstate) - 🟡 [medium]
lib/api.ts:663— getAppLicenses の degraded mode が HTTP エラーをカバーしない (confidence: 0.82,code.error_handling.http_not_degraded) - 🟡 [medium]
lib/api.ts:658— getAppMeta/getAppLicenses の API ヘルパー単体テストがない (confidence: 0.78,maintainability.test.missing_api_tests) - 🟡 [medium]
app/about/page.tsx:38— API 由来の source_url を href に未検証で設定 (confidence: 0.72,sec.sast.injection.xss) - 🟡 [medium]
app/licenses/page.tsx:56— サードパーティ URL を href に未検証で設定 (confidence: 0.70,sec.sast.injection.xss) - 🔵 [low]
__tests__/pages/about.test.tsx:1— About ページのテストが2ファイルに重複している (confidence: 0.95,maintainability.test.duplicate) - 🔵 [low]
__tests__/app/licenses.test.tsx:39— Licenses テストが app_license 表示を検証していない (confidence: 0.85,maintainability.test.incomplete_coverage) - 🔵 [low]
lib/api.ts:658— getAppMeta/getAppLicenses が API 応答を検証せず返却 (confidence: 0.65,sec.sast.input_validation.missing)
head: d80e728 | Orange Codens (P1)
1 participant
Summary
Type of Change
Testing
Checklist
Task Goal
Target Repository
Corevice/open-gitAll work for this task belongs in this repository. Do the implementation inside this repository's directory in the workspace; do not search or edit other repositories unless this task explicitly requires a cross-repo change.
Context
The /about and /licenses pages are public-facing pages required by the PRD to display product metadata and third-party OSS license information. They call the /api/meta and /api/licenses backend endpoints (implemented in the backend task). This sub-task adds API helper functions and creates both page components with tests.
Steps
open-git/lib/api-types.ts— add two new interfaces:AppMeta { app_name: string; version: string; git_commit: string; build_date: string; license: string; source_url: string; }andLicenseEntry { name: string; version: string; license: string; url: string; }andAppLicenses { app_license: string; third_party: LicenseEntry[]; }open-git/lib/api.ts— add two exported functions:export async function getAppMeta(baseURL: string): Promise<AppMeta>(fetch${baseURL}/api/meta, return json) andexport async function getAppLicenses(baseURL: string): Promise<AppLicenses>(fetch${baseURL}/api/licenses, return json); on fetch error,getAppLicensesshould catch and return{ app_license: "", third_party: [] }matching the PRD degraded-mode requirementopen-git/app/about/page.tsx—"use client"component; useuseEffect+useStateto callgetAppMeta(process.env.NEXT_PUBLIC_API_BASE_URL ?? "")on mount; display:BRANDING.appName, version from the API response (or"dev"while loading), license name with a link to the /licenses page, and a link back toBRANDING.sourceUrl; show a loading state while fetchingopen-git/app/licenses/page.tsx—"use client"component; callgetAppLicenseson mount; renderapp_licenseas the project's own license, then render a table/list ofthird_partyentries each showing nChanges Made
I'll
implement
the
about
/licenses
pages
and
API
helpers
.
First
I'm
reading
the
existing
files
and
patterns
in
the
repo
.
Implement
ing
the
six
files
now
.
Implemented
the
/ aboutand
/ lic ensespages
with
API
helpers
and
tests
.
Changes
lib/api-types.ts— AddedAppMeta,LicenseEntry, andAppLicensesinterfaces.lib/api.ts— Added:
getAppMeta(baseURL)— fetches/api/metaand returns JSONgetAppLicenses(baseURL)— fetches/api/licenses;on error returns
{ app_license: "", third_party: [] }app/about/page.tsx— Client component that loads meta on mount, showsBRANDING.appName, version ("dev"while loading), license link to/licenses, and source URL link.app/licenses/page.tsx— Clientcomponent that loads licenses, shows project license and a third-party table, or the fallback message when
third_partyisempty.
Tests —
about.test.tsxcovers loading state, app name, version, and/licenseslink;licenses.test.tsxcovers rendered entries and empty fallbacktext.
[PURPLE-MODIFIED-FILES]
open-git/lib/api-types.ts
open-git/lib/api.ts
open-git/app/about/page.tsx
open-git/app/licenses/page
.tsx
open
-git
/__
tests
__/
app
/about
.test
.tsx
open
-git
/__
tests
__/
app
/licenses
.test
.tsx
[/
P
UR
P
LE
-M
OD
IFIED
-F
ILES
]
Task ID:
019f0d7d-8aeb-74b9-8674-a65ffb0f5541Generated by Purple Codens