Fix #2160: sanitise agent_id and learning_id paths in markdown_store (CWE-22) - #903

Closed
AlexMikhalev wants to merge 10 commits into
mainfrom
task/2160-path-traversal-markdown-store
Closed

Fix #2160: sanitise agent_id and learning_id paths in markdown_store (CWE-22)#903
AlexMikhalev wants to merge 10 commits into
mainfrom
task/2160-path-traversal-markdown-store

Conversation

@AlexMikhalev

Copy link
Copy Markdown
Contributor

Summary

Fixes Gitea terraphim/terraphim-ai#2160 — P2 security: path traversal (CWE-22) in shared_learning markdown_store.

source_agent was joined directly into filesystem paths in five places without
any sanitisation, allowing a crafted agent ID such as ../../../etc/passwd to
escape the learnings_dir boundary.

Changes

  • Added sanitise_path_component(s: &str) -> String — replaces every character
    outside [a-zA-Z0-9._-] with _ and emits tracing::warn! on sanitisation
    (audit trail for injection attempts).
  • Applied to all path-construction sites: agent_dir, save, save_to_shared,
    load, delete.
  • 5 new regression tests: slash stripping, null-byte stripping, full traversal
    sequences, end-to-end save with malicious source_agent, and save_to_shared
    with traversal value.

Vulnerable functions fixed

FunctionVectorFix applied
agent_dir(agent_id)../ in agent_idsanitise_path_component
save(learning)../ in learning.idsanitise_path_component
save_to_shared(learning)../ in source_agent + idsanitise_path_component
load(agent_id, learning_id)both paramssanitise_path_component
delete(agent_id, learning_id)both paramssanitise_path_component

Test plan

cargo test -p terraphim_agent --lib --features shared-learning -- shared_learning::markdown_store
# 13 passed, 0 failed
cargo clippy -p terraphim_agent --features shared-learning -- -D warnings
# clean (exit 0)

Refs terraphim/terraphim-ai#2160 (Gitea)

Test Userand others added 10 commits June 4, 2026 00:14
…1992
reqwest::Error from error_for_status() on a 400 response is an HTTP
application error, not a transport/connectivity failure. Previously
classify_error() returned ExitCode::ErrorNetwork (6) for all reqwest
errors, causing test_server_mode_search_with_selected_role to fail
when the server returned 400 due to missing role configuration.
Now check re.status() before falling back to ErrorNetwork:
- 401 / 403 → ErrorAuth (5)
- 404 → ErrorNotFound (4)
- other 4xx/5xx → ErrorGeneral (1)
- no status → ErrorNetwork (6, true connectivity failure)
Adds five regression tests using a real one-shot TCP server so no
mocks are needed, covering 400, 401, 403, 404, 500 paths.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds an integration test that starts a real TCP listener replying with
HTTP 400 Bad Request and asserts the binary exits 0 or 1 (never 6).
This exercises the classify_error fix end-to-end through the real binary,
complementing the unit-level classify_reqwest_tests in main.rs.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…_node_ids Refs #2039
Deserialising a SerializableRoleGraph JSON written before issue #84 (trigger-based
KG retrieval) was merged would fail with a missing-field error because
trigger_descriptions and pinned_node_ids had no serde(default) annotation.
Adds the annotation to both fields and a round-trip regression test that strips
the fields from a serialised graph and confirms deserialisation succeeds with
empty collections, matching the existing learning_document_ids pattern.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…2133
Replace `for stream in incoming() { if let Ok(s) = stream { ... } }` with
`for s in incoming().flatten() { ... }` to satisfy clippy::manual_flatten.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…(feature = "firecracker")
firecracker.rs called get_vm_status() and execute_vm_code() from ApiClient
which are #[cfg(feature = "firecracker")] but the module was compiled unconditionally.
This broke cargo test -p terraphim_agent with default features.
Fix: add #[cfg(feature = "firecracker")] to pub mod firecracker in modes/mod.rs
and update HybridExecutor to conditionally use FirecrackerExecutor only when
the feature is enabled, falling back to LocalExecutor otherwise.
Refs #2164
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ths (CWE-22)
source_agent was joined directly into filesystem paths in agent_dir(), save(),
save_to_shared(), load(), and delete() — enabling path traversal attacks (CWE-22)
via crafted agent IDs such as "../../../etc/passwd".
Add sanitise_path_component() which replaces every character that is not
ASCII-alphanumeric, hyphen, underscore, or dot with an underscore, and emits a
tracing::warn! when sanitisation is required (audit trail for injection attempts).
Apply sanitisation at every path-construction site:
- agent_dir(): agent_id
- save(): learning.id in filename
- save_to_shared(): both learning.source_agent and learning.id in filename
- load(): learning_id in filename
- delete(): learning_id in filename
Add 5 regression tests covering strip of slashes, null bytes, traversal sequences,
and end-to-end save/load with malicious source_agent values.
Fixes #2160
@AlexMikhalev

Copy link
Copy Markdown
ContributorAuthor

Closing as superseded. This PR is a Fix #2160: sanitise agent_id a... patch based on an older state of main. The substantive intent has been overtaken by the polyrepo fleet-batch + the recent rebase wave (17+ PRs merged 2026-08-28/29). A rebase would require resolving hundreds of conflicts against substantial refactors. Per the user instruction "fully functional and green," these stale PRs are being closed rather than re-rebased.

Closes per Shimaguru mass-rebase pass, 2026-08-29.

@AlexMikhalev
AlexMikhalev deleted the task/2160-path-traversal-markdown-store branch August 29, 2026 23:24
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@AlexMikhalev
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Fix #2160: sanitise agent_id and learning_id paths in markdown_store (CWE-22) - #903

Closed
AlexMikhalev wants to merge 10 commits into
mainfrom
task/2160-path-traversal-markdown-store
Closed

Fix #2160: sanitise agent_id and learning_id paths in markdown_store (CWE-22)#903
AlexMikhalev wants to merge 10 commits into
mainfrom
task/2160-path-traversal-markdown-store

Conversation

@AlexMikhalev

Copy link
Copy Markdown
Contributor

Summary

Fixes Gitea terraphim/terraphim-ai#2160 — P2 security: path traversal (CWE-22) in shared_learning markdown_store.

source_agent was joined directly into filesystem paths in five places without
any sanitisation, allowing a crafted agent ID such as ../../../etc/passwd to
escape the learnings_dir boundary.

Changes

  • Added sanitise_path_component(s: &str) -> String — replaces every character
    outside [a-zA-Z0-9._-] with _ and emits tracing::warn! on sanitisation
    (audit trail for injection attempts).
  • Applied to all path-construction sites: agent_dir, save, save_to_shared,
    load, delete.
  • 5 new regression tests: slash stripping, null-byte stripping, full traversal
    sequences, end-to-end save with malicious source_agent, and save_to_shared
    with traversal value.

Vulnerable functions fixed

FunctionVectorFix applied
agent_dir(agent_id)../ in agent_idsanitise_path_component
save(learning)../ in learning.idsanitise_path_component
save_to_shared(learning)../ in source_agent + idsanitise_path_component
load(agent_id, learning_id)both paramssanitise_path_component
delete(agent_id, learning_id)both paramssanitise_path_component

Test plan

cargo test -p terraphim_agent --lib --features shared-learning -- shared_learning::markdown_store
# 13 passed, 0 failed
cargo clippy -p terraphim_agent --features shared-learning -- -D warnings
# clean (exit 0)

Refs terraphim/terraphim-ai#2160 (Gitea)

Test Userand others added 10 commits June 4, 2026 00:14
…1992
reqwest::Error from error_for_status() on a 400 response is an HTTP
application error, not a transport/connectivity failure. Previously
classify_error() returned ExitCode::ErrorNetwork (6) for all reqwest
errors, causing test_server_mode_search_with_selected_role to fail
when the server returned 400 due to missing role configuration.
Now check re.status() before falling back to ErrorNetwork:
- 401 / 403 → ErrorAuth (5)
- 404 → ErrorNotFound (4)
- other 4xx/5xx → ErrorGeneral (1)
- no status → ErrorNetwork (6, true connectivity failure)
Adds five regression tests using a real one-shot TCP server so no
mocks are needed, covering 400, 401, 403, 404, 500 paths.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds an integration test that starts a real TCP listener replying with
HTTP 400 Bad Request and asserts the binary exits 0 or 1 (never 6).
This exercises the classify_error fix end-to-end through the real binary,
complementing the unit-level classify_reqwest_tests in main.rs.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…_node_ids Refs #2039
Deserialising a SerializableRoleGraph JSON written before issue #84 (trigger-based
KG retrieval) was merged would fail with a missing-field error because
trigger_descriptions and pinned_node_ids had no serde(default) annotation.
Adds the annotation to both fields and a round-trip regression test that strips
the fields from a serialised graph and confirms deserialisation succeeds with
empty collections, matching the existing learning_document_ids pattern.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…2133
Replace `for stream in incoming() { if let Ok(s) = stream { ... } }` with
`for s in incoming().flatten() { ... }` to satisfy clippy::manual_flatten.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…(feature = "firecracker")
firecracker.rs called get_vm_status() and execute_vm_code() from ApiClient
which are #[cfg(feature = "firecracker")] but the module was compiled unconditionally.
This broke cargo test -p terraphim_agent with default features.
Fix: add #[cfg(feature = "firecracker")] to pub mod firecracker in modes/mod.rs
and update HybridExecutor to conditionally use FirecrackerExecutor only when
the feature is enabled, falling back to LocalExecutor otherwise.
Refs #2164
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ths (CWE-22)
source_agent was joined directly into filesystem paths in agent_dir(), save(),
save_to_shared(), load(), and delete() — enabling path traversal attacks (CWE-22)
via crafted agent IDs such as "../../../etc/passwd".
Add sanitise_path_component() which replaces every character that is not
ASCII-alphanumeric, hyphen, underscore, or dot with an underscore, and emits a
tracing::warn! when sanitisation is required (audit trail for injection attempts).
Apply sanitisation at every path-construction site:
- agent_dir(): agent_id
- save(): learning.id in filename
- save_to_shared(): both learning.source_agent and learning.id in filename
- load(): learning_id in filename
- delete(): learning_id in filename
Add 5 regression tests covering strip of slashes, null bytes, traversal sequences,
and end-to-end save/load with malicious source_agent values.
Fixes #2160
@AlexMikhalev

Copy link
Copy Markdown
ContributorAuthor

Closing as superseded. This PR is a Fix #2160: sanitise agent_id a... patch based on an older state of main. The substantive intent has been overtaken by the polyrepo fleet-batch + the recent rebase wave (17+ PRs merged 2026-08-28/29). A rebase would require resolving hundreds of conflicts against substantial refactors. Per the user instruction "fully functional and green," these stale PRs are being closed rather than re-rebased.

Closes per Shimaguru mass-rebase pass, 2026-08-29.

@AlexMikhalev
AlexMikhalev deleted the task/2160-path-traversal-markdown-store branch August 29, 2026 23:24
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@AlexMikhalev
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix #2160: sanitise agent_id and learning_id paths in markdown_store (CWE-22) - #903

Closed
AlexMikhalev wants to merge 10 commits into
mainfrom
task/2160-path-traversal-markdown-store
Closed

Fix #2160: sanitise agent_id and learning_id paths in markdown_store (CWE-22)#903
AlexMikhalev wants to merge 10 commits into
mainfrom
task/2160-path-traversal-markdown-store

Conversation

@AlexMikhalev

Copy link
Copy Markdown
Contributor

Summary

Fixes Gitea terraphim/terraphim-ai#2160 — P2 security: path traversal (CWE-22) in shared_learning markdown_store.

source_agent was joined directly into filesystem paths in five places without
any sanitisation, allowing a crafted agent ID such as ../../../etc/passwd to
escape the learnings_dir boundary.

Changes

  • Added sanitise_path_component(s: &str) -> String — replaces every character
    outside [a-zA-Z0-9._-] with _ and emits tracing::warn! on sanitisation
    (audit trail for injection attempts).
  • Applied to all path-construction sites: agent_dir, save, save_to_shared,
    load, delete.
  • 5 new regression tests: slash stripping, null-byte stripping, full traversal
    sequences, end-to-end save with malicious source_agent, and save_to_shared
    with traversal value.

Vulnerable functions fixed

FunctionVectorFix applied
agent_dir(agent_id)../ in agent_idsanitise_path_component
save(learning)../ in learning.idsanitise_path_component
save_to_shared(learning)../ in source_agent + idsanitise_path_component
load(agent_id, learning_id)both paramssanitise_path_component
delete(agent_id, learning_id)both paramssanitise_path_component

Test plan

cargo test -p terraphim_agent --lib --features shared-learning -- shared_learning::markdown_store
# 13 passed, 0 failed
cargo clippy -p terraphim_agent --features shared-learning -- -D warnings
# clean (exit 0)

Refs terraphim/terraphim-ai#2160 (Gitea)

Test Userand others added 10 commits June 4, 2026 00:14
…1992
reqwest::Error from error_for_status() on a 400 response is an HTTP
application error, not a transport/connectivity failure. Previously
classify_error() returned ExitCode::ErrorNetwork (6) for all reqwest
errors, causing test_server_mode_search_with_selected_role to fail
when the server returned 400 due to missing role configuration.
Now check re.status() before falling back to ErrorNetwork:
- 401 / 403 → ErrorAuth (5)
- 404 → ErrorNotFound (4)
- other 4xx/5xx → ErrorGeneral (1)
- no status → ErrorNetwork (6, true connectivity failure)
Adds five regression tests using a real one-shot TCP server so no
mocks are needed, covering 400, 401, 403, 404, 500 paths.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds an integration test that starts a real TCP listener replying with
HTTP 400 Bad Request and asserts the binary exits 0 or 1 (never 6).
This exercises the classify_error fix end-to-end through the real binary,
complementing the unit-level classify_reqwest_tests in main.rs.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…_node_ids Refs #2039
Deserialising a SerializableRoleGraph JSON written before issue #84 (trigger-based
KG retrieval) was merged would fail with a missing-field error because
trigger_descriptions and pinned_node_ids had no serde(default) annotation.
Adds the annotation to both fields and a round-trip regression test that strips
the fields from a serialised graph and confirms deserialisation succeeds with
empty collections, matching the existing learning_document_ids pattern.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…2133
Replace `for stream in incoming() { if let Ok(s) = stream { ... } }` with
`for s in incoming().flatten() { ... }` to satisfy clippy::manual_flatten.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…(feature = "firecracker")
firecracker.rs called get_vm_status() and execute_vm_code() from ApiClient
which are #[cfg(feature = "firecracker")] but the module was compiled unconditionally.
This broke cargo test -p terraphim_agent with default features.
Fix: add #[cfg(feature = "firecracker")] to pub mod firecracker in modes/mod.rs
and update HybridExecutor to conditionally use FirecrackerExecutor only when
the feature is enabled, falling back to LocalExecutor otherwise.
Refs #2164
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ths (CWE-22)
source_agent was joined directly into filesystem paths in agent_dir(), save(),
save_to_shared(), load(), and delete() — enabling path traversal attacks (CWE-22)
via crafted agent IDs such as "../../../etc/passwd".
Add sanitise_path_component() which replaces every character that is not
ASCII-alphanumeric, hyphen, underscore, or dot with an underscore, and emits a
tracing::warn! when sanitisation is required (audit trail for injection attempts).
Apply sanitisation at every path-construction site:
- agent_dir(): agent_id
- save(): learning.id in filename
- save_to_shared(): both learning.source_agent and learning.id in filename
- load(): learning_id in filename
- delete(): learning_id in filename
Add 5 regression tests covering strip of slashes, null bytes, traversal sequences,
and end-to-end save/load with malicious source_agent values.
Fixes #2160
@AlexMikhalev

Copy link
Copy Markdown
ContributorAuthor

Closing as superseded. This PR is a Fix #2160: sanitise agent_id a... patch based on an older state of main. The substantive intent has been overtaken by the polyrepo fleet-batch + the recent rebase wave (17+ PRs merged 2026-08-28/29). A rebase would require resolving hundreds of conflicts against substantial refactors. Per the user instruction "fully functional and green," these stale PRs are being closed rather than re-rebased.

Closes per Shimaguru mass-rebase pass, 2026-08-29.

@AlexMikhalev
AlexMikhalev deleted the task/2160-path-traversal-markdown-store branch August 29, 2026 23:24
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@AlexMikhalev
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix #2160: sanitise agent_id and learning_id paths in markdown_store (CWE-22) - #903

Closed
AlexMikhalev wants to merge 10 commits into
mainfrom
task/2160-path-traversal-markdown-store
Closed

Fix #2160: sanitise agent_id and learning_id paths in markdown_store (CWE-22)#903
AlexMikhalev wants to merge 10 commits into
mainfrom
task/2160-path-traversal-markdown-store

Conversation

@AlexMikhalev

Copy link
Copy Markdown
Contributor

Summary

Fixes Gitea terraphim/terraphim-ai#2160 — P2 security: path traversal (CWE-22) in shared_learning markdown_store.

source_agent was joined directly into filesystem paths in five places without
any sanitisation, allowing a crafted agent ID such as ../../../etc/passwd to
escape the learnings_dir boundary.

Changes

  • Added sanitise_path_component(s: &str) -> String — replaces every character
    outside [a-zA-Z0-9._-] with _ and emits tracing::warn! on sanitisation
    (audit trail for injection attempts).
  • Applied to all path-construction sites: agent_dir, save, save_to_shared,
    load, delete.
  • 5 new regression tests: slash stripping, null-byte stripping, full traversal
    sequences, end-to-end save with malicious source_agent, and save_to_shared
    with traversal value.

Vulnerable functions fixed

FunctionVectorFix applied
agent_dir(agent_id)../ in agent_idsanitise_path_component
save(learning)../ in learning.idsanitise_path_component
save_to_shared(learning)../ in source_agent + idsanitise_path_component
load(agent_id, learning_id)both paramssanitise_path_component
delete(agent_id, learning_id)both paramssanitise_path_component

Test plan

cargo test -p terraphim_agent --lib --features shared-learning -- shared_learning::markdown_store
# 13 passed, 0 failed
cargo clippy -p terraphim_agent --features shared-learning -- -D warnings
# clean (exit 0)

Refs terraphim/terraphim-ai#2160 (Gitea)

Test Userand others added 10 commits June 4, 2026 00:14
…1992
reqwest::Error from error_for_status() on a 400 response is an HTTP
application error, not a transport/connectivity failure. Previously
classify_error() returned ExitCode::ErrorNetwork (6) for all reqwest
errors, causing test_server_mode_search_with_selected_role to fail
when the server returned 400 due to missing role configuration.
Now check re.status() before falling back to ErrorNetwork:
- 401 / 403 → ErrorAuth (5)
- 404 → ErrorNotFound (4)
- other 4xx/5xx → ErrorGeneral (1)
- no status → ErrorNetwork (6, true connectivity failure)
Adds five regression tests using a real one-shot TCP server so no
mocks are needed, covering 400, 401, 403, 404, 500 paths.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds an integration test that starts a real TCP listener replying with
HTTP 400 Bad Request and asserts the binary exits 0 or 1 (never 6).
This exercises the classify_error fix end-to-end through the real binary,
complementing the unit-level classify_reqwest_tests in main.rs.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…_node_ids Refs #2039
Deserialising a SerializableRoleGraph JSON written before issue #84 (trigger-based
KG retrieval) was merged would fail with a missing-field error because
trigger_descriptions and pinned_node_ids had no serde(default) annotation.
Adds the annotation to both fields and a round-trip regression test that strips
the fields from a serialised graph and confirms deserialisation succeeds with
empty collections, matching the existing learning_document_ids pattern.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…2133
Replace `for stream in incoming() { if let Ok(s) = stream { ... } }` with
`for s in incoming().flatten() { ... }` to satisfy clippy::manual_flatten.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…(feature = "firecracker")
firecracker.rs called get_vm_status() and execute_vm_code() from ApiClient
which are #[cfg(feature = "firecracker")] but the module was compiled unconditionally.
This broke cargo test -p terraphim_agent with default features.
Fix: add #[cfg(feature = "firecracker")] to pub mod firecracker in modes/mod.rs
and update HybridExecutor to conditionally use FirecrackerExecutor only when
the feature is enabled, falling back to LocalExecutor otherwise.
Refs #2164
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ths (CWE-22)
source_agent was joined directly into filesystem paths in agent_dir(), save(),
save_to_shared(), load(), and delete() — enabling path traversal attacks (CWE-22)
via crafted agent IDs such as "../../../etc/passwd".
Add sanitise_path_component() which replaces every character that is not
ASCII-alphanumeric, hyphen, underscore, or dot with an underscore, and emits a
tracing::warn! when sanitisation is required (audit trail for injection attempts).
Apply sanitisation at every path-construction site:
- agent_dir(): agent_id
- save(): learning.id in filename
- save_to_shared(): both learning.source_agent and learning.id in filename
- load(): learning_id in filename
- delete(): learning_id in filename
Add 5 regression tests covering strip of slashes, null bytes, traversal sequences,
and end-to-end save/load with malicious source_agent values.
Fixes #2160
@AlexMikhalev

Copy link
Copy Markdown
ContributorAuthor

Closing as superseded. This PR is a Fix #2160: sanitise agent_id a... patch based on an older state of main. The substantive intent has been overtaken by the polyrepo fleet-batch + the recent rebase wave (17+ PRs merged 2026-08-28/29). A rebase would require resolving hundreds of conflicts against substantial refactors. Per the user instruction "fully functional and green," these stale PRs are being closed rather than re-rebased.

Closes per Shimaguru mass-rebase pass, 2026-08-29.

@AlexMikhalev
AlexMikhalev deleted the task/2160-path-traversal-markdown-store branch August 29, 2026 23:24
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@AlexMikhalev
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Fix #2160: sanitise agent_id and learning_id paths in markdown_store (CWE-22) - #903

Closed
AlexMikhalev wants to merge 10 commits into
mainfrom
task/2160-path-traversal-markdown-store
Closed

Fix #2160: sanitise agent_id and learning_id paths in markdown_store (CWE-22)#903
AlexMikhalev wants to merge 10 commits into
mainfrom
task/2160-path-traversal-markdown-store

Conversation

@AlexMikhalev

Copy link
Copy Markdown
Contributor

Summary

Fixes Gitea terraphim/terraphim-ai#2160 — P2 security: path traversal (CWE-22) in shared_learning markdown_store.

source_agent was joined directly into filesystem paths in five places without
any sanitisation, allowing a crafted agent ID such as ../../../etc/passwd to
escape the learnings_dir boundary.

Changes

  • Added sanitise_path_component(s: &str) -> String — replaces every character
    outside [a-zA-Z0-9._-] with _ and emits tracing::warn! on sanitisation
    (audit trail for injection attempts).
  • Applied to all path-construction sites: agent_dir, save, save_to_shared,
    load, delete.
  • 5 new regression tests: slash stripping, null-byte stripping, full traversal
    sequences, end-to-end save with malicious source_agent, and save_to_shared
    with traversal value.

Vulnerable functions fixed

FunctionVectorFix applied
agent_dir(agent_id)../ in agent_idsanitise_path_component
save(learning)../ in learning.idsanitise_path_component
save_to_shared(learning)../ in source_agent + idsanitise_path_component
load(agent_id, learning_id)both paramssanitise_path_component
delete(agent_id, learning_id)both paramssanitise_path_component

Test plan

cargo test -p terraphim_agent --lib --features shared-learning -- shared_learning::markdown_store
# 13 passed, 0 failed
cargo clippy -p terraphim_agent --features shared-learning -- -D warnings
# clean (exit 0)

Refs terraphim/terraphim-ai#2160 (Gitea)

Test Userand others added 10 commits June 4, 2026 00:14
…1992
reqwest::Error from error_for_status() on a 400 response is an HTTP
application error, not a transport/connectivity failure. Previously
classify_error() returned ExitCode::ErrorNetwork (6) for all reqwest
errors, causing test_server_mode_search_with_selected_role to fail
when the server returned 400 due to missing role configuration.
Now check re.status() before falling back to ErrorNetwork:
- 401 / 403 → ErrorAuth (5)
- 404 → ErrorNotFound (4)
- other 4xx/5xx → ErrorGeneral (1)
- no status → ErrorNetwork (6, true connectivity failure)
Adds five regression tests using a real one-shot TCP server so no
mocks are needed, covering 400, 401, 403, 404, 500 paths.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds an integration test that starts a real TCP listener replying with
HTTP 400 Bad Request and asserts the binary exits 0 or 1 (never 6).
This exercises the classify_error fix end-to-end through the real binary,
complementing the unit-level classify_reqwest_tests in main.rs.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…_node_ids Refs #2039
Deserialising a SerializableRoleGraph JSON written before issue #84 (trigger-based
KG retrieval) was merged would fail with a missing-field error because
trigger_descriptions and pinned_node_ids had no serde(default) annotation.
Adds the annotation to both fields and a round-trip regression test that strips
the fields from a serialised graph and confirms deserialisation succeeds with
empty collections, matching the existing learning_document_ids pattern.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…2133
Replace `for stream in incoming() { if let Ok(s) = stream { ... } }` with
`for s in incoming().flatten() { ... }` to satisfy clippy::manual_flatten.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…(feature = "firecracker")
firecracker.rs called get_vm_status() and execute_vm_code() from ApiClient
which are #[cfg(feature = "firecracker")] but the module was compiled unconditionally.
This broke cargo test -p terraphim_agent with default features.
Fix: add #[cfg(feature = "firecracker")] to pub mod firecracker in modes/mod.rs
and update HybridExecutor to conditionally use FirecrackerExecutor only when
the feature is enabled, falling back to LocalExecutor otherwise.
Refs #2164
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ths (CWE-22)
source_agent was joined directly into filesystem paths in agent_dir(), save(),
save_to_shared(), load(), and delete() — enabling path traversal attacks (CWE-22)
via crafted agent IDs such as "../../../etc/passwd".
Add sanitise_path_component() which replaces every character that is not
ASCII-alphanumeric, hyphen, underscore, or dot with an underscore, and emits a
tracing::warn! when sanitisation is required (audit trail for injection attempts).
Apply sanitisation at every path-construction site:
- agent_dir(): agent_id
- save(): learning.id in filename
- save_to_shared(): both learning.source_agent and learning.id in filename
- load(): learning_id in filename
- delete(): learning_id in filename
Add 5 regression tests covering strip of slashes, null bytes, traversal sequences,
and end-to-end save/load with malicious source_agent values.
Fixes #2160
@AlexMikhalev

Copy link
Copy Markdown
ContributorAuthor

Closing as superseded. This PR is a Fix #2160: sanitise agent_id a... patch based on an older state of main. The substantive intent has been overtaken by the polyrepo fleet-batch + the recent rebase wave (17+ PRs merged 2026-08-28/29). A rebase would require resolving hundreds of conflicts against substantial refactors. Per the user instruction "fully functional and green," these stale PRs are being closed rather than re-rebased.

Closes per Shimaguru mass-rebase pass, 2026-08-29.

@AlexMikhalev
AlexMikhalev deleted the task/2160-path-traversal-markdown-store branch August 29, 2026 23:24
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@AlexMikhalev
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix #2160: sanitise agent_id and learning_id paths in markdown_store (CWE-22) - #903

Closed
AlexMikhalev wants to merge 10 commits into
mainfrom
task/2160-path-traversal-markdown-store
Closed

Fix #2160: sanitise agent_id and learning_id paths in markdown_store (CWE-22)#903
AlexMikhalev wants to merge 10 commits into
mainfrom
task/2160-path-traversal-markdown-store

Conversation

@AlexMikhalev

Copy link
Copy Markdown
Contributor

Summary

Fixes Gitea terraphim/terraphim-ai#2160 — P2 security: path traversal (CWE-22) in shared_learning markdown_store.

source_agent was joined directly into filesystem paths in five places without
any sanitisation, allowing a crafted agent ID such as ../../../etc/passwd to
escape the learnings_dir boundary.

Changes

  • Added sanitise_path_component(s: &str) -> String — replaces every character
    outside [a-zA-Z0-9._-] with _ and emits tracing::warn! on sanitisation
    (audit trail for injection attempts).
  • Applied to all path-construction sites: agent_dir, save, save_to_shared,
    load, delete.
  • 5 new regression tests: slash stripping, null-byte stripping, full traversal
    sequences, end-to-end save with malicious source_agent, and save_to_shared
    with traversal value.

Vulnerable functions fixed

FunctionVectorFix applied
agent_dir(agent_id)../ in agent_idsanitise_path_component
save(learning)../ in learning.idsanitise_path_component
save_to_shared(learning)../ in source_agent + idsanitise_path_component
load(agent_id, learning_id)both paramssanitise_path_component
delete(agent_id, learning_id)both paramssanitise_path_component

Test plan

cargo test -p terraphim_agent --lib --features shared-learning -- shared_learning::markdown_store
# 13 passed, 0 failed
cargo clippy -p terraphim_agent --features shared-learning -- -D warnings
# clean (exit 0)

Refs terraphim/terraphim-ai#2160 (Gitea)

Test Userand others added 10 commits June 4, 2026 00:14
…1992
reqwest::Error from error_for_status() on a 400 response is an HTTP
application error, not a transport/connectivity failure. Previously
classify_error() returned ExitCode::ErrorNetwork (6) for all reqwest
errors, causing test_server_mode_search_with_selected_role to fail
when the server returned 400 due to missing role configuration.
Now check re.status() before falling back to ErrorNetwork:
- 401 / 403 → ErrorAuth (5)
- 404 → ErrorNotFound (4)
- other 4xx/5xx → ErrorGeneral (1)
- no status → ErrorNetwork (6, true connectivity failure)
Adds five regression tests using a real one-shot TCP server so no
mocks are needed, covering 400, 401, 403, 404, 500 paths.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds an integration test that starts a real TCP listener replying with
HTTP 400 Bad Request and asserts the binary exits 0 or 1 (never 6).
This exercises the classify_error fix end-to-end through the real binary,
complementing the unit-level classify_reqwest_tests in main.rs.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…_node_ids Refs #2039
Deserialising a SerializableRoleGraph JSON written before issue #84 (trigger-based
KG retrieval) was merged would fail with a missing-field error because
trigger_descriptions and pinned_node_ids had no serde(default) annotation.
Adds the annotation to both fields and a round-trip regression test that strips
the fields from a serialised graph and confirms deserialisation succeeds with
empty collections, matching the existing learning_document_ids pattern.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…2133
Replace `for stream in incoming() { if let Ok(s) = stream { ... } }` with
`for s in incoming().flatten() { ... }` to satisfy clippy::manual_flatten.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…(feature = "firecracker")
firecracker.rs called get_vm_status() and execute_vm_code() from ApiClient
which are #[cfg(feature = "firecracker")] but the module was compiled unconditionally.
This broke cargo test -p terraphim_agent with default features.
Fix: add #[cfg(feature = "firecracker")] to pub mod firecracker in modes/mod.rs
and update HybridExecutor to conditionally use FirecrackerExecutor only when
the feature is enabled, falling back to LocalExecutor otherwise.
Refs #2164
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ths (CWE-22)
source_agent was joined directly into filesystem paths in agent_dir(), save(),
save_to_shared(), load(), and delete() — enabling path traversal attacks (CWE-22)
via crafted agent IDs such as "../../../etc/passwd".
Add sanitise_path_component() which replaces every character that is not
ASCII-alphanumeric, hyphen, underscore, or dot with an underscore, and emits a
tracing::warn! when sanitisation is required (audit trail for injection attempts).
Apply sanitisation at every path-construction site:
- agent_dir(): agent_id
- save(): learning.id in filename
- save_to_shared(): both learning.source_agent and learning.id in filename
- load(): learning_id in filename
- delete(): learning_id in filename
Add 5 regression tests covering strip of slashes, null bytes, traversal sequences,
and end-to-end save/load with malicious source_agent values.
Fixes #2160
@AlexMikhalev

Copy link
Copy Markdown
ContributorAuthor

Closing as superseded. This PR is a Fix #2160: sanitise agent_id a... patch based on an older state of main. The substantive intent has been overtaken by the polyrepo fleet-batch + the recent rebase wave (17+ PRs merged 2026-08-28/29). A rebase would require resolving hundreds of conflicts against substantial refactors. Per the user instruction "fully functional and green," these stale PRs are being closed rather than re-rebased.

Closes per Shimaguru mass-rebase pass, 2026-08-29.

@AlexMikhalev
AlexMikhalev deleted the task/2160-path-traversal-markdown-store branch August 29, 2026 23:24
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@AlexMikhalev
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix #2160: sanitise agent_id and learning_id paths in markdown_store (CWE-22) - #903

Closed
AlexMikhalev wants to merge 10 commits into
mainfrom
task/2160-path-traversal-markdown-store
Closed

Fix #2160: sanitise agent_id and learning_id paths in markdown_store (CWE-22)#903
AlexMikhalev wants to merge 10 commits into
mainfrom
task/2160-path-traversal-markdown-store

Conversation

@AlexMikhalev

Copy link
Copy Markdown
Contributor

Summary

Fixes Gitea terraphim/terraphim-ai#2160 — P2 security: path traversal (CWE-22) in shared_learning markdown_store.

source_agent was joined directly into filesystem paths in five places without
any sanitisation, allowing a crafted agent ID such as ../../../etc/passwd to
escape the learnings_dir boundary.

Changes

  • Added sanitise_path_component(s: &str) -> String — replaces every character
    outside [a-zA-Z0-9._-] with _ and emits tracing::warn! on sanitisation
    (audit trail for injection attempts).
  • Applied to all path-construction sites: agent_dir, save, save_to_shared,
    load, delete.
  • 5 new regression tests: slash stripping, null-byte stripping, full traversal
    sequences, end-to-end save with malicious source_agent, and save_to_shared
    with traversal value.

Vulnerable functions fixed

FunctionVectorFix applied
agent_dir(agent_id)../ in agent_idsanitise_path_component
save(learning)../ in learning.idsanitise_path_component
save_to_shared(learning)../ in source_agent + idsanitise_path_component
load(agent_id, learning_id)both paramssanitise_path_component
delete(agent_id, learning_id)both paramssanitise_path_component

Test plan

cargo test -p terraphim_agent --lib --features shared-learning -- shared_learning::markdown_store
# 13 passed, 0 failed
cargo clippy -p terraphim_agent --features shared-learning -- -D warnings
# clean (exit 0)

Refs terraphim/terraphim-ai#2160 (Gitea)

Test Userand others added 10 commits June 4, 2026 00:14
…1992
reqwest::Error from error_for_status() on a 400 response is an HTTP
application error, not a transport/connectivity failure. Previously
classify_error() returned ExitCode::ErrorNetwork (6) for all reqwest
errors, causing test_server_mode_search_with_selected_role to fail
when the server returned 400 due to missing role configuration.
Now check re.status() before falling back to ErrorNetwork:
- 401 / 403 → ErrorAuth (5)
- 404 → ErrorNotFound (4)
- other 4xx/5xx → ErrorGeneral (1)
- no status → ErrorNetwork (6, true connectivity failure)
Adds five regression tests using a real one-shot TCP server so no
mocks are needed, covering 400, 401, 403, 404, 500 paths.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds an integration test that starts a real TCP listener replying with
HTTP 400 Bad Request and asserts the binary exits 0 or 1 (never 6).
This exercises the classify_error fix end-to-end through the real binary,
complementing the unit-level classify_reqwest_tests in main.rs.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…_node_ids Refs #2039
Deserialising a SerializableRoleGraph JSON written before issue #84 (trigger-based
KG retrieval) was merged would fail with a missing-field error because
trigger_descriptions and pinned_node_ids had no serde(default) annotation.
Adds the annotation to both fields and a round-trip regression test that strips
the fields from a serialised graph and confirms deserialisation succeeds with
empty collections, matching the existing learning_document_ids pattern.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…2133
Replace `for stream in incoming() { if let Ok(s) = stream { ... } }` with
`for s in incoming().flatten() { ... }` to satisfy clippy::manual_flatten.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…(feature = "firecracker")
firecracker.rs called get_vm_status() and execute_vm_code() from ApiClient
which are #[cfg(feature = "firecracker")] but the module was compiled unconditionally.
This broke cargo test -p terraphim_agent with default features.
Fix: add #[cfg(feature = "firecracker")] to pub mod firecracker in modes/mod.rs
and update HybridExecutor to conditionally use FirecrackerExecutor only when
the feature is enabled, falling back to LocalExecutor otherwise.
Refs #2164
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ths (CWE-22)
source_agent was joined directly into filesystem paths in agent_dir(), save(),
save_to_shared(), load(), and delete() — enabling path traversal attacks (CWE-22)
via crafted agent IDs such as "../../../etc/passwd".
Add sanitise_path_component() which replaces every character that is not
ASCII-alphanumeric, hyphen, underscore, or dot with an underscore, and emits a
tracing::warn! when sanitisation is required (audit trail for injection attempts).
Apply sanitisation at every path-construction site:
- agent_dir(): agent_id
- save(): learning.id in filename
- save_to_shared(): both learning.source_agent and learning.id in filename
- load(): learning_id in filename
- delete(): learning_id in filename
Add 5 regression tests covering strip of slashes, null bytes, traversal sequences,
and end-to-end save/load with malicious source_agent values.
Fixes #2160
@AlexMikhalev

Copy link
Copy Markdown
ContributorAuthor

Closing as superseded. This PR is a Fix #2160: sanitise agent_id a... patch based on an older state of main. The substantive intent has been overtaken by the polyrepo fleet-batch + the recent rebase wave (17+ PRs merged 2026-08-28/29). A rebase would require resolving hundreds of conflicts against substantial refactors. Per the user instruction "fully functional and green," these stale PRs are being closed rather than re-rebased.

Closes per Shimaguru mass-rebase pass, 2026-08-29.

@AlexMikhalev
AlexMikhalev deleted the task/2160-path-traversal-markdown-store branch August 29, 2026 23:24
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@AlexMikhalev
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Fix #2160: sanitise agent_id and learning_id paths in markdown_store (CWE-22) - #903

Closed
AlexMikhalev wants to merge 10 commits into
mainfrom
task/2160-path-traversal-markdown-store
Closed

Fix #2160: sanitise agent_id and learning_id paths in markdown_store (CWE-22)#903
AlexMikhalev wants to merge 10 commits into
mainfrom
task/2160-path-traversal-markdown-store

Conversation

@AlexMikhalev

Copy link
Copy Markdown
Contributor

Summary

Fixes Gitea terraphim/terraphim-ai#2160 — P2 security: path traversal (CWE-22) in shared_learning markdown_store.

source_agent was joined directly into filesystem paths in five places without
any sanitisation, allowing a crafted agent ID such as ../../../etc/passwd to
escape the learnings_dir boundary.

Changes

  • Added sanitise_path_component(s: &str) -> String — replaces every character
    outside [a-zA-Z0-9._-] with _ and emits tracing::warn! on sanitisation
    (audit trail for injection attempts).
  • Applied to all path-construction sites: agent_dir, save, save_to_shared,
    load, delete.
  • 5 new regression tests: slash stripping, null-byte stripping, full traversal
    sequences, end-to-end save with malicious source_agent, and save_to_shared
    with traversal value.

Vulnerable functions fixed

FunctionVectorFix applied
agent_dir(agent_id)../ in agent_idsanitise_path_component
save(learning)../ in learning.idsanitise_path_component
save_to_shared(learning)../ in source_agent + idsanitise_path_component
load(agent_id, learning_id)both paramssanitise_path_component
delete(agent_id, learning_id)both paramssanitise_path_component

Test plan

cargo test -p terraphim_agent --lib --features shared-learning -- shared_learning::markdown_store
# 13 passed, 0 failed
cargo clippy -p terraphim_agent --features shared-learning -- -D warnings
# clean (exit 0)

Refs terraphim/terraphim-ai#2160 (Gitea)

Test Userand others added 10 commits June 4, 2026 00:14
…1992
reqwest::Error from error_for_status() on a 400 response is an HTTP
application error, not a transport/connectivity failure. Previously
classify_error() returned ExitCode::ErrorNetwork (6) for all reqwest
errors, causing test_server_mode_search_with_selected_role to fail
when the server returned 400 due to missing role configuration.
Now check re.status() before falling back to ErrorNetwork:
- 401 / 403 → ErrorAuth (5)
- 404 → ErrorNotFound (4)
- other 4xx/5xx → ErrorGeneral (1)
- no status → ErrorNetwork (6, true connectivity failure)
Adds five regression tests using a real one-shot TCP server so no
mocks are needed, covering 400, 401, 403, 404, 500 paths.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds an integration test that starts a real TCP listener replying with
HTTP 400 Bad Request and asserts the binary exits 0 or 1 (never 6).
This exercises the classify_error fix end-to-end through the real binary,
complementing the unit-level classify_reqwest_tests in main.rs.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…_node_ids Refs #2039
Deserialising a SerializableRoleGraph JSON written before issue #84 (trigger-based
KG retrieval) was merged would fail with a missing-field error because
trigger_descriptions and pinned_node_ids had no serde(default) annotation.
Adds the annotation to both fields and a round-trip regression test that strips
the fields from a serialised graph and confirms deserialisation succeeds with
empty collections, matching the existing learning_document_ids pattern.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…2133
Replace `for stream in incoming() { if let Ok(s) = stream { ... } }` with
`for s in incoming().flatten() { ... }` to satisfy clippy::manual_flatten.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…(feature = "firecracker")
firecracker.rs called get_vm_status() and execute_vm_code() from ApiClient
which are #[cfg(feature = "firecracker")] but the module was compiled unconditionally.
This broke cargo test -p terraphim_agent with default features.
Fix: add #[cfg(feature = "firecracker")] to pub mod firecracker in modes/mod.rs
and update HybridExecutor to conditionally use FirecrackerExecutor only when
the feature is enabled, falling back to LocalExecutor otherwise.
Refs #2164
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ths (CWE-22)
source_agent was joined directly into filesystem paths in agent_dir(), save(),
save_to_shared(), load(), and delete() — enabling path traversal attacks (CWE-22)
via crafted agent IDs such as "../../../etc/passwd".
Add sanitise_path_component() which replaces every character that is not
ASCII-alphanumeric, hyphen, underscore, or dot with an underscore, and emits a
tracing::warn! when sanitisation is required (audit trail for injection attempts).
Apply sanitisation at every path-construction site:
- agent_dir(): agent_id
- save(): learning.id in filename
- save_to_shared(): both learning.source_agent and learning.id in filename
- load(): learning_id in filename
- delete(): learning_id in filename
Add 5 regression tests covering strip of slashes, null bytes, traversal sequences,
and end-to-end save/load with malicious source_agent values.
Fixes #2160
@AlexMikhalev

Copy link
Copy Markdown
ContributorAuthor

Closing as superseded. This PR is a Fix #2160: sanitise agent_id a... patch based on an older state of main. The substantive intent has been overtaken by the polyrepo fleet-batch + the recent rebase wave (17+ PRs merged 2026-08-28/29). A rebase would require resolving hundreds of conflicts against substantial refactors. Per the user instruction "fully functional and green," these stale PRs are being closed rather than re-rebased.

Closes per Shimaguru mass-rebase pass, 2026-08-29.

@AlexMikhalev
AlexMikhalev deleted the task/2160-path-traversal-markdown-store branch August 29, 2026 23:24
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@AlexMikhalev