Skip to content

Commit de770bf

Browse files
baozhoutaoclaude
andauthored
fix(cli,service-sms)!: 启动期拒绝表外的 OS_SMS_PROVIDER,而不是静默降级成 LogSmsTransport (#5713) (#5771)
* fix(cli,service-sms)!: refuse an unknown OS_SMS_PROVIDER at boot instead of silently degrading to LogSmsTransport (#5713) `os serve` read `OS_SMS_PROVIDER` / `config.sms.provider` while assembling the kernel and handed the string to `SmsServicePlugin` with nothing to compare it against. The plugin caught the resulting `makeSmsTransport: unknown provider` throw and substituted `LogSmsTransport`, so a typo booted a server whose `send()` answers `status: 'sent'` and delivers nothing. This path never reaches `SettingsService`, which is why the `sms` namespace's `select` options table, its write-path enforcement (#5131) and the env-override gate (#5204) could none of them see it. `resolveSmsCapabilityArg` is extracted (mirroring `resolveEmailCapabilityArg`) and throws on a tag outside the vocabulary; the capability loop turns that into a hard boot error for a declared `requires: ['sms']`, else a loud console.error. Credentials are deliberately NOT demanded here — the settings namespace binds them at kernel:ready. `@objectstack/service-sms` exports `SMS_TRANSPORT_PROVIDERS` / `isSmsTransportProvider`, with `SmsProviderTag` derived from the array, so the CLI reads the vocabulary rather than restating it (#5094). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DWUR56YsttL5sTF72Q75TQ * docs(sms-service): 说明 OS_SMS_PROVIDER 有两个读取时刻,启动期那次现在会拒绝表外取值 (#5713) 该页此前只把 OS_SMS_PROVIDER 描述成 settings 的 env 覆盖,读者无从得知它还会在 `os serve` 组装 kernel 时(settings 服务尚不存在)被读一次 —— 而新的启动期拒绝正 发生在那一次。补上两个读取时刻的区分、被检查的只有 provider tag(凭据仍由该命名空间 在 kernel:ready 提供)、以及 OS_SMS_PROVIDER=log 这个明确的退出方式。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DWUR56YsttL5sTF72Q75TQ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 88a6bed commit de770bf

7 files changed

Lines changed: 362 additions & 9 deletions

File tree

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
---
2+
"@objectstack/service-sms": minor
3+
"@objectstack/cli": major
4+
---
5+
6+
fix(cli,service-sms)!: `OS_SMS_PROVIDER=twilo` now fails the boot instead of silently becoming the log transport (#5713)
7+
8+
**BREAKING for one configuration: a provider tag no SMS transport can build.**
9+
`os serve` used to hand `OS_SMS_PROVIDER` (or `config.sms.provider`) straight to
10+
`SmsServicePlugin` with nothing to compare it against. The plugin then caught the
11+
`makeSmsTransport: unknown provider 'twilo'` throw, substituted `LogSmsTransport`,
12+
and booted normally — measured, not inferred:
13+
14+
```
15+
new SmsServicePlugin({ provider: 'twilo' }).init(ctx)
16+
booted_without_throw: true transport_class: 'LogSmsTransport'
17+
isConfigured(): false logger.warn × 1, logger.error × 0
18+
service.send(…) → { status: 'sent', messageId: 'dev-sms-…' }
19+
```
20+
21+
So a phone-OTP sign-in answered "code sent", the user waited for an SMS that was
22+
never dispatched, and the one `warn` line scrolled past in the boot log. That is
23+
the declared-but-not-delivered shape of Prime Directive #10, and the same one
24+
#5132 closed for **mail** in the neighbouring arm of the very same capability
25+
loop.
26+
27+
Three gates already guard the `sms` provider value and none of them could see
28+
this path: the `sms` settings namespace declares `provider` as a `select` with an
29+
options table, #5131 enforces that table on the write path, and #5204 closed the
30+
`SettingsService` env-override branch. All three live behind `SettingsService`
31+
this read happens while the kernel is being assembled, *before* a settings
32+
service exists.
33+
34+
**`resolveSmsCapabilityArg` now refuses a provider tag outside
35+
`log` / `aliyun` / `twilio`**, the way its neighbouring `resolveEmailCapabilityArg`
36+
already did, and the capability loop turns that into the loud failure it should
37+
be — a hard boot error when the app declared `requires: ['sms']`, otherwise a
38+
`console.error` and no SMS service.
39+
40+
**What it deliberately does NOT do:** demand credentials. Unlike mail, SMS
41+
provider credentials are not a boot-time input — the `sms` settings namespace
42+
binds them at `kernel:ready`, and that is their documented home. A bare
43+
`OS_SMS_PROVIDER=twilio` on a host whose Twilio keys live in Settings is a
44+
complete configuration and passes through untouched. `SmsServicePlugin`'s own
45+
fallback is likewise untouched: for a *known* provider with incomplete
46+
constructor credentials it is correct (the settings bind can still swap in a
47+
working transport), and it remains the last line of defence for hosts that
48+
construct the plugin themselves. `os serve` simply stops feeding it input it can
49+
never use.
50+
51+
**Who is affected:** deployments that set `OS_SMS_PROVIDER` (or
52+
`config.sms.provider`) to a value outside the supported three — in practice a
53+
typo, or a provider that was never implemented — and relied on the fallback to
54+
boot. An unset `OS_SMS_PROVIDER` still defaults to `log`; every supported tag
55+
still boots with or without credentials.
56+
57+
**Migration — one line, either direction:**
58+
59+
- the environment is *not* meant to send SMS → `OS_SMS_PROVIDER=log` (that
60+
explicit value is the supported way to say so, and why refusing the others is
61+
fair);
62+
- the environment *is* meant to send SMS → fix the tag to `aliyun` or `twilio`
63+
and put the credentials in Settings → SMS Delivery (or
64+
`config.sms.providerOptions`).
65+
66+
The error names the consequence and both fixes, per AGENTS.md's
67+
degradation-log-level rule.
68+
69+
`@objectstack/service-sms` gains the vocabulary the CLI reads instead of
70+
restating: `SMS_TRANSPORT_PROVIDERS` and `isSmsTransportProvider()`, with
71+
`SmsProviderTag` now derived from the array rather than declared beside it. One
72+
vocabulary, two consumers — a second literal list in the CLI is how the mail
73+
settings dropdown and the mail transports drifted apart in the first place
74+
(#5094).

content/docs/kernel/runtime-services/sms-service.mdx

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,19 @@ Every key accepts the standard settings env override (`OS_SMS_PROVIDER`,
5757
`OS_SMS_ALIYUN_ACCESS_KEY_ID`, …). The **Send test SMS** action exercises the
5858
live (or unsaved) provider configuration.
5959

60+
`OS_SMS_PROVIDER` (and `config.sms.provider`) is read twice, at two different
61+
moments: once by `os serve` while it assembles the kernel — before any settings
62+
service exists — to pick the plugin's initial transport, and again by the
63+
settings namespace once that binds at `kernel:ready`. **A provider value outside
64+
`log` / `aliyun` / `twilio` is refused at the first of those** (#5713): the boot
65+
fails when the app declares `requires: ['sms']`, and otherwise logs an error and
66+
starts without an SMS service. It used to become the `log` transport silently, so
67+
a typo like `OS_SMS_PROVIDER=twilo` produced a server that answered every send
68+
`status: 'sent'` and delivered nothing. Credentials are *not* required at boot —
69+
only the provider tag is checked, because the credentials legitimately arrive
70+
later, from this namespace. An environment that is not meant to send SMS says so
71+
with `OS_SMS_PROVIDER=log`, which is the default.
72+
6073
## Consumers
6174

6275
- **Phone-number OTP auth** — sign-in verification codes and self-service
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* framework#5713 — what `SmsServicePlugin` is constructed with on the
5+
* `os serve` path, and what happens when the provider tag cannot deliver.
6+
*
7+
* The `sms` settings namespace declares `provider` as a `select` with an
8+
* options table (`log` / `aliyun` / `twilio`), #5131 enforces that table on the
9+
* write path, and #5204 closed the `SettingsService` env-override branch that
10+
* bypassed it. None of those three gates can see this path: `os serve` reads
11+
* `OS_SMS_PROVIDER` while assembling the kernel, *before* a settings service
12+
* exists, and handed the string straight to the plugin.
13+
*
14+
* Measured on `origin/main` before this change, `provider: 'twilo'` (a plausible
15+
* misspelling of `twilio`) reached `SmsServicePlugin.init`, threw inside
16+
* `makeSmsTransport`, was caught, and became `LogSmsTransport`:
17+
*
18+
* booted_without_throw: true transport_class: 'LogSmsTransport'
19+
* isConfigured(): false send() → { status: 'sent', messageId: 'dev-sms-…' }
20+
*
21+
* — a server that answers every OTP send "sent" and delivers nothing. That is
22+
* the declared-but-not-delivered shape of Prime Directive #10, and the same one
23+
* #5132 closed for mail in the neighbouring arm of this very loop.
24+
*
25+
* These pin the invariant in one piece: a configuration this server can deliver
26+
* through reaches the plugin unchanged (credentials included — they legitimately
27+
* arrive later, from the settings namespace at `kernel:ready`), and a provider
28+
* tag it cannot deliver through throws. The counterpart the throw depends on is
29+
* pinned too: an operator who does not want SMS sent says so with
30+
* `OS_SMS_PROVIDER=log`, and that still boots.
31+
*/
32+
33+
import { describe, it, expect } from 'vitest';
34+
import { SMS_TRANSPORT_PROVIDERS } from '@objectstack/service-sms';
35+
import { resolveSmsCapabilityArg } from './serve.js';
36+
37+
describe('resolveSmsCapabilityArg', () => {
38+
it('defaults to the log provider when nothing is configured', () => {
39+
const { options } = resolveSmsCapabilityArg({}, {});
40+
expect(options).toMatchObject({ provider: 'log' });
41+
expect(options).not.toHaveProperty('providerOptions');
42+
expect(options).not.toHaveProperty('retries');
43+
});
44+
45+
it('boots on an EXPLICIT provider=log — the way to say "this environment does not send SMS"', () => {
46+
// The premise of every throw below: refusing an undeliverable provider is
47+
// only fair because "no SMS from here" has its own spelling. If this ever
48+
// stops booting, the errors elsewhere in this file stop being actionable.
49+
expect(() => resolveSmsCapabilityArg({}, { OS_SMS_PROVIDER: 'log' })).not.toThrow();
50+
expect(resolveSmsCapabilityArg({}, { OS_SMS_PROVIDER: 'log' }).options)
51+
.toMatchObject({ provider: 'log' });
52+
// …including from objectstack.config.ts.
53+
expect(resolveSmsCapabilityArg({ provider: 'log' }, {}).options).toMatchObject({ provider: 'log' });
54+
});
55+
56+
it('lets env beat config, and normalizes the case', () => {
57+
expect(resolveSmsCapabilityArg({ provider: 'aliyun' }, { OS_SMS_PROVIDER: 'twilio' }).options)
58+
.toMatchObject({ provider: 'twilio' });
59+
// `OS_SMS_PROVIDER=Twilio` is the same declaration — the guard runs on the
60+
// lower-cased value, never on the raw env string.
61+
expect(resolveSmsCapabilityArg({}, { OS_SMS_PROVIDER: 'Twilio' }).options)
62+
.toMatchObject({ provider: 'twilio' });
63+
expect(resolveSmsCapabilityArg({ provider: 'ALIYUN' }, {}).options)
64+
.toMatchObject({ provider: 'aliyun' });
65+
});
66+
67+
it('passes a deliverable provider through WITHOUT demanding credentials', () => {
68+
// Unlike mail, SMS credentials are not a boot-time input: the `sms`
69+
// settings namespace binds them at kernel:ready. A bare provider tag is a
70+
// complete configuration here, so this arm refuses the tag and nothing
71+
// else — demanding keys would break every host that stores them in
72+
// Settings, which is the documented home for them.
73+
for (const provider of SMS_TRANSPORT_PROVIDERS) {
74+
expect(() => resolveSmsCapabilityArg({}, { OS_SMS_PROVIDER: provider }), provider).not.toThrow();
75+
expect(resolveSmsCapabilityArg({}, { OS_SMS_PROVIDER: provider }).options, provider)
76+
.toMatchObject({ provider });
77+
}
78+
});
79+
80+
it('carries config.sms.providerOptions and retries through untouched', () => {
81+
const { options } = resolveSmsCapabilityArg(
82+
{ provider: 'aliyun', providerOptions: { accessKeyId: 'ak', signName: '签名' }, retries: 2 },
83+
{},
84+
);
85+
expect(options).toMatchObject({
86+
provider: 'aliyun',
87+
providerOptions: { accessKeyId: 'ak', signName: '签名' },
88+
retries: 2,
89+
});
90+
// `retries: 0` is a real declaration (no retry), not an absence.
91+
expect(resolveSmsCapabilityArg({ retries: 0 }, {}).options).toMatchObject({ retries: 0 });
92+
});
93+
94+
it('THROWS on a provider tag no transport can deliver — no silent LogSmsTransport (#5713)', () => {
95+
const boot = () => resolveSmsCapabilityArg({}, { OS_SMS_PROVIDER: 'twilo' });
96+
expect(boot).toThrow(/provider='twilo'/);
97+
// Consequence AND fix in the one message (AGENTS.md degradation-log-level).
98+
expect(boot).toThrow(/nothing would leave the box/);
99+
expect(boot).toThrow(/log \/ aliyun \/ twilio/);
100+
expect(boot).toThrow(/OS_SMS_PROVIDER=log/);
101+
// …and never the old silent rewrite.
102+
expect(boot).not.toThrow(/falling back to LogSmsTransport/);
103+
});
104+
105+
it('refuses the same tag declared through config.sms.provider', () => {
106+
// A typo in objectstack.config.ts is the same declaration by another
107+
// channel — and the one an operator cannot fix with an env var.
108+
expect(() => resolveSmsCapabilityArg({ provider: 'aliyn' }, {}))
109+
.toThrow(/provider='aliyn'/);
110+
// Retired-looking and never-supported tags land in the same arm, and the
111+
// message names the vocabulary rather than guessing at an intent.
112+
for (const tag of ['sendgrid', 'aws-sns', 'tencent', 'smtp']) {
113+
expect(() => resolveSmsCapabilityArg({}, { OS_SMS_PROVIDER: tag }), tag)
114+
.toThrow(/log \/ aliyun \/ twilio/);
115+
}
116+
});
117+
118+
it('reads its vocabulary from @objectstack/service-sms, not a second literal', () => {
119+
// #5094's lesson, pinned: if a transport is added to service-sms and this
120+
// file kept its own list, the new provider would be refused at boot while
121+
// the plugin could build it. The error message enumerates the exported
122+
// vocabulary, so this assertion goes red the day the two diverge.
123+
let message = '';
124+
try {
125+
resolveSmsCapabilityArg({}, { OS_SMS_PROVIDER: 'definitely-not-a-provider' });
126+
} catch (err) {
127+
message = (err as Error).message;
128+
}
129+
expect(message).toContain(SMS_TRANSPORT_PROVIDERS.join(' / '));
130+
});
131+
});

packages/cli/src/commands/serve.ts

Lines changed: 78 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,13 @@ import { missingProviderMessage } from '../utils/capability-preflight.js';
2828
// only (no plugin class): `os serve` loads `EmailServicePlugin` itself through
2929
// the capability loop's dynamic import, host copy first.
3030
import { isEmailTransportProvider, emailProviderRequiresApiKey, unsupportedProviderFix } from '@objectstack/plugin-email';
31+
// The SMS provider vocabulary, read from the package that materialises the
32+
// transports, for the same reason and by the same rule as the mail one above
33+
// (#5713). `resolveSmsCapabilityArg` has to refuse exactly the tags
34+
// `makeSmsTransport` cannot build — restating `log`/`aliyun`/`twilio` here would
35+
// be the second literal #5094 was filed for. Values only (no plugin class): the
36+
// capability loop dynamic-imports `SmsServicePlugin` itself, host copy first.
37+
import { isSmsTransportProvider, SMS_TRANSPORT_PROVIDERS } from '@objectstack/service-sms';
3138
import { resolveObjectStackHome } from '@objectstack/runtime';
3239
import { LOG_LEVELS, resolveLogLevel, readLogLevelEnv } from '../utils/log-level.js';
3340
import { BootLogCapture, isVerboseBootLevel } from '../utils/boot-log-capture.js';
@@ -2342,13 +2349,12 @@ export default class Serve extends Command {
23422349
// credentials normally live in the `sms` settings namespace
23432350
// (bound at kernel:ready); constructor opts cover pre-settings
23442351
// boot and hosts without the settings service.
2345-
const cfgSms = (config as any).sms ?? {};
2346-
const provider = (process.env.OS_SMS_PROVIDER || cfgSms.provider || 'log').toLowerCase();
2347-
arg = {
2348-
provider,
2349-
...(cfgSms.providerOptions ? { providerOptions: cfgSms.providerOptions } : {}),
2350-
...(cfgSms.retries != null ? { retries: cfgSms.retries } : {}),
2351-
};
2352+
//
2353+
// Throws on a provider tag no transport can deliver (#5713) — the
2354+
// catch below turns that into the boot failure / loud error it
2355+
// should be, never a LogSmsTransport substituted behind the
2356+
// operator's back. Same shape as the `email` arm above.
2357+
arg = resolveSmsCapabilityArg((config as any).sms ?? {}, process.env).options;
23522358
} else if (cap === 'storage') {
23532359
// Storage is now in the default capability slate. If the host
23542360
// hasn't configured a backend explicitly we fall back to the
@@ -3231,6 +3237,71 @@ export function resolveEmailCapabilityArg(
32313237
return { options };
32323238
}
32333239

3240+
/** Constructor options for `SmsServicePlugin`, as the capability loop builds them. */
3241+
export interface SmsCapabilityArg {
3242+
options: Record<string, unknown>;
3243+
}
3244+
3245+
/**
3246+
* Resolve `SmsServicePlugin` constructor options from `config.sms` + `OS_SMS_*`
3247+
* env, and **refuse a provider tag no transport can deliver through** (#5713).
3248+
*
3249+
* The refusal is the point. Credentials for a real provider normally arrive from
3250+
* the `sms` settings namespace at `kernel:ready`, so this function deliberately
3251+
* does NOT demand them — a bare `OS_SMS_PROVIDER=twilio` on a host whose Twilio
3252+
* keys are stored in Settings is a complete, working configuration and passes
3253+
* through untouched. What it refuses is the one thing settings can never repair:
3254+
* a provider *tag* outside `SMS_TRANSPORT_PROVIDERS`.
3255+
*
3256+
* That tag used to travel all the way into the plugin, which caught the
3257+
* `makeSmsTransport: unknown provider 'twilo'` throw and substituted
3258+
* `LogSmsTransport` behind the operator's back. Measured on `origin/main` before
3259+
* this change, `new SmsServicePlugin({ provider: 'twilo' }).init(ctx)`:
3260+
*
3261+
* - boots without throwing, registers the `sms` service;
3262+
* - transport = `LogSmsTransport`, `isConfigured() === false`;
3263+
* - one `logger.warn` line, then `send()` answers
3264+
* `{ status: 'sent', messageId: 'dev-sms-…' }`.
3265+
*
3266+
* So a phone-OTP sign-in tells the user "code sent" and nothing leaves the box —
3267+
* the same declared-but-not-delivered shape #5132 closed for mail one layer up,
3268+
* and the same door #5204 closed on the `SettingsService` env branch. This path
3269+
* never reaches `SettingsService`: it runs at kernel-assembly time, before the
3270+
* settings service exists, which is exactly why the `sms` namespace's `select`
3271+
* options table (`sms.manifest.ts`) could not see it.
3272+
*
3273+
* The plugin's fallback is left alone on purpose. For a *known* provider with
3274+
* incomplete constructor credentials it is correct — the settings bind can still
3275+
* swap in a working transport — and it stays the last line of defence for hosts
3276+
* that construct `SmsServicePlugin` themselves. `os serve` simply stops handing
3277+
* it input it cannot use.
3278+
*
3279+
* `OS_SMS_PROVIDER=log` (the default) is how an environment says "this box does
3280+
* not send SMS", which is what makes refusing the rest fair.
3281+
*/
3282+
export function resolveSmsCapabilityArg(
3283+
cfgSms: Record<string, any> = {},
3284+
env: NodeJS.ProcessEnv = process.env,
3285+
): SmsCapabilityArg {
3286+
const provider = String(env.OS_SMS_PROVIDER || cfgSms.provider || 'log').toLowerCase();
3287+
if (!isSmsTransportProvider(provider)) {
3288+
throw new Error(
3289+
`provider='${provider}' is not a transport this server can deliver through, so every OTP and `
3290+
+ "notification SMS would be answered status: 'sent' and nothing would leave the box — "
3291+
+ `pick one of ${SMS_TRANSPORT_PROVIDERS.join(' / ')} (Settings → SMS Delivery → Provider). `
3292+
+ 'On this boot path the provider is OS_SMS_PROVIDER or config.sms.provider; set '
3293+
+ 'OS_SMS_PROVIDER=log if this environment is not meant to send SMS.',
3294+
);
3295+
}
3296+
return {
3297+
options: {
3298+
provider,
3299+
...(cfgSms.providerOptions ? { providerOptions: cfgSms.providerOptions } : {}),
3300+
...(cfgSms.retries != null ? { retries: cfgSms.retries } : {}),
3301+
},
3302+
};
3303+
}
3304+
32343305
/**
32353306
* Best-effort driver introspection.
32363307
*

packages/services/service-sms/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ export {
1010
export { SmsServicePlugin, type SmsServicePluginOptions } from './sms-plugin.js';
1111
export {
1212
makeSmsTransport,
13+
SMS_TRANSPORT_PROVIDERS,
14+
isSmsTransportProvider,
1315
AliyunSmsTransport,
1416
TwilioSmsTransport,
1517
type SmsProviderTag,

0 commit comments

Comments
 (0)