fix(permission): resolve redirect targets against the cd-tracked cwd - #947

Open
Terminator666666 wants to merge 1 commit into
modelscope:mainfrom
Terminator666666:fix/redirect-cwd-tracking
Open

fix(permission): resolve redirect targets against the cd-tracked cwd#947
Terminator666666 wants to merge 1 commit into
modelscope:mainfrom
Terminator666666:fix/redirect-cwd-tracking

Conversation

@Terminator666666

Copy link
Copy Markdown

Change Summary

ShellPathValidator.check() tracks cd as it walks a compound command and passes the
resulting cwd down to _check_command(). _check_redirects() never got that treatment —
it resolves relative redirect targets against self._workspace_root regardless of where
the shell has actually moved.

So two halves of the same check disagree with each other:

cd work/sub && rm ../f -> allow (correct)
cd work/sub && echo x > ../f -> deny (wrong — this writes work/f)

Both commands touch the same file. Only one of them is judged correctly.

The mismatch goes both ways. Writes that stay inside the workspace get rejected, which is
the harmless direction. The other direction: a relative target can resolve into an allowed
directory when measured from the workspace root while the shell writes somewhere else. I
could only reproduce that with several allowed_dirs at differing depths, and it also
needs the real parent directory to already exist, so I doubt it amounts to much in
practice. Wrong either way.

The fix hands the tracked cwd to _check_redirects(). The call stays where it is, ahead
of the cd bookkeeping, because a shell resolves the redirect in cd foo > log against
the old cwd as well.

Related issue number

None.

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Run pre-commit install and pre-commit run --all-files before git commit, and passed lint check.
  • Documentation reflects the changes where applicable

Added TestRedirectCwdTracking in tests/permission/test_shell_validator.py — four cases
covering both directions plus one asserting that redirects and ordinary path arguments
agree on the cwd. Reverting shell_validator.py and keeping the tests fails three of them.

tests/permission/ is at 340 passed. Note that a plain pytest tests/permission/ gave me
30 failures on async tests before I touched anything; they need pytest-asyncio and pass
with --asyncio-mode=auto. Unrelated to this change, but it tripped me up while
establishing a baseline, so flagging it in case the CI config is worth a look.

Docs box left unchecked — the behaviour matches what the docstrings already describe, so
there was nothing to update.

ShellPathValidator tracks `cd` while walking a compound command and passes
the resulting cwd to _check_command(), but _check_redirects() ignored it and
always resolved relative redirect targets against the workspace root. The two
checks therefore disagreed about the cwd within one and the same command:
cd work/sub && rm ../f -> allow (correct)
cd work/sub && echo x > ../f -> deny (wrong, writes work/f)
The mismatch cuts both ways. Writes that stay inside the workspace are
rejected, and with several allowed_dirs at differing depths a target can
resolve into an allowed directory from the workspace root while the shell
writes outside of it.
Pass the tracked cwd into _check_redirects(). It stays ahead of the `cd`
handling on purpose, since a shell resolves the redirect in `cd foo > log`
against the old cwd as well.
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

@Terminator666666
, '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(permission): resolve redirect targets against the cd-tracked cwd - #947

Open
Terminator666666 wants to merge 1 commit into
modelscope:mainfrom
Terminator666666:fix/redirect-cwd-tracking
Open

fix(permission): resolve redirect targets against the cd-tracked cwd#947
Terminator666666 wants to merge 1 commit into
modelscope:mainfrom
Terminator666666:fix/redirect-cwd-tracking

Conversation

@Terminator666666

Copy link
Copy Markdown

Change Summary

ShellPathValidator.check() tracks cd as it walks a compound command and passes the
resulting cwd down to _check_command(). _check_redirects() never got that treatment —
it resolves relative redirect targets against self._workspace_root regardless of where
the shell has actually moved.

So two halves of the same check disagree with each other:

cd work/sub && rm ../f -> allow (correct)
cd work/sub && echo x > ../f -> deny (wrong — this writes work/f)

Both commands touch the same file. Only one of them is judged correctly.

The mismatch goes both ways. Writes that stay inside the workspace get rejected, which is
the harmless direction. The other direction: a relative target can resolve into an allowed
directory when measured from the workspace root while the shell writes somewhere else. I
could only reproduce that with several allowed_dirs at differing depths, and it also
needs the real parent directory to already exist, so I doubt it amounts to much in
practice. Wrong either way.

The fix hands the tracked cwd to _check_redirects(). The call stays where it is, ahead
of the cd bookkeeping, because a shell resolves the redirect in cd foo > log against
the old cwd as well.

Related issue number

None.

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Run pre-commit install and pre-commit run --all-files before git commit, and passed lint check.
  • Documentation reflects the changes where applicable

Added TestRedirectCwdTracking in tests/permission/test_shell_validator.py — four cases
covering both directions plus one asserting that redirects and ordinary path arguments
agree on the cwd. Reverting shell_validator.py and keeping the tests fails three of them.

tests/permission/ is at 340 passed. Note that a plain pytest tests/permission/ gave me
30 failures on async tests before I touched anything; they need pytest-asyncio and pass
with --asyncio-mode=auto. Unrelated to this change, but it tripped me up while
establishing a baseline, so flagging it in case the CI config is worth a look.

Docs box left unchecked — the behaviour matches what the docstrings already describe, so
there was nothing to update.

ShellPathValidator tracks `cd` while walking a compound command and passes
the resulting cwd to _check_command(), but _check_redirects() ignored it and
always resolved relative redirect targets against the workspace root. The two
checks therefore disagreed about the cwd within one and the same command:
cd work/sub && rm ../f -> allow (correct)
cd work/sub && echo x > ../f -> deny (wrong, writes work/f)
The mismatch cuts both ways. Writes that stay inside the workspace are
rejected, and with several allowed_dirs at differing depths a target can
resolve into an allowed directory from the workspace root while the shell
writes outside of it.
Pass the tracked cwd into _check_redirects(). It stays ahead of the `cd`
handling on purpose, since a shell resolves the redirect in `cd foo > log`
against the old cwd as well.
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

@Terminator666666
, '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(permission): resolve redirect targets against the cd-tracked cwd - #947

Open
Terminator666666 wants to merge 1 commit into
modelscope:mainfrom
Terminator666666:fix/redirect-cwd-tracking
Open

fix(permission): resolve redirect targets against the cd-tracked cwd#947
Terminator666666 wants to merge 1 commit into
modelscope:mainfrom
Terminator666666:fix/redirect-cwd-tracking

Conversation

@Terminator666666

Copy link
Copy Markdown

Change Summary

ShellPathValidator.check() tracks cd as it walks a compound command and passes the
resulting cwd down to _check_command(). _check_redirects() never got that treatment —
it resolves relative redirect targets against self._workspace_root regardless of where
the shell has actually moved.

So two halves of the same check disagree with each other:

cd work/sub && rm ../f -> allow (correct)
cd work/sub && echo x > ../f -> deny (wrong — this writes work/f)

Both commands touch the same file. Only one of them is judged correctly.

The mismatch goes both ways. Writes that stay inside the workspace get rejected, which is
the harmless direction. The other direction: a relative target can resolve into an allowed
directory when measured from the workspace root while the shell writes somewhere else. I
could only reproduce that with several allowed_dirs at differing depths, and it also
needs the real parent directory to already exist, so I doubt it amounts to much in
practice. Wrong either way.

The fix hands the tracked cwd to _check_redirects(). The call stays where it is, ahead
of the cd bookkeeping, because a shell resolves the redirect in cd foo > log against
the old cwd as well.

Related issue number

None.

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Run pre-commit install and pre-commit run --all-files before git commit, and passed lint check.
  • Documentation reflects the changes where applicable

Added TestRedirectCwdTracking in tests/permission/test_shell_validator.py — four cases
covering both directions plus one asserting that redirects and ordinary path arguments
agree on the cwd. Reverting shell_validator.py and keeping the tests fails three of them.

tests/permission/ is at 340 passed. Note that a plain pytest tests/permission/ gave me
30 failures on async tests before I touched anything; they need pytest-asyncio and pass
with --asyncio-mode=auto. Unrelated to this change, but it tripped me up while
establishing a baseline, so flagging it in case the CI config is worth a look.

Docs box left unchecked — the behaviour matches what the docstrings already describe, so
there was nothing to update.

ShellPathValidator tracks `cd` while walking a compound command and passes
the resulting cwd to _check_command(), but _check_redirects() ignored it and
always resolved relative redirect targets against the workspace root. The two
checks therefore disagreed about the cwd within one and the same command:
cd work/sub && rm ../f -> allow (correct)
cd work/sub && echo x > ../f -> deny (wrong, writes work/f)
The mismatch cuts both ways. Writes that stay inside the workspace are
rejected, and with several allowed_dirs at differing depths a target can
resolve into an allowed directory from the workspace root while the shell
writes outside of it.
Pass the tracked cwd into _check_redirects(). It stays ahead of the `cd`
handling on purpose, since a shell resolves the redirect in `cd foo > log`
against the old cwd as well.
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

@Terminator666666
, '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(permission): resolve redirect targets against the cd-tracked cwd - #947

Open
Terminator666666 wants to merge 1 commit into
modelscope:mainfrom
Terminator666666:fix/redirect-cwd-tracking
Open

fix(permission): resolve redirect targets against the cd-tracked cwd#947
Terminator666666 wants to merge 1 commit into
modelscope:mainfrom
Terminator666666:fix/redirect-cwd-tracking

Conversation

@Terminator666666

Copy link
Copy Markdown

Change Summary

ShellPathValidator.check() tracks cd as it walks a compound command and passes the
resulting cwd down to _check_command(). _check_redirects() never got that treatment —
it resolves relative redirect targets against self._workspace_root regardless of where
the shell has actually moved.

So two halves of the same check disagree with each other:

cd work/sub && rm ../f -> allow (correct)
cd work/sub && echo x > ../f -> deny (wrong — this writes work/f)

Both commands touch the same file. Only one of them is judged correctly.

The mismatch goes both ways. Writes that stay inside the workspace get rejected, which is
the harmless direction. The other direction: a relative target can resolve into an allowed
directory when measured from the workspace root while the shell writes somewhere else. I
could only reproduce that with several allowed_dirs at differing depths, and it also
needs the real parent directory to already exist, so I doubt it amounts to much in
practice. Wrong either way.

The fix hands the tracked cwd to _check_redirects(). The call stays where it is, ahead
of the cd bookkeeping, because a shell resolves the redirect in cd foo > log against
the old cwd as well.

Related issue number

None.

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Run pre-commit install and pre-commit run --all-files before git commit, and passed lint check.
  • Documentation reflects the changes where applicable

Added TestRedirectCwdTracking in tests/permission/test_shell_validator.py — four cases
covering both directions plus one asserting that redirects and ordinary path arguments
agree on the cwd. Reverting shell_validator.py and keeping the tests fails three of them.

tests/permission/ is at 340 passed. Note that a plain pytest tests/permission/ gave me
30 failures on async tests before I touched anything; they need pytest-asyncio and pass
with --asyncio-mode=auto. Unrelated to this change, but it tripped me up while
establishing a baseline, so flagging it in case the CI config is worth a look.

Docs box left unchecked — the behaviour matches what the docstrings already describe, so
there was nothing to update.

ShellPathValidator tracks `cd` while walking a compound command and passes
the resulting cwd to _check_command(), but _check_redirects() ignored it and
always resolved relative redirect targets against the workspace root. The two
checks therefore disagreed about the cwd within one and the same command:
cd work/sub && rm ../f -> allow (correct)
cd work/sub && echo x > ../f -> deny (wrong, writes work/f)
The mismatch cuts both ways. Writes that stay inside the workspace are
rejected, and with several allowed_dirs at differing depths a target can
resolve into an allowed directory from the workspace root while the shell
writes outside of it.
Pass the tracked cwd into _check_redirects(). It stays ahead of the `cd`
handling on purpose, since a shell resolves the redirect in `cd foo > log`
against the old cwd as well.
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

@Terminator666666
, '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(permission): resolve redirect targets against the cd-tracked cwd - #947

Open
Terminator666666 wants to merge 1 commit into
modelscope:mainfrom
Terminator666666:fix/redirect-cwd-tracking
Open

fix(permission): resolve redirect targets against the cd-tracked cwd#947
Terminator666666 wants to merge 1 commit into
modelscope:mainfrom
Terminator666666:fix/redirect-cwd-tracking

Conversation

@Terminator666666

Copy link
Copy Markdown

Change Summary

ShellPathValidator.check() tracks cd as it walks a compound command and passes the
resulting cwd down to _check_command(). _check_redirects() never got that treatment —
it resolves relative redirect targets against self._workspace_root regardless of where
the shell has actually moved.

So two halves of the same check disagree with each other:

cd work/sub && rm ../f -> allow (correct)
cd work/sub && echo x > ../f -> deny (wrong — this writes work/f)

Both commands touch the same file. Only one of them is judged correctly.

The mismatch goes both ways. Writes that stay inside the workspace get rejected, which is
the harmless direction. The other direction: a relative target can resolve into an allowed
directory when measured from the workspace root while the shell writes somewhere else. I
could only reproduce that with several allowed_dirs at differing depths, and it also
needs the real parent directory to already exist, so I doubt it amounts to much in
practice. Wrong either way.

The fix hands the tracked cwd to _check_redirects(). The call stays where it is, ahead
of the cd bookkeeping, because a shell resolves the redirect in cd foo > log against
the old cwd as well.

Related issue number

None.

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Run pre-commit install and pre-commit run --all-files before git commit, and passed lint check.
  • Documentation reflects the changes where applicable

Added TestRedirectCwdTracking in tests/permission/test_shell_validator.py — four cases
covering both directions plus one asserting that redirects and ordinary path arguments
agree on the cwd. Reverting shell_validator.py and keeping the tests fails three of them.

tests/permission/ is at 340 passed. Note that a plain pytest tests/permission/ gave me
30 failures on async tests before I touched anything; they need pytest-asyncio and pass
with --asyncio-mode=auto. Unrelated to this change, but it tripped me up while
establishing a baseline, so flagging it in case the CI config is worth a look.

Docs box left unchecked — the behaviour matches what the docstrings already describe, so
there was nothing to update.

ShellPathValidator tracks `cd` while walking a compound command and passes
the resulting cwd to _check_command(), but _check_redirects() ignored it and
always resolved relative redirect targets against the workspace root. The two
checks therefore disagreed about the cwd within one and the same command:
cd work/sub && rm ../f -> allow (correct)
cd work/sub && echo x > ../f -> deny (wrong, writes work/f)
The mismatch cuts both ways. Writes that stay inside the workspace are
rejected, and with several allowed_dirs at differing depths a target can
resolve into an allowed directory from the workspace root while the shell
writes outside of it.
Pass the tracked cwd into _check_redirects(). It stays ahead of the `cd`
handling on purpose, since a shell resolves the redirect in `cd foo > log`
against the old cwd as well.
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

@Terminator666666
, '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(permission): resolve redirect targets against the cd-tracked cwd - #947

Open
Terminator666666 wants to merge 1 commit into
modelscope:mainfrom
Terminator666666:fix/redirect-cwd-tracking
Open

fix(permission): resolve redirect targets against the cd-tracked cwd#947
Terminator666666 wants to merge 1 commit into
modelscope:mainfrom
Terminator666666:fix/redirect-cwd-tracking

Conversation

@Terminator666666

Copy link
Copy Markdown

Change Summary

ShellPathValidator.check() tracks cd as it walks a compound command and passes the
resulting cwd down to _check_command(). _check_redirects() never got that treatment —
it resolves relative redirect targets against self._workspace_root regardless of where
the shell has actually moved.

So two halves of the same check disagree with each other:

cd work/sub && rm ../f -> allow (correct)
cd work/sub && echo x > ../f -> deny (wrong — this writes work/f)

Both commands touch the same file. Only one of them is judged correctly.

The mismatch goes both ways. Writes that stay inside the workspace get rejected, which is
the harmless direction. The other direction: a relative target can resolve into an allowed
directory when measured from the workspace root while the shell writes somewhere else. I
could only reproduce that with several allowed_dirs at differing depths, and it also
needs the real parent directory to already exist, so I doubt it amounts to much in
practice. Wrong either way.

The fix hands the tracked cwd to _check_redirects(). The call stays where it is, ahead
of the cd bookkeeping, because a shell resolves the redirect in cd foo > log against
the old cwd as well.

Related issue number

None.

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Run pre-commit install and pre-commit run --all-files before git commit, and passed lint check.
  • Documentation reflects the changes where applicable

Added TestRedirectCwdTracking in tests/permission/test_shell_validator.py — four cases
covering both directions plus one asserting that redirects and ordinary path arguments
agree on the cwd. Reverting shell_validator.py and keeping the tests fails three of them.

tests/permission/ is at 340 passed. Note that a plain pytest tests/permission/ gave me
30 failures on async tests before I touched anything; they need pytest-asyncio and pass
with --asyncio-mode=auto. Unrelated to this change, but it tripped me up while
establishing a baseline, so flagging it in case the CI config is worth a look.

Docs box left unchecked — the behaviour matches what the docstrings already describe, so
there was nothing to update.

ShellPathValidator tracks `cd` while walking a compound command and passes
the resulting cwd to _check_command(), but _check_redirects() ignored it and
always resolved relative redirect targets against the workspace root. The two
checks therefore disagreed about the cwd within one and the same command:
cd work/sub && rm ../f -> allow (correct)
cd work/sub && echo x > ../f -> deny (wrong, writes work/f)
The mismatch cuts both ways. Writes that stay inside the workspace are
rejected, and with several allowed_dirs at differing depths a target can
resolve into an allowed directory from the workspace root while the shell
writes outside of it.
Pass the tracked cwd into _check_redirects(). It stays ahead of the `cd`
handling on purpose, since a shell resolves the redirect in `cd foo > log`
against the old cwd as well.
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

@Terminator666666
, '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(permission): resolve redirect targets against the cd-tracked cwd - #947

Open
Terminator666666 wants to merge 1 commit into
modelscope:mainfrom
Terminator666666:fix/redirect-cwd-tracking
Open

fix(permission): resolve redirect targets against the cd-tracked cwd#947
Terminator666666 wants to merge 1 commit into
modelscope:mainfrom
Terminator666666:fix/redirect-cwd-tracking

Conversation

@Terminator666666

Copy link
Copy Markdown

Change Summary

ShellPathValidator.check() tracks cd as it walks a compound command and passes the
resulting cwd down to _check_command(). _check_redirects() never got that treatment —
it resolves relative redirect targets against self._workspace_root regardless of where
the shell has actually moved.

So two halves of the same check disagree with each other:

cd work/sub && rm ../f -> allow (correct)
cd work/sub && echo x > ../f -> deny (wrong — this writes work/f)

Both commands touch the same file. Only one of them is judged correctly.

The mismatch goes both ways. Writes that stay inside the workspace get rejected, which is
the harmless direction. The other direction: a relative target can resolve into an allowed
directory when measured from the workspace root while the shell writes somewhere else. I
could only reproduce that with several allowed_dirs at differing depths, and it also
needs the real parent directory to already exist, so I doubt it amounts to much in
practice. Wrong either way.

The fix hands the tracked cwd to _check_redirects(). The call stays where it is, ahead
of the cd bookkeeping, because a shell resolves the redirect in cd foo > log against
the old cwd as well.

Related issue number

None.

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Run pre-commit install and pre-commit run --all-files before git commit, and passed lint check.
  • Documentation reflects the changes where applicable

Added TestRedirectCwdTracking in tests/permission/test_shell_validator.py — four cases
covering both directions plus one asserting that redirects and ordinary path arguments
agree on the cwd. Reverting shell_validator.py and keeping the tests fails three of them.

tests/permission/ is at 340 passed. Note that a plain pytest tests/permission/ gave me
30 failures on async tests before I touched anything; they need pytest-asyncio and pass
with --asyncio-mode=auto. Unrelated to this change, but it tripped me up while
establishing a baseline, so flagging it in case the CI config is worth a look.

Docs box left unchecked — the behaviour matches what the docstrings already describe, so
there was nothing to update.

ShellPathValidator tracks `cd` while walking a compound command and passes
the resulting cwd to _check_command(), but _check_redirects() ignored it and
always resolved relative redirect targets against the workspace root. The two
checks therefore disagreed about the cwd within one and the same command:
cd work/sub && rm ../f -> allow (correct)
cd work/sub && echo x > ../f -> deny (wrong, writes work/f)
The mismatch cuts both ways. Writes that stay inside the workspace are
rejected, and with several allowed_dirs at differing depths a target can
resolve into an allowed directory from the workspace root while the shell
writes outside of it.
Pass the tracked cwd into _check_redirects(). It stays ahead of the `cd`
handling on purpose, since a shell resolves the redirect in `cd foo > log`
against the old cwd as well.
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

@Terminator666666
, '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(permission): resolve redirect targets against the cd-tracked cwd - #947

Open
Terminator666666 wants to merge 1 commit into
modelscope:mainfrom
Terminator666666:fix/redirect-cwd-tracking
Open

fix(permission): resolve redirect targets against the cd-tracked cwd#947
Terminator666666 wants to merge 1 commit into
modelscope:mainfrom
Terminator666666:fix/redirect-cwd-tracking

Conversation

@Terminator666666

Copy link
Copy Markdown

Change Summary

ShellPathValidator.check() tracks cd as it walks a compound command and passes the
resulting cwd down to _check_command(). _check_redirects() never got that treatment —
it resolves relative redirect targets against self._workspace_root regardless of where
the shell has actually moved.

So two halves of the same check disagree with each other:

cd work/sub && rm ../f -> allow (correct)
cd work/sub && echo x > ../f -> deny (wrong — this writes work/f)

Both commands touch the same file. Only one of them is judged correctly.

The mismatch goes both ways. Writes that stay inside the workspace get rejected, which is
the harmless direction. The other direction: a relative target can resolve into an allowed
directory when measured from the workspace root while the shell writes somewhere else. I
could only reproduce that with several allowed_dirs at differing depths, and it also
needs the real parent directory to already exist, so I doubt it amounts to much in
practice. Wrong either way.

The fix hands the tracked cwd to _check_redirects(). The call stays where it is, ahead
of the cd bookkeeping, because a shell resolves the redirect in cd foo > log against
the old cwd as well.

Related issue number

None.

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Run pre-commit install and pre-commit run --all-files before git commit, and passed lint check.
  • Documentation reflects the changes where applicable

Added TestRedirectCwdTracking in tests/permission/test_shell_validator.py — four cases
covering both directions plus one asserting that redirects and ordinary path arguments
agree on the cwd. Reverting shell_validator.py and keeping the tests fails three of them.

tests/permission/ is at 340 passed. Note that a plain pytest tests/permission/ gave me
30 failures on async tests before I touched anything; they need pytest-asyncio and pass
with --asyncio-mode=auto. Unrelated to this change, but it tripped me up while
establishing a baseline, so flagging it in case the CI config is worth a look.

Docs box left unchecked — the behaviour matches what the docstrings already describe, so
there was nothing to update.

ShellPathValidator tracks `cd` while walking a compound command and passes
the resulting cwd to _check_command(), but _check_redirects() ignored it and
always resolved relative redirect targets against the workspace root. The two
checks therefore disagreed about the cwd within one and the same command:
cd work/sub && rm ../f -> allow (correct)
cd work/sub && echo x > ../f -> deny (wrong, writes work/f)
The mismatch cuts both ways. Writes that stay inside the workspace are
rejected, and with several allowed_dirs at differing depths a target can
resolve into an allowed directory from the workspace root while the shell
writes outside of it.
Pass the tracked cwd into _check_redirects(). It stays ahead of the `cd`
handling on purpose, since a shell resolves the redirect in `cd foo > log`
against the old cwd as well.
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

@Terminator666666