CodeQL: answer go/path-injection for a workspace-confined path in one declared place, instead of a dismissal per PR
Maintainers usually reply within 1 day
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 15/100
Research direction
Start with resolveWorkspacePath and judgeWorkspacePathArg in proxy/tool_args.go, then openRegularFile in proxy/file_kind.go, to see where the lexical containment test lives. Then read the CodeQL workflow in codeql.yml. The work is to extract one containment predicate, call it from both places, add a test that fails if they disagree, and add a .github/codeql/codeql-config.yml barrier declaration. Done means no new path-injection alert on a confined read and a failing alert on an unguarded read, with no dismissals.
Written by the indexing model from the issue text.
Description
What needs doing
proxy/file_kind.go:89 — the os.OpenFile inside openRegularFile — opens a path that came from a model's tool argument, and CodeQL reports that as a high go/path-injection alert: "This path depends on a user-provided value". On the first push of #680 it was the one check that was not green. On the second push it was green, with No new alerts in code changed by this pull request, and the sink had not changed between the two pushes.
So the finding was either dismissed or stopped counting as new. The alerts API is not readable with a token that carries no security-events scope, so this issue does not claim which of the two happened. What it claims is that a green check is now the only record that anyone considered the sink, and that record does not survive the next file that does the same thing.
The alert is a false positive, and the reasoning belongs in the repository rather than in a dismissal nobody can read. canonicalToolArgs in proxy/tool_args.go judges every argument listed in workspacePathFields before any handler runs; judgeWorkspacePathArg sends each one through resolveWorkspacePath, which refuses a name outside the workspace by lexical path and again by the nearest existing ancestor, then hands the handler the canonical bytes. A path that reaches openRegularFile has already been refused unless it was inside the workspace. CodeQL cannot see that: its dataflow follows the model's JSON string into os.OpenFile, and it has no model for a value canonicalised and confined three functions away, in another file, by a check it also does not model.
What I propose is one declared place that answers this class of alert, wired in from a config file so the judgement is reviewable like code:
- Name the containment decision once. The lexical test inside
resolveWorkspacePathbecomes a named predicate, andresolveWorkspacePathcalls it rather than restating it, so there is one owner and no second definition to drift. - Assert that predicate at the point of use.
readRegularFile/openRegularFiletakes the workspace root and refuses when the predicate fails, immediately before the open. This is not decoration: it makes each read independent of a caller several functions away instead of trusting it, which is the property a reviewer wants at a file boundary. It is also the shape CodeQL can see — a guard whose false result returns before the sink. - Declare the guard in models-as-data. CodeQL supports
barrierGuardModelfor Go declaratively from 2.25.2 (April 2026), so a guard can be registered as apath-injectionbarrier with no custom QL to maintain: https://codeql.github.com/docs/codeql-language-guides/customizing-library-models-for-go/ and https://github.blog/changelog/2026-04-21-codeql-now-supports-sanitizers-and-validators-in-models-as-data/ - Put the scan's settings in
.github/codeql/codeql-config.yml, passed toinitwithconfig-file:, so the declaration lives next to the workflow instead of only in inputs.
This is not the kind check from #365 and does not reopen the race that fixed. Containment is about where the name points; the kind is about what the handle holds. A name swapped for a named pipe between the guard and the open still cannot make the open wait, because the open is non-blocking and the handle is judged after it returns.
One thing is unverified, and the PR has to settle it before writing anything else: whether a data extension can model a function in this repository's own main package, github.com/itigges22/atlas-proxy, rather than a dependency of it. The barrier syntax itself is available — [email protected], the version codeql.yml pins, bundles CodeQL 2.27.2 — so the in-source question is the only open one. If it fails, the fallbacks in order are a query-filters entry scoped as narrowly as the config format allows, and then a dismissal of the single alert, which is what this issue exists to stop repeating per PR and is therefore the last resort, not the plan.
Not an RFC: no change to what the proxy reads, refuses or accepts, no new dependency, no change to the workspace contract — the guard asserts a boundary the code already enforces. It adds one filepath.Rel per read and one declaration for the scanner. If a maintainer reads a sanitizer declaration as a change to the security model, say so and this moves to the RFC form instead of arguing the classification.
Why
A dismissal leaves nothing behind for the next person. #680 is the demonstration in both directions: a correct fix whose first push was held by a finding about a path that is confined by design, and whose second push passed with no change to the finding's cause. Neither outcome is recorded anywhere but a green check. Left alone, the next file that reads a model-supplied path repeats the first, and whether it reaches a reviewer or a pass depends on who is around with the right permission.
The second cost matters more. An alert dismissed per PR teaches reviewers to dismiss, and it teaches the next contributor that the class is noise. A guard in the code, a declaration in the repository and a negative control proving the scanner was not blinded keep the class honest: a genuinely unconfined sink still fails the check, and this shape stops failing.
Done when
- The scan reads its settings from a file under
.github/codeql/, andcodeql.ymlpasses that file toinit.queries: security-and-qualitystill runs — moved into the config, or kept in the workflow with the config not contradicting it. - The containment predicate has one definition, called by both
resolveWorkspacePathand the read helper, with a test that fails if the two ever disagree. - A new Go file that opens a model-supplied, workspace-confined path raises no
go/path-injectionalert, and nobody dismisses anything to achieve it. - A negative control proves the scanner is not blinded: a branch that adds a read skipping the guard still gets a
go/path-injectionalert and a failedCodeQLcheck. This is what makes the declaration safe rather than a silencer, and it belongs in the same PR. docs/says where the declaration lives and what to do when a new alert of this class appears, in whichever doc already covers the CI and security tooling, so the next person finds it before they reach the Security tab.- The later reads tracked in #679 go through the same helper, or the PR says explicitly which of them do not and why.
Related: #680, where this was found; #365, the fault it fixes; #679, the later reads in a session; #112, the earlier CodeQL backlog pass.
@itigges22 — the alert on #680 is answered and the check is green, so this is not a request to unblock anything. It is the other half of what you said when you flagged the readLedgerBytes lines: there was nothing to re-mark on a new file, and there is still nothing in the repository that says why this sink is safe. If you would rather this class stayed a per-PR judgement, say so and I will close this; otherwise I will take it and open the PR once the remaining #365 work is in, so the two do not land in the same helpers at once.
- Dominant language
- Python
- Stars
- 2.1k
- Forks
- 197
- Avg merge
- 5h 13m
- Merged PRs (30d)
- 604
Getting set up
- Ships a Dockerfile or Docker Compose file
- Has a pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from inferstep/ATLAS
-
help wanted
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Maintainers usually reply within 1 day
-
Bootstrap: the last two steps run as the user who owns the installMay be free again @assassinationss claimed this 26 days ago, and no pull request is open. Openarea/install reliability
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
inferstep/ATLAS#412 · 4 comments ·
Maintainers usually reply within 1 day
-
area/cli
Difficulty 2/5 1-3 hours Newbie friendliness 85/100
Maintainers usually reply within 1 day
-
area/cli area/lens status/ready
Difficulty 2/5 Half a day Newbie friendliness 66/100
Maintainers usually reply within 1 day
-
area/ci status/ready
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
inferstep/ATLAS#245 · 4 comments ·
Maintainers usually reply within 1 day
Similar issues
-
needs-area-label
Difficulty 2/5 1-3 hours Newbie friendliness 66/100
Maintainers usually reply within 2 days
-
enhancement good first issue
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
Official-Space-AI/OpenPhysGraspGen#114 ·
Maintainers usually reply within 2 days
-
changelog investigate
Difficulty 2/5 1-3 hours Newbie friendliness 62/100
ramnes/notion-sdk-py#441 ·
-
qa_bug
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
NVIDIA-NeMo/Gym#5644 ·
Maintainers usually reply within 2 days
-
BUG: catalog database credentials printed to logs at startupPossibly taken @cjboyle claimed this today. Open
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
Maintainers usually reply within 1 day