health-check reads secrets.smoketest_bot_token without guarding undefined secrets #4

New issue

Open

opened 2026-06-14 23:52:12 +00:00 by forgejo-admin · 0 comments

forgejo-admin commented

2026-06-14 23:52:12 +00:00

Owner

Summary

celilo/scripts/health-check.ts accesses secrets.smoketest_bot_token directly. secrets is typed as required (Record<string, string>) and the production handler always supplies it, so this is fine in practice — but there's no defensive guard if secrets is ever undefined.

Context

This surfaced while fixing the unit tests: 6 health-check tests threw undefined is not an object (evaluating 'secrets.smoketest_bot_token') because they omitted the secrets dep. Fixed on the test side in commit 4396221 (pass secrets: {}), deliberately leaving the source contract intact rather than masking a potentially-real missing-secret with ?..

Decision to make

Is the strict contract correct (caller must always pass secrets), or should the source defensively handle undefined (treat as "no token provisioned" → skip smoke checks)? Low priority either way; filing so the choice is explicit rather than implicit.

Acceptance criteria

Decide: keep required secrets, or make access defensive (secrets?.smoketest_bot_token).
If defensive, ensure the "skip smoke checks" path is exercised by a test.

## Summary `celilo/scripts/health-check.ts` accesses `secrets.smoketest_bot_token` directly. `secrets` is typed as required (`Record<string, string>`) and the production handler always supplies it, so this is fine in practice — but there's no defensive guard if `secrets` is ever `undefined`. ## Context This surfaced while fixing the unit tests: 6 health-check tests threw `undefined is not an object (evaluating 'secrets.smoketest_bot_token')` because they omitted the `secrets` dep. Fixed on the **test** side in commit `4396221` (pass `secrets: {}`), deliberately leaving the source contract intact rather than masking a potentially-real missing-secret with `?.`. ## Decision to make Is the strict contract correct (caller must always pass `secrets`), or should the source defensively handle `undefined` (treat as "no token provisioned" → skip smoke checks)? Low priority either way; filing so the choice is explicit rather than implicit. ## Acceptance criteria - [ ] Decide: keep required `secrets`, or make access defensive (`secrets?.smoketest_bot_token`). - [ ] If defensive, ensure the "skip smoke checks" path is exercised by a test.

forgejo-admin added the

type/tech-debt

area/deploy

priority/low

labels

2026-06-14 23:52:12 +00:00