auth-simple: duplicate app keys differing in case silently shadow the tighter entry, and a backend migration moves policy authority with no observable signal
Nadie ha tomado este issue todavía.
Evaluación
- Dificultad
- 5/5
- Tiempo estimado
- Más de una semana
- Aptitud para principiantes
- 30/100
- Tipo de issue
- Nueva funcionalidad
- Claridad
- Bastante claro
- Estado de actividad
- Activo
- Stack tecnológico
- rust, typescript
Línea de trabajo
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.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
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.
- Lenguaje dominante
- Rust
- Estrellas
- 546
- Forks
- 96
- Merge medio
- 19 h 22 min
- PR fusionados (30 d)
- 109
Guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de Dstack-TEE/dstack
-
Dificultad 3/5 1-2 días Aptitud para principiantes 55/100
Dstack-TEE/dstack#1300 ·
-
Dificultad 4/5 3-5 días Aptitud para principiantes 48/100
Dstack-TEE/dstack#1299 ·
-
Dificultad 4/5 3-5 días Aptitud para principiantes 48/100
Dstack-TEE/dstack#1298 ·
-
Dificultad 5/5 Más de una semana Aptitud para principiantes 25/100
Dstack-TEE/dstack#1297 ·
-
Dificultad 5/5 Más de una semana Aptitud para principiantes 35/100
Dstack-TEE/dstack#1296 ·
Todos los issues de Dstack-TEE/dstack
Issues similares
-
risk:low runtime status:in-progress type:test
Dificultad 1/5 Menos de una hora Aptitud para principiantes 92/100
zeroclaw-labs/zeroclaw#11023 ·
-
good first issue refactor
Dificultad 2/5 1-3 horas Aptitud para principiantes 72/100
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 86/100
kwakseongjae/auto-hwp#319 ·
-
area:cli bug filter-quality good first issue priority:medium
Dificultad 2/5 1-3 horas Aptitud para principiantes 84/100
-
Dificultad 1/5 Menos de una hora Aptitud para principiantes 72/100
bevyengine/bevy#25861 ·