feat: Remote Platform Erase capability - #2407
Conversation
a1f3953 to
16567d2
Compare
86f6395 to
4f898d2
Compare
15e3a49 to
b99abac
Compare
… is done in off state
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.
Suppressed comments (11)
src/test/collections/MPS.postman_collection.json:3253
- These Postman URLs don’t match the routes registered in
src/routes/amt/index.ts(which uses/api/v1/amt/boot/remoteErase/:guidand/api/v1/amt/boot/capabilities/:guid). Update the collection paths to the actual server routes (or add matching routes if these are the intended canonical endpoints), otherwise the collection will 404 incorrectly.
"raw": "{{protocol}}://{{host}}/api/v1/amt/boot/rpe/1",
src/test/collections/MPS.postman_collection.json:3296
- These Postman URLs don’t match the routes registered in
src/routes/amt/index.ts(which uses/api/v1/amt/boot/remoteErase/:guidand/api/v1/amt/boot/capabilities/:guid). Update the collection paths to the actual server routes (or add matching routes if these are the intended canonical endpoints), otherwise the collection will 404 incorrectly.
"raw": "{{protocol}}://{{host}}/api/v1/amt/rpe/1",
src/routes/amt/setAMTFeatures.ts:92
- When
platformEraseEnabledis provided but the device reportsPlatformErase === 0, the request silently succeeds without applying the requested setting. This is inconsistent with the documented/implemented behavior insetRPE/sendRPE(which return 400 for unsupported). Consider returning a 400MPSValidationErrorwhen the client explicitly requestsplatformEraseEnabled: truebut the device doesn’t support RPE (or document the no-op behavior clearly).
if (payload.platformEraseEnabled !== undefined) {
const bootCaps = await req.deviceAction.getBootCapabilities()
const platformEraseCaps = bootCaps.Body?.AMT_BootCapabilities?.PlatformErase ?? 0
if (platformEraseCaps !== 0) {
rpeDesired = !!payload.platformEraseEnabled
await req.deviceAction.setRPE(rpeDesired)
}
}
swagger.yaml:305
- The OpenAPI spec for the remote erase POST references
RemoteEraseRequest, but the implementation accepts additional fields (notablypowerType) and enforces byte-length rules forssdPassword. Please update the schema to includepowerType(and any constraints like enum/range if applicable) and ensure the response/GET schemas match what the handlers actually return (the GET handler currently returns parsed booleans, not a raw bitmask).
/api/v1/amt/boot/remoteErase/{guid}:
get:
summary: Get Remote Platform Erase capabilities
description: Returns the platform erase capabilities bitmask for the device.
swagger.yaml:328
- The OpenAPI spec for the remote erase POST references
RemoteEraseRequest, but the implementation accepts additional fields (notablypowerType) and enforces byte-length rules forssdPassword. Please update the schema to includepowerType(and any constraints like enum/range if applicable) and ensure the response/GET schemas match what the handlers actually return (the GET handler currently returns parsed booleans, not a raw bitmask).
post:
summary: Trigger Remote Platform Erase
swagger.yaml:356
- The OpenAPI spec for the remote erase POST references
RemoteEraseRequest, but the implementation accepts additional fields (notablypowerType) and enforces byte-length rules forssdPassword. Please update the schema to includepowerType(and any constraints like enum/range if applicable) and ensure the response/GET schemas match what the handlers actually return (the GET handler currently returns parsed booleans, not a raw bitmask).
requestBody:
required: true
content:
application/json:
schema:
$ref: '#/components/schemas/RemoteEraseRequest'
src/routes/amt/sendRPE.ts:12
PLATFORM_ERASE_CSME_UNCONFIGUREis declared but not used, while the code uses the literal0x10000. Either use the constant in the mask construction or remove the constant to avoid drift/confusion.
const PLATFORM_ERASE_CSME_UNCONFIGURE = 0x10000
src/routes/amt/sendRPE.ts:26
PLATFORM_ERASE_CSME_UNCONFIGUREis declared but not used, while the code uses the literal0x10000. Either use the constant in the mask construction or remove the constant to avoid drift/confusion.
(unconfigureCSME ? 0x10000 : 0)
src/amt/DeviceAction.ts:213
- Elsewhere the code treats multiple firmware variants (
RPE,RPEEnabled,PlatformErase). HeresetRPEonly setsRPE, which may not enable the correct flag on devices that useRPEEnabled(or similar). To improve compatibility, set the appropriate supported field(s) consistently (e.g., assign bothRPEandRPEEnabledwhen present) before PUT.
async setRPE(isEnabled: boolean): Promise<void> {
logger.silly(`setRPE ${messages.REQUEST}`)
const bootOptions = await this.getBootOptions()
const current = bootOptions.AMT_BootSettingData
;(current as any).RPE = isEnabled
await this.setBootConfiguration(current)
logger.silly(`setRPE ${messages.COMPLETE}`)
}
src/routes/amt/getBootCapabilities.ts:20
- This boot capabilities route is emitting
POWER_CAPABILITIES_*messages, which makes logs/telemetry misleading. Use boot-capability-specific message keys (or add them) so metrics and troubleshooting don’t conflate power capabilities with boot/RPE capabilities.
MqttProvider.publishEvent('request', ['AMT_BootCapabilities'], messages.POWER_CAPABILITIES_REQUESTED, guid)
src/routes/amt/getBootCapabilities.ts:30
- This boot capabilities route is emitting
POWER_CAPABILITIES_*messages, which makes logs/telemetry misleading. Use boot-capability-specific message keys (or add them) so metrics and troubleshooting don’t conflate power capabilities with boot/RPE capabilities.
MqttProvider.publishEvent('success', ['AMT_BootCapabilities'], messages.POWER_CAPABILITIES_SUCCESS, guid)
res.status(200).json(capabilities).end()
} catch (error) {
logger.error(`${messages.POWER_CAPABILITIES_EXCEPTION} : ${error}`)
MqttProvider.publishEvent('fail', ['AMT_BootCapabilities'], messages.INTERNAL_SERVICE_ERROR)
res.status(500).json(ErrorResponse(500, messages.POWER_CAPABILITIES_EXCEPTION)).end()
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (10)
src/amt/DeviceAction.ts:310
- On BootSettingData PUT faults, the code logs the full request XML (
xmlPut) which can include the SSD erase password (<h:RSEPassword>...). This leaks a sensitive secret into logs; the error path should redact that field the same way the optional debug logging does.
if (putResult?.Envelope?.Body?.Fault) {
logger.error(`sendRPE BootSettingData PUT XML: ${xmlPut}`)
throw new Error(`BootSettingData PUT failed: ${JSON.stringify(putResult.Envelope.Body.Fault)}`)
src/amt/DeviceAction.ts:321
- The PR description says RPE requires an S5→S0 cycle (power action 5 / off-hard), but this implementation chooses Power On (2) or Master Bus Reset (10) based on live power state. That’s a functional discrepancy and may not meet the documented RPE sequence; also the state comments are inverted vs the conditional (
currentState === '8').
// If system is currently off (state !== '8') → Power On (2)
// If system is currently on (state === '8') → Master Bus Reset (10)
const powerStateResult = await this.getPowerState()
const currentState = powerStateResult?.PullResponse?.Items?.CIM_AssociatedPowerManagementService?.PowerState
const action: CIM.Types.PowerManagementService.PowerState = currentState === '8' ? 2 : 10
swagger.yaml:322
- The 200 response references
#/components/schemas/BootCapabilitiesResponse, but that schema is not defined inswagger.yaml, so the OpenAPI spec will not validate. Add the missing schema (or reference an existing one) for this endpoint’s response.
description: 'Boot capabilities including platform erase bitmask'
content:
application/json:
schema:
$ref: '#/components/schemas/BootCapabilitiesResponse'
src/test/collections/MPS.postman_collection.json:3299
- This Postman request URL does not match the implemented route (
POST /api/v1/amt/boot/remoteErase/{guid}); it currently calls/api/v1/amt/rpe/{guid}, which will 404. Update the URL (andpathsegments) to/api/v1/amt/boot/remoteErase/{guid}.
"url": {
"raw": "{{protocol}}://{{host}}/api/v1/amt/rpe/1",
"protocol": "{{protocol}}",
"host": [
"{{host}}"
src/amt/DeviceAction.ts:303
process.envkeys are normalized to lowercase at startup (src/index.ts), so checkingprocess.env.MPS_RPE_LOG_REDACTED_XMLwill never be true. Use the lowercased env var name (or route this through validated config) so the redacted-XML logging toggle can actually be enabled.
This issue also appears in the following locations of the same file:
- line 308
- line 317
if (process.env.MPS_RPE_LOG_REDACTED_XML != null && process.env.MPS_RPE_LOG_REDACTED_XML !== '') {
src/routes/amt/setAMTFeatures.ts:100
- When
payload.ocris provided butplatformEraseEnabledis omitted, this code defaultsrpeOntofalse, which can unintentionally disable the current RPE boot-service state (32770/32771) while the caller only intended to toggle OCR. Preserve the existing RPE bit unless the request explicitly changes it.
if (payload.ocr !== undefined) {
const ocrOn = !!payload.ocr
const rpeOn = rpeDesired ?? false
let requestedState = 32768
if (ocrOn && rpeOn) requestedState = 32771
swagger.yaml:3440
- The OpenAPI schema removes
remoteErasefromGetAMTFeaturesResponse, which is a breaking contract change for existing clients. KeepremoteErase(optionally marked deprecated) alongside the newrpe/rpeSupportedfields to preserve backward compatibility.
rpe:
type: boolean
rpeSupported:
type: boolean
src/routes/amt/getAMTFeatures.ts:52
GetAMTFeaturesResponsedrops the existingremoteErasefield from the response. Even if it was previously alwaysfalse, removing it is a breaking API change for strict clients. Consider keepingremoteEraseas a deprecated alias (e.g., mirroringrpe) while also returning the newrpe/rpeSupportedfields.
httpsBootSupported: ocrProcessResult.HTTPSBootSupported,
winREBootSupported: ocrProcessResult.WinREBootSupported,
localPBABootSupported: ocrProcessResult.LocalPBABootSupported,
rpe,
rpeSupported
swagger.yaml:306
- The endpoint description says it returns a platform erase bitmask, but the handler (
getBootCapabilities) returns a boolean map of supported options. Update the OpenAPI description/response schema to match the actual response shape (and consider also documenting the canonical/api/v1/amt/boot/capabilities/{guid}path that’s implemented in the router).
This issue also appears on line 318 of the same file.
summary: Get Remote Platform Erase capabilities
description: Returns the platform erase capabilities bitmask for the device.
tags:
src/test/collections/MPS.postman_collection.json:3255
- This Postman request targets
/api/v1/amt/boot/rpe/{guid}, but no such route is registered (RPE enabling is now done viaPOST /api/v1/amt/features/{guid}withplatformEraseEnabled). Remove or repurpose this request to avoid false-negative API test runs.
This issue also appears on line 3295 of the same file.
"url": {
"raw": "{{protocol}}://{{host}}/api/v1/amt/boot/rpe/1",
"protocol": "{{protocol}}",
"host": [
03126bf to
a606fce
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Suppressed comments (12)
src/amt/DeviceAction.ts:247
- A failed transition into RPE mode is only logged, but the code then writes erase parameters, activates the boot role, and reboots the machine. Throw here so a rejected state transition cannot produce a false 200 success response.
if (rscResult?.Envelope?.Body?.RequestStateChange_OUTPUT?.ReturnValue !== 0) {
logger.error(`sendRPE RequestStateChange(32770) failed: ${JSON.stringify(rscResult?.Envelope?.Body)}`)
}
src/amt/DeviceAction.ts:314
forceBootModereturns the rawSetBootConfigRoleresponse and does not throw even when it is null or AMT returns a nonzero value. Ignoring that result allows the host to be rebooted and the endpoint to report success without activating the RPE boot configuration. Validate the method result before proceeding.
await this.forceBootMode(1)
src/amt/DeviceAction.ts:325
- The power-action result is ignored, so a normal AMT response with a nonzero
ReturnValuestill leads to a 200 “success” response even though the erase reboot was not initiated. CheckRequestPowerStateChange_OUTPUT.ReturnValueand throw on failure.
await this.sendPowerAction(action)
src/routes/amt/setAMTFeatures.ts:100
- When
platformEraseEnabledis omitted,rpeDesired ?? falsetreats the existing RPE state as disabled. An OCR-only update on a device currently in state 32771 therefore moves it to 32769 and silently disables RPE. Read the current boot-service state and preserve its RPE bit when this optional field is absent.
if (payload.ocr !== undefined) {
const ocrOn = !!payload.ocr
const rpeOn = rpeDesired ?? false
let requestedState = 32768
if (ocrOn && rpeOn) requestedState = 32771
src/routes/amt/setAMTFeatures.ts:91
- If the device reports no
PlatformErasecapability, this branch silently skips the requested change and the handler still returns 200 “Updated AMT Features.” Return a validation error instead so callers are not told an unsupported RPE enable/disable request succeeded.
const platformEraseCaps = bootCaps.Body?.AMT_BootCapabilities?.PlatformErase ?? 0
if (platformEraseCaps !== 0) {
rpeDesired = !!payload.platformEraseEnabled
await req.deviceAction.setRPE(rpeDesired)
}
src/routes/amt/setAMTFeatures.ts:106
- This RPE-only branch always selects an OCR-off state. Because
ocris optional, enabling RPE on a device with OCR already enabled changes 32769 to 32770, while disabling RPE changes 32771 to 32768. Preserve the current OCR bit whenocris omitted.
} else if (rpeDesired !== undefined) {
// OCR not in request — set RPE-only state (32770 enabled, 32768 disabled)
await req.deviceAction.BootServiceStateChange(rpeDesired ? 32770 : 32768)
src/amt/DeviceAction.ts:323
- The PR description states that RPE requires an S5→S0 power cycle and that a warm reset is insufficient, but an on system is sent action 10 (
Master Bus Reset). This does not perform the documented off/on sequence, so RPE may never execute for the normal powered-on case. Dispatch the AMT-required power-cycle action instead.
const action: CIM.Types.PowerManagementService.PowerState = OFF_STATES.has(Number(currentState)) ? 2 : 10
src/routes/amt/getAMTFeatures.ts:52
- Removing the existing
remoteEraseproperty is a breaking response-shape change for/api/v1/amt/features/{guid}. Keep it as a deprecated alias while addingrpeandrpeSupported, and document both shapes in OpenAPI so existing clients continue to work.
rpe,
rpeSupported
swagger.yaml:322
BootCapabilitiesResponseis not defined anywhere undercomponents/schemas, leaving this OpenAPI document with an unresolved$ref. The handler returns four boolean flags, so define that schema or describe those properties inline.
$ref: '#/components/schemas/BootCapabilitiesResponse'
src/routes/amt/amtFeatureValidator.ts:20
- This adds
platformEraseEnabledto the accepted public request, butSetAMTFeaturesRequestinswagger.yamlstill ends atocrand does not expose the field. Update the OpenAPI schema and feature endpoint description in the same change so generated clients can send the new option.
check('platformEraseEnabled').optional().isBoolean().toBoolean()
src/routes/amt/index.ts:73
- The PR description promises
GET /api/v1/amt/boot/capabilities/{guid}, but only theremoteErasealias is registered, so clients using the advertised primary endpoint receive 404. Register and document the capabilities route, or correct the stated API contract if only the alias is intended.
amtRouter.get('/boot/remoteErase/:guid', ciraMiddleware, getBootCapabilities)
src/amt/DeviceAction.ts:303
- This switch cannot be enabled through the documented configuration path: startup lowercases all environment keys and stores validated settings in
Environment.Config(src/index.ts:35-41), soprocess.env.MPS_RPE_LOG_REDACTED_XMLis absent after initialization. Add a lowercase.mpsrc/loadConfigsetting and read it fromEnvironment.Configlike other tunables.
if (process.env.MPS_RPE_LOG_REDACTED_XML != null && process.env.MPS_RPE_LOG_REDACTED_XML !== '') {
0a71cae to
0b2b790
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.
Suppressed comments (10)
src/routes/amt/setAMTFeatures.ts:130
BootServiceStateChangeonly logs a missing/nonzero AMTReturnValueand still resolves, so this RPE-only branch publishes success and returns 200 even when the boot-service transition failed. Make the device action propagate failure and abort this handler.
await req.deviceAction.BootServiceStateChange(requestedState)
src/amt/DeviceAction.ts:335
- The PR description classifies Hibernate/S4 (state 7) as an off state that must use Power On, but this set excludes 7 and instead includes 12/13. A hibernating device will therefore receive Master Bus Reset, contrary to the documented erase flow.
const OFF_STATES = new Set([6, 8, 12, 13])
src/routes/amt/getAMTFeatures.ts:56
- This removes the existing
remoteEraseresponse field and replaces it withrpe, which breaks consumers of the established/api/v1/amt/features/:guidcontract. KeepremoteEraseas a compatibility alias while adding the new fields, and retain it in Swagger and tests.
rpe,
rpeSupported
src/routes/amt/setAMTFeatures.ts:96
- RPE capability validation happens after redirection and user-consent mutations at lines 75-87. If RPE is unsupported, this returns 400 after silently applying those earlier changes, leaving clients unable to know the request partially succeeded. Perform all RPE validation before the first device mutation.
This issue also appears on line 130 of the same file.
const bootCaps = await req.deviceAction.getBootCapabilities()
const platformEraseCaps = bootCaps.Body?.AMT_BootCapabilities?.PlatformErase ?? 0
if (platformEraseCaps === 0) {
throw new MPSValidationError('Device does not support Remote Platform Erase', 400)
swagger.yaml:331
- This prerequisite contradicts the implementation and PR description:
sendRPEautomatically enables RPE when needed. As written, API consumers may perform an unnecessary feature call or conclude the endpoint is unusable.
RPE must be enabled on the device first via `POST /api/v1/amt/features/{guid}`.
swagger.yaml:4583
- OpenAPI
maxLengthcounts Unicode characters, while the validator rejects passwords over 64 UTF-8 bytes. For example, 64€characters satisfy this schema but the API rejects them, so generated-client validation disagrees with the server. Align the public constraint with the runtime byte-length rule.
description: 'Optional password for encrypted SSD erase (max 64 bytes)'
maxLength: 64
swagger.yaml:4567
- The runtime requires at least one erase option to be true, but this schema permits
{}and requests where every option is false. Encode that invariant in the OpenAPI schema so generated clients and contract validation match the 400 behavior.
RemoteEraseRequest:
title: RemoteEraseRequest
properties:
src/routes/amt/index.ts:75
- Both routes are new API behavior, but the Postman update covers only POST, and the security collection/data contains neither route. Add GET coverage to the main collection and verb/security cases for both endpoints so the API test artifacts remain synchronized.
amtRouter.get('/boot/remoteErase/:guid', ciraMiddleware, getBootCapabilities)
amtRouter.post('/boot/remoteErase/:guid', sendRPEValidator(), validateMiddleware, ciraMiddleware, sendRPE)
src/amt/DeviceAction.ts:309
- This bypasses the repository config pipeline, and any nonempty value—including
MPS_RPE_LOG_REDACTED_XML=false—enables XML logging. Define a lowercase boolean config key, parse/validate it throughloadConfig(src/index.ts:40-41), and read it fromEnvironment.Configso deployments can reliably disable the diagnostic output.
if (process.env.MPS_RPE_LOG_REDACTED_XML != null && process.env.MPS_RPE_LOG_REDACTED_XML !== '') {
docker-compose.yml:17
- These database/Vault startup changes are unrelated to Remote Platform Erase and alter deployment behavior independently of the feature. Move the Compose health-check work—and the unrelated webserver filesystem fix—to focused PRs so this hardware-destructive feature can be reviewed and reverted in isolation.
db:
condition: service_healthy
vault:
condition: service_healthy
NOTE: Needs to be tested on real HW. Currently don't have an AMT 16+ device that supports RPE.
PR Checklist
What are you changing?
Adds Remote Platform Erase (RPE) support to MPS.
sendRPE)DeviceAction:BootSettingDataPUT payload construction,GET /api/v1/amt/boot/remoteErase/{guid}(capabilities alias)POST /api/v1/amt/boot/remoteErase/{guid}(trigger erase)PlatformErasesupport,platformEraseEnabled,rpeandrpeSupportedsetAMTFeaturesnow coordinates OCR + RPE boot state combinations.Anything the reviewer should know when reviewing this PR?
The sendRPE implementation follows the AMT spec sequence: set boot service to idle (32768) → GET boot settings → enable RPE if not already set → switch boot service to RPE mode (32770) → PUT BootSettingData with erase fields → SetBootConfigRole → query live power state and dispatch:
The
0x10000bit ineraseMaskis a sentinel for CSME/ConfigurationDataReset— it is not a hardware TLV target and is stripped before building the UEFI parameter array.Real hardware validation is needed on an AMT 16+ device; the
PlatformErasecapability field is not present on older firmware.