fix(i18n): define api-error-generic across all 6 locales (audit: api-error-generic-missing-fluent-key)#5322
Conversation
…error-generic-missing-fluent-key)
41+ HTTP 500 handlers in routes/{agents,tools_sessions}.rs build
their response body as:
t_args("api-error-generic", &[("error", &e.to_string())])
But api-error-generic was never defined in any locale's errors.ftl.
The fallback at i18n.rs:163-164 returns the bare key on lookup
miss, so the $error interpolation never ran and every 5xx surfaced
the literal 'api-error-generic' to the client with diagnostic
context silently dropped — affecting agent-not-found-returns-500,
rusqlite-errors-leak, and the agent_clone 500-on-AgentAlreadyExists
case folded into the same audit item.
Stopgap per docs/issues/api-error-generic-missing-fluent-key.md:
define the key in en/ja/zh-CN/de/fr/es. en uses bare { $error };
the five other locales prefix with localized Error/エラー/错误/
Fehler/Erreur/Error so the rendered string differs per locale (the
per-locale 'must differ from en' assertion in
all_languages_have_same_keys requires this).
Long-term remediation (typed MemoryRouteError helper per
routes/memory.rs:198-215) is tracked separately.
Regression guards:
- api-error-generic added to all_languages_have_same_keys
- new api_error_generic_interpolates_underlying_error exercises
every locale with a sample $error and asserts the rendered body
is not the bare key AND contains the error string verbatim
22 i18n::tests green. cargo test -p librefang-types --lib i18n.
…5322) Delete the audit doc and update INDEX.md per the convention #5265 established: decrement narrative count (119→118), rollup table Critical (7→6) + Total (120→119), remove the bullet under Critical, and renumber the Pass 1 / Pass 2 triage-order lists so the numbering stays contiguous after the item is dropped from slot #1.
…5322) Delete the audit doc and update INDEX.md per the convention #5265 established: decrement narrative count (119→118), rollup table Critical (7→6) + Total (120→119), remove the bullet under Critical, and renumber the Pass 1 / Pass 2 triage-order lists so the numbering stays contiguous after the item is dropped from slot #1.
6c06459 to
4dfd855
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4dfd85580a
ℹ️ 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".
| # Used by 41+ HTTP 500 handlers as a stopgap until each route is moved to a | ||
| # typed MemoryRouteError-style helper. Without this key, every `t_args("api-error-generic", …)` | ||
| # call returns the literal key as the response body and `$error` interpolation never runs. | ||
| api-error-generic = { $error } |
There was a problem hiding this comment.
Avoid exposing raw internal errors in api-error-generic
Binding api-error-generic directly to { $error } turns every existing ApiErrorResponse::internal(t.t_args("api-error-generic", [("error", &e.to_string())])) callsite (for example in crates/librefang-api/src/routes/tools_sessions.rs:452) into a client-visible raw error echo, so backend failures now leak internal diagnostics (DB/implementation details) in HTTP 500 bodies. This conflicts with the project’s own 5xx handling policy in crates/librefang-api/src/routes/memory.rs:187-210, which explicitly scrubs internal failures to avoid exposing deployment detail, so the new translation key introduces a broad information-disclosure regression whenever those handlers error.
Useful? React with 👍 / 👎.
…or-generic Other entries in fr/errors.ftl (e.g. `Requete invalide :`, `Echec de l'analyse de la configuration :`) all use the space-before-colon French typographic convention. The new `api-error-generic` entry rendered `Erreur:` without the space — minor visual drift from the surrounding file. One-character fix; `api_error_generic_interpolates_underlying_error` still green.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 821b7ffac7
ℹ️ 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".
| # Used by 41+ HTTP 500 handlers as a stopgap until each route is moved to a | ||
| # typed MemoryRouteError-style helper. Without this key, every `t_args("api-error-generic", …)` | ||
| # call returns the literal key as the response body and `$error` interpolation never runs. | ||
| api-error-generic = { $error } |
There was a problem hiding this comment.
Stop returning raw internal errors in api-error-generic
Defining api-error-generic as { $error } makes every t_args("api-error-generic", [("error", &e.to_string())]) callsite return unsanitized backend error text to clients on 5xx responses, which can expose deployment details such as DB paths, SQL/driver errors, or internal identifiers. This is a regression from the previous behavior and conflicts with the project’s explicit 5xx scrubbing pattern in crates/librefang-api/src/routes/memory.rs (MemoryRouteError::Internal returns a fixed "Internal server error" body), so the key should map to a stable generic message rather than echoing $error verbatim.
Useful? React with 👍 / 👎.
# Conflicts: # openapi.json # xtask/baselines/openapi.sha256
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Closes #5329
Summary
Audit:
docs/issues/api-error-generic-missing-fluent-key.md(Severity: Critical).41+ HTTP 500 handlers in
crates/librefang-api/src/routes/{agents,tools_sessions}.rsbuild their response body as:
But
api-error-genericwas never defined in any locale'serrors.ftl(
grep -rn 'api-error-generic\s*='againstcrates/librefang-types/locales/{en,ja,zh-CN,de,fr,es}/errors.ftl→ 0 hits).The fallback at
i18n.rs:163-164returns the literal key on a missinglookup, so the
{$error}interpolation never ran — every one of those5xx responses surfaced
{"error": "api-error-generic"}to the clientwith the actual
e.to_string()silently dropped.This swallowed the diagnostic context behind every 500 in
agents+tools_sessionsroutes — including the bug classesagent-not-found-returns-500,rusqlite-errors-leak, and theagent_clone500-on-AgentAlreadyExistscase folded into the sameaudit item.
Fix
Per the audit doc's stopgap: define the key in every locale.
English uses bare
{ $error }(the underlying error strings arealready in English). The five other locales prefix with a localized
"Error / エラー / 错误 / Fehler / Erreur / Error: " so the rendered
string differs across bundles — the per-locale assertion in
all_languages_have_same_keysrequires non-English bundles emit adistinct rendered output, which a bare-shared pattern would not.
The long-term remediation (route every 500 through a typed
MemoryRouteError-style helper perroutes/memory.rs:198-215) istracked separately and intentionally out of scope here.
Regression guards
api-error-genericappended to the key list inall_languages_have_same_keys. A new locale or a staleerrors.ftlcannot silently regress this without a CI failure.api_error_generic_interpolates_underlying_errorexercises every
SUPPORTED_LANGUAGESentry with a sample$errorand asserts (a) the rendered body is not the literal key and
(b) the underlying error string is contained verbatim.
Verification
Test plan
Test / Unit (lib+bin)greenconfirm the response body contains the real error text instead of
"api-error-generic"prefix appears