Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
17 changes: 13 additions & 4 deletions images/chromium-headful/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ RUN apt-get update && \
sudo \
mutter \
x11vnc \
# Recording tools
ffmpeg \
# Python/pyenv reqs
build-essential \
libssl-dev \
Expand Down Expand Up @@ -92,6 +90,17 @@ RUN apt-get update && \
unzip && \
apt-get clean

# install ffmpeg manually since the version available in apt is from the 4.x branch due to #drama.
# as of writing these static builds will be the latest 7.0.x release.
RUN set -eux; \
URL="https://johnvansickle.com/ffmpeg/releases/ffmpeg-release-amd64-static.tar.xz"; \
Copy link

Choose a reason for hiding this comment

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

Security concern: Downloading and executing binaries from an external source without integrity verification. Consider adding checksum validation or using a specific versioned URL with known checksums to ensure the downloaded binary hasn't been tampered with.

Suggested change
URL="https://johnvansickle.com/ffmpeg/releases/ffmpeg-release-amd64-static.tar.xz"; \
# install ffmpeg manually since the version available in apt is from the 4.x branch due to #drama.
# as of writing these static builds will be the latest 7.0.x release.
RUN set -eux; \
URL="https://johnvansickle.com/ffmpeg/releases/ffmpeg-release-amd64-static.tar.xz"; \
EXPECTED_SHA256="<insert_known_checksum_here>"; \
echo "Downloading FFmpeg static build from $URL"; \
curl -fsSL "$URL" -o /tmp/ffmpeg.tar.xz; \
echo "$EXPECTED_SHA256 /tmp/ffmpeg.tar.xz" | sha256sum -c -; \
tar -xJf /tmp/ffmpeg.tar.xz -C /tmp; \
install -m755 /tmp/ffmpeg-*/ffmpeg /usr/local/bin/ffmpeg; \
install -m755 /tmp/ffmpeg-*/ffprobe /usr/local/bin/ffprobe; \
rm -rf /tmp/ffmpeg*

Type: Security | Severity: High

echo "Downloading FFmpeg static build from $URL"; \
curl -fsSL "$URL" -o /tmp/ffmpeg.tar.xz; \
tar -xJf /tmp/ffmpeg.tar.xz -C /tmp; \
install -m755 /tmp/ffmpeg-*/ffmpeg /usr/local/bin/ffmpeg; \
install -m755 /tmp/ffmpeg-*/ffprobe /usr/local/bin/ffprobe; \
rm -rf /tmp/ffmpeg*

# runtime
ENV USERNAME=root
RUN set -eux; \
Expand Down Expand Up @@ -138,8 +147,8 @@ RUN git clone --branch v1.5.0 https://github.com/novnc/noVNC.git /opt/noVNC && \

# setup desktop env & app
ENV DISPLAY_NUM=1
ENV HEIGHT=1080
ENV WIDTH=1920
ENV HEIGHT=768
ENV WIDTH=1024
ENV WITHDOCKER=true

COPY xorg.conf /etc/neko/xorg.conf
Expand Down
2 changes: 1 addition & 1 deletion images/chromium-headful/neko.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# https://neko.m1k1o.net/docs/v3/configuration#file

desktop:
screen: "1920x1080@60"
screen: "1024x768@60"

member:
provider: "noauth"
Expand Down
8 changes: 8 additions & 0 deletions images/chromium-headful/wrapper.sh
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,13 @@ if [[ "${ENABLE_WEBRTC:-}" == "true" ]]; then
# use webrtc
echo "✨ Starting neko (webrtc server)."
/usr/bin/neko serve --server.static /var/www --server.bind 0.0.0.0:8080 >&2 &

# Wait for neko to be ready.
echo "Waiting for neko port 0.0.0.0:8080..."
while ! nc -z 127.0.0.1 8080 2>/dev/null; do
sleep 0.5
done
echo "Port 8080 is open"
else
# use novnc
./novnc_startup.sh
Expand Down Expand Up @@ -213,6 +220,7 @@ if [[ "${WITH_KERNEL_IMAGES_API:-}" == "true" ]]; then
sleep 5

# Attempt to click the warning's close button
echo "Clicking the warning's close button at x=$OFFSET_X y=115"
if curl -s -o /dev/null -X POST \
http://localhost:10001/computer/click_mouse \
-H "Content-Type: application/json" \
Expand Down
6 changes: 3 additions & 3 deletions server/cmd/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ func validate(config *Config) error {
if config.DisplayNum < 0 {
return fmt.Errorf("DISPLAY_NUM must be greater than 0")
}
if config.FrameRate < 0 || config.FrameRate > 120 {
return fmt.Errorf("FRAME_RATE must be greater than 0 and less than 120")
if config.FrameRate < 0 || config.FrameRate > 20 {
return fmt.Errorf("FRAME_RATE must be greater than 0 and less than or equal to 20")
}
if config.MaxSizeInMB < 0 || config.MaxSizeInMB > 1000 {
return fmt.Errorf("MAX_SIZE_MB must be greater than 0 and less than 1000")
return fmt.Errorf("MAX_SIZE_MB must be greater than 0 and less than or equal to 1000")
}
if config.PathToFFmpeg == "" {
return fmt.Errorf("FFMPEG_PATH is required")
Expand Down
4 changes: 2 additions & 2 deletions server/cmd/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ func TestLoad(t *testing.T) {
name: "custom valid env",
env: map[string]string{
"PORT": "12345",
"FRAME_RATE": "24",
"FRAME_RATE": "20",
"DISPLAY_NUM": "2",
"MAX_SIZE_MB": "250",
"OUTPUT_DIR": "/tmp",
"FFMPEG_PATH": "/usr/local/bin/ffmpeg",
},
wantCfg: &Config{
Port: 12345,
FrameRate: 24,
FrameRate: 20,
DisplayNum: 2,
MaxSizeInMB: 250,
OutputDir: "/tmp",
Expand Down
54 changes: 27 additions & 27 deletions server/lib/oapi/oapi.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions server/lib/recorder/ffmeg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ func TestFFmpegRecorder_StartAndStop(t *testing.T) {

time.Sleep(50 * time.Millisecond)

require.NoError(t, rec.Stop(t.Context()))
err := rec.Stop(t.Context())
require.Error(t, err)
assert.Contains(t, err.Error(), "exit status 101")

<-rec.exited
require.False(t, rec.IsRecording(t.Context()))
}
Expand All @@ -55,7 +58,9 @@ func TestFFmpegRecorder_ForceStop(t *testing.T) {

time.Sleep(50 * time.Millisecond)

require.NoError(t, rec.ForceStop(t.Context()))
err := rec.ForceStop(t.Context())
require.Error(t, err)

<-rec.exited
require.False(t, rec.IsRecording(t.Context()))
assert.Contains(t, rec.cmd.ProcessState.String(), "killed")
Expand Down
18 changes: 13 additions & 5 deletions server/lib/recorder/ffmpeg.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,14 @@ func (fr *FFmpegRecorder) Start(ctx context.Context) error {
// Stop gracefully stops the recording using a multi-phase shutdown process.
func (fr *FFmpegRecorder) Stop(ctx context.Context) error {
defer fr.stz.Enable(ctx)
err := fr.shutdownInPhases(ctx, []shutdownPhase{
{"wake_and_interrupt", []syscall.Signal{syscall.SIGCONT, syscall.SIGINT}, 5 * time.Second, "graceful stop"},
{"retry_interrupt", []syscall.Signal{syscall.SIGINT}, 3 * time.Second, "retry graceful stop"},
{"terminate", []syscall.Signal{syscall.SIGTERM}, 250 * time.Millisecond, "forceful termination"},
// This isn't scientific - give ffmpeg a long time to complete since encoding pipelines can
// be complex and we care more about the recording than performance. In cases where ffmpeg
// "falls behind" (e.g. it's resource constrained) it's better for our use case to wait for
// the recording to complete than it is to quickly terminate. We intentionally detach the
// shutdown process from any inbound context
err := fr.shutdownInPhases(context.Background(), []shutdownPhase{
{"wake_and_interrupt", []syscall.Signal{syscall.SIGINT}, time.Minute, "graceful stop"},
{"terminate", []syscall.Signal{syscall.SIGTERM}, 2 * time.Second, "forceful termination"},
{"kill", []syscall.Signal{syscall.SIGKILL}, 100 * time.Millisecond, "immediate kill"},
})

Expand Down Expand Up @@ -286,6 +290,8 @@ func ffmpegArgs(params FFmpegRecordingParams, outputPath string) ([]string, erro
args = append(args, []string{
// Video encoding
"-c:v", "libx264",
"-profile:v", "high", // Explicit web-compatible profile
"-pix_fmt", "yuv420p", // Web-standard pixel format

// Timestamp handling for reliable playback
"-use_wallclock_as_timestamps", "1", // Use system time instead of input stream time
Expand Down Expand Up @@ -384,7 +390,9 @@ func (fr *FFmpegRecorder) shutdownInPhases(ctx context.Context, phases []shutdow
// Wait for exit or timeout
if err := waitForChan(ctx, phase.timeout-time.Since(phaseStartTime), done); err == nil {
log.Info("ffmpeg shutdown successful", "phase", phase.name)
return nil
fr.mu.Lock()
defer fr.mu.Unlock()
return fr.ffmpegErr
}
}

Expand Down
2 changes: 1 addition & 1 deletion server/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ components:
type: integer
description: Recording framerate in fps (overrides server default)
minimum: 1
maximum: 60
maximum: 20
maxDurationInSeconds:
type: integer
description: Maximum recording duration in seconds (overrides server default)
Expand Down
Loading