Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion schemas/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,10 @@
"type": "string"
}
},
"bazel-deps-query": {
"description": "A bazel query command to run to dynamically resolve additional dependency paths. The output is parsed to extract local package paths which are merged with additional-paths. Example: \"bazel query 'deps(//my-service)'\"",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devin you should be able to construct the query based on the package name

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call! I'll update it so the query is auto-constructed from the package path. I'll make bazel-deps-query accept either:

  • true (boolean) → auto-constructs bazel query 'deps(//package-path)'
  • a string → uses it as a custom query (for advanced use cases)

So the typical config becomes just:

{
  "packages": {
    "apps/my-app": {
      "bazel-deps-query": true
    }
  }
}

"type": "string"
},
"version-file": {
"description": "Path to the specialize version file. Used by `ruby` and `simple` strategies.",
"type": "string"
Expand Down Expand Up @@ -494,6 +498,7 @@
"initial-version": true,
"exclude-paths": true,
"component-no-space": false,
"additional-paths": true
"additional-paths": true,
"bazel-deps-query": true
}
}
22 changes: 20 additions & 2 deletions src/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import {
} from './util/pull-request-overflow-handler';
import {signoffCommitMessage} from './util/signoff-commit-message';
import {CommitExclude} from './util/commit-exclude';
import {runBazelQuery} from './util/bazel-query';

type ExtraGenericFile = {
type: 'generic';
Expand Down Expand Up @@ -140,6 +141,7 @@ export interface ReleaserConfig {
// Manifest only
excludePaths?: string[];
additionalPaths?: string[];
bazelDepsQuery?: string;
}

export interface CandidateReleasePullRequest {
Expand Down Expand Up @@ -188,6 +190,7 @@ interface ReleaserConfigJson {
'initial-version'?: string;
'exclude-paths'?: string[]; // manifest-only
'additional-paths'?: string[]; // manifest-only
'bazel-deps-query'?: string; // manifest-only
'date-format'?: string;
}

Expand Down Expand Up @@ -670,14 +673,27 @@ export class Manifest {
);
}

// resolve bazel-deps-query for each package and merge with additionalPaths
const resolvedAdditionalPaths: Record<string, string[]> = {};
for (const [path, config] of Object.entries(this.repositoryConfig)) {
const staticPaths = config.additionalPaths || [];
let bazelPaths: string[] = [];
if (config.bazelDepsQuery) {
bazelPaths = runBazelQuery(config.bazelDepsQuery, path, this.logger);
}
// merge and deduplicate
const allPaths = [...new Set([...staticPaths, ...bazelPaths])];
resolvedAdditionalPaths[path] = allPaths;
}

// split commits by path
this.logger.info(`Splitting ${commits.length} commits by path`);
const cs = new CommitSplit({
includeEmpty: true,
packagePaths: Object.fromEntries(
Object.entries(this.repositoryConfig).map(([path, config]) => [
Object.entries(this.repositoryConfig).map(([path]) => [
path,
config.additionalPaths || [],
resolvedAdditionalPaths[path] || [],
])
),
});
Expand Down Expand Up @@ -1414,6 +1430,7 @@ function extractReleaserConfig(
initialVersion: config['initial-version'],
excludePaths: config['exclude-paths'],
additionalPaths: config['additional-paths'],
bazelDepsQuery: config['bazel-deps-query'],
dateFormat: config['date-format'],
};
}
Expand Down Expand Up @@ -1776,6 +1793,7 @@ function mergeReleaserConfig(
excludePaths: pathConfig.excludePaths ?? defaultConfig.excludePaths,
additionalPaths:
pathConfig.additionalPaths ?? defaultConfig.additionalPaths,
bazelDepsQuery: pathConfig.bazelDepsQuery ?? defaultConfig.bazelDepsQuery,
dateFormat: pathConfig.dateFormat ?? defaultConfig.dateFormat,
};
}
Expand Down
121 changes: 121 additions & 0 deletions src/util/bazel-query.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
// Copyright 2024 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import {execSync} from 'child_process';
import {Logger} from './logger';

/**
* Parse the output of a bazel query command into a list of local package paths.
*
* Bazel query output contains lines like:
* //path/to/package:target_name
* @external_dep//path:target
*
* This function:
* 1. Filters out external dependencies (lines starting with `@`)
* 2. Extracts the package path from local targets (`//path/to/package:target` -> `path/to/package`)
* 3. Deduplicates paths
* 4. Optionally excludes a given path (e.g., the package's own path)
*
* @param output Raw stdout from a bazel query command
* @param excludePath Optional path to exclude from results (e.g., the package's own path)
* @returns Array of unique local package paths
*/
export function parseBazelQueryOutput(
output: string,
excludePath?: string
): string[] {
const lines = output.split('\n').filter(line => line.trim().length > 0);

const paths = new Set<string>();
for (const line of lines) {
const trimmed = line.trim();

// Skip external dependencies (start with @)
if (trimmed.startsWith('@')) {
continue;
}

// Match local targets: //path/to/package:target or //path/to/package
const match = trimmed.match(/^\/\/([^:]*)/);
if (!match) {
continue;
}

const packagePath = match[1];

// Skip empty paths (e.g., //:target refers to the root)
if (!packagePath) {
continue;
}

// Normalize: remove trailing slashes
const normalized = packagePath.replace(/\/+$/, '');
if (!normalized) {
continue;
}

// Exclude the package's own path if specified
if (excludePath && normalized === excludePath.replace(/\/+$/, '')) {
continue;
}

paths.add(normalized);
}

return Array.from(paths).sort();
}

/**
* Execute a bazel query command and return the parsed local package paths.
*
* @param query The bazel query string to execute (e.g., "bazel query 'deps(//combined-service)'")
* @param excludePath Optional path to exclude from results
* @param logger Optional logger instance
* @returns Array of unique local package paths
*/
export function runBazelQuery(
query: string,
excludePath?: string,
logger?: Logger
): string[] {
logger?.info(`Running bazel deps query: ${query}`);

try {
const output = execSync(query, {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arbitrary command execution via execSync

The query parameter is passed directly to execSync, which spawns a shell and executes it as-is. Since this value comes from release-please-config.json, anyone with write access to the config file can execute arbitrary commands on the CI runner — not just bazel queries.

For example, a config value like "rm -rf / && bazel query 'deps(//app)'" would be executed. Consider either:

  1. Validating that the command starts with bazel query (e.g., /^bazel\s+query\s/), or
  2. Splitting the command and using execFileSync('bazel', ['query', ...args]) instead, which avoids shell interpretation entirely.

This was acknowledged in the PR description's review checklist, but adding a basic prefix validation would be a low-cost safeguard.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/util/bazel-query.ts
Line: 96

Comment:
**Arbitrary command execution via `execSync`**

The `query` parameter is passed directly to `execSync`, which spawns a shell and executes it as-is. Since this value comes from `release-please-config.json`, anyone with write access to the config file can execute arbitrary commands on the CI runner — not just bazel queries.

For example, a config value like `"rm -rf / && bazel query 'deps(//app)'"` would be executed. Consider either:
1. Validating that the command starts with `bazel query` (e.g., `/^bazel\s+query\s/`), or
2. Splitting the command and using `execFileSync('bazel', ['query', ...args])` instead, which avoids shell interpretation entirely.

This was acknowledged in the PR description's review checklist, but adding a basic prefix validation would be a low-cost safeguard.

How can I resolve this? If you propose a fix, please make it concise.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 8cdd609:

  • Switched to execFileSync('bazel', ['query', ...]) (no shell) to mitigate command-injection risk.
  • bazel-deps-query now supports true (auto-constructs default expression from package path) or a string (either a query expression like deps(//...) or a full bazel query '...' command, from which we extract the expression before execution).

encoding: 'utf-8',
timeout: 120000, // 2 minute timeout
stdio: ['pipe', 'pipe', 'pipe'],
});

const paths = parseBazelQueryOutput(output, excludePath);
logger?.info(
`Bazel deps query resolved ${
paths.length
} additional paths: ${JSON.stringify(paths)}`
);
return paths;
} catch (err) {
const error = err as Error & {stderr?: string};
logger?.error(
`Failed to run bazel deps query "${query}": ${error.message}`
);
if (error.stderr) {
logger?.error(`stderr: ${error.stderr}`);
}
throw new Error(
`Failed to execute bazel-deps-query "${query}": ${error.message}`
);
}
}
8 changes: 8 additions & 0 deletions test/fixtures/manifest/config/bazel-deps-query.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"release-type": "simple",
"packages": {
"apps/my-app": {
"bazel-deps-query": "bazel query 'deps(//apps/my-app)'"
}
}
}
152 changes: 152 additions & 0 deletions test/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,34 @@ describe('Manifest', () => {
'path-ignore',
]);
});
it('should read bazel-deps-query from manifest', async () => {
const getFileContentsStub = sandbox.stub(
github,
'getFileContentsOnBranch'
);
getFileContentsStub
.withArgs('release-please-config.json', 'main')
.resolves(
buildGitHubFileContent(
fixturesPath,
'manifest/config/bazel-deps-query.json'
)
)
.withArgs('.release-please-manifest.json', 'main')
.resolves(
buildGitHubFileContent(
fixturesPath,
'manifest/versions/versions.json'
)
);
const manifest = await Manifest.fromManifest(
github,
github.repository.defaultBranch
);
expect(manifest.repositoryConfig['apps/my-app'].bazelDepsQuery).to.equal(
"bazel query 'deps(//apps/my-app)'"
);
});
it('should read additional paths from manifest', async () => {
const getFileContentsStub = sandbox.stub(
github,
Expand Down Expand Up @@ -3767,6 +3795,130 @@ describe('Manifest', () => {
});
});

it('should update manifest for commits resolved by bazel-deps-query', async () => {
mockReleases(sandbox, github, []);
mockTags(sandbox, github, [
{
name: 'apps-myapp-v1.0.0',
sha: 'abc123',
},
]);
mockCommits(sandbox, github, [
{
sha: 'aaaaaa',
message: 'fix: shared-lib bugfix',
files: ['libs/shared-lib/test.txt'],
},
{
sha: 'abc123',
message: 'chore: release main',
files: [],
pullRequest: {
headBranchName: 'release-please/branches/main/components/myapp',
baseBranchName: 'main',
number: 123,
title: 'chore: release main',
body: '',
labels: [],
files: [],
sha: 'abc123',
},
},
]);
// Stub the bazel query module
const bazelQueryModule = await import('../src/util/bazel-query');
const runBazelQueryStub = sandbox
.stub(bazelQueryModule, 'runBazelQuery')
.returns(['libs/shared-lib', 'libs/other-lib']);

const manifest = new Manifest(
github,
'main',
{
'apps/my-app': {
releaseType: 'simple',
component: 'myapp',
bazelDepsQuery: "bazel query 'deps(//apps/my-app)'",
},
},
{
'apps/my-app': Version.parse('1.0.0'),
}
);
const pullRequests = await manifest.buildPullRequests();
expect(pullRequests).lengthOf(1);
const pullRequest = pullRequests[0];
expect(pullRequest.version?.toString()).to.eql('1.0.1');
expect(pullRequest.headRefName).to.eql(
'release-please--branches--main--components--myapp'
);
// Verify the bazel query was called with the right arguments
sinon.assert.calledOnce(runBazelQueryStub);
sinon.assert.calledWith(
runBazelQueryStub,
"bazel query 'deps(//apps/my-app)'",
'apps/my-app'
);
});

it('should merge bazel-deps-query results with static additionalPaths', async () => {
mockReleases(sandbox, github, []);
mockTags(sandbox, github, [
{
name: 'apps-myapp-v1.0.0',
sha: 'abc123',
},
]);
mockCommits(sandbox, github, [
{
sha: 'aaaaaa',
message: 'fix: static-lib bugfix',
files: ['libs/static-lib/test.txt'],
},
{
sha: 'abc123',
message: 'chore: release main',
files: [],
pullRequest: {
headBranchName: 'release-please/branches/main/components/myapp',
baseBranchName: 'main',
number: 123,
title: 'chore: release main',
body: '',
labels: [],
files: [],
sha: 'abc123',
},
},
]);
// Stub the bazel query module
const bazelQueryModule = await import('../src/util/bazel-query');
sandbox
.stub(bazelQueryModule, 'runBazelQuery')
.returns(['libs/dynamic-lib']);

const manifest = new Manifest(
github,
'main',
{
'apps/my-app': {
releaseType: 'simple',
component: 'myapp',
additionalPaths: ['libs/static-lib'],
bazelDepsQuery: "bazel query 'deps(//apps/my-app)'",
},
},
{
'apps/my-app': Version.parse('1.0.0'),
}
);
const pullRequests = await manifest.buildPullRequests();
expect(pullRequests).lengthOf(1);
const pullRequest = pullRequests[0];
// The commit in libs/static-lib should trigger a release
expect(pullRequest.version?.toString()).to.eql('1.0.1');
});

it('should update manifest for commits in additionalPaths', async () => {
mockReleases(sandbox, github, []);
mockTags(sandbox, github, [
Expand Down
Loading
Loading