How to review command injection without running untrusted code

You can review a command-injection claim from source without executing the application, a repository hook, or a suspicious command. Identify who controls the executable or each argument, whether a shell interprets a constructed string, what the called program does with its arguments, and what privileges the process has. Trace the actual entry point and any validation before the process call. A shell string containing user input is a strong lead; a process argument array may close the shell-injection path while leaving argument injection or file-handling risks to review.

Python's subprocess security notes say that the library does not implicitly invoke a shell and that an explicit shell makes quoting the application's responsibility. OWASP's command injection guide recommends avoiding OS commands where a suitable library operation exists, then separating command structure from data and validating allowed arguments when a process is necessary. These are design rules; a reviewer still has to prove which code path and interpreter are involved.

An illustrative report-rendering job

Invented service and code. Nothing below was executed. Cedar Reports lets an authenticated operator export a report in a selected format. A queued job eventually runs a renderer. The scanner cites this worker:

def render_report(job):
    command = (
        "/opt/report-render --format " + job.format_name
        + " --input " + job.input_path
    )
    subprocess.run(command, shell=True, check=True)

The worker builds one string and requests shell interpretation. The reviewer should trace whether format_name or input_path ultimately comes from the operator, a trusted enum, a fixed service path, or another job producer. The immediate code does not show those origins. A statement such as “remote command execution confirmed” would therefore be premature. The source does support a precise question: can an actor influence any part of the shell string before this worker uses it?

The queue boundary does not make data trusted. Check the API that creates jobs, any other producer, the serialized job schema, and the worker's validation. A validator in the web handler does not protect a second producer that writes the same queue directly. Conversely, an immutable server-side format mapping used by every producer could defeat the proposed format-input path. Record the actual call chain rather than assuming the scanner's taint label settles it.

Use this path record:

The last row matters. MITRE's CWE-78 describes commands interpreted with externally influenced special elements, but a source line alone does not reveal whether the job can reach production, what the child can read, or what action an attacker could ultimately cause.

Review shell syntax and program arguments separately

A common “fix” changes the call to an argument list:

subprocess.run(
    ["/opt/report-render", "--format", job.format_name, "--input", job.input_path],
    shell=False,
    check=True,
)

This removes the implicit shell interpretation in the ordinary Python subprocess path, which is material progress. It does not prove that job.format_name and job.input_path are safe for the renderer itself. A program can interpret a value as an option, a URL, a template, a file path, or an instruction in its own syntax. MITRE's CWE-88 and OWASP's argument-injection discussion separate this risk from shell metacharacter parsing. The called program's documented option and input semantics are therefore part of the review.

The Python documentation also notes a platform wrinkle: on Windows, batch files may be launched through a shell by the operating system even when the Python call passes an argument list. Do not assert a cross-platform guarantee from shell=False alone. If the deployment target, executable type or wrapper is unknown, mark that uncertainty rather than claiming the path safe.

An illustrative design with a narrower input boundary is:

FORMAT_FLAGS = {"pdf": "--pdf", "text": "--text"}

def render_report(job):
    flag = FORMAT_FLAGS.get(job.format_name)
    if flag is None:
        raise ValueError("unsupported format")
    input_path = path_for_server_assigned_job(job.id)
    subprocess.run(
        ["/opt/report-render", flag, input_path],
        shell=False,
        check=True,
        timeout=30,
    )

The child also inherits process context unless the caller controls it. Review its environment, working directory, configuration search paths, inherited handles and stdin: a fixed executable can still load attacker-influenced configuration or plugins. Use a deployment-owned executable and narrowly configured child process, with document parsing confined separately.

The example assumes path_for_server_assigned_job returns a path fixed by trusted job storage; that assumption must be verified in a real application. The exact renderer must also be checked to see whether a path beginning with a dash, a special protocol, or embedded content changes its behavior. Where a maintained library can render the report without spawning a process, prefer that design and review the library's handling of untrusted document content. If a process remains necessary, run it with only the access it needs. A timeout bounds one failure mode; it is not an injection defense.

Read the renderer's contract before naming the remaining flaw

Assume the fictional native renderer has this deliberately narrow interface:

report-render (--pdf | --text) ABSOLUTE_INPUT_FILE
exactly one format flag and one file; no URL or expression inputs

This is an invented contract to make the review concrete, not documentation for a real utility. For job E-7, the intended argument vector is:

["/opt/report-render", "--pdf", "/srv/jobs/E-7/input.json"]

This vector contains the executable plus two arguments: one flag and one filename. Count and position matter. The implementation above matches this fictional interface; a real binary must be checked against its own parser and version.

The job owns an opaque ID and a format key. Its creator does not choose an executable, an arbitrary flag or a filesystem path. The worker loads the job record using its authorized tenant/project context, maps the format again, and obtains the input location from trusted storage metadata. If a direct queue producer sends format_name="archive", the worker rejects it before process creation. A web-only enum would not settle that second-producer branch.

Now vary one fact at a time:

A name such as safe_render cannot answer these questions. The effective executable, wrapper and input grammar can.

A completed source-only disposition

The synthetic source packet shows the web and scheduled-job producers converging on the same worker validation, a server-owned input location, and the fixed native executable. It supports rejecting the original shell-syntax claim for the repaired revision. It does not qualify the renderer's document parser: that component's version and resource-loading configuration were not supplied.

A useful handoff therefore closes the old source path conditionally on deployment matching that revision and assigns one bounded follow-up to the rendering owner: supply the parser configuration and child-process authority. No command needs to be launched to identify this evidence gap. Functional confirmation of valid PDF/text exports belongs in the application's authorized environment, separate from reading untrusted repository code.

Validate a repair honestly

Compare old and new pinned revisions: did the shell string disappear on every relevant path, are format choices fixed, and does the job identifier resolve to a server-controlled input location? Review the renderer's documented argument semantics, the deployed executable type, and the worker's actual permissions. A security practitioner may run safe functional checks in a controlled environment using synthetic jobs if separately authorized by the application team. The examples here are unexecuted; source inspection does not authorize running repository code, hooks or dependencies.

Inspect the inputs the process receives indirectly

The explicit argument vector is only part of Cedar's process contract. The working directory, environment, inherited input, and referenced file content can also influence the renderer. The existing source packet names those as review targets; turn them into an evidence request that the deployment owner can answer without running the repository.

For job E-7, ask which component writes /srv/jobs/E-7/input.json, which identities can replace it, and whether the worker can accidentally select another job's input. Ask whether the deployment supplies a fixed renderer configuration or lets the current directory contribute settings. These are questions about the actual utility and environment, not assertions that the fictional renderer loads plugins or remote content.

The same care applies to standard input. A fixed command can accept a separate stream of instructions or document data. Determine which grammar that stream uses and which actor controls it. Removing shell parsing is a useful repair of one boundary; it does not eliminate every interpreter inside the child process.

Write a source-only review note with a useful next action

A concise Cedar note can state that the repaired worker constructs the documented argument vector, validates the format key, and selects a server-owned input location. It can then name the unresolved part: the renderer version and document-resource policy. The rendering owner receives a request for those artifacts, rather than an instruction to launch an unknown command to “see what happens.”

If the owner supplies documentation for a different renderer version, keep the version gap visible. If the source references a deployment-owned wrapper whose content is unavailable, the wrapper is the next evidence boundary. A fixed executable path in application code is not proof of what bytes occupy that path in the assessed environment.

Preserve ordinary rendering behavior in the repair

A safe remediation should still support the approved PDF and text outputs for a valid synthetic job. Define those functional expectations alongside the denial of unknown format keys and arbitrary input locations. This gives the application team a controlled validation plan without asking the reviewer to execute customer code.

For a removed legacy shell path, inspect error handling and secondary producers at the new revision. A rejected format must not fall back to the old helper, and a scheduled producer must reach the same worker decision. The closure claim can then be specific: the reviewed source paths no longer assemble a shell command from these job fields. Any remaining parser, file-access, or deployment uncertainty stays attached to its own follow-up.

If the worker moved, a second producer was not inspected, or the deployment platform remains unknown, a missing alert after the patch is not closure. Keep the outcome inconclusive until the path and control are evidenced. The next step for a reviewer is to fill the five-edge path record for the next process-launch alert. For the general interpreter method see injection review from input to operation; for a separate file-path question see reviewing unsafe path construction.