Skip to content

Commit d02a784

Browse files
CIclaude
andcommitted
fix(web): let /apps Edit reveal a masked credential instead of offering "xxx"
Since springfall2008#5053 the /apps page masks credentials through mask_secret_args() before rendering, which is right for what the page shows but also made the masked copy the row's data-original-value. Clicking Edit on a credential therefore filled the input with "xxx": the real key could be neither seen nor amended, only retyped, and saving it unchanged was (correctly) refused. The page stays masked. Rows whose value was masked now carry data-secret="1", and Edit on such a row fetches the one real value from a new GET /apps_value?path= endpoint before opening the input, replacing the row's original with it so an unchanged Apply is not seen as a change. The endpoint returns a single scalar only - a container or unknown path is refused - and exposes nothing the unmasked /debug_apps download does not already. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent ee1b056 commit d02a784

3 files changed

Lines changed: 124 additions & 6 deletions

File tree

‎apps/predbat/tests/test_web_apps_edit.py‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,14 @@ async def post(self):
6767
return self.postdata
6868

6969

70+
class FakeGetRequest:
71+
"""A minimal aiohttp-request stand-in exposing only the query string the handler reads."""
72+
73+
def __init__(self, query):
74+
"""Store the query parameters this request will hand back."""
75+
self.query = query
76+
77+
7078
def _load_yaml(path="apps.yaml"):
7179
"""Load a YAML file from the temporary working directory."""
7280
yaml = YAML()
@@ -502,6 +510,53 @@ def run_web_apps_edit_tests(my_predbat):
502510
print(" ERROR: the rotated credential was not written")
503511
failed += 1
504512

513+
# ---------------------------------------------------------------------
514+
# The page serving the mask meant clicking Edit on a credential filled the input with
515+
# "xxx": the real key could not be seen or amended, only retyped from scratch. The page
516+
# stays masked; Edit fetches the one real value through /apps_value instead.
517+
print("Test: credential rows are flagged for reveal-on-edit and still served masked")
518+
text = _render_apps_page(my_predbat, {"octopus_api_key": "sk-top-level-credential-0000", "chat": {"providers": {"openrouter": {"api_key": "sk-live-nested-credential-1234", "model": "some-model"}}}})
519+
if "sk-top-level-credential-0000" in text or "sk-live-nested-credential-1234" in text:
520+
print(" ERROR: a credential reached the rendered page in the clear")
521+
failed += 1
522+
if not re.search(r'<tr id="row_\d+" data-arg-name="octopus_api_key" data-original-value="xxx" data-secret="1">', text):
523+
print(" ERROR: the top-level credential row should be masked and flagged data-secret")
524+
failed += 1
525+
if not re.search(r"data-nested-path='chat\.providers\.openrouter\.api_key' data-nested-original='xxx' data-secret='1'", text):
526+
print(" ERROR: the nested credential row should be masked and flagged data-secret")
527+
failed += 1
528+
if re.search(r"data-nested-path='chat\.providers\.openrouter\.model'[^>]*data-secret", text):
529+
print(" ERROR: a non-credential sibling must not be flagged data-secret")
530+
failed += 1
531+
532+
print("Test: /apps_value hands back the real value of the credential being edited")
533+
web_interface = _reset_fixture(my_predbat)
534+
for path, expected in (("chat.providers.openrouter.api_key", "sk-live-nested-credential-1234"), ("forecast_solar[0].api_key", "fs-live-credential-5678"), ("forecast_solar[0].declination", "30")):
535+
result = json.loads(asyncio.run(web_interface.html_apps_value(FakeGetRequest({"path": path}))).text)
536+
if not result.get("success") or result.get("value") != expected:
537+
print(" ERROR: expected /apps_value for {} to return {}, got: {}".format(path, expected, result))
538+
failed += 1
539+
540+
print("Test: /apps_value refuses a missing path or a whole structure")
541+
for query in ({}, {"path": "chat.providers.nope"}, {"path": "chat.providers"}):
542+
result = json.loads(asyncio.run(web_interface.html_apps_value(FakeGetRequest(query))).text)
543+
if result.get("success") or "value" in result:
544+
print(" ERROR: expected /apps_value to refuse {}, got: {}".format(query, result))
545+
failed += 1
546+
547+
print("Test: Edit reveals a flagged credential before opening the input")
548+
for function, attribute in (("editValue", "originalValue"), ("editNestedValue", "nestedOriginal")):
549+
edit_src = apps_js[apps_js.index("function {}(".format(function)) :]
550+
edit_src = edit_src[: edit_src.index("\n}\n")]
551+
if "row.dataset.secret === '1'" not in edit_src or "revealSecretValue(row, '{}'".format(attribute) not in edit_src:
552+
print(" ERROR: expected {} to fetch the real value of a data-secret row, got:\n{}".format(function, edit_src))
553+
failed += 1
554+
reveal_src = apps_js[apps_js.index("async function revealSecretValue(") :]
555+
reveal_src = reveal_src[: reveal_src.index("\n}\n")]
556+
if "fetch('./apps_value?path=' + encodeURIComponent(path))" not in reveal_src or "delete row.dataset.secret" not in reveal_src:
557+
print(" ERROR: expected revealSecretValue to fetch /apps_value and replace the masked original, got:\n{}".format(reveal_src))
558+
failed += 1
559+
505560
finally:
506561
os.chdir(original_dir)
507562
shutil.rmtree(temp_dir, ignore_errors=True)

‎apps/predbat/web.py‎

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@
9191
predbat_log_file_prev,
9292
is_secret_key,
9393
)
94-
from utils import is_data_numerical, ROOT_YAML_KEY, SECRET_MASK, YAML_DUMP_WIDTH, parse_yaml_path, update_nested_yaml_value # noqa: F401 - re-exported: moved to utils.py, agent_tools.py/chat_tools.py must not import from web.py
94+
from utils import is_data_numerical, ROOT_YAML_KEY, SECRET_MASK, YAML_DUMP_WIDTH, parse_yaml_path, resolve_nested_yaml_value, update_nested_yaml_value # noqa: F401 - re-exported: moved to utils.py, agent_tools.py/chat_tools.py must not import from web.py
9595
from const import TIME_FORMAT, TIME_FORMAT_DAILY, TIME_FORMAT_HA, MANUAL_RATE_MAX_MINUTES, MANUAL_TIME_MAX_MINUTES
9696
from predbat import THIS_VERSION_DISPLAY
9797
from component_base import ComponentBase
@@ -514,6 +514,7 @@ async def start(self):
514514
app.router.add_get("/log", self.html_log)
515515
app.router.add_get("/apps", self.html_apps)
516516
app.router.add_post("/apps", self.html_apps_post)
517+
app.router.add_get("/apps_value", self.html_apps_value)
517518
app.router.add_get("/charts", self.html_charts)
518519
app.router.add_get("/config", self.html_config)
519520
app.router.add_get("/entity", self.html_entity)
@@ -2950,9 +2951,10 @@ def render_type(self, arg, value, parent_path="", row_counter=None):
29502951
actions_cell += self.render_delete_button(nested_row_id)
29512952

29522953
raw_value = self.resolve_value_raw(key, nested_value)
2954+
secret_attr = self.secret_row_attr(key, nested_value, "'")
29532955

29542956
if nested_row_id is not None:
2955-
text += f"<tr id='nested_row_{nested_row_id}' data-nested-path='{nested_path_attr}' data-nested-original='{html_module.escape(str(raw_value))}'><td><b>{key}: </b></td><td id='nested_value_{nested_row_id}'>{self.render_type(key, nested_value, nested_path, row_counter)}</td><td>{actions_cell}</td></tr>\n"
2957+
text += f"<tr id='nested_row_{nested_row_id}' data-nested-path='{nested_path_attr}' data-nested-original='{html_module.escape(str(raw_value))}'{secret_attr}><td><b>{key}: </b></td><td id='nested_value_{nested_row_id}'>{self.render_type(key, nested_value, nested_path, row_counter)}</td><td>{actions_cell}</td></tr>\n"
29562958
else:
29572959
text += "<tr><td><b>{}: </b></td><td colspan='2'>{}</td></tr>\n".format(key, self.render_type(key, nested_value, nested_path, row_counter))
29582960
text += self.render_add_row("addDictKey", [dict_path], "Add setting", row_counter)
@@ -3752,6 +3754,41 @@ def is_editable_value(self, value):
37523754
return len(value) > 0 and any(self.is_editable_value(item) for item in value)
37533755
return False
37543756

3757+
def secret_row_attr(self, key, value, quote):
3758+
"""
3759+
Return the data-secret attribute for a row whose value html_apps() served masked, else "".
3760+
3761+
The page holds SECRET_MASK for a credential rather than the credential itself, so the
3762+
browser's Edit has to fetch the real value (/apps_value) before it can offer it - this flag
3763+
is how it knows to. Only a scalar leaf is flagged: mask_secret_args() replaces a secret
3764+
key's whole value, list or not, so this is the row that holds the mask.
3765+
"""
3766+
if value == SECRET_MASK and is_secret_key(key):
3767+
return " data-secret={}1{}".format(quote, quote)
3768+
return ""
3769+
3770+
async def html_apps_value(self, request):
3771+
"""
3772+
Return the real value of one apps.yaml setting, for the /apps editor's Edit on a credential.
3773+
3774+
html_apps() serves credentials masked, so without this Edit could only offer "xxx" - the
3775+
user could neither see nor amend the key, and saving it back unchanged is refused. Only
3776+
the one path asked for is returned, and only when it is a single value, so the page itself
3777+
still never carries credentials and a whole container cannot be pulled in the clear. The
3778+
unmasked /debug_apps download already exposes the same values behind the same access.
3779+
"""
3780+
path = request.query.get("path", "")
3781+
if not path:
3782+
return web.json_response({"success": False, "message": "No path given"})
3783+
try:
3784+
value = resolve_nested_yaml_value(self.args, path)
3785+
except (KeyError, TypeError, ValueError) as e:
3786+
return web.json_response({"success": False, "message": f"Path {path} not found: {str(e)}"})
3787+
if not self.is_editable_value(value) or isinstance(value, list):
3788+
return web.json_response({"success": False, "message": f"{path} is not a single value that can be edited here"})
3789+
leaf = [segment for segment in parse_yaml_path(path) if not segment.startswith("[")][-1]
3790+
return web.json_response({"success": True, "value": str(self.resolve_value_raw(leaf, value))})
3791+
37553792
def resolve_value_raw(self, arg, value):
37563793
if isinstance(value, str) and "{" in value:
37573794
text = self.base.resolve_arg(arg, value, indirect=False, quiet=True)
@@ -3870,8 +3907,8 @@ async def html_apps(self, request):
38703907
# For numerical values, show edit button
38713908
actions_cell = f'<button class="edit-button" onclick="editValue({row_id})">Edit</button>'
38723909

3873-
text += '<tr id="row_{}" data-arg-name="{}" data-original-value="{}"><td>{}</td><td id="value_{}">{}</td><td>{}</td></tr>\n'.format(
3874-
row_id, arg_attr, html_module.escape(str(raw_value)), arg, row_id, self.render_type(arg, value, "", row_counter), actions_cell
3910+
text += '<tr id="row_{}" data-arg-name="{}" data-original-value="{}"{}><td>{}</td><td id="value_{}">{}</td><td>{}</td></tr>\n'.format(
3911+
row_id, arg_attr, html_module.escape(str(raw_value)), self.secret_row_attr(arg, value, '"'), arg, row_id, self.render_type(arg, value, "", row_counter), actions_cell
38753912
)
38763913
row_id += 1
38773914

‎apps/predbat/web_helper.py‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -940,10 +940,33 @@ def get_apps_js(all_states_json):
940940
updateChangeCounter();
941941
}
942942
943-
function editValue(rowId) {
943+
async function revealSecretValue(row, attribute, path) {
944+
// The page serves credentials masked, so a data-secret row's original is the placeholder,
945+
// not the key - fetch the real value once and make it the row's original, so the input
946+
// offers the real key and saving it unchanged is not seen as a change
947+
try {
948+
const response = await fetch('./apps_value?path=' + encodeURIComponent(path));
949+
const result = await response.json();
950+
if (!result.success) {
951+
showMessage(result.message, 'error');
952+
return false;
953+
}
954+
row.dataset[attribute] = result.value;
955+
delete row.dataset.secret;
956+
return true;
957+
} catch (error) {
958+
showMessage('Could not load the value to edit: ' + error.message, 'error');
959+
return false;
960+
}
961+
}
962+
963+
async function editValue(rowId) {
944964
const row = document.getElementById('row_' + rowId);
945965
const valueCell = document.getElementById('value_' + rowId);
946966
const argName = row.dataset.argName;
967+
if (row.dataset.secret === '1' && !(await revealSecretValue(row, 'originalValue', argName))) {
968+
return;
969+
}
947970
const originalValue = row.dataset.originalValue;
948971
949972
// Check if there's a pending change, use that value instead of original
@@ -1636,10 +1659,13 @@ def get_apps_js(all_states_json):
16361659
updateChangeCounter();
16371660
}
16381661
1639-
function editNestedValue(rowId) {
1662+
async function editNestedValue(rowId) {
16401663
const row = document.getElementById('nested_row_' + rowId);
16411664
const valueCell = document.getElementById('nested_value_' + rowId);
16421665
const nestedPath = row.dataset.nestedPath;
1666+
if (row.dataset.secret === '1' && !(await revealSecretValue(row, 'nestedOriginal', nestedPath))) {
1667+
return;
1668+
}
16431669
const originalValue = row.dataset.nestedOriginal;
16441670
16451671
// Check if there's a pending change, use that value instead of original

0 commit comments

Comments
 (0)