Browse Source

refactor(news): drop env-based config; hardcode feed URL

Make news delivery work out of the box without any infrastructure-side
env injection. Two changes that only make sense together:

- Hardcode the feed URL as `FEED_URL` in `news-cron-service.ts`
  pointing at the vendor-managed GitHub Pages endpoint. Removes the
  `process.env.NEWS_FEED_URL` read, the empty/unset skip path, and the
  `isAllowedUrl()` scheme whitelist (the URL is fixed in code, so there
  is nothing untrusted to validate).
- Drop `envVarName: 'NEWS_DELIVERY_ENABLED'` from
  `news:isDeliveryEnabled`. The flag is now DB-only: configManager
  precedence reduces to `DB > defaultValue: true`. This removes any
  reason for cloud infra to inject env vars per tenant — admin UI is
  the sole control surface.

Tests updated: drop the six URL-related cases (`NEWS_FEED_URL is not
set`, empty string, non-allowed http, https/localhost/127.0.0.1
allowlist) and replace the env setup blocks throughout. Add one
positive test asserting the cron fetches from the hardcoded vendor
URL.

Spec updates:
- requirements.md Req 1.1, Req 9 (Note + 1, 2, 7) reframed for
  hardcoded URL and DB-only flag.
- design.md NewsCronService section, flow diagram, security and
  testing-strategy bullets updated to match.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Ryotaro Nagahara 2 weeks ago
parent
commit
df383c1a44

+ 12 - 13
.kiro/specs/news-inappnotification/design.md

@@ -12,7 +12,7 @@
 
 ### Goals
 
-- 外部フィード(`NEWS_FEED_URL`)を cron で定期取得し、MongoDB にキャッシュする
+- 外部フィード(コードにハードコードされた配信元 URL)を cron で定期取得し、MongoDB にキャッシュする
 - InAppNotification パネルで通知とニュースを統合表示する
 - ニュースの既読/未読状態をユーザー単位で管理する
 - ロール別表示制御(admin/general)をサーバーサイドで強制する
@@ -75,7 +75,7 @@ graph TB
 | Layer | 選択 / バージョン | 役割 |
 |---|---|---|
 | Backend Cron | node-cron(既存) | フィード定期取得スケジューリング |
-| Backend HTTP | node `fetch` / axios(既存) | `NEWS_FEED_URL` から feed.json 取得 |
+| Backend HTTP | node `fetch` / axios(既存) | コードに内蔵された配信元 URL から feed.json 取得 |
 | Data Store | MongoDB + Mongoose(既存) | NewsItem, NewsReadStatus の永続化 |
 | Frontend Data | SWR `useSWRInfinite`(既存) | ニュース・通知の無限スクロール取得 |
 | Frontend State | React `useState`(既存パターン) | フィルタタブ・未読トグルのローカル state |
@@ -95,7 +95,6 @@ sequenceDiagram
 
   Cron->>Cron: getCronSchedule() = '0 0 * * *'(midnight 起動)
   Cron->>Cron: configManager.getConfig('news:isDeliveryEnabled') が false? → スキップ
-  Cron->>Cron: NEWS_FEED_URL 未設定? → スキップ
   Cron->>Cron: randomSleep(0–5 時間)でリクエスト時刻を分散
   Cron->>Feed: HTTP GET feed.json
   alt 取得失敗
@@ -191,7 +190,7 @@ sequenceDiagram
 **Responsibilities & Constraints**
 - 毎日 0 時に発火し、ランダムスリープで実取得時刻を 0–5 時に分散させる(cron 起動 `'0 0 * * *'` + `randomSleep(0–5h)`)
 - **配信フラグ判定**:cron 発火ごとに `configManager.getConfig('news:isDeliveryEnabled')` を読み、`false` ならフィード取得をスキップ(再起動不要、次回 tick から即時反映)
-- `NEWS_FEED_URL` 未設定時はスキップ(エラーなし)
+- **配信元 URL はコードにハードコード**(`https://growilabs.github.io/growi-news-feed/feed.json`)。env による上書き経路は持たず、ユーザー(admin 含む)・運用者ともに変更不可
 - 取得失敗時は既存 DB データを維持
 - `growiVersionRegExps` の照合はここで実施(DB には合致アイテムのみ保存)
 
@@ -201,13 +200,13 @@ sequenceDiagram
 **Dependencies**
 - Inbound: node-cron — スケジュール実行(P0)
 - Outbound: NewsService — upsert/delete(P0)
-- External: `NEWS_FEED_URL` の HTTP エンドポイント — feed.json 取得(P0)
+- External: 弊社管理の HTTP エンドポイント(コードに内蔵された URL) — feed.json 取得(P0)
 
 **Contracts**: Batch [x]
 
 ##### Batch / Job Contract
 - Trigger: `node-cron` スケジュール `'0 0 * * *'`(実取得は randomSleep を経て 0–5 時に分散)
-- Input: `NEWS_FEED_URL` 環境変数、GROWI バージョン文字列
+- Input: GROWI バージョン文字列(配信元 URL はコードに内蔵)
 - Output: MongoDB の NewsItem コレクションを最新フィードと同期
 - Idempotency: `externalId` ユニークインデックスにより冪等。再実行しても重複なし
 
@@ -223,7 +222,7 @@ const MAX_RANDOM_SLEEP_MS = 5 * 60 * 60 * 1000;  // 5 hours
 
 **Implementation Notes**
 - Integration: `server/service/cron.ts` の `CronService` を継承。`startCron()` をアプリ起動時に呼ぶ
-- Validation: `NEWS_FEED_URL` の URL 検証は以下のルールで行う。`https://` で始まる URL は常に許可。`http://localhost` または `http://127.0.0.1` で始まる URL はローカル開発用として許可。それ以外の `http://` は拒否する。`growiVersionRegExps` は try-catch で個別評価し、不正 regex はスキップ
+- Validation: 配信元 URL はコードにハードコードされており、ランタイムの URL 検証は不要(外部入力経路がない)。`growiVersionRegExps` は try-catch で個別評価し、不正 regex はスキップ
 - Risks: フィード取得タイムアウト(10秒推奨)。外部依存のため失敗を前提に設計する
 
 ---
@@ -404,7 +403,7 @@ GROWI の scope 階層は以下の意味論で運用する:
 
 **Responsibilities & Constraints**
 - `apps/app/src/server/service/config-manager/config-definition.ts` に CONFIG_KEYS と `defineConfig` の 2 箇所を追加
-- 既存先例 `OPENTELEMETRY_ENABLED`(`otel:enabled`)と完全に同形:`envVarName: 'NEWS_DELIVERY_ENABLED'` + `defaultValue: true`
+- `defineConfig` パターンを踏襲しつつ、**`envVarName` を意図的に持たせない**(`defaultValue: true` のみ)。これにより env からの上書きを禁じ、admin UI 経由の DB 操作のみが ON/OFF を変えられる経路となる
 - `defaultValue: true` をコードに内蔵 → DB に値が無い状態で全顧客が ON
 - 値の優先順は configManager の既存仕様(DB > env > defaultValue)に従う
 
@@ -413,8 +412,8 @@ GROWI の scope 階層は以下の意味論で運用する:
 - Outbound: configManager(既存)
 
 **Implementation Notes**
-- 通常運用では env を設定しないため `/admin` 環境変数一覧に現れない(DB 主体運用)
-- 開発時の override 用途で `.env.development.local` 等に `NEWS_DELIVERY_ENABLED` を書いた場合のみ env 値が効く
+- env 変数として一切暴露しないため `/admin` 環境変数一覧には決して現れない(DB 単独運用)
+- 開発時に強制的に値を変更したい場合は、ローカルで DB レコードを直接書き換えるか、コードを一時編集する
 - 設定変更時は configManager の `updateConfigs` がメモリキャッシュ更新と pubsub 通知(multi-pod 反映)を行う
 
 ---
@@ -701,7 +700,7 @@ interface INewsItemWithReadStatus {
 | カテゴリ | エラー | 対応 |
 |---|---|---|
 | Cron / External | フィード取得失敗(ネットワーク、タイムアウト) | `logger.error` + 既存 DB データ維持。次回 cron で再試行 |
-| Cron / Config | `NEWS_FEED_URL` 未設定 | スキップ(ログなし)。設定されるまで無害に動作 |
+| Cron / Config | `news:isDeliveryEnabled` が `false` | スキップ(debug ログ)。admin が再度 ON にするまで無害に停止 |
 | Cron / Validation | `growiVersionRegExps` に不正 regex | try-catch で該当アイテムをスキップ、`logger.warn` |
 | API / Auth | 未認証リクエスト | 401(`loginRequiredStrictly` が処理) |
 | API / Validation | 不正な `newsItemId` フォーマット | 400(`mongoose.isValidObjectId()` チェック) |
@@ -718,7 +717,7 @@ interface INewsItemWithReadStatus {
 
 ### Unit Tests
 
-- `NewsCronService.executeJob()`: 正常取得 → upsert、取得失敗 → DB 変更なし、`NEWS_FEED_URL` 未設定 → スキップ
+- `NewsCronService.executeJob()`: 正常取得 → upsert、取得失敗 → DB 変更なし、`news:isDeliveryEnabled` が `false` → スキップ
 - `NewsCronService.executeJob()`: `growiVersionRegExps` 一致 → 保存、不一致 → 除外
 - `NewsService.listForUser()`: `targetRoles` フィルタ(admin のみ、general 除外)
 - `NewsService.listForUser()`: `onlyUnread=true` で未読のみ返す
@@ -745,7 +744,7 @@ interface INewsItemWithReadStatus {
 - すべての `/apiv3/news/*` エンドポイントに `loginRequiredStrictly` を適用する
 - アクセストークン用 scope は **`features.in_app_notification`** を使用する(read / write)。設定 CRUD 用の `user_settings.in_app_notification` とはセマンティクスが異なるため流用しない。アクセストークン発行時にユーザーが意図した粒度でアクセスを許可できるようにする
 - `conditions.targetRoles` のフィルタリングはサーバーサイドの `NewsService.listForUser()` で強制する。クライアントから `targetRoles` パラメータを受け付けない
-- `NEWS_FEED_URL` は `https://` で始まる URL は常に許可。`http://localhost` または `http://127.0.0.1` で始まる URL はローカル開発用として許可。それ以外の `http://` は拒否する
+- 配信元 URL はコードにハードコードされており、ランタイムで変更できる経路を持たない。env 変数による上書きもサポートしない
 - フィードから取得したデータはそのまま DB に保存し、クライアントへのレスポンス時に Mongoose スキーマで型安全に扱う
 
 ## Performance & Scalability

+ 5 - 5
.kiro/specs/news-inappnotification/requirements.md

@@ -14,7 +14,7 @@ GROWI の InAppNotification にニュース配信・表示機能を追加する
 
 #### Acceptance Criteria
 
-1. When cron スケジュールの実行時刻に達した場合, the News Cron Service shall 設定された URL から JSON フィードを HTTP GET で取得する
+1. When cron スケジュールの実行時刻に達した場合, the News Cron Service shall コードに内蔵された配信元 URL から JSON フィードを HTTP GET で取得する
 2. When フィードの取得に成功した場合, the News Cron Service shall 取得したニュースアイテムをローカル MongoDB に upsert(`externalId` で重複排除)する
 3. When フィードに含まれなくなったニュースアイテムがある場合, the News Cron Service shall 該当アイテムをローカル DB から削除する
 4. When 複数の GROWI インスタンスが同時に取得を試みる場合, the News Cron Service shall ランダムスリープにより配信元へのリクエストを時間分散する
@@ -111,14 +111,14 @@ GROWI の InAppNotification にニュース配信・表示機能を追加する
 
 **Objective:** As a GROWI 管理者, I want ニュース配信のオンオフを管理画面から切り替えたい, so that 環境変数の編集や再起動なしにインスタンス単位で配信を停止/再開できる
 
-**Note:** 配信フラグは DB(`Config` コレクション)で管理し、admin が `/admin/app` UI から操作する。`OPENTELEMETRY_ENABLED` などの既存パターン(configManager + `defineConfig` + `envVarName` + `defaultValue`)を踏襲する。`defaultValue: true` をコードに内蔵することで、新規・既存インスタンスとも DB に値が無い状態で**デフォルト ON**が成立する。env レイヤーは開発時の override(例:`.env.development.local`)にのみ用い、本番運用での設定変更は UI 経由で行う。pod 再起動は不要。
+**Note:** 配信フラグは DB(`Config` コレクション)で管理し、admin が `/admin/app` UI から操作する。configManager + `defineConfig` + `defaultValue` の既存パターンを踏襲するが、**env からの上書き経路は意図的に持たない**(`envVarName` を設定しない)。これにより「インフラ側の env 注入」を不要にし、ニュース配信の意思は **DB のみ**で表現される。`defaultValue: true` をコードに内蔵することで、新規・既存インスタンスとも DB に値が無い状態で**デフォルト ON**が成立する。配信元 URL もコードにハードコードされており、ユーザー(admin 含む)・運用者ともに変更できない。pod 再起動は不要。
 
 #### Acceptance Criteria
 
-1. The configuration `news:isDeliveryEnabled` shall `defaultValue: true` を持ち、DB/env のいずれ値が無い場合は ON として扱われる
-2. The 設定値 shall configManager 経由で読み出され、優先順位は **DB > env > defaultValue**(既存仕様)に従う
+1. The configuration `news:isDeliveryEnabled` shall `defaultValue: true` を持ち、DB に値が無い場合は ON として扱われる
+2. The 設定値 shall configManager 経由で読み出される。env からの上書きは意図的にサポートしない(`envVarName` を設定しない)ため、優先順位は **DB > defaultValue** のみとなる
 3. When 管理者が `/admin/app` の UI からトグルを切り替えた場合, the GROWI shall `Config` コレクションの該当キーを更新し、再起動なしで設定値を反映する
 4. The 切替操作 shall admin 権限を持つユーザーのみに許可される
 5. When `news:isDeliveryEnabled` が `false` の場合, the News Cron Service shall 次回 cron 発火時にフィード取得をスキップする(既に取得済みの DB キャッシュは維持する)
 6. When `news:isDeliveryEnabled` が `true` に戻された場合, the News Cron Service shall 次回 cron 発火時に通常どおりフィード取得を再開する
-7. The 設定値 shall env 変数(`NEWS_DELIVERY_ENABLED`)が明示的に設定されていない通常運用では `/admin` トップの「サーバー側で設定されている環境変数一覧」に現れない(DB 主体運用のため、env を設定する必要がない)
+7. The 設定値 shall 環境変数として暴露されないため、`/admin` トップの「サーバー側で設定されている環境変数一覧」には決して現れない

+ 2 - 63
apps/app/src/features/news/server/services/news-cron-service.spec.ts

@@ -75,7 +75,6 @@ const mockResponse = (
 
 describe('NewsCronService', () => {
   let service: NewsCronService;
-  const originalEnv = process.env.NEWS_FEED_URL;
 
   beforeEach(() => {
     service = new NewsCronService();
@@ -84,10 +83,6 @@ describe('NewsCronService', () => {
     vi.spyOn(Math, 'random').mockReturnValue(0);
   });
 
-  afterEach(() => {
-    process.env.NEWS_FEED_URL = originalEnv;
-  });
-
   describe('getCronSchedule', () => {
     test('should return daily schedule at midnight', () => {
       expect(service.getCronSchedule()).toBe('0 0 * * *');
@@ -96,7 +91,6 @@ describe('NewsCronService', () => {
 
   describe('executeJob', () => {
     test('should skip when news:isDeliveryEnabled is false', async () => {
-      process.env.NEWS_FEED_URL = 'https://example.com/feed.json';
       mocks.getConfig.mockImplementationOnce((key: string) =>
         key === 'news:isDeliveryEnabled' ? false : undefined,
       );
@@ -109,7 +103,6 @@ describe('NewsCronService', () => {
     });
 
     test('should run when news:isDeliveryEnabled is true (default)', async () => {
-      process.env.NEWS_FEED_URL = 'https://example.com/feed.json';
       mocks.mockFetch.mockResolvedValue(
         mockResponse({ version: '1.0', items: [] }),
       );
@@ -120,63 +113,18 @@ describe('NewsCronService', () => {
       expect(mocks.mockFetch).toHaveBeenCalled();
     });
 
-    test('should skip when NEWS_FEED_URL is not set', async () => {
-      delete process.env.NEWS_FEED_URL;
-
-      await service.executeJob();
-
-      expect(mocks.mockFetch).not.toHaveBeenCalled();
-      expect(mocks.upsertNewsItems).not.toHaveBeenCalled();
-    });
-
-    test('should skip when NEWS_FEED_URL is empty string', async () => {
-      process.env.NEWS_FEED_URL = '';
-
-      await service.executeJob();
-
-      expect(mocks.mockFetch).not.toHaveBeenCalled();
-    });
-
-    test('should skip when NEWS_FEED_URL uses non-allowed http', async () => {
-      process.env.NEWS_FEED_URL = 'http://example.com/feed.json';
-
-      await service.executeJob();
-
-      expect(mocks.mockFetch).not.toHaveBeenCalled();
-    });
-
-    test('should allow https:// URLs', async () => {
-      process.env.NEWS_FEED_URL = 'https://example.com/feed.json';
+    test('should fetch from the hardcoded vendor URL', async () => {
       mocks.mockFetch.mockResolvedValue(mockResponse(VALID_FEED));
 
       await service.executeJob();
 
       expect(mocks.mockFetch).toHaveBeenCalledWith(
-        'https://example.com/feed.json',
+        'https://growilabs.github.io/growi-news-feed/feed.json',
         expect.any(Object),
       );
     });
 
-    test('should allow http://localhost URLs', async () => {
-      process.env.NEWS_FEED_URL = 'http://localhost:8099/feed.json';
-      mocks.mockFetch.mockResolvedValue(mockResponse(VALID_FEED));
-
-      await service.executeJob();
-
-      expect(mocks.mockFetch).toHaveBeenCalled();
-    });
-
-    test('should allow http://127.0.0.1 URLs', async () => {
-      process.env.NEWS_FEED_URL = 'http://127.0.0.1:8099/feed.json';
-      mocks.mockFetch.mockResolvedValue(mockResponse(VALID_FEED));
-
-      await service.executeJob();
-
-      expect(mocks.mockFetch).toHaveBeenCalled();
-    });
-
     test('should upsert items on successful fetch', async () => {
-      process.env.NEWS_FEED_URL = 'https://example.com/feed.json';
       mocks.mockFetch.mockResolvedValue(mockResponse(VALID_FEED));
 
       await service.executeJob();
@@ -189,7 +137,6 @@ describe('NewsCronService', () => {
     });
 
     test('should NOT update DB when fetch fails', async () => {
-      process.env.NEWS_FEED_URL = 'https://example.com/feed.json';
       mocks.mockFetch.mockResolvedValue({ ok: false, status: 500 });
 
       await service.executeJob();
@@ -199,7 +146,6 @@ describe('NewsCronService', () => {
     });
 
     test('should NOT update DB when fetch throws', async () => {
-      process.env.NEWS_FEED_URL = 'https://example.com/feed.json';
       mocks.mockFetch.mockRejectedValue(new Error('Network error'));
 
       await expect(service.executeJob()).resolves.not.toThrow();
@@ -208,7 +154,6 @@ describe('NewsCronService', () => {
     });
 
     test('should filter items by growiVersionRegExps', async () => {
-      process.env.NEWS_FEED_URL = 'https://example.com/feed.json';
       mocks.getGrowiVersion.mockReturnValue('7.5.0');
       const feedWithVersionFilter = {
         version: '1.0',
@@ -237,7 +182,6 @@ describe('NewsCronService', () => {
     });
 
     test('should skip items with invalid growiVersionRegExps', async () => {
-      process.env.NEWS_FEED_URL = 'https://example.com/feed.json';
       mocks.getGrowiVersion.mockReturnValue('7.5.0');
       const feedWithInvalidRegex = {
         version: '1.0',
@@ -275,7 +219,6 @@ describe('NewsCronService', () => {
     // cleaned up. The cron must now hand the full set of feed externalIds
     // to `deleteItemsNotInFeed`, which uses a $nin filter to remove the rest.
     test('should pass every feed externalId to deleteItemsNotInFeed (regression for stale-item bug)', async () => {
-      process.env.NEWS_FEED_URL = 'https://example.com/feed.json';
       const feed = {
         version: '1.0',
         items: [
@@ -315,7 +258,6 @@ describe('NewsCronService', () => {
     });
 
     test('should skip when response body exceeds size limit (5 MiB)', async () => {
-      process.env.NEWS_FEED_URL = 'https://example.com/feed.json';
       // Build a string that exceeds 5 MiB
       const oversizedText = 'x'.repeat(5 * 1024 * 1024 + 1);
       mocks.mockFetch.mockResolvedValue({
@@ -330,7 +272,6 @@ describe('NewsCronService', () => {
     });
 
     test('should abort when top-level shape is invalid', async () => {
-      process.env.NEWS_FEED_URL = 'https://example.com/feed.json';
       // Missing `items` field — top-level schema check fails
       mocks.mockFetch.mockResolvedValue(mockResponse({ version: '1.0' }));
 
@@ -341,7 +282,6 @@ describe('NewsCronService', () => {
     });
 
     test('should skip individual invalid items but keep valid ones', async () => {
-      process.env.NEWS_FEED_URL = 'https://example.com/feed.json';
       const feedWithMixedItems = {
         version: '1.0',
         items: [
@@ -366,7 +306,6 @@ describe('NewsCronService', () => {
     });
 
     test('should skip when response body is not valid JSON', async () => {
-      process.env.NEWS_FEED_URL = 'https://example.com/feed.json';
       mocks.mockFetch.mockResolvedValue({
         ok: true,
         text: () => Promise.resolve('not-a-json{'),

+ 7 - 23
apps/app/src/features/news/server/services/news-cron-service.ts

@@ -23,14 +23,12 @@ const FETCH_TIMEOUT_MS = 10_000;
 const MAX_RESPONSE_SIZE_BYTES = 5 * 1024 * 1024;
 
 /**
- * Check if the given URL is allowed for fetching
+ * Vendor-controlled news feed URL. Hardcoded so a fresh deployment delivers
+ * news without any infrastructure-side env injection. Users (incl. admins)
+ * cannot change this; opt-out is performed via the `news:isDeliveryEnabled`
+ * config flag managed in the admin UI.
  */
-const isAllowedUrl = (url: string): boolean => {
-  if (url.startsWith('https://')) return true;
-  if (url.startsWith('http://localhost')) return true;
-  if (url.startsWith('http://127.0.0.1')) return true;
-  return false;
-};
+const FEED_URL = 'https://growilabs.github.io/growi-news-feed/feed.json';
 
 /**
  * Check if the item matches the current GROWI version
@@ -68,7 +66,7 @@ export class NewsCronService extends CronService {
   }
 
   override async executeJob(): Promise<void> {
-    // Read the delivery toggle (DB > env > defaultValue: true) on every tick so
+    // Read the delivery toggle (DB > defaultValue: true) on every tick so
     // an admin's UI change takes effect from the next scheduled run, with no
     // pod restart required (Requirements 9.5, 9.6).
     if (!configManager.getConfig('news:isDeliveryEnabled')) {
@@ -76,26 +74,12 @@ export class NewsCronService extends CronService {
       return;
     }
 
-    const feedUrl = process.env.NEWS_FEED_URL;
-
-    if (!feedUrl || feedUrl.trim() === '') {
-      logger.debug('NEWS_FEED_URL is not set, skipping news feed sync');
-      return;
-    }
-
-    if (!isAllowedUrl(feedUrl)) {
-      logger.warn(
-        `NEWS_FEED_URL "${feedUrl}" is not allowed. Only https:// and http://localhost or http://127.0.0.1 are permitted.`,
-      );
-      return;
-    }
-
     // Random sleep to distribute requests across multiple GROWI instances
     await randomSleep(MAX_RANDOM_SLEEP_MS);
 
     let rawJson: unknown;
     try {
-      const response = await fetch(feedUrl, {
+      const response = await fetch(FEED_URL, {
         signal: AbortSignal.timeout(FETCH_TIMEOUT_MS),
       });
 

+ 0 - 1
apps/app/src/server/service/config-manager/config-definition.ts

@@ -1220,7 +1220,6 @@ export const CONFIG_DEFINITIONS = {
 
   // News Settings
   'news:isDeliveryEnabled': defineConfig<boolean>({
-    envVarName: 'NEWS_DELIVERY_ENABLED',
     defaultValue: true,
   }),