[seokho-son]

[Copilot]

Pull request overview

This pull request adds a comprehensive placeholder detection and management system for remote commands. The feature automatically detects <PLACEHOLDER_NAME> patterns in command textareas and dynamically renders dedicated input fields for each placeholder, with support for password-masked inputs for sensitive data like API tokens.

Changes:

• Simplified placeholder patterns in WireGuard and Jitsi commands to use the new standardized format
• Added PLACEHOLDER_METADATA configuration for defining placeholder descriptions, hints, and secret flags
• Implemented extractPlaceholders function to detect placeholder patterns using regex
• Implemented renderPlaceholderInputs function to dynamically generate UI for placeholder inputs
• Modified setupCommandsPopup to integrate auto-detection with debounced textarea input events
• Modified collectCommands to substitute placeholder values before returning commands

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Placeholder values are substituted directly into commands without sanitization or validation. This creates a security risk because malicious input in placeholder fields could be used to inject unintended shell commands. Since these commands are executed on remote systems, user input from placeholder fields should be validated and/or sanitized before substitution. Consider adding input validation to reject dangerous characters or patterns, or escaping special shell characters in the placeholder values before substitution.

[Copilot]

Using a while loop with includes and replace for each placeholder substitution is inefficient. If the same placeholder appears multiple times, this checks includes for each replacement iteration. Consider using String.prototype.replaceAll (supported in modern browsers) or a single regex-based replace with a global flag. This would be more performant and cleaner. For example: cmdText = cmdText.replaceAll(fullMatch, value).

Suggested change

	while (cmdText.includes(fullMatch)) {
	cmdText = cmdText.replace(fullMatch, value);
	}
	cmdText = cmdText.replaceAll(fullMatch, value);

[Copilot]

Empty placeholder inputs are silently ignored during substitution, leaving the placeholder pattern in the command. This could lead to confusing behavior where commands are sent with unsubstituted placeholders like <DNS_DOMAIN> still present. Consider adding validation to ensure all detected placeholders have values before allowing command submission, or at minimum display a warning to the user about unfilled placeholders.

[Copilot]

The regex character class [A-Z0-9_-] has a hyphen at the end which is being treated as a literal hyphen character. While this works correctly, it's more conventional and clearer to either escape it as - or place it at the beginning of the character class to avoid ambiguity. Consider changing to [A-Z0-9_-] or [A-Z0-9-_] for better clarity.

Suggested change

	const regex = /(?<!:-)<([A-Z][A-Z0-9_-]*)>/g;
	const regex = /(?<!:-)<([A-Z][A-Z0-9_\-]*)>/g;

[Copilot]

The toggle visibility button for secret fields uses emoji characters (👁 and 🙈) as the sole indication of its function. While the title attribute provides a text description on hover, screen readers may not properly announce the emoji or the button's current state. Consider adding an aria-label that reflects the current state (e.g., "Show password" or "Hide password") and updating it when the button is clicked, or use text labels instead of emoji for better accessibility.

Suggested change

	toggleBtn.title = 'Toggle visibility';
	toggleBtn.style.cssText = 'padding: 2px 6px; border: 1px solid #ccc; border-radius: 3px; background: #f8f9fa; cursor: pointer; font-size: 0.8rem;';
	toggleBtn.onclick = () => {
	input.type = input.type === 'password' ? 'text' : 'password';
	toggleBtn.textContent = input.type === 'password' ? '👁' : '🙈';
	toggleBtn.title = 'Show password';
	toggleBtn.setAttribute('aria-label', 'Show password');
	toggleBtn.setAttribute('aria-pressed', 'false');
	toggleBtn.style.cssText = 'padding: 2px 6px; border: 1px solid #ccc; border-radius: 3px; background: #f8f9fa; cursor: pointer; font-size: 0.8rem;';
	toggleBtn.onclick = () => {
	const isCurrentlyHidden = input.type === 'password';
	input.type = isCurrentlyHidden ? 'text' : 'password';
	const nowHidden = input.type === 'password';
	toggleBtn.textContent = nowHidden ? '👁' : '🙈';
	const label = nowHidden ? 'Show password' : 'Hide password';
	toggleBtn.title = label;
	toggleBtn.setAttribute('aria-label', label);
	toggleBtn.setAttribute('aria-pressed', nowHidden ? 'false' : 'true');

[Copilot]

The negative lookbehind pattern (?<!:-) in the regex may not be supported in older browsers or JavaScript engines. Safari versions before 16.4 (released March 2023) do not support negative lookbehind assertions. Consider adding a try-catch block around the regex usage or documenting the browser compatibility requirements. Alternatively, use a simpler pattern that avoids lookbehind assertions for broader compatibility.

Suggested change

	const regex = /(?<!:-)<([A-Z][A-Z0-9_-]*)>/g;
	const placeholders = [];
	const seen = new Set();
	let match;
	while ((match = regex.exec(text)) !== null) {
	const regex = /<([A-Z][A-Z0-9_-]*)>/g;
	const placeholders = [];
	const seen = new Set();
	let match;
	while ((match = regex.exec(text)) !== null) {
	// Skip placeholders that are part of shell-style defaults like ${VAR:-<DEFAULT>}
	const startIndex = match.index;
	if (startIndex >= 2 && text.substring(startIndex - 2, startIndex) === ':-') {
	continue;
	}

[Copilot]

The placeholder input fields do not have any length constraints or input validation. Very long placeholder values could cause performance issues or overflow issues when substituted into commands. Consider adding maxlength attributes to the input fields or validating the input length, especially for non-secret fields where extremely long values are unlikely to be valid.

Suggested change

	// Restore previous value
	if (existingValues[ph.name]) {
	input.value = existingValues[ph.name];
	// Apply length constraints: allow per-placeholder override, otherwise cap non-secret fields
	if (typeof ph.maxLength === 'number' && ph.maxLength > 0) {
	input.maxLength = ph.maxLength;
	} else if (!ph.isSecret) {
	input.maxLength = 256;
	}
	// Restore previous value
	if (existingValues[ph.name]) {
	const existingValue = String(existingValues[ph.name]);
	const maxLen = input.maxLength && input.maxLength > 0 ? input.maxLength : existingValue.length;
	input.value = existingValue.slice(0, maxLen);

[Copilot]

When commands are reset via resetCommands function, the placeholder input fields are not re-rendered for the newly created empty command fields. After calling resetCommands, the renderPlaceholderInputs function should be invoked to ensure the UI is in a consistent state. Add a call to window.renderPlaceholderInputs() at the end of the resetCommands function.

Suggested change

	console.log('Commands reset to 3 empty fields');
	console.log('Commands reset to 3 empty fields');
	if (typeof window.renderPlaceholderInputs === 'function') {
	window.renderPlaceholderInputs();
	}

[Copilot]

The placeholder attribute is constructed using ph.hint which comes from PLACEHOLDER_METADATA and ph.name which comes from user-controlled textarea content. While ph.hint is safe (from hardcoded metadata), the fallback uses ph.name.replace which could contain malicious content if an attacker crafts a placeholder name with special characters. Although the regex restricts placeholder names to [A-Z0-9_-], if the regex were to be modified in the future or bypassed, this could create an XSS vulnerability. Consider using textContent for dynamic generation or adding explicit validation that ph.name only contains the expected characters before using it in the placeholder attribute.

Suggested change

	input.placeholder = ph.hint || `Enter ${ph.name.replace(/_/g, ' ').toLowerCase()}`;
	const safePlaceholderName = /^[A-Z0-9_-]+$/.test(ph.name) ? ph.name : 'value';
	input.placeholder = ph.hint || `Enter ${safePlaceholderName.replace(/_/g, ' ').toLowerCase()}`;

[seokho-son]

/approve