auth-simple: duplicate app keys differing in case silently shadow the tighter entry, and a backend migration moves policy authority with no observable signal
まだ誰も着手していません。
評価
- 難易度
- 5/5
- 見積もり時間
- 1週間以上
- 初心者へのやさしさ
- 30/100
- issue の種類
- 機能追加
- 明瞭さ
- おおむね明確
- 活発さ
- 活発
- 技術スタック
- rust, typescript
調査の方向性
Start with dstack/kms/auth-simple/index.ts, docs/auth-simple-operations.md, dstack/kms/auth-simple/migration.test.ts, and kms/src/main_service.rs, then read scenario 7 in .agent/CONTRACT-SCENARIOS.md. Done requires an agreed scope for duplicate-key handling and backend-posture reporting or documentation, with the relevant tests and operational guidance updated.
索引モデルが issue の本文から書いたものです。
説明
Label: DESIGN. No test fails against the current code; the tests below pass and pin present behaviour.
Two related gaps found while walking the "migrate one app between authorization backends" scenario. Both are off-chain and cheap to fix; neither needs any contract change.
1. Duplicate app keys differing only in case — the first entry wins
auth-simple resolves an app by scanning the config map and taking the first normalized match:
// dstack/kms/auth-simple/index.ts:173
const appConfig = Object.entries(config.apps).find(
([id]) => normalizeHex(id) === appId
)?.[1];
normalizeHex lowercases and prepends 0x, but JSON keeps "0x…AA" and "0x…aa" as two distinct object keys. So a config with both — which is exactly what happens when an operator pastes a checksummed address next to an existing lowercase one — silently resolves to whichever appears first in the file. A tightened entry placed below a stale permissive one never runs:
[PASS] DELTA 6: duplicate app keys differing only in case — the first entry wins
// 0x…AA: composeHashes ['0xv1', '0xv-withdrawn'] (stale, added during debugging)
// 0x…aa: composeHashes ['0xv1'] (intended, tightened)
expect(r.isAllowed).toBe(true); // for '0xv-withdrawn'
The on-chain backends have no equivalent, because the key there is a Solidity address. This is auth-simple-specific.
Steelman. Case-insensitive matching is the right behaviour — hex addresses genuinely arrive in both forms, and normalizeHex is doing the correct thing. .find returning the first match is the obvious JS idiom, and the duplicate-key case is an operator error, not an input an attacker controls. Detecting it costs a pass over the map that the happy path does not need.
Cost. A silent config-authoring hazard in the one backend whose whole interface is a hand-edited JSON file, where the failure mode is "the tighter policy you wrote is ignored" rather than an error. docs/auth-simple-operations.md walks operators through iterative tightening of exactly this map.
Fix direction (no contract change; one commit in auth-simple):
- Normalize at load and reject duplicates. In
loadConfig, build aMapkeyed onnormalizeHex(id)and throw — or log an error and fail closed — when two raw keys collide. Loudest, and it catches the error at the moment the operator makes it. Costs one pass per request, or zero if the config is cached (it is currently re-read per call, which is deliberate — see below). - Normalize at load and take the most restrictive merge instead of the first. Avoids a hard failure but hides the mistake; probably worse.
- Validate in the zod schema with a
superRefineonappsthat rejects normalized-key collisions. Keeps the check with the rest of the config contract and is the smallest diff.
(3), with (1)'s fail-closed behaviour on violation.
2. A backend migration changes who holds app policy authority, and nothing downstream can tell
Walking the on-chain ↔ auth-simple migration end to end, two structural properties change that no per-field delta captures:
- Authority moves. On-chain, an app's compose-hash allowlist is controlled by the app owner's key, on a contract the KMS operator does not own. Under
auth-simpleit is a JSON file on the KMS operator's filesystem. Migration transfers the app's policy authority to the infrastructure operator, and back, with no signal to the app or its users. - The audit trail disappears.
PolicyChangedgives a permanent, third-party-verifiable record of every policy change. A config edit leaves no artifact outside the operator's host.
And the health endpoint cannot distinguish the two backends, because kmsContractAddr, chainId and appImplementation are free-form config values in auth-simple:
// dstack/kms/auth-simple/index.ts:44-48
kmsContractAddr: z.string().default('0x0000000000000000000000000000000000000000'),
chainId: z.number().default(0),
appImplementation: z.string().default('0x0000000000000000000000000000000000000000'),
[PASS] DELTA 5: the health endpoint can be made byte-identical to the on-chain backend
expect(json.chainId).toBe(2035);
expect(json.kmsContractAddr).toBe('0xA1B2C3D4E5F6071829304152637485960718293A');
GetInfo / GetMeta surface exactly these fields (kms/src/main_service.rs:471-473), so a migrated deployment can report the old contract address and chain id verbatim and nothing downstream can tell.
Steelman. auth-simple is a documented, supported, first-class backend — docs/auth-simple-operations.md exists for it — and choosing it is a legitimate deployment posture for air-gapped, single-tenant or pre-production settings where running a chain is not worth it. The config fields are surfaced because the KMS's response schema requires them; defaulting them to zero and letting an operator fill them in is the honest way to satisfy a shared schema from a backend that has no chain. Inventing a "backend kind" field also means every consumer has to learn to interpret it, and an operator who wants to misrepresent their posture can lie about any field you add.
The fail-closed behaviour is genuinely good and worth recording: an unreadable or missing config denies everything, matching the on-chain backend's behaviour when the RPC is unreachable. Both postures fail closed. Verified independently, not by comparison:
[PASS] DELTA 4: an unreadable config denies everything (fail-closed), like an unreachable RPC
Cost. An app cannot require on-chain governance as a condition of running, and a relying party cannot verify that the KMS it is trusting is governed by the contract it thinks it is. The posture is not attested, not measured and not reportable. Since the KMS's own app-compose is measured and its config is not, this is a property of the deployment that sits outside everything the attestation covers.
Related and already recorded, not re-reported here: K-e in AUDIT-BACKLOG.md (the compose-simple topology puts the whole authorization decision on an unauthenticated plain-HTTP channel), partly addressed by #1268; and the known advisoryIds / TCB-default divergences between the backends.
Fix direction (no contract change):
- Docs only. A "what changes when you migrate" section in
docs/auth-simple-operations.mdnaming both properties: who controls the app allowlist, and that no audit trail survives. Zero cost, immediately useful, and it is missing today. - Report the backend kind honestly. Add a non-overridable field to the health/
GetInforesponse —backend: "config"vs"ethereum"— set by the backend's own code rather than by config, and plumb it throughGetMeta. Does not stop a determined operator from running a modified backend, but it removes the accidental case and gives an honest deployment something to point at. Small and additive;GetMetaalready gained an optional field in #1268 (os_image_verification) by the same pattern. - Measure the auth config. Include a hash of the resolved auth config in the KMS's own attested measurements, so the posture is inside the attestation rather than beside it. This is the only option that actually makes the posture verifiable, and it is much larger — it needs a decision about config mutability at runtime, which conflicts with the current re-read-per-request design that gives
auth-simpleits instant revocation latency. - Refuse the impersonation case narrowly. Have
auth-simplereject a config that sets a non-zerokmsContractAddrorchainId, since it has no chain to back them. Cheap, but it breaks any operator legitimately using those fields as documentation of a prior deployment; probably only worth it alongside (2).
(1) is required regardless; (2) is the cheap honest improvement; (3) is the real fix if the team wants the posture to be verifiable.
Found during a scenario-driven review of the authorization contracts; full walk in .agent/CONTRACT-SCENARIOS.md (scenario 7), tests in dstack/kms/auth-simple/migration.test.ts.
- 主要言語
- Rust
- スター
- 546
- フォーク
- 96
- 平均マージ
- 19時間 22分
- マージ済み PR(30日)
- 109
コントリビューションガイド
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
Dstack-TEE/dstack のほかの issue
-
難易度 3/5 1〜2日 初心者へのやさしさ 55/100
Dstack-TEE/dstack#1300 ·
-
難易度 4/5 3〜5日 初心者へのやさしさ 48/100
Dstack-TEE/dstack#1299 ·
-
難易度 4/5 3〜5日 初心者へのやさしさ 48/100
Dstack-TEE/dstack#1298 ·
-
難易度 5/5 1週間以上 初心者へのやさしさ 25/100
Dstack-TEE/dstack#1297 ·
-
難易度 5/5 1週間以上 初心者へのやさしさ 35/100
Dstack-TEE/dstack#1296 ·
Dstack-TEE/dstack の issue をすべて見る
似ている issue
-
risk:low runtime status:in-progress type:test
難易度 1/5 1時間未満 初心者へのやさしさ 92/100
zeroclaw-labs/zeroclaw#11023 ·
-
good first issue refactor
難易度 2/5 1〜3時間 初心者へのやさしさ 72/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 86/100
kwakseongjae/auto-hwp#319 ·
-
area:cli bug filter-quality good first issue priority:medium
難易度 2/5 1〜3時間 初心者へのやさしさ 84/100
-
難易度 1/5 1時間未満 初心者へのやさしさ 72/100
bevyengine/bevy#25861 ·