Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 387a647355
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| isinstance(node.func, ast.Attribute) | ||
| and node.func.attr in python_path_filesystem_read_methods | ||
| and python_path_receiver_expression( | ||
| node.func.value, tree, parents | ||
| ) |
There was a problem hiding this comment.
The new receiver guard skips validation entirely when a Path constructor is aliased: factory = Path; print(factory("synthetic-private").read_text()) now returns no scanner violation, whereas the parent scanner rejected it. Any unrecognized factory or helper returning a Path similarly bypasses all filesystem-reader checks and can expose private files through packet output, so unresolved receivers using these reader names must remain fail-closed.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| elif isinstance(node, (ast.For, ast.AsyncFor, ast.comprehension)): | ||
| # Values yielded from an environment iterator remain sensitive in | ||
| # both loop bodies and comprehension elements. | ||
| assignments.append((node.target, node.iter)) |
There was a problem hiding this comment.
When an environment iterator is wrapped, recording only the outer iterable expression loses the taint: for _, value in enumerate(os.environ.values()): print(value) and the analogous zip(...) case both pass this scanner. Those snippets still print inherited values, including credentials, so iterable taint must recurse through ordinary iterator-wrapper calls rather than recognizing only a direct os.environ.values() call.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| if python_call_derived_command_alias(value, modules): | ||
| return True | ||
| if isinstance( |
There was a problem hiding this comment.
The new helper traverses literal containers but falls through on ordinary calls, so launchers = {"x": subprocess.run}; for launch in launchers.values(): launch(["gh", "workflow", "run", "ci.yml"]) is accepted. This leaves the iterable-alias finding open for dictionary views and container-conversion calls, allowing a hidden workflow replay or other process launch to pass packet certification; the iterable analysis needs to follow reviewed view/wrapper calls back to their source containers.
AGENTS.md reference: AGENTS.md:L8-L8
Useful? React with 👍 / 👎.
| for statement in module.body: | ||
| if isinstance(statement, (ast.Import, ast.ImportFrom)): | ||
| exec(compile(ast.Module(body=[statement], type_ignores=[]), "<packet-scanner-import>", "exec"), namespace) | ||
| elif isinstance(statement, (ast.FunctionDef, ast.AsyncFunctionDef)): | ||
| exec(compile(ast.Module(body=[statement], type_ignores=[]), "<packet-scanner-function>", "exec"), namespace) |
There was a problem hiding this comment.
When the Markdown scanner fence is modified, this setup executes every extracted import and function definition with full builtins before any specimen is inspected. An added import, decorator, or default-argument expression can therefore run arbitrary code merely by invoking the advertised offline regression harness, contradicting its non-executing contract; validate and whitelist these AST nodes without evaluating packet-controlled imports or definition-time expressions.
Useful? React with 👍 / 👎.
| def config_violation(assignment): | ||
| key = assignment.split("=", 1)[0].lower() | ||
| if git_config_include_key(key): | ||
| return "Git configuration includes are not allowed before read-only commands" |
There was a problem hiding this comment.
The new check covers include keys supplied as -c, --config-env, or git config operands but still accepts git config --includes --list and command-prefix injection such as GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=include.path GIT_CONFIG_VALUE_0=/tmp/inc git status. git config -h describes --[no-]includes as “respect include directives on lookup,” and a local probe confirmed the environment form loads values from the referenced file, so these forms can still read sensitive configuration or activate included Git behavior after passing certification; reject include-enabling flags and inspect Git configuration environment assignments before wrapper stripping.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| @@ -0,0 +1,582 @@ | |||
| """Offline, non-executing regression probes for the seven issue #79 findings. | |||
There was a problem hiding this comment.
This commit introduces a Python implementation without an ADR or evidence-backed exception, even though the repository requires Go unless an ADR changes that decision. Reimplement the harness using the repository's Go tooling or add the required accepted ADR before introducing this non-Go maintenance surface.
AGENTS.md reference: AGENTS.md:L14-L14
Useful? React with 👍 / 👎.
| executable = executable_basename(tokens[0]) | ||
| if executable == "set": |
There was a problem hiding this comment.
The environment-dump check handles set and export but still accepts bare env and forms such as env -0 or env -u NAME; the wrapper parser then strips env, finds no child executable, and certifies the command. env --help explicitly states, “If no COMMAND, print the resulting environment,” so an executable shell fence can still emit every inherited credential despite this hardening; reject env whenever wrapper parsing reaches the no-command case.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
|
Triage checkpoint: Codex completed review of 387a647 with seven P1 findings. All are being treated as unresolved premerge blockers; current quick-check success does not override them. Independent contract/security reviews also reproduced overlapping reader/iterator gaps. A single correction batch with focused red/green evidence is in progress; no merge until final-head independent delta review, a fresh @codex review completion and quick checks. No live operations or workflow replay are authorized by this fix batch. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 387a647355
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) | ||
| ): | ||
| raise SystemExit("Git packet blob query was outside the reviewed revision/path") | ||
| command = git_query(["show", blob_spec]) |
There was a problem hiding this comment.
When the local repository contains refs/replace/<HEAD>, this git show resolves the replacement object, while rev-parse HEAD and the remote ls-remote comparison retain the original commit ID. I reproduced a clean worktree where the original HEAD contained one packet, a replacement commit contained different bytes, and both this blob query and the worktree returned the replacement bytes while HEAD still matched the original remote SHA. git --help lists the global [--no-replace-objects] option; use it for the exact-head queries or explicitly reject replacement refs, otherwise the template can certify content that is absent from the reviewed remote head.
AGENTS.md reference: AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
| guarded = any( | ||
| isinstance(call, ast.Call) | ||
| and python_dotted_name(call.func) == "package_dir.relative_to" | ||
| and len(call.args) == 1 | ||
| and python_path_division_names(call.args[0]) == expected_root |
There was a problem hiding this comment.
When an executable fence defines source_fuzz_guard with these expected variable names but calls it with arbitrary roots, this check only proves that package_dir is below the caller-supplied go_repo_root / module_dir. I loaded the scanner and it accepted a helper invoked as source_fuzz_guard(Path("/"), Path("etc"), Path("")) that prints package_dir.glob("*"), allowing filesystem enumeration to enter packet output. Unlike the unresolved-receiver finding, this uses a directly recognized Path receiver and passes specifically through the new exemption; require provenance for the root or validate every call site rather than trusting the helper's name and parameter spelling.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dac58b4add
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if isinstance(node.func, ast.Name): | ||
| local_helpers = python_local_function_candidates( | ||
| node.func.id, node, tree, parents |
There was a problem hiding this comment.
Restricting helper resolution to calls whose target is an ast.Name lets other local callables launder inherited credentials. For example, a class method that returns dict(os.environ), followed by secret = Snapshot().read(); print(secret), receives no scanner violation and emits the environment; a lambda assigned to a name has the same result because it is absent from the function index. Resolve method/lambda bodies too, or fail closed when their return provenance is unknown.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| elif isinstance(node, (ast.For, ast.AsyncFor, ast.comprehension)): | ||
| # Values yielded from an environment iterator remain sensitive in | ||
| # both loop bodies and comprehension elements. | ||
| assignments.append((node.target, node.iter)) |
There was a problem hiding this comment.
When an outer loop consumes a comprehension, this new binding records the whole comprehension as the target's source, but that expression is not recognized as sensitive when its iterator is the call os.environ.values(). The fresh case for secret in [value for value in os.environ.values()]: print(secret) still passes after the enumerate/zip fix and prints inherited credentials; list, set, dict, and generator comprehensions need to propagate taint from their elements and generators.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| local_functions = {} | ||
| for node in ast.walk(tree): | ||
| if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): | ||
| local_functions.setdefault(node.name, []).append(node) |
There was a problem hiding this comment.
Indexing only FunctionDef/AsyncFunctionDef nodes leaves other local factories outside launcher provenance. I checked factory = lambda: subprocess.run; launch = factory(); launch(["gh", "workflow", "run", "ci.yml"]) and the equivalent class-method factory, and both receive no scanner violation even though they execute a workflow dispatch. Include lambdas and callable methods in return analysis, or reject aliases produced by unresolved local calls.
AGENTS.md reference: AGENTS.md:L8-L8
Useful? React with 👍 / 👎.
| current = node | ||
| while current in parents: | ||
| parent = parents[current] | ||
| if isinstance(parent, ast.If): | ||
| condition = condition_value(parent.test) |
There was a problem hiding this comment.
The reachability evaluator treats unrecognized constant conditions as unknown, so wrapping the containment try in if 1 == 1: pass; else: ... is accepted even though the check can never run; I applied that mutation to the canonical source-fuzz body and inspect_python_heredoc returned no violation. With an escaping package argument, execution continues to package_dir.glob without proving containment, allowing files outside the reviewed module to be inspected. Require the containment try in the direct reachable function body instead of accepting it beneath partially evaluated branches.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| def _verification_function(module: ast.Module, name: str) -> ast.FunctionDef: | ||
| for statement in module.body: | ||
| if isinstance(statement, ast.FunctionDef) and statement.name == name: | ||
| return statement |
There was a problem hiding this comment.
Returning the first matching function lets the harness validate a safe definition while Python executes a later override. I inserted a second require_packet_head_parity that simply returns immediately before its call: the packet scanner reported no violation, _verification_function still selected the original verifier, but the live template would bind and invoke the no-op definition, allowing mismatched packet bytes or intent bits to pass. Require exactly one top-level helper with this name before compiling it.
AGENTS.md reference: AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b79709bf4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( | ||
| isinstance(value, ast.Attribute) | ||
| and value.attr in python_path_filesystem_read_methods | ||
| and not python_known_non_path_reader_call(value, tree) | ||
| ): | ||
| receiver = value.value | ||
| elif isinstance(value, ast.Name) and value.id in aliases: | ||
| receiver = aliases[value.id] |
There was a problem hiding this comment.
Fresh evidence after the direct-receiver fix is def factory(): return Path("synthetic-private/file").read_text; reader = factory(); print(reader()): the current scanner returns no violation and the snippet emits arbitrary file contents. This alias analysis follows only direct attributes and existing names, not call results, even though local return analysis already exists elsewhere; propagate returned Path reader callables or fail closed on the unresolved alias.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| if ( | ||
| isinstance(value.func, ast.Attribute) | ||
| and value.func.attr in {"items", "keys", "values"} | ||
| ): |
There was a problem hiding this comment.
Fresh evidence after the dictionary-view fix is launchers = {"x": subprocess.run}; launch = launchers.pop("x"); launch(["gh", "workflow", "run", "ci.yml"]), which the scanner accepts. I checked gh 2.96.0's workflow run --help, which states that this command creates a workflow_dispatch event and triggers the workflow, so limiting attribute-call propagation to items/keys/values still permits an automatic replay through pop; mapping element-producing methods must preserve launcher provenance.
AGENTS.md reference: AGENTS.md:L8-L8
Useful? React with 👍 / 👎.
| for node in ast.walk(tree): | ||
| if not isinstance(node, ast.Call) or not isinstance(node.func, ast.Name): | ||
| continue | ||
| function = local_functions.get(node.func.id) | ||
| if function is None: | ||
| continue | ||
| for parameter, argument in call_arguments(node, function): | ||
| if python_sensitive_value_expression( | ||
| argument, sensitive_names, tree, parents | ||
| ) and parameter not in sensitive_names: | ||
| sensitive_names.add(parameter) | ||
| changed = True | ||
| for function in python_local_function_candidates( | ||
| node.func.id, node, tree, parents | ||
| ): |
There was a problem hiding this comment.
Fresh evidence exercises argument flow rather than the previously fixed return flow: class C: def emit(self, payload): print(payload); C().emit(os.environ) receives no violation and prints the inherited environment; an assigned lambda with the same body also passes. The call-site taint pass considers only calls whose target is an ast.Name and resolves only function definitions, so method and lambda parameters never become sensitive; bind arguments for those local callable forms as well.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| packet_path = Path("docs/evidence/g01-recovery-packet.md") | ||
| intent_result = run_bounded_git_query( | ||
| git_query(["ls-files", "-v", "-z", "--", packet_path.as_posix()]), | ||
| cwd=Path.cwd(), | ||
| env=git_environment, |
There was a problem hiding this comment.
When scripts/evidence_packet/issue79_regression_test.py is marked assume-unchanged or skip-worktree and then edited to weaken or skip its tests, this packet-only intent/blob check still succeeds and the later git status remains empty, so test output from modified code can be paired with the reviewed remote HEAD. I checked the installed git update-index -h, which describes these flags as marking files “not changing” or “index-only”; reject such intent bits and compare HEAD bytes for every changed evidence source, not only the Markdown packet.
AGENTS.md reference: AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9233241cbd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if isinstance(value, ast.Call) and isinstance(value.func, ast.Name): | ||
| class_names.add(value.func.id) | ||
| for returned in python_local_call_return_values(value, tree, parents): |
There was a problem hiding this comment.
Fresh evidence after the direct method-return correction is Alias = Snapshot; print(Alias().read()), where Snapshot.read returns dict(os.environ): the scanner reports no violation and prints the inherited environment. This resolver records the called name Alias as the class name without following its assignment to Snapshot, so both sensitive method returns and parameter flows can still be laundered through a class alias; resolve constructor aliases before looking up the method.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| isinstance(value.func, ast.Attribute) | ||
| and value.func.attr in {"items", "keys", "values", "pop"} | ||
| ): |
There was a problem hiding this comment.
Fresh evidence after the .pop correction is launchers = {"x": subprocess.run}; launch = launchers.get("x"); launch(["gh", "workflow", "run", "ci.yml"]), which the scanner accepts because this mapping-method list omits get. I checked gh 2.96.0's workflow run --help, which says it creates a workflow_dispatch event and triggers the workflow, so mapping lookup can still hide an automatic workflow replay; preserve launcher provenance through get as well.
AGENTS.md reference: AGENTS.md:L8-L8
Useful? React with 👍 / 👎.
| if dotted not in path_preserving_calls and not ( | ||
| isinstance(node.func, ast.Attribute) | ||
| and node.func.attr in {"as_posix", "as_uri"} | ||
| ): | ||
| return False |
There was a problem hiding this comment.
When a resolved path is converted with the built-in formatter, value = format(Path.cwd().resolve()); print(value) receives no scanner violation and emits the local worktree path. Because an unrecognized call returns False here instead of propagating path provenance from its arguments, the new output-sink protection can be bypassed by format (and similarly ascii or an explicit __str__ call); include these path-preserving conversions or conservatively trace their arguments.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| The harness must use only the standard library and explicitly selected local | ||
| Git fixture operations. Invoke it with `python3 -B` to avoid bytecode artifacts | ||
| and record the actual interpreter and test results. Adding dependencies, |
There was a problem hiding this comment.
When the maintainer environment has PYTHONPATH, a user-site sitecustomize, or another Python startup customization, the prescribed python3 -B command executes that code before the harness's import and definition-time validation. I reproduced this with a synthetic PYTHONPATH containing ast.py, which executed immediately on harness startup; Python's local --help confirms that -B only disables bytecode while -I isolates Python from the user environment. Invoke the harness with isolated mode so external startup/import code cannot access credentials or corrupt the recorded regression result.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a61c35fb8a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for statement in candidate.body | ||
| for call in ast.walk(statement) |
There was a problem hiding this comment.
When the Markdown link check nests path.relative_to(repository_root) under if False, this broad ast.walk predicate still marks the path as guarded; I applied that mutation to the canonical link-check heredoc and inspect_python_heredoc returned no violation. A repository Markdown target such as ../../private/file#x can then reach path.is_file() and anchors(path) without containment, causing certification to read outside the reviewed repository; require the containment call to be a direct reachable statement in the guarded try.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| candidates = tuple(assignments.get(value.id, ())) + tuple( | ||
| iterable_bindings.get(value.id, ()) | ||
| ) |
There was a problem hiding this comment.
When a function parameter shadows a module-level regex-match variable, this global name-based lookup still classifies the parameter as a regex result. I verified that match = re.match("x", "x"); def expose(match): print(match.group()); expose(Path("/etc/passwd")) receives no scanner violation, even though runtime dispatches Path.group() and performs an unreviewed filesystem metadata read. Scope assignment and parameter bindings before exempting .group() as a regex API.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c755af9dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if dotted == "dict" and any( | ||
| isinstance(argument, ast.Attribute) | ||
| and python_dotted_name(argument) == "os.environ" | ||
| for argument in node.args | ||
| python_sensitive_value_expression( | ||
| value, sensitive_names, tree, parents, seen.copy() | ||
| ) | ||
| for value in list(node.args) |
There was a problem hiding this comment.
In the packet's isolated Python heredocs, aliasing a preserving constructor bypasses this name-based check: maker = dict; value = maker(os.environ); print(value) receives no scanner violation and prints the inherited environment. Resolve aliases of the listed container/string constructors before testing the call target, otherwise packet certification can emit credentials despite the new taint propagation.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| python_imported_function_alias_is_stable( | ||
| node.func.id, "sys", "exit", tree | ||
| ) | ||
| or python_assigned_callable_alias(node.func.id, "sys.exit", tree) | ||
| ) |
There was a problem hiding this comment.
The name-based alias branch only resolves sys.exit, so a packet heredoc such as emit = print; emit(os.environ) receives no scanner violation and writes the entire inherited environment; emit = warnings.warn is affected similarly. Extend alias resolution to all sinks recognized by this function so known credential taint cannot be laundered merely by assigning the sink to another name.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| assignments_by_name, | ||
| seen.copy(), | ||
| ): | ||
| return True | ||
| return False |
There was a problem hiding this comment.
In the packet's isolated Python heredocs, print(Path.home()), print(Path("~").expanduser()), and print(os.path.expanduser("~")) all receive no scanner violation while emitting the maintainer's personal home path. This provenance check recognizes Path.cwd() but returns false for these other standard local-path constructors, so include home/expansion operations among resolved local path sources before output-sink certification.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| or value not in reviewed_shell_export_assignments | ||
| for value in assignments | ||
| ): | ||
| return "shell export requires explicit reviewed assignments" | ||
| return None |
There was a problem hiding this comment.
The new environment-output gate accepts every non-set/export command, but the shell allowlist includes processors with direct environment access: awk 'BEGIN { print ENVIRON["GH_TOKEN"] }' and jq -n env both receive no scanner violation, and local synthetic probes confirmed that each prints the supplied variable. Inspect or reject the environment primitives of these allowlisted processors so executable packet fences cannot emit inherited credentials.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| def _top_level_assignment(module: ast.Module, name: str) -> ast.Assign | ast.AnnAssign: | ||
| for statement in module.body: | ||
| if isinstance(statement, ast.Assign) and any( | ||
| isinstance(target, ast.Name) and target.id == name | ||
| for target in statement.targets | ||
| ): | ||
| return statement | ||
| if isinstance(statement, ast.AnnAssign) and isinstance(statement.target, ast.Name) and statement.target.id == name: |
There was a problem hiding this comment.
Fresh evidence beyond the duplicate-helper correction is a second top-level issue79_reviewed_evidence_paths assignment after the canonical one: this helper returns the first assignment and the regression assertion still sees both reviewed files, while Python executes the later value and the live parity loop can omit the harness entirely. Requiring each security-relevant template assignment to be unique prevents an assume-unchanged modified harness from being paired with the reviewed remote head after tests validated a shadowed value.
AGENTS.md reference: AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
https://github.com/1XP-AI/gh-runnerd/blob/1f5f89bc80f09393dbc44f45d20f0141e747005a/docs/evidence/g01-recovery-packet.md#L11329-L11330
Fresh evidence after the launcher-alias corrections is import subprocess; (launch := subprocess.run)(["gh", "workflow", "run", "ci.yml"]), which the current scanner accepts. The installed gh 2.96.0 help states that workflow run creates a workflow_dispatch event and triggers the workflow, so limiting NamedExpr handling here to conditional values still permits an automatic replay when the assignment expression itself is the call target; resolve that target or fail closed on such callable expressions.
AGENTS.md reference: AGENTS.md:L8-L8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "traceback.print_exception", | ||
| }: | ||
| return True | ||
| if isinstance(node.func, ast.Name) and tree is not None: |
There was a problem hiding this comment.
Fresh evidence after the assigned/import-alias fix is import os; (emit := print)(os.environ): the current scanner returns no violation, while Python immediately invokes print and emits the entire inherited environment. This branch only resolves sink aliases when the callable is an ast.Name, so a NamedExpr used directly as the call target bypasses the new protection; resolve callable expressions here or fail closed on unresolved targets.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| for value in assignments | ||
| ): | ||
| return "shell export requires explicit reviewed assignments" | ||
| return None |
There was a problem hiding this comment.
In an executable Bash fence, name=GH_TOKEN followed by printf '%s\n' "${!name}" receives no scanner violation and prints the inherited token; I confirmed this with a synthetic GH_TOKEN. The new environment-output gate accepts every command other than the special set/export cases, while the earlier parameter regex does not recognize Bash's ${!name} indirect expansion, so an allowlisted printf can still place credentials in packet output; reject indirect expansion or resolve its referenced variable before allowlisting the command.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
|
@codex review Please review exact current head The AugAssign RHS now uses existing receiver provenance before and independently of the simple local-assignment alias exemption; target/operator do not bypass it. Sensitive global-holder and harmless paired controls, local Name receiver RHS, and supported numeric/literal-list augmentation are covered. No effects engine or allowlist expansion; prior corrections unchanged. Actual evidence: RED2methods/4failedassertions/0errors0.246s (one sensitive acceptance, three harmless-policy assertions); GREEN0.241s;122focusedPASS21.382s; identical328-helper/7-regexpreflightPASS12.116s. ONE fresh fullpacket PASS958.273s/exit0/261shell86Pythonzero. Packet Two independent Codex gpt-6.1-sol / medium reviewers approved the exact SHA, parent, unchanged tested blobs, clean tree and three-file scope. Contract independently ran2methodsPASS0.237s; security2methodsPASS0.235s and a finite attribute-target/safe contrastPASS0.315s including loader. Final attestations: msg_7bc28421ec8e and msg_5071cdf5aaed; both completed/released. No specimen executed and no duplicated final-gate test run. Project #79 is In review. Your fresh completed same-head review, all-finding triage and hosted quick remain mandatory before merge. No live canary, runner/JIT/credentials/container operation or workflow dispatch; G01/G02 remain open. |
💡 Codex Reviewhttps://github.com/1XP-AI/gh-runnerd/blob/dc83077613276d4c9ffba0cae3aaddd9696fbef5/docs/evidence/g01-recovery-packet.md#L22908 An exact-head inert probe with AGENTS.md reference: AGENTS.md:L9-L9 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Completed exact-head review of |
|
@codex review Please review exact current head Existing provenance now checks BOTH target and RHS. The target Store context follows retained alias-assignment bindings through one/two hops before preserving receiver identity. RED2methods/4failures/0errors0.255s comprises two sensitive acceptances plus harmless policy pairs; direct-self and unknown controls already refused. V14+V15 GREEN4/0.276s,124focusedPASS20.848s,identical328helpers/7regexpreflightPASS12.064s. No generic operator/effects engine or allowlist expansion. Packet Fresh V15 whole-packet scan NOT RUN. Validation sequencing now follows incremental correction cadence: the one fresh whole-packet qualification will run on the stable GitHub-approved immutable head BEFORE MERGE, with result in a PR comment and no reviewed-head amendment. Historical V14 PASS958.273s is not V15 proof. This sequencing change removes no independent/GH/hosted quick/security gate and does not represent unrun checks as passed. Project #79 In review; exact-head hosted quick, completed Codex review/all-finding triage and pending whole-packet qualification all remain required. No specimen execution/live canary/runner/JIT/credential/container/workflow-dispatch operation; G01/G02 remain open. |
💡 Codex Reviewhttps://github.com/1XP-AI/gh-runnerd/blob/516de3e1f0a0753ed372aca3fae23aed2f23a34f/docs/evidence/g01-recovery-packet.md#L22908-L22909 Fresh exact-head evidence beyond the simple alias cases is AGENTS.md reference: AGENTS.md:L9-L9 https://github.com/1XP-AI/gh-runnerd/blob/516de3e1f0a0753ed372aca3fae23aed2f23a34f/docs/evidence/g01-recovery-packet.md#L23358-L23359 Fresh exact-head evidence beyond the repaired iterator/comprehension cases is AGENTS.md reference: AGENTS.md:L9-L9 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Completed exact-head review of |
|
@codex review Please review exact head Augmented targets now require supported ordinary assignment origins through existing provenance/inventory, ignoring only AugAssign store entries: a separate with-store remains unsupported through aliases/alternatives. Complete context-manager examples include enter and exit. Load-context direct subscriptions reuse existing getitem protocol-return analysis; visible sensitive alternatives refuse and known harmless custom/literal selections remain supported. No new generic effects interpreter or allowlist expansion; no general unknown-object safety claim. RED3methods/11failedassertions/0errors0.297s =6sensitive paths +5harmless-policy assertions. GREEN7methods0.339s,127focusedPASS22.000s,identical328helpers/7regexpreflightPASS12.182s. Packet Two independent Codex gpt-6.1-sol / medium reviewers approved exact SHA,parent,unchanged tested blobs,clean tree/3file scope. Direct-subscription reviewer ran8finitecontrastsPASS0.297s; binding reviewer6selectedmethodsPASS0.310s plus two same-binding contrasts confirming unknown with-store preservation. Final attestations msg_1056866e2ac5 and msg_08a3a338e140; both settled/released. Fresh V16 whole-packet NOT RUN. One fresh qualification remains mandatory on stable GitHub-approved immutable head BEFORE MERGE, recorded in a PR comment without changing source; historical V14 full is not current proof and V15 full was not run. Same-head hosted quick, completed Codex review and all-finding triage also remain mandatory. Project In review; no specimen execution, live canary, runner/JIT/credentials/container operations or workflow dispatch; G01/G02 remain open. |
💡 Codex Reviewhttps://github.com/1XP-AI/gh-runnerd/blob/4218cd063a4cba8fc64242e953f13ecce24f82e2/docs/evidence/g01-recovery-packet.md#L23379-L23380 When AGENTS.md reference: AGENTS.md:L9-L9 https://github.com/1XP-AI/gh-runnerd/blob/4218cd063a4cba8fc64242e953f13ecce24f82e2/docs/evidence/g01-recovery-packet.md#L23379-L23380 When AGENTS.md reference: AGENTS.md:L9-L9 https://github.com/1XP-AI/gh-runnerd/blob/4218cd063a4cba8fc64242e953f13ecce24f82e2/docs/evidence/g01-recovery-packet.md#L23379-L23380 A class subscription can bypass this new check entirely: AGENTS.md reference: AGENTS.md:L9-L9 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Completed exact-head review of |
|
@codex review Please review exact head 130 focused methods PASS22.416s; identical expanded preflight PASS12.802s; zero new collisions/lost proofs. Two independent Codex gpt-6.1-sol/medium reviews approved this immutable SHA (contract7methods0.377s+4inert contrasts; security3methods0.269s+12inert contrasts). Ledger preserves historical findings/limitations. No newer inline or review-object findings observed during paginated audit. Fresh full-packet qualification remains NOT RUN: one scan will run on the stable GitHub-approved exacthead before merge, recorded separately without amending reviewed source. Hosted quick is pending. No specimen execution, live action or merge; G01/G02 remain open. |
💡 Codex Reviewhttps://github.com/1XP-AI/gh-runnerd/blob/8994900ca4181aa3341601d6466c669326e388b4/docs/evidence/g01-recovery-packet.md#L23418-L23420 Fresh exact-head evidence beyond the repaired direct and alias cases is a local AGENTS.md reference: AGENTS.md:L9-L9 https://github.com/1XP-AI/gh-runnerd/blob/8994900ca4181aa3341601d6466c669326e388b4/docs/evidence/g01-recovery-packet.md#L22864-L22868 When AGENTS.md reference: AGENTS.md:L9-L9 https://github.com/1XP-AI/gh-runnerd/blob/8994900ca4181aa3341601d6466c669326e388b4/docs/evidence/g01-recovery-packet.md#L22860-L22861 Fresh exact-head evidence beyond direct class-body methods is AGENTS.md reference: AGENTS.md:L9-L9 https://github.com/1XP-AI/gh-runnerd/blob/8994900ca4181aa3341601d6466c669326e388b4/docs/evidence/g01-recovery-packet.md#L22864-L22868 An exact-head heredoc can define AGENTS.md reference: AGENTS.md:L9-L9 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Completed exact-head review of |
|
@codex review Please review exact head Actual final evidence:136focused PASS21.747s; expanded compatibility preflight PASS12.256s206/328helpers7regexzero collisions/lostproofs; two independent Codex gpt-6.1-sol/medium SOURCE approvals on this SHA. Contract original HOLD now refuses, alias methodPASS0.252s+16inertcontrasts; security aliasmethodPASS0.270s with prior scoped generator/state evidence preserved. Immutable ledger distinguishes sensitive RED from harmless unsupported policy, and initial versus final source verification. Paginated issue/inline/review-object audit found no newer untriaged findings. Fresh whole-packet qualification NOT RUN: one stable GitHub-approved-head scan is required before merge, recorded separately without amending reviewed source. Same-head hosted quick pending. No specimen execution, live operation or merge; G01/G02 remain open. |
💡 Codex Reviewhttps://github.com/1XP-AI/gh-runnerd/blob/d6e6686ee840bf483cbc790101482420d242edea/docs/evidence/g01-recovery-packet.md#L22872-L22875 Fresh exact-head evidence beyond the repaired tuple/iterator wrappers is AGENTS.md reference: AGENTS.md:L9-L9 https://github.com/1XP-AI/gh-runnerd/blob/d6e6686ee840bf483cbc790101482420d242edea/docs/evidence/g01-recovery-packet.md#L22823-L22825 Fresh exact-head evidence beyond the repaired direct and literal-container aliases is AGENTS.md reference: AGENTS.md:L9-L9 https://github.com/1XP-AI/gh-runnerd/blob/d6e6686ee840bf483cbc790101482420d242edea/docs/evidence/g01-recovery-packet.md#L22922-L22925 Fresh exact-head evidence beyond the repaired nested method definitions is AGENTS.md reference: AGENTS.md:L9-L9 https://github.com/1XP-AI/gh-runnerd/blob/d6e6686ee840bf483cbc790101482420d242edea/docs/evidence/g01-recovery-packet.md#L22809-L22814 Fresh exact-head evidence beyond direct callable-attribute mutation is a factory that creates AGENTS.md reference: AGENTS.md:L9-L9 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Completed exact-head review of |
|
@codex review Please review exact head 140 focused PASS21.635s; expanded preflight PASS12.363s206/328helpers7regexzero collisions/lostproofs. Two independent Codex gpt-6.1-sol/medium exact-SHA SOURCE approvals: contract4methods0.267s+15inert contrasts; security4methods0.276s/16methods0.515s+7contrasts. Ledger preserves every source-scoped receipt and distinguishes sensitive RED from harmless unsupported policy; aliased nested-list tuple checks refuse, without general-proof claims. No newer inline/review-object findings were found in paginated audit. Fresh whole-packet qualification NOT RUN: mandatory once on stable GitHub-approved head before merge, receipt in PR without source amendment. Hosted quick pending. No specimen execution/live action/merge; G01/G02 stay open. |
💡 Codex Reviewhttps://github.com/1XP-AI/gh-runnerd/blob/3b44ecc7628a98b90f2d7a5a94e5e1c8a5d1adc9/docs/evidence/g01-recovery-packet.md#L22882-L22885 Fresh exact-head evidence beyond the repaired positional-mapping cases is AGENTS.md reference: AGENTS.md:L9-L9 https://github.com/1XP-AI/gh-runnerd/blob/3b44ecc7628a98b90f2d7a5a94e5e1c8a5d1adc9/docs/evidence/g01-recovery-packet.md#L22794-L22797 Fresh exact-head evidence beyond the repaired local-name case is AGENTS.md reference: AGENTS.md:L9-L9 https://github.com/1XP-AI/gh-runnerd/blob/3b44ecc7628a98b90f2d7a5a94e5e1c8a5d1adc9/docs/evidence/g01-recovery-packet.md#L22932-L22937 Fresh exact-head evidence beyond the class-suite binder cases is a descriptor whose AGENTS.md reference: AGENTS.md:L9-L9 https://github.com/1XP-AI/gh-runnerd/blob/3b44ecc7628a98b90f2d7a5a94e5e1c8a5d1adc9/docs/evidence/g01-recovery-packet.md#L22827-L22830 Fresh exact-head evidence for the new callable-state proof is a AGENTS.md reference: AGENTS.md:L9-L9 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Completed exact-head review of |
|
@codex review Please review exact head Final145focused PASS22.030s and exactexpandedpreflight PASS12.456s206/328helpers7regexzero collisions/lostproofs. Two independent Codex gpt-6.1-sol/medium SOURCE approvals on this SHA: contract5methods0.289s+8inertcontrasts, security22methods0.639s+12contrasts. Ledger accurately scopes initial versus final results and policy-only keyword-only callback controls. Paginated inline/review-object audit found no newer untriaged findings. Fresh full-packet qualification remains NOT RUN, mandatory once on stable GitHub-approved exacthead before merge; receipt will not amend reviewed source. Hosted quick pending. No specimen execution/live action/merge; G01/G02 stay open. |
💡 Codex Reviewhttps://github.com/1XP-AI/gh-runnerd/blob/66c33aa19ab72603e48a8deb67fbe7d11064da6f/docs/evidence/g01-recovery-packet.md#L22893 When a mapping passed through AGENTS.md reference: AGENTS.md:L9-L9 https://github.com/1XP-AI/gh-runnerd/blob/66c33aa19ab72603e48a8deb67fbe7d11064da6f/docs/evidence/g01-recovery-packet.md#L22822-L22824 Fresh evidence beyond the repaired direct-parameter case is AGENTS.md reference: AGENTS.md:L9-L9 https://github.com/1XP-AI/gh-runnerd/blob/66c33aa19ab72603e48a8deb67fbe7d11064da6f/docs/evidence/g01-recovery-packet.md#L22794 Fresh evidence beyond the repaired literal-member case is AGENTS.md reference: AGENTS.md:L9-L9 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
The exact- |
V21 security/compatibility HOLD — no new candidate pushedPR head remains Two independent Codex The additional minimal correction reproduced 12 sensitive failures, then passed eight targeted methods. However, the combined 151-method run FAILED one canonical safe-positive test (17.588s, zero errors); the compatibility preflight passed (12.397s, 206/328/7 checks, no collisions/lost proofs). A separate scope-collision positive also reproduced a failure (0.227s). Fresh whole-packet qualification was NOT RUN; earlier full-pass receipts are historical only. Both independent reviewers withhold final source approval: existing unordered store/provenance analysis cannot establish safe selected values after overwrites and later alias/call effects. Fixing this while retaining existing safe positives requires explicitly scoped new key-domain/order-sensitive overwrite and alias/call-invalidation proofs, adversarial TDD, and fresh independent review. No key/path allowlist, unknown-as-safe shortcut, or canonical-fixture modification was accepted. Local source/tests/evidence are preserved uncommitted; no V21 push, merge, live canary, token, registration or runner operation occurred. Issue #79 remains open and moves to Blocked pending authorization of that bounded design expansion. Codex's reusable summary still reported the old head as running at the last audit; the findings report is not being represented as completed final-head approval. |
V22 resumed with maintainer authorizationThe maintainer explicitly approved the design and implementation expansion identified in the V21 HOLD: mapping value overwrite order, complete finite key domains, and alias/mutation/call invalidation. Issue #79 is now In progress. One writer and two independent reviewers are working in the existing issue worktree using the maintainer-selected Codex PR head is still This authorization covers local design, implementation and verification followed by the previously authorized push/review/merge process. Actual live canary execution still requires separate approval. |
|
V22 checkpoint — HOLD, not a merge candidate. Local source and inert regression specimens are preserved. Corrections now cover the reproduced declared-writer, nearer loop/comprehension binder, augmented-subscript and mutated update-provider gaps. The current focused batch still has known positive-case failures for the primary direct-subscript overwrite boundary and owner-local environment forwarding; no passing final batch is claimed. The mandatory second independent security review could not be completed after a provider interruption. No final source approval, replacement review, new candidate push or merge has been performed. The shared output-boundary change remains unimplemented; prior design-only approval is not source approval. The latest completed unchanged canonical qualification remains a failure; later bounded instrumented diagnostics are not qualification. V22 combined focused, compatibility preflight and fresh whole-packet qualification remain outstanding. The published PR head is still Project #79 is Blocked pending availability of the required review and completion of the remaining implementation/verification. G01/G02 and live canary execution remain separate, incomplete gates. No workflow dispatch, runner change or live execution was performed. |
1 participant
Issue #79 — keyword, member, hook and callback proof closure
Head:
66c33aa19ab72603e48a8deb67fbe7d11064da6f. Refs #79; G01/G02 remain open.The immutable ledger retains historical source-scoped findings and addresses the four-P1 exact-3b44ecc report.
Actual evidence
Initial RED4methods15failed assertions/zero errors0.287s (9sensitive6policy); additional computed-hook closure RED1method4failures0.249s (2sensitive2policy). Final GREEN22methods0.632s; 145 focused methods PASS22.030s; identical expanded preflight PASS12.456s206/328helpers7regexzero collisions/lostproofs. Earlier144-method receipts retain their own hashes.
Packet
9d9a0dfba93d08f04c62f5cb9f527e48cad5375a; harness76a232bff88a0da828c6f52f60c329b0a91c1462; ledger3b7806ef34b59b26fcb1609de2571b514e43736f. Exact three-file hygiene and clean candidate verified. Two independent Codexgpt-6.1-sol / mediumexact-SHA source approvals under maintainer override: contract5methods0.289s+8contrasts, security22methods0.639s+12contrasts; keyword-only callback contrasts explicitly policy-only.Fresh V20 whole-packet qualification: NOT RUN / pending. One scan remains mandatory on the stable GitHub-approved immutable candidate BEFORE MERGE, recorded in a PR comment without amending source. Historical V14 full PASS958.273s is not current evidence; V15–V19 full scans were not run.
Remaining gates / safety
Completed same-head GitHub Codex review, all-finding triage, hosted quick and stable-head packet qualification remain mandatory. Public CI remains postmerge integration evidence.
All specimens/recipes/descriptors/callbacks stayed inert AST data. No live canary, runner/JIT/credential/token/container operation, manual workflow dispatch or specimen execution occurred. Rollback is a reviewed bounded revert preserving historical evidence and reopening removed coverage.