Prevent secrets referencing themselves to loop over and over again, compute them only once

This commit is contained in:
Carlos Monastyrski
2025-09-12 21:02:17 -03:00
parent c24b222ff0
commit 369c620fcf
2 changed files with 76 additions and 10 deletions

View File

@@ -622,11 +622,15 @@ export const expandSecretReferencesFactory = ({
const stackTrace = { ...dto, key: "root", children: [] } as TSecretReferenceTraceNode; const stackTrace = { ...dto, key: "root", children: [] } as TSecretReferenceTraceNode;
if (!dto.value) return { expandedValue: "", stackTrace }; if (!dto.value) return { expandedValue: "", stackTrace };
const stack = [{ ...dto, depth: 0, trace: stackTrace }];
// Track visited secrets to prevent circular references
const createSecretId = (env: string, secretPath: string, key: string) => `${env}:${secretPath}:${key}`;
const stack = [{ ...dto, depth: 0, trace: stackTrace, visitedSecrets: new Set<string>() }];
let expandedValue = dto.value; let expandedValue = dto.value;
while (stack.length) { while (stack.length) {
const { value, secretPath, environment, depth, trace } = stack.pop()!; const { value, secretPath, environment, depth, trace, visitedSecrets } = stack.pop()!;
// eslint-disable-next-line no-continue // eslint-disable-next-line no-continue
if (depth > MAX_SECRET_REFERENCE_DEPTH) continue; if (depth > MAX_SECRET_REFERENCE_DEPTH) continue;
@@ -700,17 +704,27 @@ export const expandSecretReferencesFactory = ({
trace trace
}; };
const shouldExpandMore = INTERPOLATION_TEST_REGEX.test(referencedSecretValue); // Check for circular reference
const referencedSecretId = createSecretId(
referencedSecretEnvironmentSlug,
referencedSecretPath,
referencedSecretKey
);
const isCircular = visitedSecrets.has(referencedSecretId);
const newVisitedSecrets = new Set([...visitedSecrets, referencedSecretId]);
const shouldExpandMore = INTERPOLATION_TEST_REGEX.test(referencedSecretValue) && !isCircular;
if (dto.shouldStackTrace) { if (dto.shouldStackTrace) {
const stackTraceNode = { ...node, children: [], key: referencedSecretKey, trace: null }; const stackTraceNode = { ...node, children: [], key: referencedSecretKey, trace: null };
trace?.children.push(stackTraceNode); trace?.children.push(stackTraceNode);
// if stack trace this would be child node // if stack trace this would be child node
if (shouldExpandMore) { if (shouldExpandMore) {
stack.push({ ...node, trace: stackTraceNode }); stack.push({ ...node, trace: stackTraceNode, visitedSecrets: newVisitedSecrets });
} }
} else if (shouldExpandMore) { } else if (shouldExpandMore) {
// if no stack trace is needed we just keep going with root node // if no stack trace is needed we just keep going with root node
stack.push(node); stack.push({ ...node, visitedSecrets: newVisitedSecrets });
} }
if (referencedSecretValue) { if (referencedSecretValue) {

View File

@@ -29,17 +29,54 @@ const INTERPOLATION_SYNTAX_REG = /\${([^}]+)}/;
export const hasSecretReference = (value: string | undefined) => export const hasSecretReference = (value: string | undefined) =>
value ? INTERPOLATION_SYNTAX_REG.test(value) : false; value ? INTERPOLATION_SYNTAX_REG.test(value) : false;
const createNodeId = (node: TSecretReferenceTraceNode): string =>
`${node.environment}:${node.secretPath}:${node.key}`;
const isCircularReference = (
node: TSecretReferenceTraceNode,
visitedPath: Set<string>
): boolean => {
const nodeId = createNodeId(node);
return visitedPath.has(nodeId);
};
const hasCircularReferences = (
node: TSecretReferenceTraceNode,
visitedPath: Set<string> = new Set()
): boolean => {
const nodeId = createNodeId(node);
if (visitedPath.has(nodeId)) {
return true;
}
const newVisitedPath = new Set([...visitedPath, nodeId]);
return node.children.some((child) => hasCircularReferences(child, newVisitedPath));
};
export const SecretReferenceNode = ({ export const SecretReferenceNode = ({
node, node,
isRoot, isRoot,
secretKey secretKey,
visitedPath = new Set()
}: { }: {
node: TSecretReferenceTraceNode; node: TSecretReferenceTraceNode;
isRoot?: boolean; isRoot?: boolean;
secretKey?: string; secretKey?: string;
visitedPath?: Set<string>;
}) => { }) => {
const [isOpen, setIsOpen] = useState(false); const [isOpen, setIsOpen] = useState(false);
const hasChildren = node.children.length > 0;
const nodeId = createNodeId(node);
const isCircular = !isRoot && isCircularReference(node, visitedPath);
const newVisitedPath = isCircular ? visitedPath : new Set([...visitedPath, nodeId]);
const safeChildren = isCircular
? []
: node.children.filter((child) => !isCircularReference(child, newVisitedPath));
const hasChildren = safeChildren.length > 0;
return ( return (
<li> <li>
@@ -77,8 +114,12 @@ export const SecretReferenceNode = ({
<Collapsible.Content className={twMerge("mt-4", style.collapsibleContent)}> <Collapsible.Content className={twMerge("mt-4", style.collapsibleContent)}>
{hasChildren && ( {hasChildren && (
<ul> <ul>
{node.children.map((el, index) => ( {safeChildren.map((el, index) => (
<SecretReferenceNode node={el} key={`${el.key}-${index + 1}`} /> <SecretReferenceNode
node={el}
key={`${el.key}-${index + 1}`}
visitedPath={newVisitedPath}
/>
))} ))}
</ul> </ul>
)} )}
@@ -102,6 +143,9 @@ export const SecretReferenceTree = ({ secretPath, environment, secretKey }: Prop
const tree = data?.tree; const tree = data?.tree;
const secretValue = data?.value; const secretValue = data?.value;
// Check if the tree contains circular references
const hasCirculars = tree ? hasCircularReferences(tree) : false;
useEffect(() => { useEffect(() => {
if (error instanceof AxiosError) { if (error instanceof AxiosError) {
const err = error?.response?.data as TApiErrors; const err = error?.response?.data as TApiErrors;
@@ -132,7 +176,15 @@ export const SecretReferenceTree = ({ secretPath, environment, secretKey }: Prop
return ( return (
<div> <div>
<FormControl label="Expanded value"> <FormControl
label="Expanded value"
tooltipText={
hasCirculars
? "This secret contains circular references. Value shown is resolved once, with circular paths truncated in the reference tree below."
: undefined
}
tooltipClassName="max-w-md break-words"
>
<SecretInput <SecretInput
key="value-overriden" key="value-overriden"
isReadOnly isReadOnly