Feat/news feed GitHub hf wechat#199
Conversation
davidliuk
left a comment
There was a problem hiding this comment.
Review comments from local pass.
| // UI-supplied API tokens override env vars. Stored in plain JSON config | ||
| // (same trust model as the rest of news settings), and only applied to the | ||
| // child process — never echoed back over the API. | ||
| if (sourceName === 'github' && typeof config.api_token === 'string' && config.api_token.trim()) { |
There was a problem hiding this comment.
High: this stores GitHub/HF access tokens in the regular news config file, and GET /api/news/config/:source returns that config unchanged. That means saved tokens are persisted in plaintext JSON and echoed back to any caller that can read news settings, bypassing the existing credentialsDb pattern used elsewhere for secrets. Please move these tokens into the credential store or, at minimum, redact them from config responses and avoid writing raw token values into the news config JSON.
| account = account.strip() | ||
| instance = (instance or DEFAULT_INSTANCE).rstrip("/") | ||
|
|
||
| if account.startswith(("http://", "https://")): |
There was a problem hiding this comment.
High: allowing accounts to be a full http:// or https:// URL makes this endpoint an authenticated server-side fetch primitive. A user can point the backend at localhost or private network services, and RSS/Atom-shaped responses may then be reflected into results/logs. Please restrict full URLs to the configured RSSHub host, or disallow arbitrary full URLs and validate scheme/host before fetching.
| if (sourceName === 'github' && typeof config.api_token === 'string' && config.api_token.trim()) { | ||
| env.GITHUB_TOKEN = config.api_token.trim(); | ||
| } | ||
| if (sourceName === 'huggingface' && typeof config.api_token === 'string' && config.api_token.trim()) { |
There was a problem hiding this comment.
Medium: the UI/config now accepts an HF token and passes it as HF_TOKEN, but paper mode still fetches Daily Papers with a hardcoded unauthenticated header in search_huggingface.py rather than hf_auth_headers(). As a result, the token only affects models/datasets/spaces while the default papers mode remains unauthenticated. Please route Daily Papers through the same auth header builder or make the settings copy explicit about the limited scope.
There was a problem hiding this comment.
High — secrets in plaintext config + echoed via GET
server/routes/news.js
- New
SECRET_FIELDS_BY_SOURCEregistry maps each source's secret field to a credential type:news_github_token,news_huggingface_token,news_wechat_access_key. HelpersupsertSingleNewsCredential/deleteNewsCredentialsByType/readActiveNewsCredentialroute throughcredentialsDb(the same per-user store used elsewhere for tokens). - GET
/config/:source: never returns the secret. Instead returns<field>_set: boolflags. On first read,migrateLegacySecretsInPlacemoves any plaintext token already on disk intocredentialsDband rewrites the JSON file without the secret — so existing installs are auto-scrubbed. - PUT
/config/:source: extracts secret fields from the body and routes them tocredentialsDb. Semantics: non-empty string → upsert,null→ delete, empty/absent → no-op (so accidentally re-saving other settings can't wipe a stored token). handleSearch: tokens are read fromcredentialsDbperreq.user.id, then injected asGITHUB_TOKEN/HF_TOKENenv into the spawned Python child only. WeChat's--access-keyis also pulled from the credential store.- The
defaultConfigentries forapi_token/access_keyare removed so the JSON shape can't accidentally re-introduce plaintext storage.
SourceSettingsDialog.tsx + i18n — TokenInput now accepts isSaved and onClear. When a credential is stored: shows a "Saved to your credential store" badge, a "Clear saved token" link, and a placeholder hint that pasting will replace it. Clearing sets the field to null, which the backend treats as an explicit delete; UI shows a "Will be removed when you save" warning until the user saves.
High — SSRF via WeChat accounts
server/scripts/research-news/search_wechat.py — normalize_account_to_url now enforces same-origin between full-URL accounts entries and the configured RSSHub instance (scheme + netloc must match exactly). Bare IDs and relative paths remain inherently scoped to instance. Smoke test confirmed:
Medium — HF Daily Papers ignored HF_TOKEN
server/scripts/research-news/search_huggingface.py — fetch_daily_papers now builds its headers via hf_auth_headers({...}) instead of a hardcoded UA-only dict, so the per-user token forwarded by the Node route is applied to the papers mode the same way it's applied to models / datasets / spaces.
Verification
ReadLintsclean across all four edited files.npx tsc --noEmit -p .passes.- Python AST parse passes for both edited scripts.
- All three i18n JSONs parse.
- SSRF guard smoke-tested above.
| if last_modified: | ||
| try: | ||
| last_dt = datetime.fromisoformat(last_modified.replace("Z", "+00:00")) | ||
| except (ValueError, TypeError): |
Added GitHub, hugging face, and WeChat news feed.