Skip to content

refactor(security): default useTLS to true on device creation - #1169

Merged
graikhel-intel merged 9 commits into
mainfrom
CM-351
Aug 20, 2026
Merged

refactor(security): default useTLS to true on device creation#1169
graikhel-intel merged 9 commits into
mainfrom
CM-351

Conversation

@DevipriyaS17

@DevipriyaS17 DevipriyaS17 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes WSMAN connections to AMT devices now default to TLS
(port 16993/HTTPS) instead of plaintext (port 16992/HTTP) when a
device is created without an explicit useTLS value.

Problem

The POST /api/v1/devices endpoint accepted whatever useTLS value
arrived in the request body. Since Go zero-initialises bool fields,
omitting useTLS silently defaulted to false, causing plaintext
WSMAN traffic as evidenced by:

  • Console logs: Post "http://test:16992/wsman"
  • tcpdump: full plaintext SOAP XML payload visible on the wire

Root Cause

useTLS is a bool in dto.Device. Omitting the field from the
JSON body results in false — no explicit default existed at the
API boundary.

Behaviour Matrix

Scenario Before After
Create device, useTLS omitted HTTP/16992 HTTPS/16993
Create device, "useTLS": false explicit HTTP/16992 HTTP/16992
Create device, "useTLS": true explicit HTTPS/16993 HTTPS/16993

Linked URL

device-management-toolkit/sample-web-ui#3480

@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 69.44444% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.24%. Comparing base (3003aa0) to head (a2a5a7d).

Files with missing lines Patch % Lines
internal/controller/httpapi/v1/devices.go 65.78% 7 Missing and 6 partials ⚠️
internal/controller/httpapi/v1/profiles.go 22.22% 5 Missing and 2 partials ⚠️
internal/controller/openapi/devices.go 0.00% 1 Missing ⚠️
pkg/logger/logger.go 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1169      +/-   ##
==========================================
+ Coverage   50.19%   50.24%   +0.05%     
==========================================
  Files         147      147              
  Lines       13616    13656      +40     
==========================================
+ Hits         6834     6862      +28     
- Misses       6190     6199       +9     
- Partials      592      595       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens device creation security by defaulting WSMAN connections to TLS (HTTPS/16993) when useTLS is omitted from POST /api/v1/devices, while still honoring explicit useTLS: false.

Changes:

  • Update the v1 device insert handler to detect whether useTLS was present in the JSON body and default it to true when omitted.
  • Add/adjust unit tests to cover the new defaulting behavior and the explicit useTLS: false case.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
internal/controller/httpapi/v1/devices.go Switches to body-caching JSON bind, inspects provided JSON keys, and defaults UseTLS to true when omitted.
internal/controller/httpapi/v1/devices_test.go Adds tests for defaulting behavior / explicit false, and updates an existing insert test expectation to include UseTLS: true.

Comment thread internal/controller/httpapi/v1/devices.go Outdated
Comment thread internal/controller/httpapi/v1/devices_test.go Outdated
Comment thread internal/controller/httpapi/v1/devices.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

integration-test/collections/console_mps_apis.postman_collection.json:1637

  • The Postman "Create Device" request explicitly sends "useTLS": true, so it doesn't actually exercise the new defaulting behavior when useTLS is omitted. To validate the regression end-to-end, remove the useTLS property from the request body and keep the response assertion jsonData.useTLS === true.
							"raw": "{\r\n    \"guid\": \"143e4567-e89b-12d3-a456-426614174000\",\r\n    \"friendlyName\": \"friendlyName\",\r\n    \"hostname\": \"hostname\",\r\n    \"tags\": [],\r\n    \"mpsusername\": \"admin\",\r\n    \"useTLS\": true,\r\n    \"deviceInfo\": {\r\n        \"fwVersion\": \"16.1.30\",\r\n        \"fwBuild\": \"3400\",\r\n        \"fwSku\": \"11\",\r\n        \"discovered\": true,\r\n        \"firstDiscovered\": \"2026-05-20T00:00:00Z\",\r\n        \"currentMode\": \"Admin\",\r\n        \"features\": \"SOL,IDER,KVM\",\r\n        \"ipAddress\": \"10.0.0.12\",\r\n        \"lastSynced\": \"2026-05-21T00:00:00Z\",\r\n        \"tlsMode\": \"TLS 1.2\",\r\n        \"upid\": {\r\n            \"oemPlatformIdType\": \"Not Set (0)\",\r\n            \"oemId\": \"\",\r\n            \"csmeId\": \"4A45A39C5ED9462082510000\"\r\n        },\r\n        \"amtEnabledInBIOS\": true,\r\n        \"meInterfaceVersion\": \"16.1.25.2124\",\r\n        \"dhcpEnabled\": true,\r\n        \"certHashes\": [\r\n            \"a1b2c3\",\r\n            \"d4e5f6\"\r\n        ],\r\n        \"lmsInstalled\": true,\r\n        \"lmsVersion\": \"2410.5.0.0\",\r\n        \"osName\": \"linux\",\r\n        \"osVersion\": \"6.8.0-51-generic\",\r\n        \"osDistro\": \"Ubuntu 24.04 LTS\",\r\n        \"cpuModel\": \"Intel(R) Core(TM) Ultra 7 165H\",\r\n        \"osIpAddress\": \"10.49.76.163\",\r\n        \"ethernetAdapterCount\": 2,\r\n        \"monitorConnected\": true,\r\n        \"ieee8021xEnabled\": false\r\n    }\r\n}",

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (2)

internal/controller/httpapi/v1/devices.go:211

  • In the create-device handler, calling providedJSONFields() just to detect whether top-level useTLS was present does extra work (it lowercases and recursively flattens nested objects) on every device create. A lighter check against the top-level JSON keys would avoid unnecessary recursion/cpu while keeping the same defaulting behavior.
	fields, err := providedJSONFields(c)

internal/controller/openapi/devices.go:113

  • The OpenAPI route description changed here; per CLAUDE.md (OpenAPI spec section) the generated spec (doc/openapi.json) should be regenerated and committed whenever spec-reachable routes/descriptions change, otherwise SwaggerHub/integrators may see stale docs.
		fuego.OptionDescription("Create a new device. If useTLS is omitted, it defaults to true."),

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

internal/controller/openapi/devices.go:113

  • The route description mentions the default, but the OpenAPI schema for the request body likely still has no field-level default for useTLS (it’s a plain bool in internal/entity/dto/v1/device.go). If you want this default to be visible to generated clients/tooling, consider adding a struct tag default (similar to default:"0" in dto/v1/alarm.go:10 or default:"All" in dto/v1/profile.go:46) so the generated spec captures it as default: true rather than only in free-text.
		fuego.OptionDescription("Create a new device. If useTLS is omitted, it defaults to true."),

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

sudhir-intc
sudhir-intc previously approved these changes Aug 5, 2026
@sudhir-intc sudhir-intc changed the title fix(security): default useTLS to true on device creation refactor(security): default useTLS to true on device creation Aug 14, 2026
@madhavilosetty-intel

Copy link
Copy Markdown
Contributor

@DevipriyaS17 allowSelfSigned should also default to true, along with useTLS, to keep the behavior consistent with the sample-web-ui PR device-management-toolkit/sample-web-ui#3480 where both options are enabled by default.

Also, please add a Postman test with "useTLS": false and "allowSelfSigned": false and verify both remain false. This will make sure the explicit-false behavior doesn’t regress later.

@graikhel-intel
graikhel-intel merged commit 001de52 into main Aug 20, 2026
21 checks passed
@graikhel-intel
graikhel-intel deleted the CM-351 branch August 20, 2026 22:27
@RosieAMT

Copy link
Copy Markdown

🎉 This PR is included in version 1.40.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@RosieAMT

Copy link
Copy Markdown

🎉 This PR is included in version 1.40.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

1 similar comment
@RosieAMT

Copy link
Copy Markdown

🎉 This PR is included in version 1.40.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

sudhir-intc added a commit that referenced this pull request Aug 25, 2026
commit 7e5511e
Merge: 86d029f c28d7a3
Author: Sudhir Pola <sudhir.pola@intel.com>
Date:   Tue Aug 25 10:10:48 2026 +0530

    Merge branch 'main' into feat/http-tenant-header

commit c28d7a3
Author: Madhavi Losetty <madhavi.losetty@intel.com>
Date:   Mon Aug 24 10:16:23 2026 -0700

    refactor(config): check admin password strength at startup (#1193)

    Warn when the configured admin password is shorter than 8 characters
    or misses a lowercase letter, uppercase letter, digit, or symbol.
    Startup continues either way.

    Generated passwords now draw one character from each required class
    and shuffle with crypto/rand, so a fresh install satisfies the same
    rule the warning describes.

    The generator draws its symbols from @ and * only, because the value
    gets pasted verbatim into files that mangle punctuation: $ and !
    expand in sh, # truncates the value in make (the Makefile does
    -include .env, where quoting does not help), and % ^ & break
    cmd.exe's set. Operators may still use any of those themselves.

    There is no upper length bound, and any non-alphanumeric counts as a
    symbol: this password is only compared against the login request, so
    a long passphrase or one using - or _ must not be called weak.

    Existing passwords are untouched: the generator only runs when no
    password is configured, and a weak value only warns.

    Co-authored-by: Ganesh Raikhelkar <ganesh.raikhelkar@intel.com>

commit 374987e
Author: Amarnath C <amarnath.c@intel.com>
Date:   Mon Aug 24 22:21:20 2026 +0530

    fix: align timeout budget with wsman client for slow devices (#1082) (#1153)

    1. httpserver read/write timeout 15s to 40s so the wsman client
      (30s) times out first with a clean 504.
    2. waitForAuth 3s to 30s so concurrent handlers share one Target
       instead of forking, preserving the library concurrency cap.
    3. expireAfter 30s to 60s to keep the authenticated Target cached
       across rapid page refreshes.

commit cc951f4
Author: Amarnath C <amarnath.c@intel.com>
Date:   Mon Aug 24 22:06:36 2026 +0530

    build(deps): bump go-wsman-messages to v2.50.2 (#1219)

    Bumps go-wsman-messages from v2.50.1 to v2.50.2.

commit 3022f37
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Mon Aug 24 06:41:41 2026 -0700

    build(deps): bump golang from 1.26-alpine to 1.27-alpine (#1215)

    Bumps golang from 1.26-alpine to 1.27-alpine.

    ---
    updated-dependencies:
    - dependency-name: golang
      dependency-version: 1.27-alpine
      dependency-type: direct:production
    ...

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
    Co-authored-by: Madhavi Losetty <madhavi.losetty@intel.com>

commit f455536
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Mon Aug 24 06:27:30 2026 -0700

    build(deps): bump github.com/getkin/kin-openapi from 0.146.0 to 0.147.0 (#1216)

    Bumps [github.com/getkin/kin-openapi](https://github.com/getkin/kin-openapi) from 0.146.0 to 0.147.0.
    - [Release notes](https://github.com/getkin/kin-openapi/releases)
    - [Commits](getkin/kin-openapi@v0.146.0...v0.147.0)

    ---
    updated-dependencies:
    - dependency-name: github.com/getkin/kin-openapi
      dependency-version: 0.147.0
      dependency-type: direct:production
      update-type: version-update:semver-minor
    ...

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
    Co-authored-by: Madhavi Losetty <madhavi.losetty@intel.com>

commit 4cfb69f
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Mon Aug 24 06:17:22 2026 -0700

    build(deps): bump modernc.org/sqlite from 1.56.0 to 1.57.0 (#1217)

    Bumps [modernc.org/sqlite](https://gitlab.com/cznic/sqlite) from 1.56.0 to 1.57.0.
    - [Changelog](https://gitlab.com/cznic/sqlite/blob/master/CHANGELOG.md)
    - [Commits](https://gitlab.com/cznic/sqlite/compare/v1.56.0...v1.57.0)

    ---
    updated-dependencies:
    - dependency-name: modernc.org/sqlite
      dependency-version: 1.57.0
      dependency-type: direct:production
      update-type: version-update:semver-minor
    ...

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit 356828b
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Mon Aug 24 06:12:50 2026 -0700

    build(deps): bump the codeql-action group with 4 updates (#1218)

    Bumps the codeql-action group with 4 updates: [github/codeql-action/init](https://github.com/github/codeql-action), [github/codeql-action/autobuild](https://github.com/github/codeql-action), [github/codeql-action/analyze](https://github.com/github/codeql-action) and [github/codeql-action/upload-sarif](https://github.com/github/codeql-action).

    Updates `github/codeql-action/init` from 4.37.7 to 4.37.8
    - [Release notes](https://github.com/github/codeql-action/releases)
    - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
    - [Commits](github/codeql-action@ff2f1c6...db488dd)

    Updates `github/codeql-action/autobuild` from 4.37.7 to 4.37.8
    - [Release notes](https://github.com/github/codeql-action/releases)
    - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
    - [Commits](github/codeql-action@ff2f1c6...db488dd)

    Updates `github/codeql-action/analyze` from 4.37.7 to 4.37.8
    - [Release notes](https://github.com/github/codeql-action/releases)
    - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
    - [Commits](github/codeql-action@ff2f1c6...db488dd)

    Updates `github/codeql-action/upload-sarif` from 4.37.7 to 4.37.8
    - [Release notes](https://github.com/github/codeql-action/releases)
    - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
    - [Commits](github/codeql-action@ff2f1c6...db488dd)

    ---
    updated-dependencies:
    - dependency-name: github/codeql-action/init
      dependency-version: 4.37.8
      dependency-type: direct:production
      update-type: version-update:semver-patch
      dependency-group: codeql-action
    - dependency-name: github/codeql-action/autobuild
      dependency-version: 4.37.8
      dependency-type: direct:production
      update-type: version-update:semver-patch
      dependency-group: codeql-action
    - dependency-name: github/codeql-action/analyze
      dependency-version: 4.37.8
      dependency-type: direct:production
      update-type: version-update:semver-patch
      dependency-group: codeql-action
    - dependency-name: github/codeql-action/upload-sarif
      dependency-version: 4.37.8
      dependency-type: direct:production
      update-type: version-update:semver-patch
      dependency-group: codeql-action
    ...

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit 7e02b81
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Fri Aug 21 11:57:47 2026 -0700

    build(deps): bump github.com/stretchr/testify from 1.12.0 to 1.12.1 (#1213)

    Bumps [github.com/stretchr/testify](https://github.com/stretchr/testify) from 1.12.0 to 1.12.1.
    - [Release notes](https://github.com/stretchr/testify/releases)
    - [Commits](stretchr/testify@v1.12.0...v1.12.1)

    ---
    updated-dependencies:
    - dependency-name: github.com/stretchr/testify
      dependency-version: 1.12.1
      dependency-type: direct:production
      update-type: version-update:semver-patch
    ...

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit 6e0906e
Author: Natalie Gaston <natalie.gaston@intel.com>
Date:   Thu Aug 20 18:38:13 2026 -0700

    fix(config): validate HTTP port and harden Windows browser launch (#1198)

commit a493e12
Author: Sinchana S R <sinchana.s.r@intel.com>
Date:   Fri Aug 21 04:18:50 2026 +0530

    refactor(api): prevent integer overflow in query parameters (#1180)

    * Add validation to prevent integer overflow in query parameters (top, skip, count).
    * Returns 400 instead of 500 on invalid input.

    Co-authored-by: Sudhir Pola <sudhir.pola@intel.com>

commit fe0cf5c
Author: ShradhaGupta31 <shradha.gupta@intel.com>
Date:   Fri Aug 21 04:14:50 2026 +0530

    refactor: reject JWT expiration value at startup if less than 1 sec (#1172)

    - Modified config.go to validate jwtExpiration while console startup
    - Reject non-positive values of jwtExpiration

    Signed-off-by: ShradhaGupta31 <shradha.gupta@intel.com>
    Co-authored-by: Madhavi Losetty <madhavi.losetty@intel.com>
    Co-authored-by: Ganesh Raikhelkar <ganesh.raikhelkar@intel.com>

commit 365531d
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Thu Aug 20 22:38:15 2026 +0000

    build(deps): bump github.com/stretchr/testify from 1.11.1 to 1.12.0 (#1205)

    Bumps [github.com/stretchr/testify](https://github.com/stretchr/testify) from 1.11.1 to 1.12.0.
    - [Release notes](https://github.com/stretchr/testify/releases)
    - [Commits](stretchr/testify@v1.11.1...v1.12.0)

    ---
    updated-dependencies:
    - dependency-name: github.com/stretchr/testify
      dependency-version: 1.12.0
      dependency-type: direct:production
      update-type: version-update:semver-minor
    ...

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
    Co-authored-by: Natalie Gaston <natalie.gaston@intel.com>

commit 4c585b1
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Thu Aug 20 15:35:21 2026 -0700

    build(deps): bump step-security/harden-runner from 2.20.1 to 2.21.0 (#1208)

    Bumps [step-security/harden-runner](https://github.com/step-security/harden-runner) from 2.20.1 to 2.21.0.
    - [Release notes](https://github.com/step-security/harden-runner/releases)
    - [Commits](step-security/harden-runner@b09bb98...05e3151)

    ---
    updated-dependencies:
    - dependency-name: step-security/harden-runner
      dependency-version: 2.21.0
      dependency-type: direct:production
      update-type: version-update:semver-minor
    ...

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
    Co-authored-by: DevipriyaS17 <devipriya.s@intel.com>

commit 9414597
Author: DevipriyaS17 <devipriya.s@intel.com>
Date:   Fri Aug 21 04:00:43 2026 +0530

    build(docker): default dev service port bindings to loopback (#1168)

    * build(docker): default dev service port bindings to loopback

    * build(docker): address review comments

    ---------

    Co-authored-by: Sudhir Pola <sudhir.pola@intel.com>

commit 001de52
Author: DevipriyaS17 <devipriya.s@intel.com>
Date:   Fri Aug 21 03:57:13 2026 +0530

    refactor(security): default useTLS to true on device creation (#1169)

    * fix(security): default useTLS to true on device creation

    * fix: add code coverage

    * refactor(security): address review comments

    * refactor(security): fix the codeql error

    ---------

    Co-authored-by: Ganesh Raikhelkar <ganesh.raikhelkar@intel.com>

commit 3003aa0
Author: Madhavi Losetty <madhavi.losetty@intel.com>
Date:   Thu Aug 20 15:20:56 2026 -0700

    refactor: address golangci-lint 2.13.1 findings (#1212)

commit 60d074d
Author: Madhavi Losetty <madhavi.losetty@intel.com>
Date:   Thu Aug 20 13:02:13 2026 -0700

    refactor(config): move config to user dir and enforce owner-only perms (#1078)

    Relocate config to the user directory and harden seedConfig error
    handling, enforcing owner-only file permissions.

    Co-authored-by: MadhaviLosetty <madhavi.losetty@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants