fix: harden anonymization privacy paths (#754)

This commit is contained in:
Thomas Durieux
2026-07-22 04:28:01 +02:00
committed by GitHub
parent 5ef10dee9e
commit aab6ccf7fc
5 changed files with 262 additions and 50 deletions
+73 -38
View File
@@ -7,7 +7,11 @@ import got from "got";
import Repository from "./Repository"; import Repository from "./Repository";
import { RepositoryStatus } from "./types"; import { RepositoryStatus } from "./types";
import config from "../config"; import config from "../config";
import { anonymizePath, isTextFile } from "./anonymize-utils"; import {
anonymizePath,
hasCustomTermReplacement,
isTextFile,
} from "./anonymize-utils";
import AnonymousError from "./AnonymousError"; import AnonymousError from "./AnonymousError";
import { handleError } from "../server/routes/route-utils"; import { handleError } from "../server/routes/route-utils";
import FileModel from "./model/files/files.model"; import FileModel from "./model/files/files.model";
@@ -17,6 +21,27 @@ import { createLogger, serializeError } from "./logger";
const logger = createLogger("anonymized-file"); const logger = createLogger("anonymized-file");
function escapeRegex(value: string): string {
return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
}
function defaultMaskCandidateRegex(value: string): RegExp {
const mask = new RegExp(
`${escapeRegex(config.ANONYMIZATION_MASK)}(?:-[0-9]+)?`,
"g"
);
let source = "^";
let lastIndex = 0;
let match: RegExpExecArray | null;
while ((match = mask.exec(value)) !== null) {
source += escapeRegex(value.slice(lastIndex, match.index));
source += "[^/]+";
lastIndex = match.index + match[0].length;
}
source += escapeRegex(value.slice(lastIndex)) + "$";
return new RegExp(source);
}
// Map a streamer error response to an AnonymousError that preserves the // Map a streamer error response to an AnonymousError that preserves the
// upstream status and error code instead of collapsing every failure into a // upstream status and error code instead of collapsing every failure into a
// generic 404. Without this, a corrupt cache, a 5xx from the streamer, an // generic 404. Without this, a corrupt cache, a 5xx from the streamer, an
@@ -129,24 +154,33 @@ export default class AnonymizedFile {
if (fileDir.endsWith("/")) fileDir = fileDir.slice(0, -1); if (fileDir.endsWith("/")) fileDir = fileDir.slice(0, -1);
const filename = basename(this.anonymizedPath); const filename = basename(this.anonymizedPath);
if (!this.anonymizedPath.includes(config.ANONYMIZATION_MASK)) { if (this.anonymizedPath == "") {
if (this.anonymizedPath == "") { return {
return { name: "",
name: "", path: "",
path: "",
repoId: this.repository.repoId,
};
}
const query: FilterQuery<IFile> = {
repoId: this.repository.repoId, repoId: this.repository.repoId,
path: fileDir,
}; };
if (filename != "") query.name = filename; }
const res = await FileModel.findOne(query);
if (res) { // Always try the path verbatim first. Most paths contain no configured
this._file = res; // term, even when the repository uses custom replacements.
return res; const exactQuery: FilterQuery<IFile> = {
} repoId: this.repository.repoId,
path: fileDir,
};
if (filename != "") exactQuery.name = filename;
const exact = await FileModel.findOne(exactQuery);
if (exact) {
this._file = exact;
return exact;
}
const terms = this.repository.options.terms || [];
const usesDefaultMask = this.anonymizedPath.includes(
config.ANONYMIZATION_MASK
);
const usesCustomReplacement = hasCustomTermReplacement(terms);
if (!usesDefaultMask && !usesCustomReplacement) {
// The stored tree can be incomplete: GitHub truncates tree listings of // The stored tree can be incomplete: GitHub truncates tree listings of
// very large repositories, and folders recorded in `truncatedFolders` // very large repositories, and folders recorded in `truncatedFolders`
// have entries that never made it into the database. Ask GitHub // have entries that never made it into the database. Ask GitHub
@@ -164,36 +198,37 @@ export default class AnonymizedFile {
}); });
} }
const pathQuery = fileDir // Custom replacements do not carry a marker that can be reversed into a
.split("/") // narrow Mongo query. Fetch the repository's paths and verify them by
.map((p) => { // re-applying anonymization. Default XXXX-N masks retain the optimized,
if (p.includes(config.ANONYMIZATION_MASK)) { // anchored query.
return "[^/]+"; const candidates = usesCustomReplacement
} ? await FileModel.find({ repoId: this.repository.repoId }).exec()
return p; : await FileModel.find({
}) repoId: this.repository.repoId,
.join("/"); path: defaultMaskCandidateRegex(fileDir),
const nameQuery = filename.replace( name: defaultMaskCandidateRegex(filename),
new RegExp(config.ANONYMIZATION_MASK + "(-[0-9]+)?"), }).exec();
"[^/]+"
);
const candidates = await FileModel.find({
repoId: this.repository.repoId,
path: new RegExp(pathQuery),
name: new RegExp(nameQuery),
}).exec();
for (const candidate of candidates) { for (const candidate of candidates) {
const candidatePath = join(candidate.path, candidate.name); const candidatePath = join(candidate.path, candidate.name);
if ( if (
anonymizePath(candidatePath, this.repository.options.terms || []) == anonymizePath(candidatePath, terms) == this.anonymizedPath
this.anonymizedPath
) { ) {
this._file = candidate; this._file = candidate;
return candidate; return candidate;
} }
} }
// If applying the configured terms does not alter the requested path, it
// may simply be absent from a truncated tree and can be recovered as-is.
if (anonymizePath(this.anonymizedPath, terms) === this.anonymizedPath) {
const recovered = await this.recoverTruncatedFile(fileDir);
if (recovered) {
this._file = recovered;
return recovered;
}
}
throw new AnonymousError("file_not_found", { throw new AnonymousError("file_not_found", {
object: this, object: this,
httpStatus: 404, httpStatus: 404,
+11 -2
View File
@@ -5,7 +5,11 @@ import * as sha1 from "crypto-js/sha1";
import User from "./User"; import User from "./User";
import GitHubStream from "./source/GitHubStream"; import GitHubStream from "./source/GitHubStream";
import Zip from "./source/Zip"; import Zip from "./source/Zip";
import { anonymizePathCompiled, compileTerms } from "./anonymize-utils"; import {
anonymizePathCompiled,
compileTerms,
hasCustomTermReplacement,
} from "./anonymize-utils";
import UserModel from "./model/users/users.model"; import UserModel from "./model/users/users.model";
import { IAnonymizedRepositoryDocument } from "./model/anonymizedRepositories/anonymizedRepositories.types"; import { IAnonymizedRepositoryDocument } from "./model/anonymizedRepositories/anonymizedRepositories.types";
import { AnonymizeTransformer } from "./anonymize-utils"; import { AnonymizeTransformer } from "./anonymize-utils";
@@ -141,6 +145,7 @@ export default class Repository {
force: false, force: false,
} }
): Promise<IFile[]> { ): Promise<IFile[]> {
const terms = this._model.options.terms || [];
let hasFile = await FileModel.exists({ repoId: this.repoId }).exec(); let hasFile = await FileModel.exists({ repoId: this.repoId }).exec();
// Files created by GitHubDownload don't carry a valid 40-char GitHub // Files created by GitHubDownload don't carry a valid 40-char GitHub
// blob SHA. When the source type later switches to GitHubStream the // blob SHA. When the source type later switches to GitHubStream the
@@ -183,7 +188,11 @@ export default class Repository {
).exec(); ).exec();
} }
} }
if (opt.path?.includes(config.ANONYMIZATION_MASK)) { if (
opt.path &&
(opt.path.includes(config.ANONYMIZATION_MASK) ||
hasCustomTermReplacement(terms))
) {
const f = new AnonymizedFile({ const f = new AnonymizedFile({
repository: this, repository: this,
anonymizedPath: opt.path, anonymizedPath: opt.path,
+95 -10
View File
@@ -144,7 +144,9 @@ export class AnonymizeTransformer extends Transform {
// byte, isn't silently corrupted by a UTF-8 round-trip through the // byte, isn't silently corrupted by a UTF-8 round-trip through the
// StringDecoder. See discussion in #493. // StringDecoder. See discussion in #493.
private pendingBytes: Buffer = Buffer.alloc(0); private pendingBytes: Buffer = Buffer.alloc(0);
private static readonly OVERLAP = 4096; private static readonly DEFAULT_OVERLAP = 4096;
private readonly overlap: number;
private readonly bufferWholeStream: boolean;
constructor( constructor(
readonly opt: { readonly opt: {
@@ -160,6 +162,26 @@ export class AnonymizeTransformer extends Transform {
this.nameVerdict = classifyByName(this.opt.filePath); this.nameVerdict = classifyByName(this.opt.filePath);
if (this.nameVerdict !== null) this.isText = this.nameVerdict; if (this.nameVerdict !== null) this.isText = this.nameVerdict;
this.anonimizer = new ContentAnonimizer(this.opt); this.anonimizer = new ContentAnonimizer(this.opt);
const termPatterns = (opt.terms || []).map((term) => parseTermSpec(term).term);
// Streaming replacement can only safely emit a prefix when every possible
// match is shorter than the retained suffix. Regex quantifiers such as
// `*`, `+`, and `{m,n}` can exceed any fixed overlap, as can the URL/image
// removal patterns. Buffer those uncommon configurations in full (bounded
// by MAX_FILE_SIZE) so anonymization fails closed instead of leaking a
// match that straddles the streaming boundary.
this.bufferWholeStream =
opt.link === false ||
opt.image === false ||
termPatterns.some(termNeedsWholeStream);
const longestFixedPattern = Math.max(
0,
...termPatterns.map((term) => term.length),
(opt.repoName?.length || 0) + (opt.branchName?.length || 0) + 128
);
this.overlap = this.bufferWholeStream
? Number.POSITIVE_INFINITY
: Math.max(AnonymizeTransformer.DEFAULT_OVERLAP, longestFixedPattern + 32);
} }
get wasAnonimized() { get wasAnonimized() {
@@ -174,7 +196,7 @@ export class AnonymizeTransformer extends Transform {
return reencoded.length === candidate.length && reencoded.equals(candidate); return reencoded.length === candidate.length && reencoded.equals(candidate);
} }
_transform(chunk: Buffer, encoding: string, callback: () => void) { _transform(chunk: Buffer, encoding: string, callback: (error?: Error) => void) {
if (this.nameVerdict === null) { if (this.nameVerdict === null) {
// Name didn't decide. isbinaryfile inspects the first 512 bytes for // Name didn't decide. isbinaryfile inspects the first 512 bytes for
// null bytes and non-printable ratio and returns a decisive boolean. // null bytes and non-printable ratio and returns a decisive boolean.
@@ -196,8 +218,19 @@ export class AnonymizeTransformer extends Transform {
this.pending += this.decoder.write(chunk); this.pending += this.decoder.write(chunk);
this.pendingBytes = Buffer.concat([this.pendingBytes, chunk]); this.pendingBytes = Buffer.concat([this.pendingBytes, chunk]);
if (this.pending.length > AnonymizeTransformer.OVERLAP) { if (
let split = this.pending.length - AnonymizeTransformer.OVERLAP; this.bufferWholeStream &&
this.pendingBytes.length > config.MAX_FILE_SIZE
) {
return callback(
new Error(
`Text file exceeded ${config.MAX_FILE_SIZE} bytes while buffering an unbounded anonymization pattern`
)
);
}
if (this.pending.length > this.overlap) {
let split = this.pending.length - this.overlap;
// Avoid splitting a UTF-16 surrogate pair. // Avoid splitting a UTF-16 surrogate pair.
const code = this.pending.charCodeAt(split); const code = this.pending.charCodeAt(split);
if (code >= 0xdc00 && code <= 0xdfff) { if (code >= 0xdc00 && code <= 0xdfff) {
@@ -311,6 +344,45 @@ function hasCatastrophicBacktracking(src: string): boolean {
return false; return false;
} }
function termNeedsWholeStream(term: string): boolean {
try {
new RegExp(term, "i");
} catch {
// Invalid regular expressions are escaped and treated literally.
return false;
}
if (hasCatastrophicBacktracking(term)) {
// Catastrophic expressions are also escaped and treated literally.
return false;
}
let escaped = false;
let inCharacterClass = false;
for (let i = 0; i < term.length; i++) {
const char = term[i];
if (escaped) {
escaped = false;
continue;
}
if (char === "\\") {
escaped = true;
continue;
}
if (char === "[") {
inCharacterClass = true;
continue;
}
if (char === "]") {
inCharacterClass = false;
continue;
}
if (!inCharacterClass && (char === "*" || char === "+" || char === "{")) {
return true;
}
}
return false;
}
function compileTerms(terms: string[] | undefined): CompiledTermVariant[] { function compileTerms(terms: string[] | undefined): CompiledTermVariant[] {
if (!terms || terms.length === 0) return []; if (!terms || terms.length === 0) return [];
const compiled: CompiledTermVariant[] = []; const compiled: CompiledTermVariant[] = [];
@@ -379,13 +451,16 @@ export class ContentAnonimizer {
) { ) {
this.compiledTerms = compileTerms(opt.terms); this.compiledTerms = compileTerms(opt.terms);
if (opt.repoName && opt.branchName) { if (opt.repoName && opt.branchName) {
const r = opt.repoName; const r = escapeRegex(opt.repoName);
const b = opt.branchName; const b = escapeRegex(opt.branchName);
this.selfLinkRegexes = [ this.selfLinkRegexes = [
new RegExp(`https://raw.githubusercontent.com/${r}/${b}\\b`, "gi"), new RegExp(
new RegExp(`https://github.com/${r}/blob/${b}\\b`, "gi"), `https://raw\\.githubusercontent\\.com/${r}/${b}(?=$|[/?#])`,
new RegExp(`https://github.com/${r}/tree/${b}\\b`, "gi"), "gi"
new RegExp(`https://github.com/${r}`, "gi"), ),
new RegExp(`https://github\\.com/${r}/blob/${b}(?=$|[/?#])`, "gi"),
new RegExp(`https://github\\.com/${r}/tree/${b}(?=$|[/?#])`, "gi"),
new RegExp(`https://github\\.com/${r}(?=$|[/?#])`, "gi"),
]; ];
} }
} }
@@ -454,6 +529,12 @@ export function anonymizePath(path: string, terms: string[]) {
return anonymizePathCompiled(path, compileTerms(terms)); return anonymizePathCompiled(path, compileTerms(terms));
} }
export function hasCustomTermReplacement(terms: string[] | undefined): boolean {
return (terms || []).some(
(term) => parseTermSpec(term).replacement !== null
);
}
// Variant that accepts pre-compiled term regexes — call sites that anonymize // Variant that accepts pre-compiled term regexes — call sites that anonymize
// many paths in a row (tree traversal) should compile once and reuse. // many paths in a row (tree traversal) should compile once and reuse.
export function anonymizePathCompiled( export function anonymizePathCompiled(
@@ -468,3 +549,7 @@ export function anonymizePathCompiled(
export { compileTerms }; export { compileTerms };
export type { CompiledTermVariant }; export type { CompiledTermVariant };
function escapeRegex(value: string): string {
return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
}
+37
View File
@@ -2,6 +2,10 @@ const { expect } = require("chai");
const { Transform } = require("stream"); const { Transform } = require("stream");
const { StringDecoder } = require("string_decoder"); const { StringDecoder } = require("string_decoder");
require("ts-node/register/transpile-only"); require("ts-node/register/transpile-only");
const {
ContentAnonimizer: RealContentAnonimizer,
AnonymizeTransformer: RealAnonymizeTransformer,
} = require("../src/core/anonymize-utils");
const { const {
withWordBoundaries, withWordBoundaries,
termVariants, termVariants,
@@ -449,6 +453,20 @@ describe("ContentAnonimizer", function () {
expect(result).to.include("anonymous.4open.science/r/abc123"); expect(result).to.include("anonymous.4open.science/r/abc123");
}); });
it("escapes regex characters in repository and branch names", function () {
const anon = new RealContentAnonimizer({
repoName: "secret-owner/secret.repo",
branchName: "release+candidate",
repoId: "abc123",
});
const result = anon.anonymize(
"https://raw.githubusercontent.com/secret-owner/secret.repo/release+candidate/src/index.ts"
);
expect(result).to.equal(
"https://anonymous.4open.science/r/abc123/src/index.ts"
);
});
it("is case-insensitive for repo name", function () { it("is case-insensitive for repo name", function () {
const anon = new ContentAnonimizer({ const anon = new ContentAnonimizer({
repoName: "Owner/Repo", repoName: "Owner/Repo",
@@ -647,6 +665,25 @@ describe("AnonymizeTransformer (streaming)", function () {
expect(result).to.equal("Created by XXXX-1 at 2025/01/01"); expect(result).to.equal("Created by XXXX-1 at 2025/01/01");
}); });
it("does not leak matches longer than the normal streaming overlap", async function () {
const secret = "A".repeat(5000);
const transformer = new RealAnonymizeTransformer({
filePath: "fixture.txt",
terms: ["A{5000}"],
});
const chunks = [];
const done = new Promise((resolve, reject) => {
transformer.on("data", (chunk) => chunks.push(Buffer.from(chunk)));
transformer.on("end", () =>
resolve(Buffer.concat(chunks).toString("utf8"))
);
transformer.on("error", reject);
});
transformer.end(Buffer.from(` ${secret} `));
const result = await done;
expect(result).to.equal(" XXXX-1 ");
});
// Regression for #342/#349: zip download was constructing the transformer // Regression for #342/#349: zip download was constructing the transformer
// and then assigning opt.filePath after the fact, but isText is decided in // and then assigning opt.filePath after the fact, but isText is decided in
// the constructor — so every entry was treated as binary and passed through // the constructor — so every entry was treated as binary and passed through
+46
View File
@@ -0,0 +1,46 @@
const { expect } = require("chai");
require("ts-node/register/transpile-only");
const AnonymizedFile = require("../src/core/AnonymizedFile").default;
const FileModel = require("../src/core/model/files/files.model").default;
describe("AnonymizedFile custom replacement paths", function () {
let originalFindOne;
let originalFind;
beforeEach(function () {
originalFindOne = FileModel.findOne;
originalFind = FileModel.find;
});
afterEach(function () {
FileModel.findOne = originalFindOne;
FileModel.find = originalFind;
});
it("maps a custom replacement back to the original file", async function () {
const original = {
repoId: "repo-1",
path: "src/secret",
name: "index.ts",
sha: "abc",
size: 10,
};
FileModel.findOne = async () => null;
FileModel.find = () => ({ exec: async () => [original] });
const repository = {
repoId: "repo-1",
options: { terms: ["secret=>hidden"] },
model: { truncatedFolders: [] },
source: {},
};
const file = new AnonymizedFile({
repository,
anonymizedPath: "src/hidden/index.ts",
});
expect(await file.getFileInfo()).to.equal(original);
expect(await file.originalPath()).to.equal("src/secret/index.ts");
});
});