Skip to content

Commit 57c3f0c

Browse files
committed
fix: trust HA Ingress auth and build proper climate payload for hvac overrides
Two HA add-on fixes: - Web UI mutations (label edits, .cbz/XML imports) now work on a default add-on install with no web_api_key. Requests proxied through Home Assistant Ingress are already authenticated by HA, so the Supervisor-injected X-Ingress-Path header is trusted, but only when the server runs in ingress mode (basePath set) so a spoofed header can't bypass auth on a directly-exposed port. A configured web_api_key still takes precedence. - A per-group "hvac" type override now publishes a full climate entity (temperature/mode topics) via _createHvacDiscovery instead of a broken generic payload, and the climate config topic is tracked for stale cleanup. Docs clarify that only Lighting (56) is discovered by default and how to reclassify lighting-app groups via type_overrides.
1 parent 1b99143 commit 57c3f0c

6 files changed

Lines changed: 192 additions & 10 deletions

File tree

README.md

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -119,13 +119,14 @@ When enabled, `cgateweb` queries the C-Gate network structure (`TREEXML`) and pu
119119
Supported Device Types:
120120

121121
* **Lights:** C-Bus Lighting Application groups (typically App ID 56) are discovered as Home Assistant `light` entities, supporting on/off and brightness control.
122-
* **Covers:** Devices using the configured `ha_discovery_cover_app_id` (default: `203`) are discovered as `cover` entities (device class `shutter`), supporting:
122+
* **Covers:** Devices using the configured `ha_discovery_cover_app_id` (default: `null` — disabled; commonly set to `203` for Enable Control) are discovered as `cover` entities (device class `shutter`), supporting:
123123
- **Position control:** Set position 0-100% (0=closed, 100=fully open)
124124
- **Stop:** Stop the cover at its current position
125125
- **Open/Close:** Basic open/close commands
126126
* **Switches:** Devices using the configured `ha_discovery_switch_app_id` (default: `null`) are discovered as `switch` entities.
127127
* **Relays:** Devices using the configured `ha_discovery_relay_app_id` (default: `null`) are discovered as `switch` entities with device class `outlet`.
128128
* **PIR Motion Sensors:** Devices using the configured `ha_discovery_pir_app_id` (default: `null`) are discovered as `binary_sensor` entities with device class `motion`.
129+
* **HVAC / Climate:** Devices using the configured `ha_discovery_hvac_app_id` (default: `null` — disabled; commonly Air Conditioning App `201`) are discovered as `climate` entities.
129130

130131
**Configuration (`settings.js`):**
131132

@@ -138,11 +139,14 @@ module.exports = {
138139
ha_discovery_prefix: 'homeassistant', // Default HA discovery topic prefix
139140
ha_discovery_networks: ['254'], // List C-Bus network IDs to scan (e.g., ['254', '200'])
140141
141-
// Application IDs for specific device types (MUST match your C-Bus project configuration)
142-
ha_discovery_cover_app_id: '203', // App ID for Covers (e.g., Enable Control)
142+
// Application IDs for specific device types (MUST match your C-Bus project configuration).
143+
// All of these default to `null` (disabled). Only the Lighting application (56) is
144+
// discovered out of the box, and every Lighting group is published as a `light`.
145+
ha_discovery_cover_app_id: '203', // App ID for Covers (e.g., Enable Control) - null to disable (default)
143146
ha_discovery_switch_app_id: null, // App ID for Switches (e.g., Enable Control, Trigger Control) - null to disable
144147
ha_discovery_relay_app_id: null, // App ID for Relays (e.g., Enable Control) - null to disable
145-
ha_discovery_pir_app_id: null // App ID for PIR Motion Sensors (e.g., Trigger Control) - null to disable
148+
ha_discovery_pir_app_id: null, // App ID for PIR Motion Sensors (e.g., Trigger Control) - null to disable
149+
ha_discovery_hvac_app_id: null // App ID for HVAC/climate zones (e.g., Air Conditioning 201) - null to disable
146150
};
147151
```
148152

@@ -159,7 +163,8 @@ The crucial step is setting the correct `ha_discovery_*_app_id` values to match
159163

160164
**Important Notes:**
161165

162-
* Discovery for Switches, Relays, and PIRs is **disabled by default** (`null`). You *must* set the corresponding `ha_discovery_*_app_id` in `settings.js` to the correct C-Bus Application ID to enable discovery for these types.
166+
* Discovery for Covers, Switches, Relays, PIRs, and HVAC is **disabled by default** (`null`). Only the Lighting application (56) is discovered automatically, and **every** Lighting group is published as a `light`. You *must* set the corresponding `ha_discovery_*_app_id` in `settings.js` to the correct C-Bus Application ID to enable the other types.
167+
* **Devices that live on the Lighting application (56) but are not lights** — e.g. shutter-relay units (blinds use lighting group addresses) or a thermostat exposed on app 56 — will appear as `light` entities because classification is purely by Application ID. To reclassify an individual group, add a per-group `type_overrides` entry in your labels file (`"<net>/<app>/<group>": "cover" | "switch" | "relay" | "pir" | "hvac"`) or via the web UI; this is the only way to change the type of a group that shares the Lighting application with real lights.
163168
* If multiple discovery types (e.g., Cover and Switch) are configured with the *same* Application ID, `cgateweb` prioritizes discovery in this order: Cover > Switch > Relay > PIR. Only the first matching type will be discovered for a given C-Bus group using that Application ID.
164169
* For more technical details, see `docs/project-homeassistant-discovery.md`.
165170

homeassistant-addon/DOCS.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,8 @@ Disable auto-discovery (`auto_discover_networks: false`) if:
158158

159159
| Option | Type | Default | Description |
160160
|--------|------|---------|-------------|
161-
| `web_api_key` | password | (empty) | API key required for write operations (`PUT/PATCH/POST`) on label-management endpoints. |
162-
| `web_allow_unauthenticated_mutations` | boolean | `false` | Unsafe override to allow write operations without API key authentication. |
161+
| `web_api_key` | password | (empty) | API key required for write operations (`PUT/PATCH/POST`) on label-management endpoints when accessed **directly** (not via Ingress). Requests through Home Assistant Ingress are already authenticated by HA and do not need this key. |
162+
| `web_allow_unauthenticated_mutations` | boolean | `false` | Unsafe override to allow write operations without authentication on the **directly-exposed** port. Not needed for the Ingress UI. |
163163
| `web_allowed_origins` | list | `[]` | Optional CORS allowlist of browser origins (e.g. `https://ha.example.com`). Empty disables cross-origin access. |
164164
| `web_mutation_rate_limit_per_minute` | integer | `120` | Per-client write rate limit for label mutation endpoints. |
165165

@@ -360,11 +360,11 @@ Set `ha_discovery_hvac_app_id: 201` to enable HVAC discovery. Use `ha_hvac_tempe
360360

361361
This add-on runs with `host_network: false`.
362362

363-
- Ingress is enabled and routes the label editor UI through Home Assistant.
363+
- Ingress is enabled and routes the label editor UI through Home Assistant. Requests arriving via Ingress are already authenticated by Home Assistant (the Supervisor injects an `X-Ingress-Path` header), so label edits and `.cbz`/XML imports work out of the box with no `web_api_key`.
364364
- Port `8080/tcp` is exposed by the add-on for direct access if needed.
365365
- Outbound connections to remote C-Gate and MQTT still work normally from the add-on container.
366366

367-
If you expose `8080`, set `web_api_key` and keep `web_allow_unauthenticated_mutations: false`.
367+
If you expose `8080` for direct (non-Ingress) access, set `web_api_key` and keep `web_allow_unauthenticated_mutations: false`. Direct requests never carry the Ingress header, so they always require the key.
368368

369369
## Stale Device Detection
370370

src/haDiscovery.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -675,7 +675,14 @@ class HaDiscovery {
675675
const config = getDiscoveryConfig(typeOverride);
676676
if (config) {
677677
this.logger.debug(`Type override: ${labelKey} -> ${typeOverride}`);
678-
this._createDiscovery(networkId, appId, groupId, group.Label, config);
678+
if (config.isHvac) {
679+
// HVAC needs the dedicated climate payload (temperature/mode
680+
// topics); the generic builder would publish a broken
681+
// climate entity with no thermostat controls.
682+
this._createHvacDiscovery(networkId, appId, groupId, group.Label);
683+
} else {
684+
this._createDiscovery(networkId, appId, groupId, group.Label, config);
685+
}
679686
// Remove any stale retained light config for this group.
680687
// This covers the case where the type changes within the same session
681688
// (e.g. first run saw it as a light; this run sees the type override).
@@ -857,6 +864,7 @@ class HaDiscovery {
857864
};
858865

859866
this._publish(discoveryTopic, JSON.stringify(payload), MQTT_RETAINED_STATE_OPTIONS);
867+
if (this._currentRunTopics) this._currentRunTopics.add(discoveryTopic);
860868
this.discoveryCount++;
861869
}
862870

src/webServer.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,16 @@ class WebServer {
648648

649649
_isAuthorizedMutation(req) {
650650
if (!this.apiKey) {
651+
// Requests proxied through Home Assistant Ingress have already been
652+
// authenticated by HA (only logged-in HA users can reach the ingress
653+
// URL). The Supervisor injects an X-Ingress-Path header on every such
654+
// request, which the directly-exposed port never carries. Trusting it
655+
// lets the bundled label UI import/edit on a default add-on install
656+
// (no web_api_key) without opening up the raw port. A configured
657+
// web_api_key still takes precedence below.
658+
if (this._isIngressRequest(req)) {
659+
return true;
660+
}
651661
return this.allowUnauthenticatedMutations;
652662
}
653663

@@ -665,6 +675,16 @@ class WebServer {
665675
return crypto.timingSafeEqual(providedBuf, expectedBuf);
666676
}
667677

678+
_isIngressRequest(req) {
679+
// Only trust the ingress marker when the server was actually started in
680+
// ingress mode (INGRESS_ENTRY -> basePath). This prevents a spoofed
681+
// X-Ingress-Path header from bypassing auth on a standalone deployment
682+
// that is not fronted by the Supervisor proxy.
683+
if (!this.basePath) return false;
684+
const ingressPath = req.headers['x-ingress-path'];
685+
return typeof ingressPath === 'string' && ingressPath.length > 0;
686+
}
687+
668688
_setCorsHeaders(req, res) {
669689
const requestOrigin = req.headers.origin;
670690
if (this.allowedOrigins && this.allowedOrigins.length > 0) {

tests/haDiscovery.test.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1152,6 +1152,41 @@ describe('HaDiscovery', () => {
11521152
expect(payload.device.name).toBe('Pond Pump');
11531153
});
11541154

1155+
it('should override a lighting group to hvac (climate) type with a full climate payload', () => {
1156+
// A thermostat wired on the lighting application (app 56) can only be
1157+
// reclassified via a per-group type override. The resulting entity must
1158+
// be a real HA climate device with temperature/mode topics — not a
1159+
// generic payload published to the climate component (which renders as a
1160+
// broken thermostat in HA).
1161+
const labelData = {
1162+
labels: new Map([['254/56/12', 'Hallway Thermostat']]),
1163+
typeOverrides: new Map([['254/56/12', 'hvac']]),
1164+
entityIds: new Map(),
1165+
exclude: new Set()
1166+
};
1167+
const ha = new HaDiscovery(mockSettings, mockPublishFn, mockSendCommandFn, labelData);
1168+
ha._publishDiscoveryFromTree('254', MOCK_TREEXML_RESULT_NET254);
1169+
1170+
// The stale light config for this group should be cleared.
1171+
const lightCall = mockPublishFn.mock.calls.find(
1172+
c => c[0] === 'testhomeassistant/light/cgateweb_254_56_12/config'
1173+
);
1174+
expect(lightCall).toBeDefined();
1175+
expect(lightCall[1]).toBe('');
1176+
1177+
const climateCall = mockPublishFn.mock.calls.find(
1178+
c => c[0] === 'testhomeassistant/climate/cgateweb_254_56_12/config'
1179+
);
1180+
expect(climateCall).toBeDefined();
1181+
const payload = JSON.parse(climateCall[1]);
1182+
expect(payload.device.name).toBe('Hallway Thermostat');
1183+
expect(payload.current_temperature_topic).toBe('cbus/read/254/56/12/current_temperature');
1184+
expect(payload.temperature_command_topic).toBe('cbus/write/254/56/12/setpoint');
1185+
expect(payload.temperature_state_topic).toBe('cbus/read/254/56/12/setpoint');
1186+
expect(payload.mode_command_topic).toBe('cbus/write/254/56/12/hvacmode');
1187+
expect(payload.modes).toEqual(expect.arrayContaining(['off', 'auto', 'cool', 'heat', 'fan_only']));
1188+
});
1189+
11551190
it('should inject default_entity_id with domain prefix when entity_ids has an entry', () => {
11561191
const labelData = {
11571192
labels: new Map([['254/56/10', 'Kitchen']]),

tests/webServer.test.js

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,120 @@ describe('WebServer', () => {
541541
});
542542
});
543543

544+
describe('Ingress authentication (HA-authenticated requests)', () => {
545+
let ingressServer;
546+
let ingressPort;
547+
const BASE = '/api/hassio_ingress/abc123';
548+
549+
afterEach(async () => {
550+
if (ingressServer) await ingressServer.close();
551+
ingressServer = null;
552+
});
553+
554+
const patchThroughIngress = (port, headers = {}) => new Promise((resolve, reject) => {
555+
const req = http.request({
556+
hostname: '127.0.0.1',
557+
port,
558+
path: `${BASE}/api/labels`,
559+
method: 'PATCH',
560+
headers: { 'Content-Type': 'application/json', ...headers }
561+
}, (res) => {
562+
let data = '';
563+
res.on('data', (chunk) => { data += chunk; });
564+
res.on('end', () => resolve({ status: res.statusCode, body: JSON.parse(data) }));
565+
});
566+
req.on('error', reject);
567+
req.write(JSON.stringify({ '254/56/10': 'Patched' }));
568+
req.end();
569+
});
570+
571+
it('allows a mutation that arrives via HA Ingress when no API key is set (default add-on install)', async () => {
572+
// Reproduces the user bug: a default HA add-on install has no
573+
// web_api_key and web_allow_unauthenticated_mutations=false. The
574+
// bundled UI is served through ingress, which HA has already
575+
// authenticated, so the Supervisor-injected X-Ingress-Path header
576+
// must be trusted. Previously this returned 401 "Unauthorized".
577+
ingressServer = new WebServer({
578+
port: 0,
579+
basePath: BASE,
580+
labelLoader,
581+
getStatus: () => ({})
582+
});
583+
await ingressServer.start();
584+
ingressPort = ingressServer._server.address().port;
585+
586+
const res = await patchThroughIngress(ingressPort, { 'X-Ingress-Path': BASE });
587+
expect(res.status).toBe(200);
588+
});
589+
590+
it('still rejects a non-ingress unauthenticated mutation when no API key is set', async () => {
591+
// A direct request (no X-Ingress-Path) must remain blocked so an
592+
// exposed port is not opened up by the ingress trust.
593+
ingressServer = new WebServer({
594+
port: 0,
595+
basePath: BASE,
596+
labelLoader,
597+
getStatus: () => ({})
598+
});
599+
await ingressServer.start();
600+
ingressPort = ingressServer._server.address().port;
601+
602+
const res = await patchThroughIngress(ingressPort);
603+
expect(res.status).toBe(401);
604+
});
605+
606+
it('ignores X-Ingress-Path when the server is not running in ingress mode', async () => {
607+
// Without basePath the server is not behind ingress; a spoofed
608+
// X-Ingress-Path header must not grant access.
609+
ingressServer = new WebServer({
610+
port: 0,
611+
labelLoader,
612+
getStatus: () => ({})
613+
});
614+
await ingressServer.start();
615+
ingressPort = ingressServer._server.address().port;
616+
617+
const res = await new Promise((resolve, reject) => {
618+
const req = http.request({
619+
hostname: '127.0.0.1',
620+
port: ingressPort,
621+
path: '/api/labels',
622+
method: 'PATCH',
623+
headers: { 'Content-Type': 'application/json', 'X-Ingress-Path': '/spoofed' }
624+
}, (response) => {
625+
let data = '';
626+
response.on('data', (chunk) => { data += chunk; });
627+
response.on('end', () => resolve({ status: response.statusCode, body: JSON.parse(data) }));
628+
});
629+
req.on('error', reject);
630+
req.write(JSON.stringify({ '254/56/10': 'Patched' }));
631+
req.end();
632+
});
633+
634+
expect(res.status).toBe(401);
635+
});
636+
637+
it('still enforces a configured API key even for ingress requests', async () => {
638+
// When an operator has explicitly set web_api_key (e.g. to harden an
639+
// exposed port), it must always win over ingress trust.
640+
ingressServer = new WebServer({
641+
port: 0,
642+
basePath: BASE,
643+
labelLoader,
644+
apiKey: 'secret-key',
645+
getStatus: () => ({})
646+
});
647+
await ingressServer.start();
648+
ingressPort = ingressServer._server.address().port;
649+
650+
const res = await patchThroughIngress(ingressPort, { 'X-Ingress-Path': BASE });
651+
expect(res.status).toBe(401);
652+
653+
const ok = await patchThroughIngress(ingressPort, { 'X-Ingress-Path': BASE, 'X-API-Key': 'secret-key' });
654+
expect(ok.status).toBe(200);
655+
});
656+
});
657+
544658
describe('Mutation rate limiting', () => {
545659
let limitedServer;
546660
let limitedPort;

0 commit comments

Comments
 (0)